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

Avoid unnecessary StyleColor copying in animation wrappers #12609

Conversation

anttijk
Copy link
Contributor

@anttijk anttijk commented Apr 11, 2023

7baa7bf

Avoid unnecessary StyleColor copying in animation wrappers
https://bugs.webkit.org/show_bug.cgi?id=255278
rdar://107875297

Reviewed by Antoine Quint.

There is some unnecessary copying of this not-quite-trivial type.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::PropertyWrapperStyleColor::PropertyWrapperStyleColor):
(WebCore::PropertyWrapperStyleColor::value const):

Remove templating and have separate classes for StyleColor and Color cases.

(WebCore::PropertyWrapperColor::PropertyWrapperColor):
(WebCore::PropertyWrapperColor::value const):
(WebCore::PropertyWrapperVisitedAffectedStyleColor::PropertyWrapperVisitedAffectedStyleColor):
(WebCore::PropertyWrapperVisitedAffectedColor::PropertyWrapperVisitedAffectedColor):
(WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
(WebCore::PropertyWrapperColor::unresolvedValue const): Deleted.

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

601bc42

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
βœ… πŸ›  tv βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ api-gtk
βœ… πŸ§ͺ mac-wk2   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@anttijk anttijk requested a review from graouts as a code owner April 11, 2023 10:14
@anttijk anttijk self-assigned this Apr 11, 2023
@graouts
Copy link
Contributor

graouts commented Apr 11, 2023

Cc @weinig who's always interested in color-related things.

@anttijk anttijk changed the title ??? Avoid unnecessary StyleColor copying in animation wrappers Apr 11, 2023
@anttijk anttijk added the Animations Bugs related to CSS + SVG animations and transitions label Apr 11, 2023
Comment on lines 1779 to 1780
auto result = blendFunc(value(from), value(to), context);
(destination.*m_setter)(result);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit sad that we can't move this. I guess it gets copied down the line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can.

@anttijk anttijk force-pushed the eng/Avoid-unnecessary-StyleColor-copying-in-animation-wrappers branch from 3e7d6aa to 601bc42 Compare April 11, 2023 11:41
@anttijk anttijk added the merge-queue Applied to send a pull request to merge-queue label Apr 11, 2023
https://bugs.webkit.org/show_bug.cgi?id=255278
rdar://107875297

Reviewed by Antoine Quint.

There is some unnecessary copying of this not-quite-trivial type.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::PropertyWrapperStyleColor::PropertyWrapperStyleColor):
(WebCore::PropertyWrapperStyleColor::value const):

Remove templating and have separate classes for StyleColor and Color cases.

(WebCore::PropertyWrapperColor::PropertyWrapperColor):
(WebCore::PropertyWrapperColor::value const):
(WebCore::PropertyWrapperVisitedAffectedStyleColor::PropertyWrapperVisitedAffectedStyleColor):
(WebCore::PropertyWrapperVisitedAffectedColor::PropertyWrapperVisitedAffectedColor):
(WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
(WebCore::PropertyWrapperColor::unresolvedValue const): Deleted.

Canonical link: https://commits.webkit.org/262822@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Avoid-unnecessary-StyleColor-copying-in-animation-wrappers branch from 601bc42 to 7baa7bf Compare April 11, 2023 12:21
@webkit-commit-queue
Copy link
Collaborator

Committed 262822@main (7baa7bf): https://commits.webkit.org/262822@main

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

@webkit-commit-queue webkit-commit-queue merged commit 7baa7bf into WebKit:main Apr 11, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Animations Bugs related to CSS + SVG animations and transitions
Projects
None yet
4 participants