[go: up one dir, main page]

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

stroke-miterlimit: invalid value behavior? #545

Closed
AmeliaBR opened this issue Sep 17, 2018 · 7 comments
Closed

stroke-miterlimit: invalid value behavior? #545

AmeliaBR opened this issue Sep 17, 2018 · 7 comments

Comments

@AmeliaBR
Copy link
Contributor

The section on stroke-miterlimit currently says:

<number>
The limit on the extent of a miter, miter-clip, or arcs line join as a multiple of the stroke-width value. The value of stroke-miterlimit must be a <number> greater than or equal to 1. Any other value is an error (see Error processing).

If it was truly an error handled by the general error processing rules, then the shape with an invalid value for the property would not be drawn at all. I'm sure that's not what happens, and not what we want to happen.

But we need to define whether value clamping happens at parse time or used value time. Which means checking to see what implementations currently do.

e.g, for:

path {
  stroke-miterlimit: 2;
  stroke-miterlimit: 0.5;
}

Is the second value thrown out at parse time, so the used value is 2? Or is it accepted then clamped to the minimum value of 1? Or is it accepted then thrown out at rendering time, so that the used value is the initial value (4)?

Does any of this change if a negative or zero value is declared?

@fsoder
Copy link
fsoder commented Sep 18, 2018

In Blink none of these values would be considered "invalid" as such, and will be passed all the way to the actual rendering layer (the graphics library.) Ditto if the value was zero. If the value is negative then I believe the rendering layer will reject it and use 4. So I guess that's "at used value time".

@AmeliaBR
Copy link
Contributor Author

@fsoder, I actually made a test & that's not quite what's happening.
https://codepen.io/AmeliaBR/pen/dqQqyQ?editors=0100

So to describe what you're seeing: The pink polyline is being animated (with SMIL, or a JS polyfill if SMIL isn't supported) to change the miter distance, from 1 to 10+. The pattern of blue lines in the background mark out integer values of the miter limit, relative to the vertex point (purple circle). When the miter hits the miter limit, it will suddenly be cut back to a bevel corner. You can also check DevTools to see which values are being thrown out by the parser, or any changes in the computed style.

Results:

  • Chrome (v69) and Safari (v11): negative values for stroke-miterlimit are discarded by the parser. Values between 0 and 1 are clamped to 1 for rendering/used-value time.
  • Edge (v17): all number values are accepted by the parser, anything less than 1 is clamped to 1 at rendering.
  • Firefox (v63): any value less than 1 is discarded by the parser.

Going by the (vaguely worded) general rule on clamping values in SVG 1.1 Implementation Requirements (defer error-checking as late as possible, with recommendations for using clamping for the specific examples), I'd say the Edge implementation is correct (don't clamp until as late as possible). But based on the text in the stroke-miterlimit property itself (values must be 1 or greater, other values are errors), I'd say that Firefox is correct.

@fsoder
Copy link
fsoder commented Sep 18, 2018

Yes, seems I missed to notice the "non-negative" restriction while trying to find the relevant bits in the code. Values in the [0, 1] range are technically not clamped, they're just not observable (i.e if the limit is in that range you'll get a bevel join regardless.)

I have no strong opinion on the matter as whole, although I'm generally not a fan of having this type of "semantic checking" in the parser (usually you end up redoing it at later stages anyway.)

@AmeliaBR
Copy link
Contributor Author

Values in the [0, 1] range are technically not clamped, they're just not observable (i.e if the limit is in that range you'll get a bevel join regardless.)

That's a good point. You get the same rendering as with a limit of 1, but don't actually need to clamp explicitly. That might change (or need some careful wording about expected behavior) if some of the other line join options in the spec ever get implemented.

@css-meeting-bot
Copy link
Member

The SVG Working Group just discussed stroke-miterlimit: invalid value behavior, and agreed to the following:

  • RESOLUTION: stroke-miterlimit should accept any non-negative number
The full IRC log of that discussion <AmeliaBR> Topic: stroke-miterlimit: invalid value behavior
<AmeliaBR> github: https://github.com//issues/545
<AmeliaBR> Amelia: stroke-miterlimit is currently defined to be => 1, but only Firefox enforces at parser level
<AmeliaBR> ... Blink & webkit accept any positive value, Edge accepts any number
<AmeliaBR> ... As @fsoder notes, there is no rendering difference between values between 0-1 versus 1
<AmeliaBR> Chris: I suspect Firefox behavior is just matching spec, could be changed. Everyone okay with making it any positive number (matching Blink & Webkit)?
<AmeliaBR> RESOLUTION: stroke-miterlimit should accept any non-negative number
<AmeliaBR> Amelia: I can do the edits. But also need browser bugs filed.
<AmeliaBR> Tav: And tests.

@AmeliaBR
Copy link
Contributor Author

Okay, I've pushed the edits to the basic syntax definition, with an informative note about the change.

I didn't make any changes to the algorithms for the new miter types (miter-clip and arcs), since I thought it could make sense to actually want to clip those miters closer to the vertex than the normal stroke width. @Tavmjong, you might want to look into your sample implementations.

Web platform tests still need to be written, and bugs on Firefox and MS Edge still need to be filed, so I'm leaving the issue open.

@AmeliaBR AmeliaBR removed their assignment Sep 28, 2018
@dirkschulze
Copy link
Contributor

We have the "Needs testing" flag set. With writing tests, implementations will get notified automatically. Right now, it is easier to organize still open issues for the next CR/PR.

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