I've tried animating the points of a polygon but it does not work.
Created attachment 24093 [details] Testcase
We have to differentiate between two issues on your example. The first one is the wrong handling on missing 'from' attributes. The second one is the animation itself. I take this bug to handle the animation problem. See also W3C test: http://www.w3.org/Graphics/SVG/Test/20061213/svggen/animate-elem-34-t.svg
Created attachment 51861 [details] Patch
Attachment 51861 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/svg/SVGAnimateElement.cpp:149: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #4) > Attachment 51861 [details] did not pass style-queue: > > Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 > WebCore/svg/SVGAnimateElement.cpp:149: Tests for true/false, null/non-null, > and zero/non-zero should all be done without equality comparisons. > [readability/comparison_to_zero] [5] > Total errors found: 1 in 9 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. The value can be negative, so ... == 0 check is ok here.
Attachment 51861 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/1611047
Created attachment 51885 [details] Patch with build-fix for chromium
Attachment 51885 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/svg/SVGAnimateElement.cpp:149: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 51885 [details] Patch Nice patch, some small comments before landing: > Index: WebCore/svg/SVGPathSegList.h > =================================================================== > --- WebCore/svg/SVGPathSegList.h (revision 56693) > +++ WebCore/svg/SVGPathSegList.h (working copy) > @@ -45,6 +45,8 @@ namespace WebCore { > SVGPathSegList(const QualifiedName&); > }; > > + float blendFunc(float from, float to, float progress); Can you rename this function to something more descriptive? > Index: WebCore/svg/SVGPointList.cpp > + const FloatPoint from = fromList->getItem(n, ec); > + const FloatPoint to = toList->getItem(n, ec); Can't you omit const here? Excellent testcase, r=me.
Attachment 51885 [details] was posted by a committer and has review+, assigning to Dirk Schulze for commit.
Unsure of the status of this patch. It's been in pending-commit for over a month. Updates?
Landed in http://trac.webkit.org/changeset/56795.
The test case for this bug doesn't work in Chrome Version 21.0.1180.82 or Safari Version 6.0 (8536.25). Isn't the fix incorporated in the latest Safari & Chrome releases?
The code base changed a lot since the landing. Niko? Philip? Any ideas?
Hi guys. Do you have any updates about the last two comments? I tried it in Chrome 25.0.1364.99 (Mac OS) and Safari 6.0.2 (Mac OS). May be better to reopen the bug?
This is broken once again in latest Chrome (31.0.1650.57) and Safari 7. Works fine in Firefox 25.0.1.
Reopening since it's broken in Safari 7 on Mavericks.
I did a little poking around yesterday and found that the SVGPointsList gets animated correctly. It seems the issue is at the painting level, either the dirty rect is not correct or the animated path is not obtained.
It doesn't seem like the right list of points is retrieved when updatePathFromPolygonElement() is called. We always get the base list instead of the animated list.
Created attachment 217421 [details] Patch
Comment on attachment 217421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217421&action=review > Source/WebCore/rendering/svg/SVGPathData.cpp:77 > static void updatePathFromPolygonElement(SVGElement* element, Path& path) You could modernize this to take SVGElement& since you are here (the argument can't be null). > Source/WebCore/rendering/svg/SVGPathData.cpp:92 > static void updatePathFromPolylineElement(SVGElement* element, Path& path) Here too. > LayoutTests/svg/animations/polyline-set-expected.txt:6 > +layer at (0,0) size 800x600 > + RenderView at (0,0) size 800x600 > +layer at (0,0) size 800x600 > + RenderSVGRoot {svg} at (10,0) size 210x220 > + RenderSVGPath {polyline} at (10,0) size 210x220 [stroke={[type=SOLID] [color=#FF0000] [stroke width=20.00]}] [points="10 10 210 10 210 210 10 210"] > + RenderSVGPath {polyline} at (10,0) size 210x220 [stroke={[type=SOLID] [color=#008000] [stroke width=20.00]}] [points="10 10 110 10 110 110 10 110"] Could you make a reftest instead? Render tree dumps are fragile.
(In reply to comment #21) > (From update of attachment 217421 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217421&action=review > > > Source/WebCore/rendering/svg/SVGPathData.cpp:77 > > static void updatePathFromPolygonElement(SVGElement* element, Path& path) > > You could modernize this to take SVGElement& since you are here (the argument can't be null). > > > Source/WebCore/rendering/svg/SVGPathData.cpp:92 > > static void updatePathFromPolylineElement(SVGElement* element, Path& path) > > Here too. > > > LayoutTests/svg/animations/polyline-set-expected.txt:6 > > +layer at (0,0) size 800x600 > > + RenderView at (0,0) size 800x600 > > +layer at (0,0) size 800x600 > > + RenderSVGRoot {svg} at (10,0) size 210x220 > > + RenderSVGPath {polyline} at (10,0) size 210x220 [stroke={[type=SOLID] [color=#FF0000] [stroke width=20.00]}] [points="10 10 210 10 210 210 10 210"] > > + RenderSVGPath {polyline} at (10,0) size 210x220 [stroke={[type=SOLID] [color=#008000] [stroke width=20.00]}] [points="10 10 110 10 110 110 10 110"] > > Could you make a reftest instead? Render tree dumps are fragile. No problem, I'll make those changes as I land, thanks for the review.
Created attachment 217422 [details] Patch for landing
Comment on attachment 217422 [details] Patch for landing Clearing flags on attachment: 217422 Committed r159559: <http://trac.webkit.org/changeset/159559>
All reviewed patches have been landed. Closing bug.
Awesome, thanks everyone!
FYI, this new test is super-flaky for Blink. We merged your change in: https://codereview.chromium.org/79063004/ http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20Blink&showAllRuns=true&tests=fast%2Fjs%2Fglobal-constructors.html%2Cfast%2Fpreloader%2Fdocument-write-noscript.html%2Csvg%2Fanimations%2Fpolygon-set.svg%2Cwebaudio%2Foscillator-sine.html I suspect it may be flaky for WebKit as well.
(In reply to comment #27) > FYI, this new test is super-flaky for Blink. We merged your change in: > https://codereview.chromium.org/79063004/ > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20Blink&showAllRuns=true&tests=fast%2Fjs%2Fglobal-constructors.html%2Cfast%2Fpreloader%2Fdocument-write-noscript.html%2Csvg%2Fanimations%2Fpolygon-set.svg%2Cwebaudio%2Foscillator-sine.html > > I suspect it may be flaky for WebKit as well. Eric, you already had a fix for a similar bug with https://codereview.chromium.org/55893002/ (fix by Erik Dahlström). This just changed the code a little bit but should have the same results. In fact now I think you have dead code since pointsCurrentValue is not longer in use. I'm not sure if this test is flaky with WebKit, but I'll look into it.
(In reply to comment #28) > I'm not sure if this test is flaky with WebKit, but I'll look into it. I'm told these two tests have not caused flakiness since they landed.
*** Bug 126050 has been marked as a duplicate of this bug. ***