[webkit-reviews] review denied: [Bug 46493] [chromium] Add mipmap support for ImageLayerChromium : [Attachment 69041] Patch

[bugzilla-daemon at webkit.org]

Vangelis Kokkevis <vangelis at chromium.org> has denied W. James MacLean
<wjmaclean at chromium.org>'s request for review:
Bug 46493: [chromium] Add mipmap support for ImageLayerChromium
https://bugs.webkit.org/show_bug.cgi?id=46493

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

------- Additional Comments from Vangelis Kokkevis <vangelis at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69041&action=review

> WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:109
> +

can you use String::contains() here instead of converting to a char* ?

> WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:269
> +	   GLC(context, context->texParameterf(GraphicsContext3D::TEXTURE_2D,
GraphicsContext3D::TEXTURE_MIN_FILTER, 

The filter mode setting can move up to the part that allocates the texture
(where you call context->texImage2D) so you don't execute it every time.  You
also need to make sure that you reset the filter modes when the tests fail as
the layer size can change.

Also, it should be calling texParameteri instead of texParameterf (I think we
have the same problem elsewhere in the code)