Bug 17284 - [CAIRO] Introduce single-pixel image optimizations
Summary: [CAIRO] Introduce single-pixel image optimizations
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Nobody
Keywords: Cairo, Gtk
Depends on:
Reported: 2008-02-10 08:49 PST by Alp Toker
Modified: 2009-03-11 10:48 PDT (History)
0 users

See Also:

single pixel optimization (2.56 KB, patch)
2009-03-02 12:11 PST, Dirk Schulze
eric: review-
Details | Formatted Diff | Diff
single pixel optimization with CairoColor (12.56 KB, patch)
2009-03-03 08:30 PST, Dirk Schulze
no flags Details | Formatted Diff | Diff
single pixel optimization with CairoColor (9.71 KB, patch)
2009-03-04 12:11 PST, Dirk Schulze
no flags Details | Formatted Diff | Diff
single pixel optimization with Color (8.12 KB, patch)
2009-03-05 08:33 PST, Dirk Schulze
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alp Toker 2008-02-10 08:49:34 PST
Single-pixel optimizations are not yet implemented and can be useful for speeding up pathological cases like single-pixel GIFs used as padding.

While at it, we can consider borrowing some of the techniques recently developed by these guys:

Comment 1 Dirk Schulze 2009-03-02 12:11:44 PST
Created attachment 28192 [details]
single pixel optimization

implementation of checkForSolidColor for Cairo after Adam Treat s patch: r41358.
Comment 2 Eric Seidel (no email) 2009-03-02 13:21:30 PST
Comment on attachment 28192 [details]
single pixel optimization

I don't really know enough about Cairo to know if your color extraction code is correct.  I think I would have abstracted it into a nice little static function though:

     uint32_t* pixel = reinterpret_cast<uint32_t*>(data);
 208     int alpha, red, green, blue;
 210     if (alpha = (*pixel & 0xff000000) >> 24) {
 211         red = ((*pixel & 0x00ff0000) >> 16) * 255 / alpha;
 212         green = ((*pixel & 0x0000ff00) >> 8) * 255 / alpha;
 213         blue = (*pixel & 0x000000ff) * 255 / alpha;
 214     } else {
 215         red = (*pixel & 0x00ff0000) >> 16;
 216         green = (*pixel & 0x0000ff00) >> 8;
 217         blue = (*pixel & 0x000000ff);
 218     }

Color color = colorFromCairoPixel(*pixel);

I think someone who knows more about Cairo should comment before I can mark this r+.

Also... is (*pixel & 0xff000000) >> 24) the cleanest way to get the alpha out of a cairo pixel?  Talk about ugly, error-prone math...

I think actually I'm going to r- for the way you grab colors out of the pixel.  Even if that's the only way, we should make those static inlines so that they're not so error prone.
Comment 3 Dirk Schulze 2009-03-03 08:30:23 PST
Created attachment 28221 [details]
single pixel optimization with CairoColor

I talked to eseidel in IRC and he mentioned to add a new typedef or somthing like that to avoid failured and make the code reading better.
I added a new color schema to Cairo: CairoColor. CairoColor attends to premultiplying and every other calculation. I added some small callers to have access to the color values of a pixel. You can easily get the unpremutiplied colors of a pixel or set them by adding unpremultiplied colors.
We just support image surfaces with 32bit for now. There are asserts before every direct manipulation of surfaces. But I don't see a problem to support other surfaces in the future if wanted.
Comment 4 Dirk Schulze 2009-03-04 12:11:11 PST
Created attachment 28279 [details]
single pixel optimization with CairoColor

this patch doesn't add a new class but uses Color.cpp instead.
Comment 5 Dirk Schulze 2009-03-05 08:33:29 PST
Created attachment 28302 [details]
single pixel optimization with Color

just added two calls to premultiply or unpremultiply the color of a pixel to Color.cpp.
Comment 6 Eric Seidel (no email) 2009-03-05 12:25:57 PST
Comment on attachment 28302 [details]
single pixel optimization with Color

halves, not halfes

 148 Color colorFromPremultipliedARGB(RGBA32);

should use unsigned, not RGBA32

This looks good.  It would be nice if someone could check your pixel math before you land.  Like Oliver?
Comment 7 Dirk Schulze 2009-03-10 11:48:12 PDT
landed in r41561.
Comment 8 Pam Greene (IRC:pamg) 2009-03-11 10:48:00 PDT
Marking fixed so it drops out of the commit queue.