Bug 185829

Summary:

[css-masking] Fully support -webkit-clip-path on SVG elements

Product:

WebKit

Reporter:

Dirk Schulze <krit>

Component:

SVG

Assignee:

Dirk Schulze <krit>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, lforschler, simon.fraser, zimmermann

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

126207

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews123 for ios-simulator-wk2	none
Archive of layout-test-results from ews125 for ios-simulator-wk2	none
Archive of layout-test-results from ews204 for win-future	none
Patch	none
Archive of layout-test-results from ews202 for win-future	none
Patch for landing	none

Dirk Schulze

Reported 2018-05-21 12:09:18 PDT

Currently there is just some rudimentary support for -webkit-clip-path on SVG elements. E.g: * -webkit-clip-path does not contribute to hit-testing. * References to <clipPath> do not work. * <shape-box> only values do not work.

Attachments
Patch (26.71 KB, patch) 2018-06-28 13:07 PDT, Dirk Schulze	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews123 for ios-simulator-wk2 (58.66 MB, application/zip) 2018-06-28 15:37 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (65.98 MB, application/zip) 2018-06-28 17:24 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews204 for win-future (12.81 MB, application/zip) 2018-06-28 23:21 PDT, EWS Watchlist	no flags	Details
Patch (27.70 KB, patch) 2018-06-29 02:03 PDT, Dirk Schulze	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews202 for win-future (13.07 MB, application/zip) 2018-06-29 07:11 PDT, EWS Watchlist	no flags	Details
Patch for landing (27.77 KB, patch) 2018-07-14 01:40 PDT, Dirk Schulze	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Dirk Schulze

Comment 1 2018-06-28 13:07:28 PDT

Created attachment 343839 [details] Patch

EWS Watchlist

Comment 2 2018-06-28 15:37:00 PDT

Comment on attachment 343839 [details] Patch Attachment 343839 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8373725 New failing tests: svg/dynamic-updates/SVGClipPathElement-prefixed-transform-influences-hitTesting.html svg/dynamic-updates/SVGClipPath-prefixed-path-influences-hitTesting.html svg/dynamic-updates/SVGClipPathElement-prefixed-css-transform-influences-hitTesting.html svg/dynamic-updates/SVGClipPath-prefixed-influences-hitTesting.html

EWS Watchlist

Comment 3 2018-06-28 15:37:03 PDT

Created attachment 343864 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

EWS Watchlist

Comment 4 2018-06-28 17:24:02 PDT

Comment on attachment 343839 [details] Patch Attachment 343839 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8375317 New failing tests: svg/dynamic-updates/SVGClipPathElement-prefixed-transform-influences-hitTesting.html svg/dynamic-updates/SVGClipPath-prefixed-path-influences-hitTesting.html svg/dynamic-updates/SVGClipPathElement-prefixed-css-transform-influences-hitTesting.html svg/dynamic-updates/SVGClipPath-prefixed-influences-hitTesting.html

EWS Watchlist

Comment 5 2018-06-28 17:24:05 PDT

Created attachment 343875 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4

EWS Watchlist

Comment 6 2018-06-28 23:21:31 PDT

Comment on attachment 343839 [details] Patch Attachment 343839 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8378703 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html http/tests/preload/onload_event.html

EWS Watchlist

Comment 7 2018-06-28 23:21:42 PDT

Created attachment 343895 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Dirk Schulze

Comment 8 2018-06-29 02:03:01 PDT

Created attachment 343897 [details] Patch

EWS Watchlist

Comment 9 2018-06-29 07:10:52 PDT

Comment on attachment 343897 [details] Patch Attachment 343897 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8382738 New failing tests: http/tests/preload/onload_event.html

EWS Watchlist

Comment 10 2018-06-29 07:11:03 PDT

Created attachment 343912 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

Dirk Schulze

Comment 11 2018-06-29 22:31:03 PDT

The failing win tests are unrelated.

Simon Fraser (smfr)

Comment 12 2018-07-11 11:08:00 PDT

Comment on attachment 343897 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343897&action=review > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:408 > +inline FloatRect clipPathReferenceBox(const RenderElement& renderer, const CSSBoxType& boxType) boxType can be passed by value. It's just an enum. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:410 > + // Depends on https://bugs.webkit.org/show_bug.cgi?id=185797 This comment can be removed. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:411 > + FloatRect referenceBox { }; No need for { } > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:413 > + if (boxType == CSSBoxType::StrokeBox || boxType == CSSBoxType::BorderBox > + || boxType == CSSBoxType::MarginBox) Don't wrap the lines this narrow. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:417 > + } else if (boxType == CSSBoxType::ViewBox && renderer.element()) { Maybe this would be better as a switch, so the compiler tells you if you forgot a value. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:421 > + referenceBox.setWidth(viewportSize.width()); > + referenceBox.setHeight(viewportSize.height()); Can this be setSize(viewportSize.width.size())? > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:432 > + FloatRect referenceBox {clipPathReferenceBox(renderer, clipPath.referenceBox())}; Would be simpler as FloatRect referenceBox = clipPathReferenceBox(renderer, clipPath.referenceBox()); > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:439 > + FloatRect referenceBox {clipPathReferenceBox(renderer, clipPath.referenceBox())}; Same. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:442 > + return clipPath.pathForReferenceRect(FloatRoundedRect {referenceBox}).contains(point); Is the explicit FloatRoundedRect necessary? > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:453 > + FloatRect referenceBox {clipPathReferenceBox(renderer, clipPath.referenceBox())}; Ditto. > Source/WebCore/rendering/svg/SVGRenderSupport.cpp:459 > + FloatRect referenceBox {clipPathReferenceBox(renderer, clipPath.referenceBox())}; > + context.clipPath(clipPath.pathForReferenceRect(FloatRoundedRect {referenceBox})); Ditto.

Dirk Schulze

Comment 13 2018-07-14 01:40:43 PDT

Created attachment 345032 [details] Patch for landing

WebKit Commit Bot

Comment 14 2018-07-14 02:19:54 PDT

Comment on attachment 345032 [details] Patch for landing Clearing flags on attachment: 345032 Committed r233835: <https://trac.webkit.org/changeset/233835>

WebKit Commit Bot

Comment 15 2018-07-14 02:19:55 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 16 2018-08-01 21:44:44 PDT

<rdar://problem/42841841>

Note You need to log in before you can comment on or make changes to this bug.