Skip to content

Conversation

@Constellation
Copy link
Member

@Constellation Constellation commented Oct 20, 2023

95809c7

[SVG] Add strokeContains fast path
https://bugs.webkit.org/show_bug.cgi?id=263430
rdar://problem/117439322

Reviewed by Cameron McCormack.

This is one step of the patch series implementing approximate repainting rect for SVG path, derived from blink's work[1].

This patch adds strokeContains optimization. The key is that we can use approximateStrokeBoundingBox to filter out the candidates
Instead of strokeBoundingBox since approximateStrokeBoundingBox is always larger than strokeBoundingBox. And approximateStrokeBoundingBox
is incredibly faster than strokeBoundingBox. We also extend our fast path handling for SVG Rect.

[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=435097

* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGEllipse.cpp:
(WebCore::LegacyRenderSVGEllipse::updateShapeFromElement):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGPath.cpp:
(WebCore::LegacyRenderSVGPath::updateShapeFromElement):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGRect.cpp:
(WebCore::LegacyRenderSVGRect::updateShapeFromElement):
(WebCore::LegacyRenderSVGRect::canUseStrokeHitTestFastPath const):
(WebCore::LegacyRenderSVGRect::definitelyHasSimpleStroke const):
(WebCore::LegacyRenderSVGRect::shapeDependentStrokeContains):
(WebCore::LegacyRenderSVGRect::shapeDependentFillContains const):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGRect.h:
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGShape.cpp:
(WebCore::LegacyRenderSVGShape::strokeContains):
(WebCore::LegacyRenderSVGShape::approximateStrokeBoundingBox const):
(WebCore::LegacyRenderSVGShape::updateRepaintBoundingBox):
(WebCore::LegacyRenderSVGShape::hasSmoothStroke const): Deleted.
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGShape.h:

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

d468c64

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

@Constellation Constellation self-assigned this Oct 20, 2023
@Constellation Constellation added the SVG For bugs in the SVG implementation. label Oct 20, 2023
@Constellation Constellation force-pushed the eng/SVG-Add-strokeContains-fast-path branch from 7fd4966 to 89ec2fa Compare October 24, 2023 22:22
@Constellation Constellation requested a review from heycam October 24, 2023 22:23
@Constellation Constellation marked this pull request as ready for review October 24, 2023 22:23
@Constellation Constellation force-pushed the eng/SVG-Add-strokeContains-fast-path branch from 89ec2fa to bb2f030 Compare October 24, 2023 22:29
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL stroke-miterlimit actually applies to rect and not just path, line, and polygon!

Copy link
Member Author

Choose a reason for hiding this comment

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

:D

Comment on lines 86 to 87
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is now out of date and I think we can remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good! Removed.

Comment on lines 83 to 84
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done :)

@Constellation Constellation force-pushed the eng/SVG-Add-strokeContains-fast-path branch from bb2f030 to d468c64 Compare October 24, 2023 23:35
@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 25, 2023
https://bugs.webkit.org/show_bug.cgi?id=263430
rdar://problem/117439322

Reviewed by Cameron McCormack.

This is one step of the patch series implementing approximate repainting rect for SVG path, derived from blink's work[1].

This patch adds strokeContains optimization. The key is that we can use approximateStrokeBoundingBox to filter out the candidates
Instead of strokeBoundingBox since approximateStrokeBoundingBox is always larger than strokeBoundingBox. And approximateStrokeBoundingBox
is incredibly faster than strokeBoundingBox. We also extend our fast path handling for SVG Rect.

[1]: https://bugs.chromium.org/p/chromium/issues/detail?id=435097

* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGEllipse.cpp:
(WebCore::LegacyRenderSVGEllipse::updateShapeFromElement):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGPath.cpp:
(WebCore::LegacyRenderSVGPath::updateShapeFromElement):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGRect.cpp:
(WebCore::LegacyRenderSVGRect::updateShapeFromElement):
(WebCore::LegacyRenderSVGRect::canUseStrokeHitTestFastPath const):
(WebCore::LegacyRenderSVGRect::definitelyHasSimpleStroke const):
(WebCore::LegacyRenderSVGRect::shapeDependentStrokeContains):
(WebCore::LegacyRenderSVGRect::shapeDependentFillContains const):
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGRect.h:
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGShape.cpp:
(WebCore::LegacyRenderSVGShape::strokeContains):
(WebCore::LegacyRenderSVGShape::approximateStrokeBoundingBox const):
(WebCore::LegacyRenderSVGShape::updateRepaintBoundingBox):
(WebCore::LegacyRenderSVGShape::hasSmoothStroke const): Deleted.
* Source/WebCore/rendering/svg/legacy/LegacyRenderSVGShape.h:

Canonical link: https://commits.webkit.org/269745@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/SVG-Add-strokeContains-fast-path branch from d468c64 to 95809c7 Compare October 25, 2023 01:21
@webkit-commit-queue
Copy link
Collaborator

Committed 269745@main (95809c7): https://commits.webkit.org/269745@main

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

@webkit-commit-queue webkit-commit-queue merged commit 95809c7 into WebKit:main Oct 25, 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 Oct 25, 2023
@Constellation Constellation deleted the eng/SVG-Add-strokeContains-fast-path branch October 25, 2023 05:52
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

Development

Successfully merging this pull request may close these issues.

4 participants