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.

Sign up for GitHub

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

Jump to bottom

For accounting for the d presentation attribute when resolving the computed style. #30743

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

For accounting for the d presentation attribute when resolving the computed style. #30743

wants to merge 2 commits into from

Conversation

graouts
Copy link
Contributor

@graouts graouts commented Jul 12, 2024 •
edited by webkit-early-warning-system
Loading

fab6397

For accounting for the `d` presentation attribute when resolving the computed style.

aeb5a4a

[svg] enable support for the 'd' property by default
https://bugs.webkit.org/show_bug.cgi?id=276172
rdar://129035240

Reviewed by NOBODY (OOPS!).

Turn on the support for the `d` CSS property by default.

In order to be able to do that, we need to address the performance regression
in the Speedometer3 subtest `React-Stockcharts-SVG` by selectively calling
`setPresentationalHintStyleIsDirty()` under `SVGPathElement::svgAttributeChanged()`
only if we've previously seen a change in the `d` CSS property. In other words,
we only kick in the presentation attribute machinery if we've determined that
the CSS `d` property was used.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::svgAttributeChanged):
(WebCore::SVGPathElement::pathDidChange):
* Source/WebCore/svg/SVGPathElement.h:

0fc023c

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ❌ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
❌ 🛠 tv ✅ 🧪 jsc-armv7-tests
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@graouts graouts self-assigned this Jul 12, 2024
@graouts graouts added the SVG For bugs in the SVG implementation. label Jul 12, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 12, 2024 •
edited
Loading

EWS run on previous version of this PR (hash e3fad7f)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from e3fad7f to 6729637 Compare July 12, 2024 12:19
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 12, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 6729637)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 12, 2024
@graouts graouts removed the merging-blocked Applied to prevent a change from being merged label Jul 12, 2024
@graouts graouts changed the title [svg] enable support for the 'd' property by default For accounting for the d presentation attribute when resolving the computed style. Jul 12, 2024
@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from 6729637 to fab6397 Compare July 12, 2024 17:15
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 12, 2024 •
edited
Loading

EWS run on previous version of this PR (hash fab6397)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from fab6397 to e394ee9 Compare July 14, 2024 10:48
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 14, 2024 •
edited
Loading

EWS run on previous version of this PR (hash e394ee9)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv ✅ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from e394ee9 to f5e0c1f Compare July 17, 2024 07:15
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash f5e0c1f)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style   🛠 ios   🛠 mac   🛠 wpe   🛠 wincairo
✅ 🧪 bindings   🛠 ios-sim   🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
  🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
✅ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🛠 gtk
✅ 🛠 🧪 jsc-arm64   🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
  🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
⏳ 🧪 vision-wk2   🛠 jsc-armv7
  🛠 tv   🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from f5e0c1f to 4c149e3 Compare July 17, 2024 07:19
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 4c149e3)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ❌ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
❌ 🛠 tv ✅ 🧪 jsc-armv7-tests
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 17, 2024
@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from 4c149e3 to 5041096 Compare August 26, 2024 06:38
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 26, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 5041096)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ❌ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
❌ 🛠 tv ✅ 🧪 jsc-armv7-tests
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@graouts
[svg] enable support for the 'd' property by default
bfa0e70 
https://bugs.webkit.org/show_bug.cgi?id=276172
rdar://129035240

Reviewed by NOBODY (OOPS!).

Turn on the support for the `d` CSS property by default.

In order to be able to do that, we need to address the performance regression
in the Speedometer3 subtest `React-Stockcharts-SVG` by selectively calling
`setPresentationalHintStyleIsDirty()` under `SVGPathElement::svgAttributeChanged()`
only if we've previously seen a change in the `d` CSS property. In other words,
we only kick in the presentation attribute machinery if we've determined that
the CSS `d` property was used.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::svgAttributeChanged):
(WebCore::SVGPathElement::pathDidChange):
* Source/WebCore/svg/SVGPathElement.h:
@graouts
For accounting for the d presentation attribute when resolving the …
0fc023c 
…computed style.
@graouts graouts force-pushed the enable-svg-d-presentation-attribute branch from 5041096 to 0fc023c Compare August 28, 2024 06:41
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 28, 2024 •
edited
Loading

EWS run on current version of this PR (hash 0fc023c)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 wincairo
✅ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ❌ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ❌ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
❌ 🛠 tv ✅ 🧪 jsc-armv7-tests
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rniwa rniwa Awaiting requested review from rniwa rniwa will be requested when the pull request is marked ready for review rniwa is a code owner

@cdumez cdumez Awaiting requested review from cdumez cdumez will be requested when the pull request is marked ready for review cdumez is a code owner

Assignees

@graouts graouts

Labels
merging-blocked Applied to prevent a change from being merged SVG For bugs in the SVG implementation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@graouts @webkit-early-warning-system @webkit-ews-buildbot