Enable VsyncScheduler again

RESOLVED FIXED in Firefox 54

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 months ago
Since RenderThread was enabled, VsyncScheduler was not used. Instead, new composition is triggered for each WebRenderBridgeParent::HandleDPEnd() call. It sometimes causes too often composition.
(Assignee)

Updated

8 months ago
Assignee: nobody → sotaro.ikeda.g
(Assignee)

Updated

8 months ago
Blocks: 1311790
(Assignee)

Comment 1

8 months ago
Created attachment 8840829 [details] [diff] [review]
patch - Enable VsyncScheduler again
(Assignee)

Comment 2

8 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25e328a1ef498e533cc933f158917489a3e34914
(Assignee)

Updated

8 months ago
Attachment #8840829 - Flags: review?(bugmail)
Comment on attachment 8840829 [details] [diff] [review]
patch - Enable VsyncScheduler again

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

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +475,5 @@
>    MOZ_ASSERT((uint32_t)(size.width * 4) == stride);
>  
> +  if (mCompositorScheduler->NeedsComposite()) {
> +    mCompositorScheduler->CancelCurrentCompositeTask();
> +    mApi->GenerateFrame();

I think we should replace this call to GenerateFrame() with mCompositorScheduler->ForceComposeToTarget(nullptr, nullptr);

It does the same thing, but it will also update the CompositorVsyncScheduler's mLastCompose timestamp which I think is important to avoid needlessly recompositing.

::: gfx/webrender_bindings/src/bindings.rs
@@ +170,5 @@
>                                preserve_frame_state);
> +}
> +
> +#[no_mangle]
> +pub unsafe extern fn wr_api_generate_frame(api: &mut RenderApi) {

does this need to be unsafe?
Attachment #8840829 - Flags: review?(bugmail) → review+
(Assignee)

Comment 4

8 months ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3)
> Comment on attachment 8840829 [details] [diff] [review]
> patch - Enable VsyncScheduler again
> 
> Review of attachment 8840829 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/wr/WebRenderBridgeParent.cpp
> @@ +475,5 @@
> >    MOZ_ASSERT((uint32_t)(size.width * 4) == stride);
> >  
> > +  if (mCompositorScheduler->NeedsComposite()) {
> > +    mCompositorScheduler->CancelCurrentCompositeTask();
> > +    mApi->GenerateFrame();
> 
> I think we should replace this call to GenerateFrame() with
> mCompositorScheduler->ForceComposeToTarget(nullptr, nullptr);
> 
> It does the same thing, but it will also update the
> CompositorVsyncScheduler's mLastCompose timestamp which I think is important
> to avoid needlessly recompositing.

Yea, I will update it in a next patch.
 
> ::: gfx/webrender_bindings/src/bindings.rs
> @@ +170,5 @@
> >                                preserve_frame_state);
> > +}
> > +
> > +#[no_mangle]
> > +pub unsafe extern fn wr_api_generate_frame(api: &mut RenderApi) {
> 
> does this need to be unsafe?

It is not necessary.
(Assignee)

Comment 5

8 months ago
Created attachment 8841093 [details] [diff] [review]
patch - Enable VsyncScheduler again
Attachment #8840829 - Attachment is obsolete: true
Attachment #8841093 - Flags: review+
(Assignee)

Comment 6

8 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f95477fb628e16bf04145da695fb039a4086b90

Needs to update also how to delete ImageKeys.
(Assignee)

Comment 7

8 months ago
(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=4f95477fb628e16bf04145da695fb039a4086b90
> 
> Needs to update also how to delete ImageKeys.

Ah, since Bug 1342246, ImageKeys allocation often causes conflicts. It is a regression of 1342246.
(Assignee)

Comment 8

8 months ago
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> 
> Ah, since Bug 1342246, ImageKeys allocation often causes conflicts. It is a
> regression of 1342246.

Bug 1342754 is created for it.

Comment 9

8 months ago
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/99933cb429ce
Enable VsyncScheduler again r=kats
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/99933cb429ce
status-firefox54: --- → fixed
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.