1088498 - Jittery images on www.neontv.co.nz

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: log.txt

Matthew noticed this and made a reduced testcase: http://flim.org/~kinetik/neontv/test.html

Hovering over the image with the mouse makes it scale, and the scaling is noticeably jittery.

I debugged this a bit. On my machine, at least, we're not layerizing the transform, which is unfortunate and probably deserves its own bug. The transforms look correct, not jittery. I think the problem is in the layout code that controls image drawing.

I set a breakpoint here and logged 'params' over a series of those animations:
  aImage->Draw(ctx, params.size, params.region, imgIContainer::FRAME_CURRENT,
               aGraphicsFilter, ToMaybe(aSVGContext), aImageFlags);
The results are attached.

Table of the _11, _22, and _31 values of imageSpaceToDeviceSpace as the image grows in size:
1		1		-59
1.0103744268	1.0103744268	-61.3653693199
1.0147877932	1.0147877932	-62.3716168404
1.0194369555	1.0194369555	-64.431625843
1.0242179632	1.0242179632	-65.5216956139
1.0270940065	1.0270940065	-65.1774334908
1.0286402702	1.0286402702	-66.5299816132
1.0301533937 	1.0301533937	-66.874973774
1.0312439203	1.0312439203	-66.1236138344
1.0320833921	1.0320833921	-66.3150134087
1.0326309204	1.0326309204	-67.4398498535
1.0329128504	1.0329128504	-67.5041298866
1.0329998732	1.0329998732	-66.5239710808
Notice that the _31 value --- which corresponds to the x-coordinate of the left edge of the image --- does not decrease uniformly. It increases from -65.5216956139 to -65.1774334908 and it also increases from -66.5299816132 to -66.1236138344. That seems wrong! I'm pretty sure it's the cause of the jitter. I assume it's the way image snapping works.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Note that the CSS has
  background: url("previewTile_r1_c1.jpg") no-repeat scroll center 0px;
so the anchor point is at the center of the image ... or would be if the image width was even, which it isn't.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Er, the image width *is* even, of course.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The block container is 315px wide. The centered anchor point is then at 157.5px. In ComputeSnappedImageDrawingParameters, we snap the anchor point after transforming it to device pixels by the current transform; since the transform origin is the center of the element, the anchor point in device pixels should remain stationary, but due to numerical error the value jiggles around slightly less or slightly more than 157.5px, and therefore can round in different directions at different points during the animation.

It's not immediately clear to me how to handle this.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In my case, it's not being layerized because I'm using BasicLayers which is forcing layers to be inactive to preserve subpixel antialiasing.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Treat anchor offsets just less than 0.5 as 0.5 when rounding

Seth, what do you think of this? I'm not thrilled with it but I can't think of anything better right now.

Comment 6

[Emanuel Hoogeveen [:ehoogeveen]]

Probably orthogonal to this bug, but I noticed that BasePoint::Round [1] does floor(x + 0.5). This will incorrectly round the greatest number less than 0.5 up to 1, and will also give an incorrect result for some very large numbers (e.g. for double, (2^53)-1 should round to itself). SM ran into the former problem fairly recently in bug 1000606 - their fixed implementation is at [2].

[1] http://hg.mozilla.org/mozilla-central/annotate/9781037ac408/gfx/2d/BasePoint.h#l78
[2] http://hg.mozilla.org/mozilla-central/annotate/9781037ac408/js/src/jsmath.cpp#l836

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think that matters here.

Comment 8

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8510986 [details] [diff] [review]
Treat anchor offsets just less than 0.5 as 0.5 when rounding

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

I don't like this either, but I can't see a better solution.

With rounding there's always going to be a jump discontinuity that will negatively affect some content. FWIW I think there is a somewhat principled view of this change: we're adding a slight bias to move that jump discontinuity away from numbers that have a very high probability of being used by content, and towards numbers that are less likely.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d67de20b8612

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d67de20b8612