[webkit-reviews] review denied: [Bug 17284] [CAIRO] Introduce single-pixel image optimizations : [Attachment 28192] single pixel optimization

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Dirk Schulze <krit at webkit.org>'s
request for review:
Bug 17284: [CAIRO] Introduce single-pixel image optimizations
https://bugs.webkit.org/show_bug.cgi?id=17284

Attachment 28192: single pixel optimization
https://bugs.webkit.org/attachment.cgi?id=28192&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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;
 209 
 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.