Status

()

defect
P3
normal
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
mozilla66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

()

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

6 months ago
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

Comment 1

6 months ago
(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?
Assignee

Comment 3

5 months ago
I am going to take a look.
Assignee

Updated

5 months ago
Assignee: nobody → sotaro.ikeda.g
Assignee

Comment 4

5 months ago
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.
Assignee

Comment 5

5 months ago
(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.
Assignee

Comment 7

5 months ago
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.
Assignee

Comment 8

5 months ago
(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.
Assignee

Comment 9

5 months ago
(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.
Assignee

Comment 10

4 months ago

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.
Assignee

Comment 12

4 months ago

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

Assignee

Comment 13

4 months ago

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

https://perfht.ml/2H5MB8l

Assignee

Comment 15

4 months ago

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

Assignee

Comment 16

4 months ago
Attachment #9035842 - Attachment is obsolete: true
Assignee

Comment 17

4 months ago

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

Assignee

Comment 19

4 months ago

: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)
Assignee

Comment 21

4 months ago

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.

Assignee

Comment 22

4 months ago
Attachment #9036507 - Attachment is obsolete: true
Assignee

Comment 24

4 months ago

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)
Assignee

Comment 26

4 months ago

(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

Assignee

Comment 27

4 months ago

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

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

Comment 28

4 months ago

Created Bug 1520705 for enabling mochitest-chrome on WebRender.

Assignee

Updated

4 months ago
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

Comment 30

4 months ago
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d1b533985274
Relax aysnc animation size restriction with WebRender r=mattwoodrow

Comment 31

4 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Updated

4 months ago
Depends on: 1521205
You need to log in before you can comment on or make changes to this bug.