1642079 - CSS `transition: scale` causes jitter and not smooth

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[nayinain]

Attachment: testcase.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

1. Open the testcase.html

Actual results:

jitter and not smooth.

Expected results:

---

Sorry for my bad English.

Comment 1

[nayinain]

Attachment: Capture 2020-05-30.mp4

Comment 2

[nayinain]

I used mozregression to find the regression bug that causing the jitter behavior is Bug 1635406.
But the issue of not smooth scaling transition seems to exist all the time.

Regression window:

2020-05-30T14:56:27.285000: INFO : application_version: 78.0a1
2020-05-30T14:56:27.285000: INFO : platform_buildid: 20200527192654
2020-05-30T14:56:27.285000: INFO : platform_changeset: 156d42f01488fb24914199fe4670c70cfffe4323
2020-05-30T14:56:27.285000: INFO : platform_repository: https://hg.mozilla.org/integration/autoland
2020-05-30T14:56:27.285000: INFO : platform_version: 78.0a1
2020-05-30T14:56:34.074000: INFO : Narrowed integration regression window from [820ae8ff, a8499f9c] (3 builds) to [156d42f0, a8499f9c] (2 builds) (~1 steps left)
2020-05-30T14:56:34.081000: DEBUG : Starting merge handling...
2020-05-30T14:56:34.081000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a8499f9c0d024b7287f3e0cb13bc56f5f3f57753&full=1
2020-05-30T14:56:34.081000: DEBUG : redo: attempt 1/3
2020-05-30T14:56:34.081000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=a8499f9c0d024b7287f3e0cb13bc56f5f3f57753&full=1',), kwargs: {}, attempt #1
2020-05-30T14:56:34.084000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2020-05-30T14:56:36.354000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=a8499f9c0d024b7287f3e0cb13bc56f5f3f57753&full=1 HTTP/1.1" 200 None
2020-05-30T14:56:36.400000: DEBUG : Found commit message:
Bug 1635406 - Snap reference frame transforms if animated or zooms. r=aosmond

Bug 1620014 attempted to fix an issue where an animated visual
viewport offset (eg due to scrolling while being zoomed in) was
causing the fractional offset of a descendant scroll frame's content
transform to change, causing too much picture cache invalidation....

Differential Revision: https://phabricator.services.mozilla.com/D76298

2020-05-30T14:56:36.400000: DEBUG : Did not find a branch, checking all integration branches
2020-05-30T14:56:36.401000: INFO : The bisection is done.
2020-05-30T14:56:36.404000: INFO : Stopped

Comment 3

[Darkspirit]

Thanks for the report!
last good: mozregression --repo autoland --launch 156d42f01488fb24914199fe4670c70cfffe4323 --pref gfx.webrender.all:true -a https://bug1642079.bmoattachments.org/attachment.cgi?id=9152930
first bad: mozregression --repo autoland --launch a8499f9c0d024b7287f3e0cb13bc56f5f3f57753 --pref gfx.webrender.all:true -a https://bug1642079.bmoattachments.org/attachment.cgi?id=9152930

Comment 4

[Darkspirit]

(In reply to nayinain from comment #2)

But the issue of not smooth scaling transition seems to exist all the time.

KDE, X11, Debian Testing, Macbook Pro, Intel Iris 6100 (Broadwell GT3)
For me, "last good" looks as smooth as Chrome in the screencast, but the reduced testcase doesn't seem to work with Chrome Dev as it doesn't support scale yet: https://developer.mozilla.org/en-US/docs/Web/CSS/scale

Comment 5

[nayinain]

Attachment: testcase2.html

(In reply to Jan Andre Ikenmeyer [:darkspirit] from comment #4)

(In reply to nayinain from comment #2)

But the issue of not smooth scaling transition seems to exist all the time.

KDE, X11, Debian Testing, Macbook Pro, Intel Iris 6100 (Broadwell GT3)
For me, "last good" looks as smooth as Chrome in the screencast, but the reduced testcase doesn't seem to work with Chrome Dev as it doesn't support scale yet: https://developer.mozilla.org/en-US/docs/Web/CSS/scale

Sorry, I uploaded the wrong test case (In addition: scale is another usage of transform: scale)，I have uploaded the testcase2.html.

Comment 6

[Darkspirit]

Thanks! Confirmed, Chrome Dev is even smoother than "last good".

Comment 7

[Jamie Nicol [:jnicol]]

Ugh, I'm beginning to understand your pain now Andrew!

So the fact that we cause this regression makes me think that maybe we never want to snap animations? It's inevitably going to cause jumpy animations like this.

This endeavour started out in bug 1620014, but snapping animations wasn't the original intention. The original intention was just to avoid invalidating the picture cache when scrolling (due to the subpixel offset changing). Maybe we should tackle this at the source, and ensure that we only ever scroll in whole pixel increments? This would avoid webrender having to snap things, and might solve bug 1642308 too. I assume there's probably terrible unforeseen consequences to that approach too, though

Comment 8

[Jamie Nicol [:jnicol]]

As a short term fix, I think I should modify the patch to only apply to pinch-zoom transforms rather than all animations. There might still be some jittery snapping when zooming on android, but that's probably an okay compromise until we find a better solution

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

Yes, nothing but unintended consequences with every snap. That said, I thought we already were snapping the scroll offsets, at least parent_accumulated_scroll_offset, whenever it was merged into a transform. Certainly it would probably be good if we were consistent with any other scroll offsets. Often these inconsistencies cause grief in the first place.

Okay with changing it to only apply to pinch-zoom transforms; we already talked about that as a more conservative change :).

Comment 10

[Jamie Nicol [:jnicol]]

Yes, in bug 1589669 we started snapping parent_accumulated_scroll_offset and everything was great. Until we landed bug 1609002, which means there are now 2 types of scroll offsets: the layout viewport offset and the visual viewport offset. The latter of which is sent to webrender as part of the transform animation used for pinch zooming, and is what I'm really struggling to snap correctly.

Comment 11

[Jamie Nicol [:jnicol]]

Attachment: Bug 1642079 - Only snap animated transforms for zoom reference frames? r?aosmond

Bug 1635406 made it so that webrender snaps the offsets of animated
transforms before accumulating them in to the reference frame
transform. Unfortunately, however, this causes jittery animations. The
original intention was just to snap the visual viewport offset when
scrolling, to avoid excessive picture cache invalidation.

To avoid this, make it so that we only snap for reference frames of
kind ReferenceFrameKind::Zoom. This will mean that most animations are
unaffected. There may however still be some jitter when zooming, but
this is outweighed by the benefit of not invalidating picture cache
tiles every frame when scrolling.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/439f76e56dda
Only snap animated transforms for zoom reference frames? r=aosmond

Comment 13

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/439f76e56dda

Comment 14

[Jamie Nicol [:jnicol]]

I'm going to let this bake on nightly for a couple days then request uplift to beta

Comment 15

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #26107 (as of Tue, 02 Jun 2020 11:10:10 GMT) ==

Improvements:

9% tscrollx windows10-64-shippable-qr opt e10s stylo 0.80 -> 0.73
3% tp5o_scroll linux64-shippable-qr opt e10s stylo 2.32 -> 2.25

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26107

Comment 16

[Jamie Nicol [:jnicol]]

Comment on attachment 9153162 [details]
Bug 1642079 - Only snap animated transforms for zoom reference frames? r?aosmond

Beta/Release Uplift Approval Request

• User impact if declined: Jittery animations on all platforms, tscrollx performance regression on windows
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Effectively just reverts to previous behaviour prior to bug 1635406 which caused this regression.
• String changes made/needed:

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9153162 [details]
Bug 1642079 - Only snap animated transforms for zoom reference frames? r?aosmond

webrender regression fix, approved for 78.0b3

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6bef5b173976

Comment 19

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly build 79.0a1 (2020-06-05) as well as Beta 78.0b3.