1008221 - CanvasClient2D is creating a new TextureClient for each frame.

Status: RESOLVED DUPLICATE of bug 1167235

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

We should recycle them instead, do double/triple buffering or anything smarter that what we currently do.

Comment 1

[Nicolas Silva [:nical]]

Attachment: Make CanvasClient2D double buffered

This relies on the fact that Matt is throttling the content-thread to avoid over-production (not sure if it has landed already, but I'll find out and this bug should be blocked on that).

Sotaro, I saw that in some places in CanvasClient2D (namely Clear() and OnDetach()), we were just nulling out mBuffer without calling RemoveFromCompositable on it. Was that intentional?
This patch adds calls to RemoveFromCompositable in these places, let me know if I was wrong to add it.

Comment 2

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #1)
> 
> Sotaro, I saw that in some places in CanvasClient2D (namely Clear() and
> OnDetach()), we were just nulling out mBuffer without calling
> RemoveFromCompositable on it. Was that intentional?

It is not intentional, I seems just forgot to add all places. Though, if we do not recycle the buffer, it is not a problem in this case.

Comment 3

[Sotaro Ikeda [:sotaro]]

On b2g, if we want to recycle the buffer, we need to handle Fence delivery to TextureClient before reuse it.
This bug might have a dependency to Bug 988956 and Bug 1006957.

Comment 4

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8420196 [details] [diff] [review]
Make CanvasClient2D double buffered

Review of attachment 8420196 [details] [diff] [review]:
-----------------------------------------------------------------

Add some comment. feedback+ if comment is addressed.

::: gfx/layers/client/CanvasClient.cpp
@@ +120,4 @@
>    }
> +  RefPtr<TextureClient> temp = mFrontBuffer;
> +  mFrontBuffer = mBackBuffer;
> +  mBackBuffer = temp;

If we want to reuse the front buffer as backbuffer. We need to ensure the buffer's ownership by somehow. On thebes layer case, it is done by sync transaciton.

On PImageBridge, we can do it by async transaction by using, AsyncTransactionTracker. On PLayerTransaction, we do not have a good way to ensure the ownership by async transaciton.

One indirect way to ensure it is Bug 854421. But it does not directly ensure the buffer owner ship. I am suspect that Bug 854421 could fix the all problems like bug 1006164.

Therefore, I like more direct ownership ensurance. How do you think about it?

I am going to impelement some basics for it in Bug 1006957 and bug 988941.

Comment 5

[Nicolas Silva [:nical]]

Thanks for the feedback Sotaro. I'll either implement proper fence delivery in CanvasClient2D or make it not double-buffered on Gonk as a first step. We'll need the fence stuff at some point either way.

Comment 6

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #5)
> Thanks for the feedback Sotaro. I'll either implement proper fence delivery
> in CanvasClient2D or make it not double-buffered on Gonk as a first step.
> We'll need the fence stuff at some point either way.

The recycling problem on gonk can be fixed by using a method in Bug 1006957.

Comment 7

[Nicolas Silva [:nical]]

Comment on attachment 8420196 [details] [diff] [review]
Make CanvasClient2D double buffered

Review of attachment 8420196 [details] [diff] [review]:
-----------------------------------------------------------------

this is a bit outdated now