Closed
Bug 1342380
Opened 8 years ago
Closed 8 years ago
Enable VsyncScheduler again
Categories
(Core :: Graphics: WebRender, defect)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: sotaro, Assigned: sotaro)
References
Details
Attachments
(1 file, 1 obsolete file)
11.29 KB,
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
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 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8840829 -
Flags: review?(bugmail)
Comment 3•8 years ago
|
||
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 years 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 years ago
|
||
Attachment #8840829 -
Attachment is obsolete: true
Attachment #8841093 -
Flags: review+
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4f95477fb628e16bf04145da695fb039a4086b90
Needs to update also how to delete ImageKeys.
Assignee | ||
Comment 7•8 years 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 years 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.
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/99933cb429ce
Enable VsyncScheduler again r=kats
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment 10•8 years ago
|
||
status-firefox54:
--- → fixed
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•