New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 700790 link

Starred by 6 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Trembling text during CSS animations when visibility is animated

Reported by findawa...@gmail.com, Mar 13 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce the problem:
1. Set a CSS animation onto an element using `@keyframes`
2. The animation involves `transform: scale()`
3. As the animation is running, unexpected trembling is produced.

[Bug example]
https://jsfiddle.net/af94sn38/3/

What is the expected behavior?
Implementing the same behavior with transition will not cause this problem.

[Correctly working example]
https://jsfiddle.net/rhfw0zxh/2/

What went wrong?
`@keyframe` animation with transform: scale() causes add fluctuation of the layer.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 56.0.2924.87  Channel: n/a
OS Version: 10.0
Flash Version: Shockwave Flash 24.0 r0
 
Just ran another test figured out that animating with CSS `visibility` property causes this. Removing all `visibility` declaration within `@keyframes` will not cause this same behavior.

Comment 2 by hdodda@chromium.org, Mar 14 2017

Cc: hdodda@chromium.org
Components: Blink>Animation
Labels: -Type-Bug -Pri-2 M-59 OS-Linux OS-Mac Pri-1 Type-Bug-Regression
Status: Untriaged (was: Unconfirmed)
Tested the issue on windows 7 & 10, ubuntu 14.04 and mac os 10.12.2 using chrome stable M57 #57.0.2987.98 and M59 #59.0.3040.0 issue is reproduced.

This is a regression issue broken in M49 .

Using the old bisect script providing the bisect results,
Good build:49.0.2573.0(Revision:361233).
Bad build: 49.0.2575.0(Revision:361776).

You are probably looking for a change made after 361528 (known good), but no later than 361566 (first known bad).

CHANGELOG URL:

 https://chromium.googlesource.com/chromium/src/+log/2ff76a86882308a81d17ac8bbeb57a95c3578a3e..98a0970e6672c6a0638f3d7c72ac4b5edf3aefe2

From the above cl , unable to find the correct suspect , could anyone help us to assign it to the concern owner.

Marking it as untraiged , for further inputs on this.

Thanks!


Components: -Blink>Animation Blink>Paint
This is working with tranistions, but not animations.  It is unlikely an animations bug, more likely related to Paint.  One possible explanation for the difference between animation and transitions is that one is running on the compositor and the other on the main thread.
Cc: danakj@chromium.org wkorman@chromium.org
Components: Internals>Compositing>Animation
Labels: -Type-Bug-Regression Hotlist-ThreadedRendering Type-Bug
Status: Available (was: Untriaged)
Summary: Trembling CSS animations when visibility is animated (was: Trembling CSS animations (transform))
My guess is that this is the interplay between compositor and main frame systems. Visibility animation is the triggering factor.

I think the change that caused the breakage is probably danakj@'s removal of the raster at scale 1 heuristic, and now we are re-rastering at various sizes in a jumpy fashion.
Labels: PaintTeamTriaged-20170315 BugSource-User

Comment 6 by danakj@chromium.org, Mar 15 2017

Cc: chrishtr@chromium.org vmp...@chromium.org
We filed bugs @ skia about raster at diff scales moving things around, but I'm not sure where they are. There is https://bugs.chromium.org/p/chromium/issues/detail?id=596382 which is marked Fixed but it was more general. It mentions this around https://bugs.chromium.org/p/chromium/issues/detail?id=596382#c43 but I don't see any CLs there that addressed it.. chrishtr@ or vmpstr@ do you have more memory of what happened?

Comment 7 by danakj@chromium.org, Mar 15 2017

Oh, and the jsfiddle here is text not images. So maybe it is https://bugs.chromium.org/p/chromium/issues/detail?id=521364 ?

Comment 8 by danakj@chromium.org, Mar 15 2017

Cc: trchen@chromium.org

Comment 9 by sunxd@chromium.org, Apr 12 2017

@trchen, is this related to crbug.com/521364
Nope, unlikely. I haven't research it but it feels like glyph hinting issue?

Comment 11 by enne@chromium.org, Apr 14 2017

Labels: -Pri-1 Pri-2
Owner: danakj@chromium.org
Dana, sorry to drop this on you but since you showed some knowledge here can you help this bug find a proper owner? In it's current state it is likely to languish (no new thoughts since mid-March).
I concur with schenney on the point that animating from visibility: visible; to visibility: visible; causes the misbehavior, while omitting visibility from the animation causes it to behave properly.  Opacity changes in the fiddle are an unnecessary distraction.
Summary: Trembling text during CSS animations when visibility is animated (was: Trembling CSS animations when visibility is animated)
I looked at it again, and if you color the background red and remove the text, the size does not seem to change smoothly. Maybe it changes 2 pixels at a time instead of 1? On the other hand, the div is auto-sized from the text content, I think, so maybe that's a red herring.

Does animating visible get it kicked off the compositor animation path?


> Does animating visible get it kicked off the compositor animation path?

Idk why it would but maybe?

Does --show-composited-layer-borders show it with a layer, maybe jumping in and out of compositing mode? 
It is composited while animating according to --show-composited-layer-borders. So much for that theory.
Simplified repro that removes fiddling with opacity attached.

tremble.html
1.1 KB View Download
Weird -- in the reduced test case the trembling scale-out is not instantiating a blink::CompositorAnimation, whereas the non-trembling scale-out and scale-in are.

To double check I moved the original fiddles to straight html and otherwise left them as-is and the original trembling fiddle also doesn't instantiate a CompositorAnimation.

But I agree that adding --show-composited-layer-borders makes it appear as if the trembling animation is composited w.r.t. separate layer coming into existence and moving along with it.

danakj@ I'd be happy to take this bug if you'd like.
tremble-orig.html
1.5 KB View Download
no-tremble-orig.html
980 bytes View Download
Animations incorporating visibility look like they're not considered an animatable value.

CompositorAnimations::IsCandidateForAnimationOnCompositor scrutinizes all properties on the keyframe effect. When it considers the visibility property we see nullptr for the animatable value and return false thus preventing placement of the animation on the compositor.

Call stack leading into this is:

Animation::PreCommit
Animation::MaybeStartAnimationOnCompositor
KeyframeEffectReadOnly::MaybeStartAnimationOnCompositor
KeyframeEffectReadOnly::IsCandidateForAnimationOnCompositor
CompositorAnimations::IsCandidateForAnimationOnCompositor

Looking into why GetAnimatableValue() for keyframe property for visibility is nullptr.

In any case, conceptually, a non-composited animation seems like it still shouldn't jitter, but that may end up being due to one of the bugs mentioned earlier in this discussion.
Cc: alancutter@chromium.org
Have to learn the purpose of AnimatableValue on Keyframe. I see this comment on Keyframe::PopulateAnimatableValue:

// FIXME: Remove this once CompositorAnimations no longer depends on
// AnimatableValues

Added in early 2015 in http://crrev.com/955583003. Adding author in case they have insight while I investigate further.
Owner: wkorman@chromium.org
Status: Assigned (was: Available)
Thanks!
KeyframeEffectModelBase::SnapshotAllCompositorKeyframes builds a cache of AnimatableValue in the keyframe properties via calling Keyframe::PopulateAnimatableValue on each keyframe.

However, it only iterates over keyframes for properties in CompositorAnimations::kCompositableProperties which includes seven specific properties and in particular does not include visibility.

It seems like we could add support for animating visibility from visible to hidden, or vice versa, by treating it functionally as the same as moving from/to opacity 0 to 1. I don't have much background on how we came up with the current set of seven properties. Filed http://crbug.com/713438 to investigate that.

Remaining for this bug, the question is now further qualified as, for non-composited animations, understand why the text jitters when scaled. I am expecting visibility is not actually relevant there, and is just a trigger that decomposites the animation.
I am familiar with the Keyframe related code. You are correct that the presence of "visibility" is pulling the animation off the compositor.

Having the animation run on the main thread is no different (from a graphical pipeline point of view) to having it done by Javascript.
This same problem exists with scripted animations: https://jsfiddle.net/af94sn38/5/

I suspect the problem comes down to us re-rasterising the text and doing device pixel alignment. In a composited animation we would simply be transforming the already rasterised layer.
Sounds like it might be related to issue 521364 after all then.
Cc: drott@chromium.org fmalita@chromium.org suzyh@chromium.org
Solving this bug requires that scale-animating text looks identical regardless of whether it's run as a composited animation.

Even if we solve issue 521364 which is mainly about how text looks at a specific static position on page I believe we will still raster text potentially quite differently anti-aliasing-wise at each different scale point. The only way to exactly match composited animation would be something like:

- ensure we raster the text just once
- ensure when we scale, we use scaling algorithm that matches what compositor would do and apply it to the previously rastered bitmap

As long as we continuously re-raster at each scale point when non-composited, then based on my understanding of font rastering internals, I don't think there's any guarantee or expectation warranted that the pixels we raster at each scale size will conform exactly to what we get when we run as a composited animation and scale down the rastered-once bitmap.

Am sure it's feasible to accomplish above in some manner but I don't have good sense of (1) what scope looks like, (2) how high priority this should really be, (3) whether there would be memory/computation resource overhead that might make us choose not to pursue. For (3) we should consider high-dpi devices, and SPv2 compositing work underway which likely means we'd only implement for SPv2.

Two next steps proposed:

- solicit input from font raster experts re: their take on my thought above (adding two to bug)

- solicit input from animation experts re: scope of work and priority (one already on bug, adding another)

If conclusion is that it's a good chunk of work and priority is not high, perhaps we should close this as WontFix. Or could gate it as follow-up once SPv2 ships.
Firefox handles scripted animation of transform a bit differently to us.
Here's an updated example to play with that includes a pause button: https://jsfiddle.net/af94sn38/6/

Firefox will smoothly scale the text without rerasterising until the transform is stable and there's a user interaction.

Chrome will reraster the text on every frame resulting in the jittery text behaviour.

Comment 27 by drott@chromium.org, Apr 21 2017

I think Alan's comment in #23 is helpful.

Looking at tremble.html I mostly see jumps in what seems like a rounded/pixel-snapped baseline position. There are smaller differences in advance widths, for example differences in distance between the e, x and t of "text", which might originate from hinting. 

When the animation starts we switch the antialiasing mode from RGB to grayscale, and we should probably switch off hinting as well, I am not sure whether that's done.

We do snap the baseline to pixels as having subpixel positioned baselines usually degrades text rendering quality, especially on non hi-dpi displays.

Text rerasterizing at different font sizes is definitely different from scaling down a pre-rasterized text, that's for reasons of antialiasing filerting, hinting, and other dynamic properties of OpenType layout. We won't get the exact same as out of bitmap scaling.

What might be worth trying here to remove one major source of "glitchiness" is to do subpixel baseline positioning during the animation. The remaining rasterization differences in advance widths or hinting might be neglible? If hinting still interferes and causes horizontal advance width jitter, we may need to ensure that it's off during the animation, but this may lead to artifacts at the beginning and end of the animation.

Comment 28 by drott@chromium.org, Apr 21 2017

...that is if we choose to keep an approach where there are multiple re-rasterizations.
In general it's not possible (or performant) to re-raster on every frame and have
it look the same. Instead we should avoid raster during a declarative animation.

If an animation is main-thread but the content is composited, then every frame
of the animation which does not actually need to invalidate the content shouldn't,
right? IOW if between two frames of animation, visibility doesn't change, then
don't re-raster.

Does a main-thread transform animation that uses percentage transform+a composited
layer re-raster on every frame? It shouldn't need to do that either I think.
I don't think we have a signal to tell cc::Layer that we're changing the transform but animating so don't reraster. It's treated like js changing a transform and assumes it should reraster. Unless something was added that I'm forgetting.
We do - the signal for "will-change: transform", which is on cc::Layer, could
be re-used
Owner: ----
Status: Available (was: Assigned)
Unassigning self from bugs that I don't expect to be able to get to soon in case someone else is able to pick them up.
Cc: flackr@chromium.org
Re #31, agreed I think we should be able to tell that the property is animating, but I think we also need to know the maximum scale from the animation so that we don't show a blurry layer but instead make the same raster decision as for composited animations. Can we pass over and use a raster scale as a layer param which is usually based on the effective transform but for animations uses the max scale throughout the animation?
Re 33: composited animations already have this concept. Can't it be reused?
Cc: sunxd@chromium.org
Perhaps, it looks like we'd have to set up enough animation data to get the animation scale, but we wouldn't want to actually run the animation since it's animated on main.
Re 31,30 it'd be nice to have a general solution for this case as well (JS animating) but we wouldn't know the scale range. I'd think you'd want to lock the scale at the size when will-change: transform was applied but I'm sure there are many sites for which this will result in blurry images.
Owner: flackr@chromium.org
Sounds like there is still a decision to make about how to best fix this, and that likely involves flackr@.  After that I expect it will be assigned to someone else.

Comment 38 by suzyh@chromium.org, Jun 13 2017

Cc: -suzyh@chromium.org
I'm seeing a very similar jitter when using transition with transform and scale. I believe it is related to this bug, but I can file a new one if it is outside of the scope of this one. 

Clicking the input element below causes the jitter to occur on the label as it moves up. 
https://material-components-web.appspot.com/text-field.html
Status: Assigned (was: Available)
Ping on this (happened to run across the bug). Is there a good solution?

Sign in to add a comment