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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 2 13:21:30 PST 2009

```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

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.
```