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-shapes][motion][svg] CSS shapes functions with/without fill-rule parameters #3468

Open
AmeliaBR opened this issue Dec 24, 2018 · 6 comments

Comments

@AmeliaBR
Copy link
Contributor

AmeliaBR commented Dec 24, 2018

(As Chris Coyier has regularly noted on CSS-Tricks, our current specs and implementation of CSS Shapes functions are a bit of a mess, with certain shapes functions working in some properties but not others. I started writing an explanation of why it's a mess, and ended up with good summaries of two possible solutions. Copying that comment here, in the hopes of finally coming to a decision & getting everything spec'd and shipped!)

The tricky part is fill-rule. The polygon() function includes fill-rule keywords as an optional first parameter:

clip-path: polygon(evenodd, 
    50% 0, 0 0, 50% 100%, 100% 0, 50% 0, 0 100%, 100% 100%, 50% 0);

https://codepen.io/AmeliaBR/pen/GgWBOy

But a <path> element uses the keywords set by the fill-rule OR clip-rule properties, depending on the shape's context. So having a keyword inside the d property would create a conflict.

The path() function as currently spec'd for offset-path doesn't include a keyword parameter, because motion only uses the outline, not the fill.

We have agreed to use that syntax for d (<path> shape) as a property.

But for clip-path (and future stuff like shape-inside to define the text wrapping area as a shape), we need to know which fill rule to use.

One idea I mused about (but never wrote down) is to define two different CSS data types, one of which is a super class of the other:

  • <outline-shape> doesn't have fill-rule keywords

  • <filled-shape> = <outline-shape> (with default fill-rule) | polygon() and path() with keywords

So, with this option, the d property would take an <outline-shape> function, no keywords allowed, and would still use the fill-rule/clip-rule properties with no conflict.

Another option is to define an auto value for the keyword inside the functions, and make that the default. In clip-path, an auto value would behave just like the current default (nonzero), But in d, it would behave as "check the fill-rule or clip-rule property according to context and use that". If you did specify a different keyword in a function inside d, it would override the other properties:

.even-odd {
  fill-rule: nonzero; /* ignored if the `d` value is supported */
  clip-rule: nonzero; /* ditto */
  d: path(evenodd, "M0,0 C80,40 -30,40 50,0 L25,50 Z");
}

A side benefit, in my opinion, is that this means we could long-term plan to deprecate usage of the fill-rule / clip-rule properties, which are already super annoying in the way they depend on context. If you want context-specific keyword values in the CSS function notation, you could use inherited CSS variable values.

But the most important benefit of either of these approaches is that they would allow all the shape functions (possibly minus fill-rule keywords) to be used in all the shape-related properties!!!

@AmeliaBR
Copy link
Contributor Author

AmeliaBR commented Dec 24, 2018

Since I'm going to be sending people to this issue to give feedback, let's do it via a GitHub reactions poll:

React to this comment with the following symbols to express a vote:

  • ❤️ (heart) for the outline-shape vs filled-shape system with different syntax for different properties

  • 🎉 (tada/hooray/party) for the auto vs override system, where keywords are allowed in any of the properties, and override the existing SVG properties

  • 👍 (+1/thumbs up) for "I don't care which option, just please pick it & ship it!"

Of course, if you have detailed use cases, other alternatives, or any other constructive comments, please feel free to leave them below!

@tabatkins
Copy link
Member

Agenda+ to decide on a final answer. It looks like people are okay with the proposal to:

  • make polygon() and path() take an optional fill keyword
  • add "auto" to the set of keywords, meaning either "defer to the appropriate contextual fill info" or "default to nonzero", depending on context

@astearns astearns added the css-shapes-1 Current Work label Jan 9, 2020
@AmeliaBR
Copy link
Contributor Author

Thanks for adding this to the agenda, @tabatkins! Based on the current schedule, I won't be on the call but I think the summary above still covers everything.

Once we pick the final syntax, other things to consider:

  • How much of this should be back-ported to Shapes 1 vs Shapes 2 (auto keyword in polygon()?)
  • What are implementation plans for harmonizing the shapes allowed in clip-path, shape-outside, and offset-path?

(For now, I'm assuming that d will continue to only accept path(). But I myself wouldn't mind if it accepted other functions if that's easier to implement!)

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Motion - Final approach for shapes, and agreed to the following:

  • RESOLVED: make an optional keyword with just the two values, and default to even-odd or "lookup depending on context"
The full IRC log of that discussion <fantasai> Topic: Motion - Final approach for shapes
<myles> ScribeNick: myles
<astearns> github: https://github.com//issues/3468
<myles> AmeliaBR: for the various properties that use shape(), some only care about the outline of the shape. Others care about the actual fill area of the shape
<myles> AmeliaBR: So for motion-path, you only care about the outline, but for clip-path, you care about which parts are inside and outside. This is an issue when you have polygons or paths with cris-crossing lines and inside/outside isn't so clear
<myles> AmeliaBR: Enter fill-rule. even-odd and nonzero.
<myles> AmeliaBR: It was originally defined as polygon(keyword, ...)
<myles> AmeliaBR: path() was defined in motion-path, and didn't include the keyword
<myles> AmeliaBR: Things get complicated with <path> because this had 2 separate properties for the fill rule. Filling vs what's the effect when it's in a clip-path.
<myles> AmeliaBR: How do we deal with this conflict between having a keyword as part of the shape function vs having a separate property which doesn't make sense for <clipPath>
<myles> AmeliaBR: I came up with a couple options. The one that seemed to have people most support is that the keyword within the shape function includes "auto" as the default, and the default would look up the other SVG properties. But if you set the value otherwise, it would override the old SVG properties and we maybe eventually deprecate the SVG properties.
<myles> AmeliaBR: If you specify a fill-rule keyword in motion-path, it's ignored, but that's not a problem. The only place where it's a problem with <shape> where it gets filled. We're specifying where if you set a fill rule in the function, it overrules the fill-rule property.
<myles> AmeliaBR: The default behavior is defined in all other cases to match the current behavior.
<myles> heycam: I like that. I'm not sure we need an explicit "auto" as opposed to just its absence.
<myles> TabAtkins: We do, for ... some case. There is a case related to <path>
<myles> TabAtkins: If path() takes a keyword, where the winding rule is determined by context, you need to be able to say "go grab from the other property explicitly"
<fantasai> +1 to heycam
<myles> heycam: This is a component of one value, and it's optional, can we just use its optionality?
<myles> AmeliaBR: So the auto behavior is the "no keyword specified" behavior
<myles> TabAtkins: That would work. It just runs into my dislike of having omitted values being unwritable
<myles> heycam: There are a lot of properties that have optional keywords
<astearns> q?
<myles> fantasai: <lists them>
<myles> TabAtkins: I get touchy
<myles> TabAtkins: I won't fight it
<TabAtkins> s/touchy/tetchy/
<fantasai> s/I get/Most of them are booleans, but when more than one value
<myles> AmeliaBR: The benefit of heycam's approach is that shipping for shapes in clip-path and shape-outside, we don't need to change anything, because the change would only come in paths where the author behavior is different from the current default behavior
<myles> heycam: Are their other elements other than <path> where we might want to have a default value that's not "go and look at the fill-rule property"?
<myles> AmeliaBR: All the other cases the default value will be to just use one of the existing keywords.
<myles> TabAtkins: even-odd is the default
<myles> heycam: There's no value in adding an explicit auto keyword to say "look at the fill-rule property" for these other cases?
<myles> TabAtkins: Those other cases don't have a property.
<myles> AmeliaBR: It wouldn't make sense to have a div with both a clip-path and a shape-outside and also set a fill-rule in another property. That would be confusing
<myles> TabAtkins: There's only the two things that have the information defined by another property, and there isn't a use case to have arbitrary things rely on those two, it's just due to SVG's existing behavior to rely on those two
<myles> TabAtkins: and that's it.
<myles> astearns: The proposed resolution is to make this an optional keyword with just the two values, and default to even-odd or "lookup depending on context"
<myles> AmeliaBR: Withing the context of d= then not specifying the keyword would the the SVG legacy beahvior. Specifying the keyword behavior would mean "ignore the fill-rule and clip-rule properties"
<myles> astearns: Any objections?
<myles> RESOLVED: make an optional keyword with just the two values, and default to even-odd or "lookup depending on context"

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 15, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1837305
gecko-commit: 6f345b692ecbd7a6de126296efb8811b6bd8597d
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 15, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 15, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1837305
gecko-commit: 6f345b692ecbd7a6de126296efb8811b6bd8597d
gecko-reviewers: emilio
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jun 15, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jun 16, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625

UltraBlame original commit: 6f345b692ecbd7a6de126296efb8811b6bd8597d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jun 16, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625

UltraBlame original commit: 6f345b692ecbd7a6de126296efb8811b6bd8597d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jun 16, 2023
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625

UltraBlame original commit: 6f345b692ecbd7a6de126296efb8811b6bd8597d
@cdoublev
Copy link
Collaborator

In order to close this issue, it seems that defining how to handle <'fill-rule'> in some contexts still needs to be defined.

People may be confused without a clarification in the related specs for these contexts (see w3c/fxtf-drafts#512 and #7390), now that path() and polygon() are defined to accept an optional <'fill-rule'> in CSS Shapes, whose default value is nonzero instead of evenodd, as noted in the above resolution, which I assume is intentional.

If I am not mistaken, it boils down to defining that:

  • <'fill-rule'> must be ignored and omitted in the serialization when the context is offset-path (Motion 1)
  • <'fill-rule'> must take precedence over fill-rule and clip-rule when the context is d (SVG 2), assuming its value definition is none | <path()> (as currently implemented in browsers) instead of none | <string>

mrobinson pushed a commit to mrobinson/stylo that referenced this issue Mar 1, 2024
For outline shapes, fill-rule should be ignored. We add the flag in
the parser of BasicShape, so offset-path can use this flag to ignore
fill-rule.

Note: "ShapeType" means this property uses filled shapes or outline
shapes. For outline shapes, we ignore fill-rule. This is from the
concept of `<outline-shape>` and `<filled-shape>` in
w3c/csswg-drafts#3468 (comment)

No behavir change in this patch, just add the ability for offset-path to
ignore `<fill-rule>` when combining all basic shapes into offset-path.

Differential Revision: https://phabricator.services.mozilla.com/D179625
@astearns
Copy link
Member

Added the omitted default handling in 5ebf7df

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants