CSS animations in SVG do not repaint when SVG is zoomed #9138
Conversation
|
NOTE - I didn't took the test case from Blink patch because in PR4727, @shallawa mentioned that this will lead to flaky issue due to timeout of 220. Although, in previous attempt, @shallawa did local testing and the patch was not fixing the issue but upon later local testing, I verified that this does fix the issue and it might be due to removing CSS border-width zoom quirk in this commit. Hence, I thought to do PR again after local testing without test-case. |
|
EWS run on previous version of this PR (hash d1e0959) |
webkit-ews-buildbot
added
the
merging-blocked
|
NOTE - 'windows' build failure error seems to suggest 'JavaScriptCoreSharedScripts.rule' exited with code 1.', so nothing seems to suggest that this could be because of this change. |
Ahmad-S792
force-pushed
the
fix199364-svg-render-image-css-animation-fix
branch
from
d1e0959
to
024ce43
Compare
Ahmad-S792
added
Images
|
EWS run on previous version of this PR (hash 024ce43)
|
Ahmad-S792
added
the
request-merge-queue
CSS animations in SVG do not repaint when SVG is zoomed https://bugs.webkit.org/show_bug.cgi?id=199364 rdar://problem/52459928 Reviewed by NOBODY (OOPS!). Partial Merge - https://src.chromium.org/viewvc/blink?view=revision&revision=174874 This patch changes the coordinate system used by RenderImage for invalidation rects to account for zoom. When an SVG image issues invalidations to its ImageResource clients, the invalidation rect does not contain zoom. This is due to how SVG images work internally: images are laid out without zoom so that relative sizes are resolved correctly; as a result the invalidation rects do not include zoom. This patch simply removes the zoom from the image size in RenderImage::repaintOrMarkForLayout and clarifies the comment. This change should not affect bitmap image invalidation because bitmaps (even animated gifs) do not use invalidation rects. From local build testing, I have verified that this does indeed fix an issue described on test case from the bug. * Source/WebCore/rendering/RenderImage.cpp: (RenderImage::repaintOrMarkForLayout): Update Comment and simplify logic to remove for 'zoom' factor * LayoutTests/svg/repaint/resources/animate-center.svg: Add Test Case Resource * LayoutTests/svg/repaint/image-animation-with-zoom.html: Add Test Case * LayoutTests/svg/repaint/image-animation-with-zoom-expected.txt: Add Test Case Expectation
Ahmad-S792
force-pushed
the
fix199364-svg-render-image-css-animation-fix
branch
from
024ce43
to
84bc61d
Compare
|
EWS run on current version of this PR (hash 84bc61d)
|
|
@smfr - I took test from Chromium patch and tried to apply two tricks to make it less flaky fo future: 1) Small Size of Image 2) Shorter set-timeout. Can you have suggest any other? |
@smfr - I am closing my PR. If you want me to reopen, happy to do so and do any changes, I will keep my branch for time being. I have uploaded video on bug as well that it does indeed fix bug. |
| @keyframes keyframes { | ||
| 0% { fill: pink } | ||
| 100% { fill: green } | ||
| } |
There was a problem hiding this comment.
If you use the same value from from and to, and provided it's different to the underlying value (not red), does this help with the flakiness? You'd also need to ensure that this modified version of the test also fails prior to your patch.
Ahmad-S792
removed
the
request-merge-queue
84bc61d