Closed Bug 1508522 Opened 6 years ago Closed 5 years ago

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

()

Details

Attachments

(1 file, 3 obsolete files)

Slowest part of the scene was about 3 fps on Win10(P50, Intel(R) HD Graphics P530).

The page used a lot of SVGs.

profiled data
 https://perfht.ml/2A8xaFA
Priority: -- → P3
(In reply to Brian Birtles (:birtles) from https://github.com/mozanime/taketori/issues/1#issuecomment-441837682)
> Bug 1100357 is also intended to help with this.
Depends on: 1100357
Changing layout.animation.prerender.absolute-limit-{x,y} and layout.animation.prerender.viewport-ratio-limit-{x,y} values helps?
I am going to take a look.
Assignee: nobody → sotaro.ikeda.g
I checked the performance. The performance was very bad because async animation was not used for rendering because svg images sizes were very big. Then sve images were rendered for every frame by BasicLayerManager.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Changing layout.animation.prerender.absolute-limit-{x,y} and
> layout.animation.prerender.viewport-ratio-limit-{x,y} values helps?

It could be used to adjust async animation usages. But it seems not fit well to https://mozanime.github.io/taketori/, since we needed to change layout.animation.prerender.viewport-ratio-limit-{x,y} to very big value.
Yeah, we are hoping to tile the content instead using the mechanism introduced in bug 1321412. However, it still needs work before it can be turned on (e.g. bug 1324591). Last I tried, it was quite buggy too.
When async animation was not used, whole webrender images were rendered via nsDisplayImage::CreateWebRenderCommands() with WebRender. But non-WebRender case, only dirty rect was rendered by nsImageFrame::PaintImage(). It seems to make WebRender's performance worse than non-WebRender.
(In reply to Brian Birtles (:birtles) from comment #6)
> Yeah, we are hoping to tile the content instead using the mechanism
> introduced in bug 1321412. However, it still needs work before it can be
> turned on (e.g. bug 1324591). Last I tried, it was quite buggy too.

When I set pref layout.animation.prerender.partial:true, the performance became a lot better.
(In reply to Sotaro Ikeda [:sotaro] from comment #8)
> When I set pref layout.animation.prerender.partial:true, the performance
> became a lot better.

And I also saw partially un-rendered animated images.

It seems not easy to address the partial pre-rendering of content with an animated transform. Instead, it seems simpler just relax, the size check in nsDisplayTransform::ShouldPrerenderTransformedContent() when the painting is for WebRender because of the following.

  • When webrender is enabled, gecko already allocates whole svg image at nsDisplayImage::CreateWebRenderCommands() and the image does not needs additional TextureClient allocation for async animation.
  • When webrender is enabled, content process normally does not allocate TextureClient except canvas and video.
  • When disiplay list does not support Webrender, part of the display list is rendered as blob image. and does not allocate TextureClient in content process.

With attachment 9035842 [details] [diff] [review], fps was improved from 3fps to 40fps.

Got profile with attachment 9035842 [details] [diff] [review]. Cpu usage in content process was decreased.

https://perfht.ml/2H5MB8l

attachment 9035842 [details] [diff] [review] had a problem. absoluteLimit was not big enough to trigger async animation on my laptop(P50 Win10).

Attachment #9035842 - Attachment is obsolete: true

attachment 9036507 [details] [diff] [review] relaxed more. With it, async animation happened more on my laptop(P50 Win10) and fps was around 46fps.

:hiro, do you know why attachment 9036507 [details] [diff] [review] could cause test_restyles.html test failure?

Flags: needinfo?(hikezoe)

It's hard to tell without the information which test actually fails. My best guess is that the last one [1].
Now the transform animation on a big element in the test runs on the compositor due to the relaxed restrictions?

[1] https://hg.mozilla.org/mozilla-central/file/e49161da5784/dom/animation/test/mozilla/file_restyles.html#l1857

Flags: needinfo?(hikezoe)

Ah, the test faild at "ok(!SpecialPowers.wrap(animation).isRunningOnCompositor)" in flush_layout_for_transform_animations(), because attachment 9036507 [details] [diff] [review] permits async animation of large frames.

Attachment #9036507 - Attachment is obsolete: true

Comment on attachment 9036804 [details] [diff] [review]
patch - Relax aysnc animation restriction with WebRender

:hiro, can you feedback to the patch?

Attachment #9036804 - Flags: feedback?(hikezoe)

Comment on attachment 9036804 [details] [diff] [review]
patch - Relax aysnc animation restriction with WebRender

I am afraid that I am not familiar with WebRender to understand the reasoning in comment 10. Could you please get feedbacks from WebRender guys?

One thing from animation perspective I am still wondering why some test cases in dom/animation/test/chrome/test_animation_performance_warning.html still pass with this patch. There are also test cases of transform animations on large elements.

Attachment #9036804 - Flags: feedback?(hikezoe)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #25)

One thing from animation perspective I am still wondering why some test cases in dom/animation/test/chrome/test_animation_performance_warning.html still pass with this patch. There are also test cases of transform animations on large elements.

I confirmed that mochitest chrome did not run with WebRender. Just enabling it caused several test failures including dom/canvas/test/chrome/test_drawWindow_widget_layers.html like the following.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66f310b8ae03324f64d1927613eb032b3c6608e9&selectedJob=222379155

Actually the following mochitests are not enabled on linux-qr-tests yet.

  • mochitest-browser-chrome
  • mochitest-chrome
  • mochitest-clipboard
  • mochitest-devtools-chrome

Created Bug 1520705 for enabling mochitest-chrome on WebRender.

Attachment #9036804 - Attachment is obsolete: true
Attachment #9037150 - Attachment description: Bug 1508522 - Relax aysnc animation restriction with WebRender → Bug 1508522 - Relax aysnc animation size restriction with WebRender
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d1b533985274
Relax aysnc animation size restriction with WebRender r=mattwoodrow
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Depends on: 1521205

For records, this change was backed out in bug 1587713, but the site in question should work fine since bug 1656418. It's much smoother with enabling layout.animation.prerender.partial at least on my Window 10 laptop on a local debug build.

The performance seems a lot better now. I still see graphical glitches and content that appears to be rendered at a resolution that is too low, but those should probably be treated as separate bugs.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: