[webkit-reviews] review denied: [Bug 91169] Chromium Mac: Add TEXTURE_RECTANGLE_ARB support to CCVideoLayerImpl : [Attachment 152801] Patch

[bugzilla-daemon at webkit.org]

Adrienne Walker <enne at google.com> has denied Sailesh Agrawal
<sail at chromium.org>'s request for review:
Bug 91169: Chromium Mac: Add TEXTURE_RECTANGLE_ARB support to CCVideoLayerImpl
https://bugs.webkit.org/show_bug.cgi?id=91169

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

------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=152801&action=review


This patch is much improved.  Thanks for all the changes.  Along with rebasing
to get this to apply to ToT, I have one more small change, but then this should
be good:

> Source/WebCore/platform/graphics/chromium/cc/CCIOSurfaceLayerImpl.cpp:102
> -    quadList.append(CCIOSurfaceDrawQuad::create(sharedQuadState, quadRect,
m_ioSurfaceSize, m_ioSurfaceTextureId));
> +    quadList.append(CCIOSurfaceDrawQuad::create(sharedQuadState, quadRect,
m_ioSurfaceSize, m_ioSurfaceTextureId, true));

Ack, boolean parameter trap
(http://ariya.ofilabs.com/2011/08/hall-of-api-shame-boolean-trap.html).  Even
if you store flipped as a boolean member variable, can you add an enum {
Flipped, Unflipped } to the create function signature so it's obvious what the
parameter means at the callsite?