[webkit-reviews] review granted: [Bug 88363] [chromium] Separate CCVideoDrawQuad and from the layer tree and video provider by removing ManagedTexture and WebVideoFrame pointers from the quad : [Attachment 146897] Patch

[bugzilla-daemon at webkit.org]

Adrienne Walker <enne at google.com> has granted Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 88363: [chromium] Separate CCVideoDrawQuad and from the layer tree and
video provider by removing ManagedTexture and WebVideoFrame pointers from the
quad
https://bugs.webkit.org/show_bug.cgi?id=88363

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

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


R=me.

> Source/WebCore/platform/graphics/chromium/cc/CCVideoDrawQuad.h:53
> +    unsigned m_frameProviderTextureId;
>      GC3Denum m_format;

I'm still interested in having fewer overloaded quads, but maybe that can be a
follow-up patch.  I think we could have a YUV video quad (3 planes), streaming
texture (id and matrix), and use CCTextureDrawQuad (id, texture transform) for
RGBA and native.  I also wonder if anybody is using that texture matrix for
anything more than uv translation and scale.

> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.cpp:248
> +void CCVideoLayerImpl::FramePlane::freeData(CCRenderer* layerRenderer)

Can you assert in the FramePlane destructor that this function has been called
and textureId is 0?