[webkit-reviews] review granted: [Bug 107526] Optimize some operations for float type in texture format conversions of WebGL : [Attachment 183934] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jun Jiang <jun.a.jiang at intel.com>'s
request for review:
Bug 107526: Optimize some operations for float type in texture format
conversions of WebGL
https://bugs.webkit.org/show_bug.cgi?id=107526

Attachment 183934: Patch
https://bugs.webkit.org/attachment.cgi?id=183934&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=183934&action=review


Optimizations look fine. I’m a little concerned that we truncate rather than
rounding after multiplying by scale factors. I’m not sure that’s the correct
calculation. I also wonder if there’s an even faster branch-free way to compute
those scale factors?

Someone should fix the style mistake all over this file, calling the type
"unsigned int" when the WebKit coding style suggests calling it just
"unsigned".

> Source/WebCore/platform/graphics/GraphicsContext3D.cpp:1177
> +    ASSERT(source && destination);

Two thoughts:

1) We never do ASSERT with && in it. We always prefer two separate assertions
in such cases so we can more easily figure out which one has failed.
2) It seems unnecessary to assert these pointers are non-zero here. Such
pointers could be scattered all over the code; there’s no particular reason to
expect null pointers here or to assume it would be hard to diagnose a null
pointer related crash inside memcpy.

Based on that, I suggest removing this line of code.