Closed Bug 1009693 Opened 10 years ago Closed 1 year ago

Animating left / top repaints due to differing fractional layer pixel alignment

Categories

(Core :: Graphics: Layers, defect)

defect

Tracking

()

RESOLVED INVALID

People

(Reporter: mstange, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

When animating the left or top property of an element with position:absolute, the intermediate animation steps are likely to have varying fractional parts. This means that, even though we layerize the element, it gets repainted on every animation step and does not benefit from the layerization.

How do we get around this problem for animated transforms? Can we do something similar here?

I've noticed this on several pages:
 - The rr presentation at http://rr-project.org/rr.html when changing slides
 - The navigation menu in the top left corner of http://www.w3.org/conf/2013sf/ during the initial slide-in animation
 - The vertically sliding phone that appears when you scroll to the very bottom of  http://www.mozilla.org/en-US/firefox/partners/
If we layerize and animate the left/top transform then the layer's position will be drawn at subpixel through OGL Layers. Can we make sure we don't invalidate the layer then and rely on that instead?
Flags: needinfo?(roc)
(In reply to Benoit Girard (:BenWa) from comment #1)
> If we layerize and animate the left/top transform then the layer's position
> will be drawn at subpixel through OGL Layers. Can we make sure we don't
> invalidate the layer then and rely on that instead?

Resampling the layer at a fractional offset is significantly worse visually than redrawing at a subpixel offset. Also, currently we snap translation coordinates in the compositor anyway. 

These are CSS transitions/animations so we can have the animation code snap to integers if we want. But, I'm pretty sure we don't hit this for animated transforms, so why are we hitting it for animated abs-pos top/left?
Flags: needinfo?(roc)
The problem with drawing at a (different) fractional position is that we'll have to resample and the quality (especially of text) will drop. Depending on the step amounts we might alternate between resampling and not which could look poor.

We have the same problem with scrolling, and we solve it there by trying to adjust requested scroll positions to the nearest value that would result in a whole-pixel shift.

I think for transforms we try snap to the nearest pixel for translations, and just give up for more complex ones.
If I write a CSS animation of top between X and Y I don’t expect to get some janky animation that kills my CPU to re-rasterize at fractional pixel offsets so that my text can look good. I don't see what's wrong with matching the behavior of CSS transform.

If the animation is very slow then maybe we want to draw with subpixel alignment.
I think nobody is saying that we shouldn't match CSS transforms.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> These are CSS transitions/animations so we can have the animation code snap
> to integers if we want. But, I'm pretty sure we don't hit this for animated
> transforms, so why are we hitting it for animated abs-pos top/left?

I don't know where exactly we snap transforms, but for animated abs-pos top/left I'm pretty sure that this is where the invalidation is coming from:
http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=7f5a8526b55a#1553
Transform establish a 'reference frame' in the display list, so content within the transform never changes coordinates.

We try snap the layer transform in DefaultComputeEffectiveTransforms
When creating a ThebesLayer, for an 'active geometry root' (the element being moved with .top will be this) positioned at X.Y, we create the ThebesLayer with a transform of X, and start drawing the content at 0.Y. When we change X, we can avoid redrawing, but changes to Y require repainting.

I guess for animated content (but probably not scrolled content) we could change this to set a ThebesLayer transform of X.Y and draw the content at 0.

I think there's some code that assumes the transform is always an integer, but I think it's fixable.
Attached file testcase
The upper two squares should match the lower two squares during the animation.
Attachment #8422634 - Attachment is patch: false
Attachment #8422634 - Attachment mime type: text/plain → text/html
Blocks: 982277
Blocks: 302909
Blocks: 754475
I guess there's no movement on this at the moment, but this would be a great fix to have for b2g 3.0 (I assume it's far too late for 2.2).

n?milan to get gfx team input, see if it fits in schedule anywhere. Failing that, quite tempted to take a look, I think this is the reason Twitter's pull-to-refresh is absolutely awful on b2g.
Flags: needinfo?(milan)
Chris, I'm sure nobody will complain if you follow up on this :)  Benoit is not around for a few months, and I'm not sure if Markus or Matt have time to take a look and when.  As far as the 2.2 timing, it would depend on the size of the fix and the impact it has on the Twitter and perhaps other apps, but I wouldn't quite slam the door on it yet :)
Flags: needinfo?(mstange)
Flags: needinfo?(milan)
Flags: needinfo?(matt.woodrow)
I probably won't have time for this before the end of Q1. But I'd really like to see it fixed!

While thinking some more about this, I think I found another problem that we'll probably encounter when we try to fix this: All the appunit-to-layerpixel snapping that we do in FrameLayerBuilder.cpp happens with respect to the reference frame's origin, i.e. we assume that our layer pixels are aligned with the reference frame's origin. Once we anchor the layer at the origin of the animated geometry root, this snapping might lead to rects that don't align  with layer pixels any more.
Flags: needinfo?(mstange)
If I have a positioned (position: relative / absolute) element, is it possible for things outside this element to interleave the contents of the positioned element?

If not, could we make positioned elements with an animated position become reference frames and have them create ContainerLayers for themselves?
Flags: needinfo?(roc)
Timothy told me that positioned elements only form stacking contexts when they have a z-index that's different from auto. That's unfortunate.
Flags: needinfo?(roc)
Seems to me a good way to fix this would be in the animation of 'top' and 'left': if the animation is fast enough, try to align 'left' and 'top' values with layer pixels. We do this for scroll offsets already.
Severity: normal → S3

Is this still a problem? The test case in comment 8 seems to look the same in every browser these days.

Flags: needinfo?(matt.woodrow) → needinfo?(mstange.moz)

I'd say it's not a problem anymore. These cases still invalidate, but that's ok because painting is now pretty fast in general.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(mstange.moz)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: