1495352 - Make wr_notifier_nop_frame_done() notify DidComposite

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P2)

Description

[Sotaro Ikeda [:sotaro]]

Non WR case, DidComposite is notified even the following cases.
- [1] When LayerManagerComposite does not have root.
- [2] when LayerManagerComposite skips composite because of no invalidation.

But with WR [1] does not notify DidComposite. [2] is going to be implemented by Bug 1461239, but wr_notifier_nop_frame_done() does not notify DidComposite.

For Bug 1461239, wr_notifier_nop_frame_done() needs to notify DidComposite.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Make wr_notifier_nop_frame_done() notify DidComposite

Comment 2

[Sotaro Ikeda [:sotaro]]

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

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 9013220 [details] [diff] [review]
patch - Make wr_notifier_nop_frame_done() notify DidComposite

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

r=me if you are positive we don't need to do anything special when UpdateAndRender returns false.

::: gfx/webrender_bindings/RenderThread.cpp
@@ +369,5 @@
>  
>    auto& renderer = it->second;
>  
> +  if (aRender) {
> +    renderer->UpdateAndRender(aReadback);

So there's nothing to do if we didn't manage to render?

Comment 4

[Sotaro Ikeda [:sotaro]]

Comment on attachment 9013220 [details] [diff] [review]
patch - Make wr_notifier_nop_frame_done() notify DidComposite

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

::: gfx/webrender_bindings/RenderThread.cpp
@@ +369,5 @@
>  
>    auto& renderer = it->second;
>  
> +  if (aRender) {
> +    renderer->UpdateAndRender(aReadback);

We do not do error handling in RenderThread::UpdateAndRender() level. The current error handling is just not notify did composite. It is not good. Error handling is already done in RendererOGL::UpdateAndRender() level.

Comment 5

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> We do not do error handling in RenderThread::UpdateAndRender() level.

Correction:
> We do not need to do error handling in RenderThread::UpdateAndRender() level.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa6baa1cd577
Make wr_notifier_nop_frame_done() notify DidComposite r=nical

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/aa6baa1cd577