1412249 - Fix DidComposite timing for EmptyTransaction

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Sotaro Ikeda [:sotaro]]

When WebRenderBridgeParent::RecvEmptyTransaction() is implemented, DidComposite handling was implemented as temporal. DidComposite needs to be call at correct timing.

https://dxr.mozilla.org/mozilla-central/source/gfx/layers/wr/WebRenderBridgeParent.cpp#627

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix DidComposite timing for EmptyTransaction

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix DidComposite timing for EmptyTransaction

Comment 3

[Sotaro Ikeda [:sotaro]]

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

Comment 4

[Nicolas Silva [:nical]]

Comment on attachment 8927242 [details] [diff] [review]
patch - Fix DidComposite timing for EmptyTransaction

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

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +663,5 @@
> +    // Send empty UpdatePipelineResources to WebRender just to notify a new epoch.
> +    // The epoch is used to know a timing of calling DidComposite().
> +    // This is much simpler than tracking an epoch of AsyncImagePipeline.
> +    wr::ResourceUpdateQueue resourceUpdates;
> +    mApi->UpdatePipelineResources(resourceUpdates, mPipelineId, wr::NewEpoch(wrEpoch));

Thankfully I have something in progress that will make this nicer (you will be able send new epochs without sending empty resource updates).

@@ +671,5 @@
> +    // If WebRenderBridgeParent does not have pending DidComposites,
> +    // send DidComposite now.
> +    if (mPendingTransactionIds.empty()) {
> +      TimeStamp now = TimeStamp::Now();
> +      mCompositorBridge->DidComposite(wr::AsUint64(mPipelineId), now, now);

Why do we need to send DidComposite here?

Comment 5

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8927242 [details] [diff] [review]
patch - Fix DidComposite timing for EmptyTransaction

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

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +671,5 @@
> +    // If WebRenderBridgeParent does not have pending DidComposites,
> +    // send DidComposite now.
> +    if (mPendingTransactionIds.empty()) {
> +      TimeStamp now = TimeStamp::Now();
> +      mCompositorBridge->DidComposite(wr::AsUint64(mPipelineId), now, now);

In this case, we do not need to defer to send DidComposite to client side, since RecvEmptyTransaction() did not update AsyncImagePipeline and there is no pending DidComposite. If WebRenderBridgeParent has pending DidComposites, sending DidComposite needs to be deferred after the pending DidComposites.

In this case, RecvEmptyTransaction() does not trigger updating Webrender, but client side needs DidComposite.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cc53112a8d5
Fix DidComposite timing for EmptyTransaction r=nical

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/4cc53112a8d5