Bug 50775 – Jittering when animating a rotated image

• Component: Canvas
• Status: RESOLVED CONFIGURATION CHANGED

[Daniel Rice]

Created attachment 76102 [details]
Reproducible test case

The attached html/js file displays an animation of a rotating checkerboard image, along with a magnified view of each frame as it is drawn.  When the image is at a rotation angle near 45 degrees, it appears to shake.  Playing the same animation on Firefox gives a smooth appearance, where the antialiasing around the internal edges changes in a steady manner from frame to frame.

[Matthew Delaney]

Hi Daniel, I'm not seeing any shaking on my current build on my machine. Though, I'm not quite sure what you mean by shaking - could you maybe elaborate a little more. As a baseline, firefox 4 beta does the same as my Safari build.

Are the details you listed in the bug all correct? (i.e. Are you on Leopard, running a nightly?) If so, which nightly? Any other relevant info for me to try to reproduce would be great.

Also, if you could try the latest webkit nightly on a Leopard or Snow Leopard install and let me know if you still see the jittering that'd be great. Thanks!

[Dmitry Chestnykh]

Matthew, (In reply to comment #1)

Here's the similar bug I filed in Chromium bug tracker: http://code.google.com/p/chromium/issues/detail?id=67555

You can reproduce it by running WebKit's 32-bit build.

[Dmitry Chestnykh]

This commit might be related: http://trac.webkit.org/changeset/15470

[Stephen White]

(In reply to comment #3)
> This commit might be related: http://trac.webkit.org/changeset/15470

Looks like this all started back in http://trac.webkit.org/changeset/14739, to fix "Pixel cracks in weather widget at 1.83 scaling" (in Radar, unfortunately, so I have no way to repro this).  I have noticed similar problems with CoreGraphics image scaling before, but I don't think these apply to Skia, but I could be wrong.

I'm tempted to

[Stephen White]

I'm tempted to say let's just rip it out on the Skia path (which is what Android have done, with no apparent ill effects).  But obviously that won't help Chrome/Mac, nor Safari.

[Dmitry Chestnykh]

Moreover, change 14739 seem to no longer fix what it supposed to fix, at least on 10.6.6. The weather widget is broken at scale factor 1.5: http://i.imgur.com/IiT7S.png

[Stephen White]

Created attachment 83193 [details]
Patch

[Stephen White]

Some more data: the test case in https://bugs.webkit.org/show_bug.cgi?id=15720 (non-canvas draws, which do not yet have this coordinate adjustment) shows cracks in Chrome/Mac and Safari, but not in Chrome/Win.  This confirms that this is a CoreGraphics-specific issue, and doesn't apply to Skia.  So I think it is safe to land the attached patch.

For the CoreGraphics path, how about this for a compromise:  If the matrix contains a rotation, do not snap to integral pixel boundaries (ie., return the original coordinates).  Then there would be no cracks in the common case (scales, translations) such as that in the OSX weather widget, and rotations wouldn't show jitter, although they would show cracks if multiple parallel images were rotated together.  The latter seems like a less common case compared to the others.

[Dmitry Chestnykh]

Stephen, but the compromise won't fix the zooming jitter (as this test case: http://code.google.com/p/chromium/issues/detail?id=67555#c18), will it?

[Stephen White]

(In reply to comment #9)
> Stephen, but the compromise won't fix the zooming jitter (as this test case: http://code.google.com/p/chromium/issues/detail?id=67555#c18), will it?

True.  Zooming would still jitter.

[Kenneth Russell]

Comment on attachment 83193 [details]
Patch

Looks fine to me. If this is the solution desired, we could always #ifdef the similar code in GraphicsContextCG.cpp with !PLATFORM(CHROMIUM).

[Stephen White]

Landed as http://trac.webkit.org/changeset/79437.  Leaving the bug open while we decide what to do on other platforms.

[Vangelis Kokkevis]

(In reply to comment #10)
> (In reply to comment #9)
> > Stephen, but the compromise won't fix the zooming jitter (as this test case: http://code.google.com/p/chromium/issues/detail?id=67555#c18), will it?
> 
> True.  Zooming would still jitter.

I'm confused.. why would zooming still jitter if you're only only apply the fix when the matrix has a rotation?  Do you mean if it has both rotation and scale? 

I actually applied a patch locally to eliminate the code in roundToDevicePixels and the zooming example from the crbug seems nice and smooth.

[Dmitry Chestnykh]

(In reply to comment #13)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Stephen, but the compromise won't fix the zooming jitter (as this test case: http://code.google.com/p/chromium/issues/detail?id=67555#c18), will it?
> > 
> > True.  Zooming would still jitter.
> 
> I'm confused.. why would zooming still jitter if you're only only apply the fix when the matrix has a rotation?  Do you mean if it has both rotation and scale? 

The proposed fix for CG was to _skip_ roundToDevicePixels when the matrix has rotation. But in case of zooming this will not work (as roundToDevicePixels still applies in this case).
 
> I actually applied a patch locally to eliminate the code in roundToDevicePixels and the zooming example from the crbug seems nice and smooth.

That's because you eliminated roundToDevicePixels, and that's the goal. However, there seem to be the case that eliminating roundToDevicePixels completely in CoreGraphics version introduces pixel cracks.

[Vangelis Kokkevis]

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #10)
> > > (In reply to comment #9)
> > > > Stephen, but the compromise won't fix the zooming jitter (as this test case: http://code.google.com/p/chromium/issues/detail?id=67555#c18), will it?
> > > 
> > > True.  Zooming would still jitter.
> > 
> > I'm confused.. why would zooming still jitter if you're only only apply the fix when the matrix has a rotation?  Do you mean if it has both rotation and scale? 
> 
> The proposed fix for CG was to _skip_ roundToDevicePixels when the matrix has rotation. But in case of zooming this will not work (as roundToDevicePixels still applies in this case).
> 
> > I actually applied a patch locally to eliminate the code in roundToDevicePixels and the zooming example from the crbug seems nice and smooth.
> 
> That's because you eliminated roundToDevicePixels, and that's the goal. However, there seem to be the case that eliminating roundToDevicePixels completely in CoreGraphics version introduces pixel cracks.

Oh, I see.

Is there any way to test that cracks are still an issue for the CG path? The fix/workaround: http://trac.webkit.org/changeset/14739   was checked in 5 years ago.  Anybody have access to the original rdar repro case?

[Stephen White]

Created attachment 88224 [details]
Patch

[Kenneth Russell]

Comment on attachment 88224 [details]
Patch

Looks good to me as long as it's been tested.

[Stephen White]

Landed the second patch as r83490.  Leaving the bug open in case other ports (e.g., Safari) want to do something about this.

[Brent Fulgham]

Safari, Chrome, and Firefox all agree on rendering for this test case. I don't believe there is any remaining compatibility issue.

[Radar WebKit Bug Importer]

<rdar://problem/97218274>