Closed Bug 633097 Opened 9 years ago Closed 5 years ago

Transitioned objects jitter with scaling transform

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jk1700, Assigned: lsalzman)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(7 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre
Build Identifier: 

Slow transitions using "scale" transform make transitioned object jittering


Reproducible: Always

Steps to Reproduce:
1. Open testcase
2. Click mouse

Actual Results:  
Div's border and text start to transform and there is some kind of trembling


Expected Results:  
Transision should be smooth


Chrome looks worse than Fx, but Opera is perfeclty smooth
Version: unspecified → Trunk
Attached file testcase
Keywords: testcase
I confirm this is visible in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Confirmed on Mozilla/5.0 (Windows NT 6.1; rv:2.0b13pre) Gecko/20110317 Firefox/4.0b13pre
Blocks: 770810
Hey Joe, not sure you're the right person for this but I'm hoping to get a bit more visibility into this. I'm trying to use this but the jittering is bad and makes this unusable in Firefox. Thanks!
Status: UNCONFIRMED → NEW
Ever confirmed: true
I think this is the same as bug 663776.
FWIW, despite Bug 663776 yet not as smooth as Opera (with HWA on there).
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121230 Firefox/20.0 ID:20121230030830
I noticed something similar (I think) on a website using `transform`. Attached two videos of the observed behavior in firefox and chrome where it looks fine.

The spinning globe in the top right corner is jittering.
Site: http://trifork.se/jobs 

Firefox: 37.0.1 also have the same problem in nightly
(In reply to eric from comment #7)
> I noticed something similar (I think) on a website using `transform`.
> Attached two videos of the observed behavior in firefox and chrome where it
> looks fine.
> 
> The spinning globe in the top right corner is jittering.
> Site: http://trifork.se/jobs 
> 
> Firefox: 37.0.1 also have the same problem in nightly

Forgot to mention that I'm running linux so not just a windows issue.
(In reply to eric from comment #7)
> I noticed something similar (I think) on a website using `transform`.
> Attached two videos of the observed behavior in firefox and chrome where it
> looks fine.
> 
> The spinning globe in the top right corner is jittering.
> Site: http://trifork.se/jobs 
> 
> Firefox: 37.0.1 also have the same problem in nightly

This particular site appears to be fixed for me on Windows 8.1 and Firefox 40.

The original test case still shows some jumpiness (presumably as we snap to pixel boundaries) but it looks a lot better than Chrome.
(In reply to Brian Birtles (:birtles) from comment #11)
> (In reply to eric from comment #7)
> > I noticed something similar (I think) on a website using `transform`.
> > Attached two videos of the observed behavior in firefox and chrome where it
> > looks fine.
> > 
> > The spinning globe in the top right corner is jittering.
> > Site: http://trifork.se/jobs 
> > 
> > Firefox: 37.0.1 also have the same problem in nightly
> 
> This particular site appears to be fixed for me on Windows 8.1 and Firefox
> 40.

I'm still seeing the issue on ff 38.0.5 and on nightly. Fedora 22, Linux 4.0.4
(In reply to eric from comment #12)
> I'm still seeing the issue on ff 38.0.5 and on nightly. Fedora 22, Linux
> 4.0.4

Can you try with Nightly (41) or Aurora/Dev Edition (40)?
(In reply to Brian Birtles (:birtles) from comment #13)
> (In reply to eric from comment #12)
> > I'm still seeing the issue on ff 38.0.5 and on nightly. Fedora 22, Linux
> > 4.0.4
> 
> Can you try with Nightly (41) or Aurora/Dev Edition (40)?

I did :)

> > I'm still seeing the issue on ff 38.0.5 and on nightly.

So yeah I'm still seeing this on nightly 41.0a1 on linux.
I boiled this test case down from the trifork website.

To reproduce, basic layers must be used, so layers acceleration must be disabled. So far, I have found Cairo, Skia, and CG backends to have problem with this, but I haven't tested on other platforms.

It uses an animated rotate transform to spin the letter Q, over a transparent background. There is also some text after the letter Q that helps to force the layer builder code to decide to flatten it.

When the animated text gets flattened, the transform ultimately goes straight to our content backend, which can't deal with the generated subpixel positioned glyphs.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
As discussed with Matt Woodrow and Markus Stange, a temporary solution for now is to just disable flattening when animated text appears over a transparent background.

This has a side-effect of, when basic layers is used, possibly disabling subpixel AA over transparent backgrounds when flattening is disabled.

But as discussed, the general desire (as evident in bug 1097464) is to just get rid of flattening eventually, this will impact fewer cases for now than just removing flattening entirely.
Attachment #8621670 - Flags: review?(matt.woodrow)
Attachment #8621670 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8621670 [details] [diff] [review]
Fix jittering animated text by disallowing flattening into a container layer that has animated text.

Found a bug - the condition accidentally checks for only alpha or transform, not both. Working on fix.
Attachment #8621670 - Flags: review+ → review-
Depends on: 771367
The transform check had to be slightly widened in scope due to the fact that the CONTENT_COMPONENT_ALPHA flag will never get set if the transform has non-integer translations, which causes subpixel AA to be disabled, and thus no component alpha content anymore...

By itself this fixes the non-pseudo-element test case. The pseudo-element test case requires the fix from bug 771367 to fully fix the reported issue.
Attachment #8621670 - Attachment is obsolete: true
Attachment #8623165 - Flags: review?(matt.woodrow)
Comment on attachment 8623165 [details] [diff] [review]
Fix jittering animated text by disallowing flattening into a container layer that has animated text.

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

::: layout/base/FrameLayerBuilder.cpp
@@ +4624,5 @@
> +      // Ensure animated text does not get flattened, even if it forces other
> +      // content in the container to be layerized. The content backend might
> +      // not support subpixel positioning of text that animated transforms can
> +      // generate. bug 633097
> +      if (contentFlags & Layer::CONTENT_MAY_CHANGE_TRANSFORM) {

CONTENT_MAY_CHANGE_TRANSFORM is a flag that is set when we've 'pre-rendered' a transform (that is, drawn the entirety of the transformed content even if it isn't all currently visible) to indicate that we can use an optimization where we modify the transform on the layer without going through layer building/painting again.

Although this will often overlap will animated transforms, it isn't always the same thing.

We should instead define a new layer flag (HAS_ANIMATED_TRANSFORM?), and set it (if applicable) in nsDisplayTransform::BuildLayer.
As discussed, this version of the patch no longer relies upon the CONTENT_MAY_CHANGE_TRANSFORM flag, and instead adds a new layer flag to independently track if there is any component alpha content in the layer that should not be flattened. This version also correctly sets it only for component alpha content inside an animated transform.
Attachment #8623165 - Attachment is obsolete: true
Attachment #8623165 - Flags: review?(matt.woodrow)
Attachment #8624702 - Flags: review?(matt.woodrow)
Comment on attachment 8624702 [details] [diff] [review]
Fix jittering animated text by disallowing flattening into a container layer that has animated text.

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

Sorry about the slow response!

::: layout/base/FrameLayerBuilder.cpp
@@ +479,5 @@
>    bool mOpaqueForAnimatedGeometryRootParent;
> +  /**
> +   * Set if there is text visible in the layer that must avoid being flattened.
> +   */
> +  bool mForceComponentAlpha;

How about mDisallowFlattening or similar? We sort of abuse 'component alpha' as a name, as it's really just an implementation detail (for some layer backends) of how we handle transparent layers with text.

@@ +3134,5 @@
>    } else if (data->mNeedComponentAlpha && !hidpi) {
>      flags |= Layer::CONTENT_COMPONENT_ALPHA;
>    }
> +  if (data->mForceComponentAlpha) {
> +    flags |= Layer::CONTENT_FORCE_COMPONENT_ALPHA;

Rather than putting this into the Layers API, let's just add a bool to NewLayerEntry which is accessible from ::Finish().

@@ +4560,5 @@
>      if (!layer->GetVisibleRegion().IsEmpty()) {
>        textContentFlags |=
>          layer->GetContentFlags() & (Layer::CONTENT_COMPONENT_ALPHA |
> +                                    Layer::CONTENT_COMPONENT_ALPHA_DESCENDANT |
> +                                    Layer::CONTENT_FORCE_COMPONENT_ALPHA);

We'll want an extra outparam for this if it's a bool on NewLayerEntry rather than a layer flag.
(In reply to Matt Woodrow (:mattwoodrow) from comment #22)
 
> @@ +3134,5 @@
> >    } else if (data->mNeedComponentAlpha && !hidpi) {
> >      flags |= Layer::CONTENT_COMPONENT_ALPHA;
> >    }
> > +  if (data->mForceComponentAlpha) {
> > +    flags |= Layer::CONTENT_FORCE_COMPONENT_ALPHA;
> 
> Rather than putting this into the Layers API, let's just add a bool to
> NewLayerEntry which is accessible from ::Finish().

We looked at this, and it doesn't work unfortunately.

The value gets propagated within the transform ContainerLayer, but doesn't have a way to propagate up the the outer container around the transform (through BuildLayer).

Sticking with the layer flag fixes this, so we'll just go with that and rip it out again when we remove flattening.
Comment on attachment 8624702 [details] [diff] [review]
Fix jittering animated text by disallowing flattening into a container layer that has animated text.

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

r=me with renaming to 'disallow flattening' and a clear comment on the Layer flag explaining that it's a temporary thing and for FrameLayerBuilder internal use only.
Attachment #8624702 - Flags: review?(matt.woodrow) → review+
Did rename of layer flag and updated content to note that flag is for temporary FrameLayerBuilder usage only

Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2742dfaf87ec
Attachment #8626770 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c36a4d16e798
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
I did a quick check on this using Firefox 41 Beta 2 (BuildID=20150817163452), on Windows 7 x64, and only one testcase appears to work as expected:
- comment 1 
 * https://bugzilla.mozilla.org/attachment.cgi?id=511303 - still doesn't look smooth
- comment 15 
 * https://bugzilla.mozilla.org/attachment.cgi?id=8621666 - looks OK now (layers acceleration disabled)
- comment 18 
 * https://bugzilla.mozilla.org/attachment.cgi?id=8621701 - still jitters (layers acceleration disabled)

Is all this OK here? Was only the second testcase supposed to work?
Flags: needinfo?(lsalzman)
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #28)
> I did a quick check on this using Firefox 41 Beta 2
> (BuildID=20150817163452), on Windows 7 x64, and only one testcase appears to
> work as expected:
> - comment 1 
>  * https://bugzilla.mozilla.org/attachment.cgi?id=511303 - still doesn't
> look smooth
> - comment 15 
>  * https://bugzilla.mozilla.org/attachment.cgi?id=8621666 - looks OK now
> (layers acceleration disabled)
> - comment 18 
>  * https://bugzilla.mozilla.org/attachment.cgi?id=8621701 - still jitters
> (layers acceleration disabled)
> 
> Is all this OK here? Was only the second testcase supposed to work?

The fix works by layerizing the animation. So, if you disable layerizing it, then you're disabling the fix. 

Architecturally, there is no way around this since if the text must be rendered each frame with different subpixel offsets, you will get jitter from any text backend that can't render to subpixel offsets. So I would imagine this falls under WONTFIX.
Flags: needinfo?(lsalzman)
You need to log in before you can comment on or make changes to this bug.