Bug 4096 - Speedup png reading
Summary: Speedup png reading
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: png functions (show other bugs)
Version: 0.9.3
Hardware: x86 (IA32) Linux (All)
: high minor
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-08-14 19:29 UTC by Billy Biggs
Modified: 2005-08-22 00:15 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch (1.56 KB, patch)
2005-08-14 19:29 UTC, Billy Biggs
Details | Splinter Review

Description Billy Biggs 2005-08-14 19:29:31 UTC
The png code is very convenient, however it spends a long time performing
premultiplication of the input.  The attached patch speeds it up a fair bit for me.

I think alpha premultiplication is something useful enough to belong in
libpixman (I even have my own MMX implementation), however this patch is small
and straightforward so I think it's worth just putting in for now.
Comment 1 Billy Biggs 2005-08-14 19:29:49 UTC
Created attachment 2859 [details] [review]
Patch
Comment 2 Carl Worth 2005-08-18 10:57:28 UTC
The patch looks fairly good to me. Here are a few (very minor) comments:

Why does this use "__inline__" rather than the C99 "inline"? Or it could
be a macro similar to the other multiplication macro (macros?) we have
elsewhere in the implementation.

I don't like the single-letter ientifiers "a" and "r" if they're intended
to refer to specific things. Maybe alpha? and color?

But if the arguments are interchangable, then single-letter generic names
would be better. "a" and "b" ? And if so, perhaps "multiply_alpha" should
also have a more generic name.

Anyway, feel free to resolve those as you see fit, then commit.
Comment 3 Billy Biggs 2005-08-19 23:08:58 UTC
Released to HEAD with modifications.

I don't know what the correct thing is to do for asking the compiler to inline
things.  I would think that if it's OK to depend on C99, using that "inline"
would be a good thing.  However, since I am unsure, and since pixman uses
__inline__ for gcc, I used that for this commit (with a macro in cairoint.h). 
Hopefully someone who knows these things better can fix it in both cairo and
pixman if it's wrong.
Comment 4 Carl Worth 2005-08-22 17:15:24 UTC
Move bugs against "cvs" version to "0.9.3" so we can remove the "cvs" version.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.