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

[LBSE] Fix pattern related assertion failure in debug builds #23605

Conversation

nikolaszimmermann
Copy link
Contributor

@nikolaszimmermann nikolaszimmermann commented Jan 31, 2024

b0b381f

[LBSE] Fix pattern related assertion failure in debug builds
https://bugs.webkit.org/show_bug.cgi?id=268488

Reviewed by Rob Buis.

Fix incorrect cycle handling for pattern resources, not correctly taking
chained resource (href/xlink:href references) into account, leading to
the following assertion in debug builds in svg/custom/recursive-pattern.svg
and svg/custom/pattern-content-inheritance-cycle.svg.

Covered by existing tests.

Combined changes:
* LayoutTests/platform/glib/svg/custom/recursive-pattern-expected.png: Added.
* LayoutTests/platform/glib/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/platform/ios/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/recursive-pattern-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/recursive-pattern-expected.txt: Removed.
* LayoutTests/platform/mac-sonoma-wk2-pixel/svg/custom/recursive-pattern-expected.png: Removed.
* LayoutTests/platform/mac/svg/custom/recursive-pattern-expected.png:
* LayoutTests/platform/mac/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/svg/custom/recursive-pattern.svg:
* Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp:
(WebCore::RenderSVGResourcePattern::prepareFillOperation):
(WebCore::RenderSVGResourcePattern::prepareStrokeOperation):
(WebCore::RenderSVGResourcePattern::createTileImage const):

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

0db06b0

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

@nikolaszimmermann nikolaszimmermann self-assigned this Jan 31, 2024
@nikolaszimmermann nikolaszimmermann added the SVG For bugs in the SVG implementation. label Jan 31, 2024
Copy link
Contributor

@rwlbuis rwlbuis left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe correct the commit message from "the following assertion" to "the assertion mentioned in the bug" or something like that.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 31, 2024
@nikolaszimmermann
Copy link
Contributor Author

Stupid me, changing the test alone is not an option, it also needs to run with LBSE or we need two variants of the patch.
Since LBSE now matches the other browser vendors, I think I'll just change to run the svg/custom/recursive-pattern.svg test with LBSE.

@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Feb 1, 2024
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Fix-pattern-related-assertion-failure-in-debug-builds branch from b08c711 to 3645920 Compare February 1, 2024 14:12
@webkit-early-warning-system
Copy link
Collaborator

Starting EWS tests for 3645920. Live statuses available at the PR page, #23605

@nikolaszimmermann
Copy link
Contributor Author

Need to wait for EWS to grab platform specific results for iOS, Gtk, WPE, win,...

@nikolaszimmermann
Copy link
Contributor Author

LGTM. Maybe correct the commit message from "the following assertion" to "the assertion mentioned in the bug" or something like that.

Will do upon the next iteration.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 1, 2024
@nikolaszimmermann nikolaszimmermann removed the merging-blocked Applied to prevent a change from being merged label Feb 1, 2024
@nikolaszimmermann nikolaszimmermann force-pushed the eng/LBSE-Fix-pattern-related-assertion-failure-in-debug-builds branch from 3645920 to 0db06b0 Compare February 1, 2024 21:08
@nikolaszimmermann nikolaszimmermann added the merge-queue Applied to send a pull request to merge-queue label Feb 1, 2024
https://bugs.webkit.org/show_bug.cgi?id=268488

Reviewed by Rob Buis.

Fix incorrect cycle handling for pattern resources, not correctly taking
chained resource (href/xlink:href references) into account, leading to
the following assertion in debug builds in svg/custom/recursive-pattern.svg
and svg/custom/pattern-content-inheritance-cycle.svg.

Covered by existing tests.

Combined changes:
* LayoutTests/platform/glib/svg/custom/recursive-pattern-expected.png: Added.
* LayoutTests/platform/glib/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/platform/ios/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/recursive-pattern-expected.png:
* LayoutTests/platform/mac-sonoma-wk2-lbse-text/svg/custom/recursive-pattern-expected.txt: Removed.
* LayoutTests/platform/mac-sonoma-wk2-pixel/svg/custom/recursive-pattern-expected.png: Removed.
* LayoutTests/platform/mac/svg/custom/recursive-pattern-expected.png:
* LayoutTests/platform/mac/svg/custom/recursive-pattern-expected.txt:
* LayoutTests/svg/custom/recursive-pattern.svg:
* Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp:
(WebCore::RenderSVGResourcePattern::prepareFillOperation):
(WebCore::RenderSVGResourcePattern::prepareStrokeOperation):
(WebCore::RenderSVGResourcePattern::createTileImage const):

Canonical link: https://commits.webkit.org/273944@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/LBSE-Fix-pattern-related-assertion-failure-in-debug-builds branch from 0db06b0 to b0b381f Compare February 1, 2024 23:09
@webkit-commit-queue webkit-commit-queue merged commit b0b381f into WebKit:main Feb 1, 2024
@webkit-commit-queue
Copy link
Collaborator

Committed 273944@main (b0b381f): https://commits.webkit.org/273944@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 1, 2024
@nikolaszimmermann nikolaszimmermann deleted the eng/LBSE-Fix-pattern-related-assertion-failure-in-debug-builds branch February 2, 2024 11:49
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