Skip to content

2D context stroke, fill color parsing spends time resolving script execution context#44998

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
kkinnunen-apple:colorresolutiondelegate-no-redundant-functions-1
May 7, 2025
Merged

2D context stroke, fill color parsing spends time resolving script execution context#44998
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
kkinnunen-apple:colorresolutiondelegate-no-redundant-functions-1

Conversation

@kkinnunen-apple
Copy link
Copy Markdown
Contributor

@kkinnunen-apple kkinnunen-apple commented May 6, 2025

48f9487

2D context stroke, fill color parsing spends time resolving script execution context
https://bugs.webkit.org/show_bug.cgi?id=292604
rdar://150757978

Reviewed by Simon Fraser.

General parsing CSS colors needs the CSS value pool from script
execution context. The fast path does not need this, but 2D context
code would resolve the context anyway. For significant number of
strokeStyle/fillStyle assignments, this would result in significant
amount of work.

Simplify the fast/slow path invocation by exposing the specific fast
path function. There's only few call sites and the previous functor
based approach resulted in more complex and less efficient code.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.cpp:
(WebCore::CSSPropertyParserHelpers::parseColorRaw):
(WebCore::CSSPropertyParserHelpers::parseColorRawSlow): Deleted.
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+ColorInlines.h:
(WebCore::CSSPropertyParserHelpers::parseColorRawFast):
(WebCore::CSSPropertyParserHelpers::parseColorRaw):
* Source/WebCore/html/ColorInputType.cpp:
(WebCore::parseColorValue):
(WebCore::colorParsingParameters): Deleted.
* Source/WebCore/html/canvas/CanvasStyle.cpp:
(WebCore::CanvasStyleColorResolutionDelegate::currentColor const):
(WebCore::parseColor):
(WebCore::elementlessColorParsingParameters): Deleted.
(WebCore::colorParsingParameters): Deleted.
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::setUnderPageBackgroundColorOverride):

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

4ad7efa

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this May 6, 2025
@kkinnunen-apple kkinnunen-apple added the Canvas Bugs related to the canvas element. label May 6, 2025
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #34657 (8752161)

❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@kkinnunen-apple kkinnunen-apple force-pushed the colorresolutiondelegate-no-redundant-functions-1 branch from 8752161 to 81b639b Compare May 6, 2025 14:07
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 6, 2025
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label May 6, 2025
@kkinnunen-apple kkinnunen-apple force-pushed the colorresolutiondelegate-no-redundant-functions-1 branch from 81b639b to f50c06e Compare May 6, 2025 19:36
@kkinnunen-apple kkinnunen-apple force-pushed the colorresolutiondelegate-no-redundant-functions-1 branch from f50c06e to 1851615 Compare May 6, 2025 19:43
@kkinnunen-apple kkinnunen-apple force-pushed the colorresolutiondelegate-no-redundant-functions-1 branch from 1851615 to 4ad7efa Compare May 6, 2025 19:48
@kkinnunen-apple kkinnunen-apple added the merge-queue Applied to send a pull request to merge-queue label May 7, 2025
…ecution context

https://bugs.webkit.org/show_bug.cgi?id=292604
rdar://150757978

Reviewed by Simon Fraser.

General parsing CSS colors needs the CSS value pool from script
execution context. The fast path does not need this, but 2D context
code would resolve the context anyway. For significant number of
strokeStyle/fillStyle assignments, this would result in significant
amount of work.

Simplify the fast/slow path invocation by exposing the specific fast
path function. There's only few call sites and the previous functor
based approach resulted in more complex and less efficient code.

* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.cpp:
(WebCore::CSSPropertyParserHelpers::parseColorRaw):
(WebCore::CSSPropertyParserHelpers::parseColorRawSlow): Deleted.
* Source/WebCore/css/parser/CSSPropertyParserConsumer+Color.h:
* Source/WebCore/css/parser/CSSPropertyParserConsumer+ColorInlines.h:
(WebCore::CSSPropertyParserHelpers::parseColorRawFast):
(WebCore::CSSPropertyParserHelpers::parseColorRaw):
* Source/WebCore/html/ColorInputType.cpp:
(WebCore::parseColorValue):
(WebCore::colorParsingParameters): Deleted.
* Source/WebCore/html/canvas/CanvasStyle.cpp:
(WebCore::CanvasStyleColorResolutionDelegate::currentColor const):
(WebCore::parseColor):
(WebCore::elementlessColorParsingParameters): Deleted.
(WebCore::colorParsingParameters): Deleted.
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::setUnderPageBackgroundColorOverride):

Canonical link: https://commits.webkit.org/294607@main
@webkit-commit-queue webkit-commit-queue force-pushed the colorresolutiondelegate-no-redundant-functions-1 branch from 4ad7efa to 48f9487 Compare May 7, 2025 07:37
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 294607@main (48f9487): https://commits.webkit.org/294607@main

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

@webkit-commit-queue webkit-commit-queue merged commit 48f9487 into WebKit:main May 7, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Canvas Bugs related to the canvas element.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants