Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SVG] animateMotion accumulate doesn't work properly with rotate: auto / auto-reverse #14149

Merged

Conversation

Ahmad-S792
Copy link
Contributor

@Ahmad-S792 Ahmad-S792 commented May 20, 2023

b3b7524

[SVG] animateMotion accumulate doesn't work properly with rotate: auto / auto-reverse

https://bugs.webkit.org/show_bug.cgi?id=256928
rdar://problem/109489241

Reviewed by Simon Fraser.

This patch aligns WebKit behavior with Gecko / Firefox and Blink / Chromium.

Merge: https://src.chromium.org/viewvc/blink?view=revision&;revision=154375

We need not accumulate the angle for animation as we have a method to get normal angle at a given length on the path.
So, only position needs to be accumulated. Also made some changes to simplify the logic for accumulation.

* Source/WebCore/svg/SVGAnimateMotionElement.cpp:
(SVGAnimateMotionElement::buildTransformForProgress): Remove 'angle' accumulation from here
(SVGAnimateMotionElement::calculateAnimatedValue): Move here
* LayoutTests/svg/animations/animateMotion-accumulate-1a.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1a-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-1b.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1b-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-1c.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1c-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-2a.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-2a-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-2b.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-2b-expected.svg: Add Test Case Expectation

All tests have slight pixel tolerances to accommodate for platform differences.

Canonical link: https://commits.webkit.org/264595@main

9460ac3

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac   πŸ›  wpe   πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ›  gtk
  πŸ§ͺ ios-wk2-wpt   πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk2   πŸ§ͺ api-gtk
βœ… πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2
  πŸ›  tv-sim
  πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge   πŸ›  watch-sim

@Ahmad-S792 Ahmad-S792 added the SVG For bugs in the SVG implementation. label May 20, 2023
@Ahmad-S792 Ahmad-S792 self-assigned this May 20, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@Ahmad-S792 Ahmad-S792 force-pushed the fix256928-svg-animation-case branch from 91ad570 to 3d35f27 Compare May 21, 2023 09:42
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@Ahmad-S792 Ahmad-S792 marked this pull request as ready for review May 21, 2023 09:43
@Ahmad-S792 Ahmad-S792 force-pushed the fix256928-svg-animation-case branch from 3d35f27 to b784c86 Compare May 21, 2023 09:50
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@Ahmad-S792 Ahmad-S792 added request-merge-queue Request a pull request to be added to merge-queue once ready and removed merging-blocked Applied to prevent a change from being merged labels May 21, 2023
@Ahmad-S792 Ahmad-S792 requested a review from rwlbuis May 26, 2023 10:00
@Ahmad-S792 Ahmad-S792 force-pushed the fix256928-svg-animation-case branch from b784c86 to 9460ac3 Compare May 26, 2023 17:49
@Ahmad-S792 Ahmad-S792 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels May 26, 2023
…o / auto-reverse

https://bugs.webkit.org/show_bug.cgi?id=256928
rdar://problem/109489241

Reviewed by Simon Fraser.

This patch aligns WebKit behavior with Gecko / Firefox and Blink / Chromium.

Merge: https://src.chromium.org/viewvc/blink?view=revision&revision=154375

We need not accumulate the angle for animation as we have a method to get normal angle at a given length on the path.
So, only position needs to be accumulated. Also made some changes to simplify the logic for accumulation.

* Source/WebCore/svg/SVGAnimateMotionElement.cpp:
(SVGAnimateMotionElement::buildTransformForProgress): Remove 'angle' accumulation from here
(SVGAnimateMotionElement::calculateAnimatedValue): Move here
* LayoutTests/svg/animations/animateMotion-accumulate-1a.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1a-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-1b.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1b-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-1c.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-1c-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-2a.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-2a-expected.svg: Add Test Case Expectation
* LayoutTests/svg/animations/animateMotion-accumulate-2b.svg: Add Test Case
* LayoutTests/svg/animations/animateMotion-accumulate-2b-expected.svg: Add Test Case Expectation

All tests have slight pixel tolerances to accommodate for platform differences.

Canonical link: https://commits.webkit.org/264595@main
@webkit-commit-queue webkit-commit-queue changed the title [SVG] animateMotion doesn't work properly with rotate: auto / auto-reverse [SVG] animateMotion accumulate doesn't work properly with rotate: auto / auto-reverse May 26, 2023
@webkit-commit-queue
Copy link
Collaborator

Committed 264595@main (b3b7524): https://commits.webkit.org/264595@main

Reviewed commits have been landed. Closing PR #14149 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit b3b7524 into WebKit:main May 26, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 26, 2023
@Ahmad-S792 Ahmad-S792 deleted the fix256928-svg-animation-case branch May 26, 2023 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SVG For bugs in the SVG implementation.
Projects
None yet
5 participants