Bug 89667 – [chromium] Use CCScopedTexture for video layers and YUV video quads

• Component: New Bugs
• Status: RESOLVED INVALID

[Dana Jansens]

[chromium] Use CCScopedTexture for video layers and YUV video quads

[Dana Jansens]

Here's a practical example of using CCScopedTexture in practice. I replaced the textureId/size/format from CCVideoLayerImpl with a CCScopedTexture. It allows us to create/free the texture without using a TextureAllocator, and makes the ownership and freeing semantics simpler IMO.

[Dana Jansens]

Created attachment 148811 [details]
Patch

[Adrienne Walker]

Comment on attachment 148811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148811&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.h:72
> +        CCScopedTexture texture;

So now CCYUVDrawQuad has a RefPtr<GraphicsContext3D>?

[Dana Jansens]

Comment on attachment 148811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148811&action=review

>> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.h:72
>> +        CCScopedTexture texture;
> 
> So now CCYUVDrawQuad has a RefPtr<GraphicsContext3D>?

ahhh that's true, not good.

[Dana Jansens]

(In reply to comment #4)
> (From update of attachment 148811 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148811&action=review
> 
> >> Source/WebCore/platform/graphics/chromium/cc/CCVideoLayerImpl.h:72
> >> +        CCScopedTexture texture;
> > 
> > So now CCYUVDrawQuad has a RefPtr<GraphicsContext3D>?
> 
> ahhh that's true, not good.

It would actually always be null, since this is a cloned copy of the texture and doesn't own the id. But yah.

[Eric Penner]

Comment on attachment 148811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148811&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCScopedTexture.h:54
> +    void cloneFrom(const CCScopedTexture&);

If there isn't two owners, couldn't this return the id? It seems like all the safety of the own pointer is gone in the cloned pointer anyway.

[Eric Penner]

Comment on attachment 148811 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148811&action=review

>> Source/WebCore/platform/graphics/chromium/cc/CCScopedTexture.h:54
>> +    void cloneFrom(const CCScopedTexture&);
> 
> If there isn't two owners, couldn't this return the id? It seems like all the safety of the own pointer is gone in the cloned pointer anyway.

Oh, I think I get the purpose now. You want to keep the size/format etc. in the new object. Maybe there should just be a super basic class that contains these things together. Or even just a struct that we use universally everywhere (like the TextureInfo struct from the original TextureManager).

[Alexandre Elias]

Agreed cloneFrom is a bad idea.  If introducing a new TextureInfo struct is enough to solve your problem, note that it would also let us replace the "maybeBytes()" method with a clearer method TextureInfo::bytes().