Closed Bug 1383825 Opened 7 years ago Closed 7 years ago

Workday homepage load animation laggy

Categories

(Core :: Graphics, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jyc, Assigned: lsalzman)

References

Details

(Keywords: perf, Whiteboard: [gfx-noted])

Attachments

(1 file)

The expanding-circle animation Workday uses when clicking on one of the icons on the home screen (e.g. "Inbox" or "Personal Information") is very laggy: while in Safari and Chrome the circle smoothly animates to fill the screen, in Firefox (at least on macOS) only maybe five frames of the animation are shown.

I captured a profile of the page: https://perfht.ml/2v0Hr6D. I also tried to reproduce this by making my own expanding circle animation, but couldn't seem to trigger it.

There was a previous bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1250037) for this marked as resolved, but maybe the issue behind that was fixed.
Here is a recording of what it looks like: https://www.eqv.io/dl/workday-laggy.mov
Realized I should upload a recording of what it looks like in Safari for reference: http://eqv.io/dl/workday-smooth.mov
I think we're probably doing a very massive blur here and that's taking all of the time.
See Also: → 1250037
The expanding-circle animation is pretty nasty for me on my linux desktop, too.

Profile (with 0.1ms samples, as recommended for linux profiles): https://perfht.ml/2vTgNJq  (the expanding-circle animation is the middle ~third of that profile timeline.)

Definitely some long rasterize times there (longest one 358ms), with the rasterize time mostly spent in AlphaBoxBlur code (agreeing with Jeff's observation) and also skdraw::drawpaint.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> I think we're probably doing a very massive blur here and that's taking all
> of the time.

Yeah we don't do anything special for non-squared box shadows. I wasn't sure how Chrome was doing it so fast
See Also: → 1267584
Keywords: perf
Priority: -- → P3
Whiteboard: [gfx-noted]
Workday is rendering a box shadow animation by tweaking the scale in the DOM every frame. These scales can range up to 64 and beyond for high DPI screens, yielding insanely big box shadows of 10Kx10K and beyond.

It seems ordinarily, for a straight-forwardly animated nsDisplayTransform with scaling, we would have been able to rely on compositor scaling for this. However, since Workday is being evil here, we bypass that case and hit the fallback.

This patch institutes growth limits for that fallback case, to ensure we don't scale the underlying size of the layer beyond what is close to the current display size. When box shadows get much larger than this, they start taking so much time to render that successive frames grow in scale too fast for any inter-frame reuse to be possible.

With this, we avoid that and no longer get crushed by re-rendering titanic box shadows every single frame.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8890049 - Flags: review?(mstange)
Comment on attachment 8890049 [details] [diff] [review]
limit the growth of scaling for animated nsDisplayTransform in fallback case

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

I'd rather have Matt review this, because I was very involved in the writing of this patch.

Matt: We don't hit the usual displaySize limit because Workday is animating the transform using JavaScript instead of using a CSS animation.

The reason that the box shadow surface is so much bigger than the window on Workday is
 1. because we're rounding up the scale to the next power of two,
 2. because box-shadows want to cache the whole shadow even if only part of the shadow is drawn, and
 3. because the box-shadow surface is enlarged by the blur radius
Attachment #8890049 - Flags: review?(mstange) → review?(matt.woodrow)
Comment on attachment 8890049 [details] [diff] [review]
limit the growth of scaling for animated nsDisplayTransform in fallback case

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

::: layout/painting/FrameLayerBuilder.cpp
@@ +5405,5 @@
> +    displaySize.height = NSIntPixelsToAppUnits(widgetSize.height, p2a);
> +  } else {
> +    displaySize = presContext->GetVisibleArea().Size();
> +  }
> +  return displaySize;

You could just do returns directly from within the if branches if you wanted.
Attachment #8890049 - Flags: review?(matt.woodrow) → review+
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2948eef41706
limit the growth of scaling for animated nsDisplayTransform in fallback case. r=mattwoodrow
https://hg.mozilla.org/mozilla-central/rev/2948eef41706
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1407470
You need to log in before you can comment on or make changes to this bug.