Make wr_notifier_nop_frame_done() notify DidComposite

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P2
normal
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

9 months ago
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.
Assignee

Updated

9 months ago
Assignee: nobody → sotaro.ikeda.g
Assignee

Updated

9 months ago
Blocks: 1461239
Assignee

Updated

9 months ago
Attachment #9013220 - Flags: review?(nical.bugzilla)
Priority: -- → P2
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?
Attachment #9013220 - Flags: review?(nical.bugzilla) → review+
Assignee

Comment 4

9 months ago
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.
Assignee

Comment 5

9 months ago
(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

9 months ago
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa6baa1cd577
Make wr_notifier_nop_frame_done() notify DidComposite r=nical

Comment 7

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aa6baa1cd577
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.