[webkit-reviews] review denied: [Bug 11405] Linux/Gdk build fixes : [Attachment 11320] linux\gdk build fixes

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Wed Nov 1 19:13:01 PST 2006


Maciej Stachowiak <mjs at apple.com> has denied Maciej Stachowiak
<mjs at apple.com>'s request for review:
Bug 11405: Linux/Gdk build fixes
http://bugs.webkit.org/show_bug.cgi?id=11405

Attachment 11320: linux\gdk build fixes
http://bugs.webkit.org/attachment.cgi?id=11320&action=edit

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
Thanks for the patch! A few comments:

What warning do you get that resulted in these changes:

-	     return NaN64AsBits;
+	     return (uintptr_t)NaN64AsBits;

It would be nicer if NanAsBits was templatized on sizeof(uintptr_t) so that
this cast wasn't necessary. It's kind of sloppy to rely on the dead code paths
to be optimized out in the first place (admittedly a pre-existing flaw in the
code).

There is a more complete AffineTransformCairo attached to
<http://bugs.webkit.org/show_bug.cgi?id=11433>, I suggest starting with that.
There's one bug in that version which I pointed out in review comments. (I
think the right way to map a rect with a transform is to map each corner and
find the bounding box). However, the other version doesn't have all the
notImplemented() stuff this does, so you may want to start with that.

Otherwise, this looks great to me.

r- to consider the two points I raised. If you can't figure out how to do the
template solution for JSImmediate.h that's ok, that can be done later, but I
think it is worth taking the better AffineTransform.



More information about the webkit-reviews mailing list