1408348 - Fix lifetime handling of WebRenderTextureHost

Status: RESOLVED WONTFIX

(Core :: Graphics: WebRender, enhancement)

Description

[Sotaro Ikeda [:sotaro]]

Since Bug 1383786 fix, the way of updating display list and usage of wr::ImageKey were changed. Current lifetime handling of WebRenderTextureHost does not fit to them. We need to address the lifetime handling.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix lifetime handling of WebRenderTextureHost

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix lifetime handling of WebRenderTextureHost

Comment 3

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=03c52b30371c1559dd2f6ea53017b7518d7a0e06

Comment 4

[Sotaro Ikeda [:sotaro]]

Current AsyncImagePipelineManager holds the TexureHosts based on Epoch. To update epoch, we need to update DisplayList. It is not good from performance point of view.

attachment 8920080 [details] [diff] [review] changes AsyncImagePipelineManager as to hold TextureHosts based on frame count. It fit well to Bug 1383786 change and AsyncImagePipelineManager could be simplified.

Comment 5

[Sotaro Ikeda [:sotaro]]

With attachment 8920080 [details] [diff] [review], requesting frame count could be get by return value of RenderThread::IncPendingFrameCount(). Completed frame count could be get by RenderThread::GetCompletedFrameCount(). With these frame counts values, AsyncImagePipelineManager could know how long AsyncImagePipelineManager needs to hold TextureHosts.

Comment 6

[Sotaro Ikeda [:sotaro]]

With Comment 5, only WebRenderImageHost::SetCurrentTextureHost() needs to call AsyncImagePipelineManager::HoldExternalImage().

Comment 7

[Nicolas Silva [:nical]]

I just have one issue with basing the texture lifetimes on frame counts: we are in the process of making changes to webrender were scene building would become asynchronous, so that we can keep scrolling scene N-1 while building scene N. When we get there, display list and resource updates will come with new epochs but scroll/animation frames will not.

This is not to say that using frame counts is bad, but I just want to make sure we keep in mind how the asynchronous frame building would interact with it.
For example imagine there is an update on big (long to build) scene and some scrolling happening at the same time. What can happen is that the render backend receives the display list, schedules the scene to be built on another thread and resumes handling scrolling on the current scene for a few frames until the new scene is ready. In this situation, after calling update_texture on the compositor thread there is no guarantee that the next frame rendered will actually use that texture.
I hope the explanation isn't too confusing.

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Nicolas Silva [:nical] from comment #7)
> This is not to say that using frame counts is bad, but I just want to make
> sure we keep in mind how the asynchronous frame building would interact with
> it.
> For example imagine there is an update on big (long to build) scene and some
> scrolling happening at the same time. What can happen is that the render
> backend receives the display list, schedules the scene to be built on
> another thread and resumes handling scrolling on the current scene for a few
> frames until the new scene is ready. In this situation, after calling
> update_texture on the compositor thread there is no guarantee that the next
> frame rendered will actually use that texture.

:nical, in this case, epoch appearing to RendererOGL::FlushRenderedEpochs() is also deferred? How can we know when webrender stop to use a specific external image?

Comment 9

[Nicolas Silva [:nical]]

Yes, epochs will be deferred along with the display list.

Example:

> set display list or update resources (epoch=1)
> display list ready (epoch=1)
> generate frame (flush rendered epoch 1)
> generate frame (flush rendered epoch 1)
> set display list or update resources (epoch=2)
> generate frame (flush rendered epoch 1)
> generate frame (flush rendered epoch 1)
> display list ready (epoch=2)
> generate frame (flush rendered epoch 2)


The information about whether an external image is in use should be in sync with the rendered epoch.

Comment 10

[Sotaro Ikeda [:sotaro]]

Thanks, make sense:)