[webkit-reviews] review denied: [Bug 91169] Chromium Mac: Add TEXTURE_RECTANGLE_ARB support to CCVideoLayerImpl : [Attachment 152114] 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 152114: Patch
https://bugs.webkit.org/attachment.cgi?id=152114&action=review

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


>> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:1675
>> +const LayerRendererChromium::TextureRectProgramFlip*
LayerRendererChromium::textureRectProgramFlip()
> 
> bikeshed: s/Flip/Flipped/ ?

Do we really need another program? Can't the rect that's being sent to the
shader be adjusted?

> Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h:204
> -    typedef ProgramBinding<VertexShaderPosTexTransform,
FragmentShaderRGBATexRectFlipAlpha> TextureIOSurfaceProgram;
> +    typedef ProgramBinding<VertexShaderPosTexTransform,
FragmentShaderRGBATexRectAlpha> TextureRectProgram;
> +    typedef ProgramBinding<VertexShaderPosTexTransform,
FragmentShaderRGBATexRectFlipAlpha> TextureRectProgramFlip;

bikshed: I'll take other suggestions, but I don't like this renaming. 
TextureRect is just too ambiguous, since TextureProgram and TextureProgramFlip
take texture rects as uniforms.

> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:211
> +    case GraphicsContext3D::TEXTURE_2D:
> +    case Extensions3D::TEXTURE_RECTANGLE_ARB: {

Can you reuse the CCIOSurfaceDrawQuad rather than shoehorning this into the
CCTextureDrawQuad?