RESOLVED CONFIGURATION CHANGED 50775
Jittering when animating a rotated image
https://bugs.webkit.org/show_bug.cgi?id=50775
Summary Jittering when animating a rotated image
Daniel Rice
Reported 2010-12-09 12:11:07 PST
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.
Attachments
Reproducible test case (2.39 KB, text/html)
2010-12-09 12:11 PST, Daniel Rice
no flags
Patch (3.08 KB, patch)
2011-02-21 11:07 PST, Stephen White
no flags
Patch (1.84 KB, patch)
2011-04-05 07:12 PDT, Stephen White
no flags
Matthew Delaney
Comment 1 2010-12-13 15:22:18 PST
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
Comment 2 2011-02-14 16:38:42 PST
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
Comment 3 2011-02-14 17:17:16 PST
This commit might be related: http://trac.webkit.org/changeset/15470
Stephen White
Comment 4 2011-02-21 08:37:48 PST
(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
Comment 5 2011-02-21 08:40:06 PST
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
Comment 6 2011-02-21 08:59:21 PST
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
Comment 7 2011-02-21 11:07:39 PST
Stephen White
Comment 8 2011-02-22 08:25:40 PST
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
Comment 9 2011-02-22 08:34:36 PST
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
Comment 10 2011-02-22 08:51:03 PST
(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 11 2011-02-22 11:40:42 PST
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
Comment 12 2011-02-23 04:16:32 PST
Landed as http://trac.webkit.org/changeset/79437. Leaving the bug open while we decide what to do on other platforms.
Vangelis Kokkevis
Comment 13 2011-02-25 18:07:46 PST
(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
Comment 14 2011-02-26 03:19:32 PST
(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
Comment 15 2011-03-07 18:39:19 PST
(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
Comment 16 2011-04-05 07:12:52 PDT
Kenneth Russell
Comment 17 2011-04-06 17:16:19 PDT
Comment on attachment 88224 [details] Patch Looks good to me as long as it's been tested.
Stephen White
Comment 18 2011-04-11 13:57:56 PDT
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
Comment 19 2022-07-18 13:56:53 PDT
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
Comment 20 2022-07-18 13:57:17 PDT
Note You need to log in before you can comment on or make changes to this bug.