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

[css-masking] Not clear how clip-path applies to fragmented elements #6383

Open
heycam opened this issue Jun 15, 2021 · 2 comments
Open

[css-masking] Not clear how clip-path applies to fragmented elements #6383

heycam opened this issue Jun 15, 2021 · 2 comments

Comments

@heycam
Copy link
Contributor

heycam commented Jun 15, 2021

https://drafts.fxtf.org/css-masking-1/#the-clip-path

I don't see anything in the CSS Masking spec that defines how clip-path interacts with box-decoration-break, or more broadly how multiple fragments influence the clipping that's applied. My expectation is that it would work like masks, where the reference box (specified in the clip-path property, unlike for masks where it's in mask-origin) is either computed separately for each fragment (when box-decoration-break is slice) or once over a hypothetical unfragmented box (when box-decoration-break is clone).

There's a very brief reference to box-decoration-break in https://drafts.fxtf.org/css-masking-1/#mask-positioning-area but nothing similar for clip-path.

Here's a test that doesn't have interop:

  • Safari applies a single clip to both fragments, and uses the bounding box of the fragments as the reference box.

Screen Shot 2021-06-15 at 2 29 24 pm

  • Firefox applies a single clip to both fragments, and uses the content(?) box of the second fragment as the reference box.

Screen Shot 2021-06-15 at 2 29 46 pm

  • Chrome applies a single clip to both fragments, and uses the content(?) box of the first fragment as the reference box.

Screen Shot 2021-06-15 at 2 30 04 pm

The way I guess it should work is:

  • For the box-decoration-break: slice case, the inset(25% 2ch 25% 0) would first be applied to the unfragmented element, which would result in both fragments having their top and bottom quarters chopped off and the second fragment having its last two characters chopped off. The blue border should be shown on the left of the first fragment, since the default reference box for the clip-path should be the border box.
  • For the box-decoration-break: clone case, both fragments would have their top and bottom quarters chopped off and both would have their last two characters chopped off. The blue border should be shown on the left of both fragments, since the default reference box for the clip-path should be the border box.
@heycam
Copy link
Contributor Author

heycam commented Jun 15, 2021

i.e. it should look like this:

Screen Shot 2021-06-15 at 2 38 16 pm

@smfr smfr added the Agenda+ label Oct 18, 2021
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Oct 19, 2021
…iling

https://bugs.webkit.org/show_bug.cgi?id=231916

Reviewed by Antti Koivisto.
Source/WebCore:

In r284336 I made clip-path not apply to non-RenderBoxes, which disabled it on inlines,
breaking the system-preview/badge.html test.

The spec says it applies to all elements, so this behavior change was incorrect.

Revert back to using calculateLayerBounds() as the fallback rect to use for inlines, and add
tests for this. This rectangle is obviously incorrect (for example, it's affected by text
shadow), but leave it for now until w3c/csswg-drafts#6383 is
resolved.

I also failed to see that computeClipPath() was already computing the reference box
internally, so clean up the code with some comments to make it more clear that the result of
calculateLayerBounds() is used only as the fallback for inlines, as well as for the buffer
bounds for the applyClippingToContext() code path.

Tests: css3/masking/clip-path-on-inline.html
       css3/masking/clip-path-on-split-inline.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::computeClipPath const):
(WebCore::RenderLayer::setupClipPath):
* rendering/RenderLayer.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateMaskingLayerGeometry):

LayoutTests:

Tests for clip-path on a single-box inline, and a split inline.

* css3/masking/clip-path-on-inline-expected.html: Added.
* css3/masking/clip-path-on-inline.html: Added.
* css3/masking/clip-path-on-split-inline-expected.html: Added.
* css3/masking/clip-path-on-split-inline.html: Added.

Canonical link: https://commits.webkit.org/243244@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284490 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg pushed a commit to Igalia/webkit that referenced this issue Oct 19, 2021
…iling

https://bugs.webkit.org/show_bug.cgi?id=231916

Reviewed by Antti Koivisto.
Source/WebCore:

In r284336 I made clip-path not apply to non-RenderBoxes, which disabled it on inlines,
breaking the system-preview/badge.html test.

The spec says it applies to all elements, so this behavior change was incorrect.

Revert back to using calculateLayerBounds() as the fallback rect to use for inlines, and add
tests for this. This rectangle is obviously incorrect (for example, it's affected by text
shadow), but leave it for now until w3c/csswg-drafts#6383 is
resolved.

I also failed to see that computeClipPath() was already computing the reference box
internally, so clean up the code with some comments to make it more clear that the result of
calculateLayerBounds() is used only as the fallback for inlines, as well as for the buffer
bounds for the applyClippingToContext() code path.

Tests: css3/masking/clip-path-on-inline.html
       css3/masking/clip-path-on-split-inline.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::computeClipPath const):
(WebCore::RenderLayer::setupClipPath):
* rendering/RenderLayer.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateMaskingLayerGeometry):

LayoutTests:

Tests for clip-path on a single-box inline, and a split inline.

* css3/masking/clip-path-on-inline-expected.html: Added.
* css3/masking/clip-path-on-inline.html: Added.
* css3/masking/clip-path-on-split-inline-expected.html: Added.
* css3/masking/clip-path-on-split-inline.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@284490 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Fragmented elements with clip-path, and agreed to the following:

  • RESOLVED: masking should follow same rules as backgrounds with regards to box-decoration-break
  • RESOLVED: clip-path and masking should follow same rules as backgrounds with regards to box-decoration-break
The full IRC log of that discussion <fantasai> Topic: Fragmented elements with clip-path
<fantasai> github: https://github.com//issues/6383
<dael> scribenick: dael
<dael> heycam: There's nothing in the css masking spec that says how clippath interacts with box decoration
<dael> heycam: slight reference to rbeak in section for mask image that implies it applies to masks on fragmented elements
<dael> heycam: Did some testing with clip-path and found safari, ff, and chrome different
<dael> heycam: What I would expect is similar to what happens to borders where if you have bo-decoration break slice you would slice up the clip and apply with different offsets
<dael> heycam: break clone evlauate size and position differently for each fragment
<fantasai> +1 to honoring box-decoration-break as heycam described
<dael> heycam: FF and Chrome similar in that they apply the clip=path computed based on one fragement, but using different elements. WK computes bounding box of all and evaluate clip-path.
<dael> heycam: I think none are exactly what we want
<dael> heycam: Wanted to see if you think it's reasonable or if clip-paths are different enough
<dael> fantasai: Agree with matching to way backgrounds and borders are handled. Makes a lot of sense
<dael> astearns: Seeing head nodding
<dael> florian: Agree. If in long run need to distinguish we could add longhands
<dael> iank_: Note that you don't need fragmenetation to trigger. Inline like a span with a clip-path triggers this
<dael> fantasai: That's still fragmenetation.
<dael> iank_: Sure
<dael> astearns: Hearing consensus. Anyone against?
<dael> chrishtr: iank_ do you have believe if this has impact on fragmeenation in [missed] architecture?
<dael> heycam: Basically behavrio is same as box-decoration break. If you have box-decoration-break slice you compute box for unfragmented thing and slice clip-path and apply separately
<dael> iank_: What happens if you have a fragment that is varying width...this is the problem in...case is inline when you have a fragmeneted span. span can have different heights. What happens there?
<dael> heycam: What happens with backgrounds?
<dael> fantasai: Spans don't have different heights on different lines b/c set by first available font
<dael> fantasai: As far as backgrounds and block level fragmentation it's defined in break. You recalc your size on each fragment. Maintain a concept of proress through element
<fantasai> https://www.w3.org/TR/css-break-3/#varying-size-boxes
<fantasai> s/proress/progress/
<dael> astearns: Background is sliced...compute for unfragmented and then you slice it up. First fragment ends up shorter does the bg resize per fragment after slicing?
<chrishtr> q+
<dael> fantasai: For slicing you do layout first. Then you assemble it. You've accounted for the spacing. You have an oval clip-rect. You do the fragmenetion, layout, and then draw background around that
<astearns> ack fantasai
<dael> fantasai: It's not that you layout as if there was no fragmenation. That would be different spacing
<astearns> ack chrishtr
<dael> chrishtr: Does the same problem, heycam , apply to other visual effects? masking, filters, and so on?
<dael> heycam: Masking and clipping seem they should do the same. Filters I had no considered
<dael> heycam: Gut feeling is that filters feel a little more different and it's not so obvious we should apply same rules. Maybe not the case
<dael> chrishtr: alisonmaher thoughts?
<dael> alisonmaher: I didn't work too much with this part of fragmentation. Keeping consistent feels like it would make sense
<dael> astearns: Sounds like there are some complications to work out.
<dael> astearns: General conensus that masking should follow same as backgrounds with regards to box-decoration-break
<dael> astearns: Anyone obj to trying this out?
<dael> RESOLVED: masking should follow same rules as backgrounds with regards to box-decoration-break
<dael> astearns: Definitely clip-path. Masking should be the same
<dael> RESOLVED: clip-path and masking should follow same rules as backgrounds with regards to box-decoration-break
<dael> astearns: We can think on filters separately. Might be better to try and reasond for clip-path and see if it would work first

JonWBedard pushed a commit to WebKit/WebKit that referenced this issue Dec 1, 2021
    Regression(r284336): [ iOS 15 ] system-preview/badge.html is image failing
    https://bugs.webkit.org/show_bug.cgi?id=231916

    Reviewed by Antti Koivisto.
    Source/WebCore:

    In r284336 I made clip-path not apply to non-RenderBoxes, which disabled it on inlines,
    breaking the system-preview/badge.html test.

    The spec says it applies to all elements, so this behavior change was incorrect.

    Revert back to using calculateLayerBounds() as the fallback rect to use for inlines, and add
    tests for this. This rectangle is obviously incorrect (for example, it's affected by text
    shadow), but leave it for now until w3c/csswg-drafts#6383 is
    resolved.

    I also failed to see that computeClipPath() was already computing the reference box
    internally, so clean up the code with some comments to make it more clear that the result of
    calculateLayerBounds() is used only as the fallback for inlines, as well as for the buffer
    bounds for the applyClippingToContext() code path.

    Tests: css3/masking/clip-path-on-inline.html
           css3/masking/clip-path-on-split-inline.html

    * rendering/RenderLayer.cpp:
    (WebCore::RenderLayer::computeClipPath const):
    (WebCore::RenderLayer::setupClipPath):
    * rendering/RenderLayer.h:
    * rendering/RenderLayerBacking.cpp:
    (WebCore::RenderLayerBacking::updateMaskingLayerGeometry):

    LayoutTests:

    Tests for clip-path on a single-box inline, and a split inline.

    * css3/masking/clip-path-on-inline-expected.html: Added.
    * css3/masking/clip-path-on-inline.html: Added.
    * css3/masking/clip-path-on-split-inline-expected.html: Added.
    * css3/masking/clip-path-on-split-inline.html: Added.

    Canonical link: https://commits.webkit.org/243244@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284490 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/243155.7@safari-613.1.6-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-613.1.6-branch@284499 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

3 participants