[Webkit-unassigned] [Bug 19243] [CAIRO] Masker in SVG

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=19243


eric at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #21333|                            |review-
               Flag|                            |




------- Comment #4 from eric at webkit.org  2008-05-27 10:31 PDT -------
(From update of attachment 21333)
This could be cleaner.

For:
+    // TODO: check if there is a difference between little and big endian
machines (for this code :-))

Just check the docs for CAIRO_FORMAT_ARGB32.  They should tell you what format
you are to expect the pixels in.

+            int alpha = (*pixel & 0xff000000) >> 24;

Should really be encapsulated into static inline funtions (or use the existing
ones in Cairo).

unsigned char alpha = alphaValue(pixel);

etc.

+            int luma = (int) (red * 0.3 + green * 0.59 + blue * 0.11);

Please add a comment to explain why you're choosing those multipliers.

Also, if you're going to be using alpha, red, green, etc. as floats, you might
as well store them in floats to begin with (instead of the ints you're
currently using, or the unsigned chars in my above example)

Again, something like this would be clearer:
*pixel = pixelFromValues(alpha, luma, luma, luma);

you could even have used something like this above:
float alpha, red, green, blue;
valuesFromPixel(alpha, red, green, blue);

I think some simple inlines like that would make this code much more readable
(and less error prone).

m_mask.set(m_ownerElement->drawMaskerContent(boundingBox,
m_maskRect).release())
drawMaskerContent returns a PassRefPtr, no need to call release() on it, as it
will automatically release into the RefPtr (that's the point of a PRP).

This comment doesn't help:
+    // Transform the mask-surface for use of cairo_mask()


Otherwise looks OK.  I'd rather see the TODOs just fixed now, instead of
landing this partial version.

r- due the fact that this code should be cleaner/more readable.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.