Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compositing: Stop compositing unnecessarily after each animation frame. #9663

Merged
merged 1 commit into from Feb 24, 2016

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Feb 16, 2016

Instead, schedule a delayed composite after each frame of an animation.

The previous code would cause jank, because the following sequence
frequently occurred:

  1. The page uses requestAnimationFrame() to request a frame.
  2. The compositor receives the message, schedules a composite,
    dispatches the rAF message to the script thread, composites, and goes to
    sleep waiting for vblank (frame 1).
  3. The script makes a change and sends it through the pipeline.
    Eventually it gets painted and is sent to the compositor, but the
    compositor is sleeping.
  4. The compositor wakes up, sees the new painted content, page flips,
    and goes to sleep (frame 2). Repeat from step 1.

The problem is that we have two composition frames, not just one. This
halves Web apps' framerate!

This commit fixes the problem by scheduling the composite in step 2 to
12 ms in the future. We already have this delayed-composition
functionality in the form of the scrolling timer, which I repurposed and
renamed to the "delayed composition timer" for this task. This change
gives the page 12 ms to prepare the frame, which seems to usually be
enough, especially with WebRender.

Note that simply removing the scheduled composite after rAF is not the
correct solution. If this is done, then pages that call rAF and don't
modify the page won't receive future rAFs, since the compositor will be
sleeping and won't be notified of vblank.

Fixes a bunch of jank in browser.html. The remaining jank seems to be a
problem with browser.html itself.

r? @glennw

Review on Reviewable

@highfive
Copy link

highfive commented Feb 16, 2016

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
timestamp = this_timestamp;
break
}
_ => break,

This comment has been minimized.

Copy link
@samlh

samlh Feb 16, 2016

Contributor

Should this have a case for exit? e.g.

    Ok(ToDelayedCompositionTimerMsg::Exit) => break 'outer,
@bors-servo
Copy link
Contributor

bors-servo commented Feb 18, 2016

The latest upstream changes (presumably #9589) made this pull request unmergeable. Please resolve the merge conflicts.

@glennw
Copy link
Member

glennw commented Feb 24, 2016

@pcwalton This looks good to me once the comment / question is resolved, will need a rebase though.

Instead, schedule a delayed composite after each frame of an animation.

The previous code would cause jank, because the following sequence
frequently occurred:

1. The page uses `requestAnimationFrame()` to request a frame.

2. The compositor receives the message, schedules a composite,
dispatches the rAF message to the script thread, composites, and goes to
sleep waiting for vblank (frame 1).

3. The script makes a change and sends it through the pipeline.
Eventually it gets painted and is sent to the compositor, but the
compositor is sleeping.

4. The compositor wakes up, sees the new painted content, page flips,
and goes to sleep (frame 2). Repeat from step 1.

The problem is that we have two composition frames, not just one. This
halves Web apps' framerate!

This commit fixes the problem by scheduling the composite in step 2 to
12 ms in the future. We already have this delayed-composition
functionality in the form of the scrolling timer, which I repurposed and
renamed to the "delayed composition timer" for this task. This change
gives the page 12 ms to prepare the frame, which seems to usually be
enough, especially with WebRender.

Note that simply removing the scheduled composite after rAF is not the
correct solution. If this is done, then pages that call rAF and don't
modify the page won't receive future rAFs, since the compositor will be
sleeping and won't be notified of vblank.

Fixes a bunch of jank in browser.html. The remaining jank seems to be a
problem with browser.html itself.
@pcwalton pcwalton force-pushed the pcwalton:browser-html-jank-fix branch from a05ab3c to 9b4cc41 Feb 24, 2016
@pcwalton
Copy link
Contributor Author

pcwalton commented Feb 24, 2016

@bors-servo: r=glennw

@bors-servo
Copy link
Contributor

bors-servo commented Feb 24, 2016

📌 Commit 9b4cc41 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Feb 24, 2016

Testing commit 9b4cc41 with merge 9d47b34...

bors-servo added a commit that referenced this pull request Feb 24, 2016
compositing: Stop compositing unnecessarily after each animation frame.

Instead, schedule a delayed composite after each frame of an animation.

The previous code would cause jank, because the following sequence
frequently occurred:

1. The page uses `requestAnimationFrame()` to request a frame.

2. The compositor receives the message, schedules a composite,
dispatches the rAF message to the script thread, composites, and goes to
sleep waiting for vblank (frame 1).

3. The script makes a change and sends it through the pipeline.
Eventually it gets painted and is sent to the compositor, but the
compositor is sleeping.

4. The compositor wakes up, sees the new painted content, page flips,
and goes to sleep (frame 2). Repeat from step 1.

The problem is that we have two composition frames, not just one. This
halves Web apps' framerate!

This commit fixes the problem by scheduling the composite in step 2 to
12 ms in the future. We already have this delayed-composition
functionality in the form of the scrolling timer, which I repurposed and
renamed to the "delayed composition timer" for this task. This change
gives the page 12 ms to prepare the frame, which seems to usually be
enough, especially with WebRender.

Note that simply removing the scheduled composite after rAF is not the
correct solution. If this is done, then pages that call rAF and don't
modify the page won't receive future rAFs, since the compositor will be
sleeping and won't be notified of vblank.

Fixes a bunch of jank in browser.html. The remaining jank seems to be a
problem with browser.html itself.

r? @glennw

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9663)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 24, 2016

@bors-servo bors-servo merged commit 9b4cc41 into servo:master Feb 24, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.