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

SVGGeometry.prototype.isPointInStroke vs. vector-effects #559

Closed
fsoder opened this issue Oct 18, 2018 · 12 comments
Closed

SVGGeometry.prototype.isPointInStroke vs. vector-effects #559

fsoder opened this issue Oct 18, 2018 · 12 comments

Comments

@fsoder
Copy link

fsoder commented Oct 18, 2018

The section on isPointInStroke lists a number of stroke-related properties that should be considered as part of the isPointInStroke operation - it does not mention vector-effect. Excluding it seems reasonable to me, but looking at the issues leading up to the current spec text (like #456) I didn't see it being (explicitly) considered, so I just wanted make sure that its exclusion from the list of properties was deliberate. If that's the case I can probably make sure that a test is added for that case.

@fsoder
Copy link
Author

fsoder commented Oct 19, 2018

Extending this query to also include 'pathLength'.

@AmeliaBR
Copy link
Contributor

I think it was definitely overlooked.

From an author perspective, the method is certainly less useful if it doesn't factor in non-scaling-stroke. While most of the other vector effects apply as transformations on the element as a whole (and therefore transform the definition of all points), a non-scaling-stroke doesn't affect the coordinates used for the element.

So the question is: would it be burdensome for implementations to factor in non-scaling-stroke effects when calculating isPointInStroke ?

Extending this query to also include 'pathLength'.

Not sure how you mean? In the sense that pathLength affects stroke dashing patterns, yes, it should affect isPointInStroke.

Perhaps it would be more helpful if the method definition simply referenced the stroke-shape algorithm instead of trying to enumerate the properties that affect the stroke.

@fsoder
Copy link
Author

fsoder commented Oct 19, 2018

So the question is: would it be burdensome for implementations to factor in non-scaling-stroke effects when calculating isPointInStroke?

I wouldn't think so in general.

Not sure how you mean? In the sense that pathLength affects stroke dashing patterns...

Yes, exactly.

Perhaps it would be more helpful if the method definition simply referenced the stroke-shape algorithm instead of trying to enumerate the properties that affect the stroke.

Yes, that would make sense. non-scaling-stroke would still need to be handled explicitly though I think, so it's not a silver bullet.

@dirkschulze
Copy link
Contributor

dirkschulze commented Oct 27, 2018

I think we have an issue for exactly this question already. I’ll link it when I find it.

@fsoder in WebKit and Blink it is a purely graphical effect. The stroke gets rendered within the „root coordinate system“ and the path transformed to the actual coordinate space temporarily at render time.

Have you checked if WebKit or Blink even include it in the Hit testing code? If yes, you are right and it wouldn’t be a big thing. Otherwise, it might be less performant than the no-vector-effect case. I wonder if the difference is measurable. If it is, there might be a security issue. This should be checked as well before we include it in the calculation.

@dirkschulze dirkschulze self-assigned this Oct 27, 2018
@dirkschulze
Copy link
Contributor

dirkschulze commented Oct 27, 2018

Here a test: https://codepen.io/krit/pen/ePXmwx Firefox and Blink do take non-scaling-stroke into account for hit testing.

@fsoder
Copy link
Author

fsoder commented Oct 28, 2018

Yes, I'm aware of what hit-testing and existing implementations do wrt vector-effect - which I was curious about why it had been left out for isPointInStroke.

@dirkschulze
Copy link
Contributor

@fsoder It was added on the agenda a while back but not discussed yet.

Independent of that, since it is a CSS property it would be important to know if the additional computational effort is measurable in JS. I doubt it but would it be possible for you to test?

@fsoder
Copy link
Author

fsoder commented Oct 28, 2018

I'd expect that it could be observable, but to what degree will likely depend on implementation details and the size of the input (i.e complexity of the path et.c.) I can probably do some measurements tomorrow.

@dirkschulze
Copy link
Contributor

@fsoder my concern is about possible attacking patterns with :visited pseudo selector (
https://developer.mozilla.org/en-US/docs/Web/CSS/Privacy_and_the_:visited_selector) on the privacy. I could be wrong but would like to rule it out and, if necessary, mention the risk with a fallback path for implementations. Again, we would need to think about attack patterns if there are any.

@fsoder
Copy link
Author

fsoder commented Oct 28, 2018

vector-effect would not be applied by :visited [1], so I think that should be fine (else I'd suspect stroke-dasharray could easily have the same issue.)

[1] As indicated by https://developer.mozilla.org/en-US/docs/Web/CSS/Privacy_and_the_:visited_selector#Limits_to_visited_link_styles

@dirkschulze
Copy link
Contributor

@fsoder ah Right. It does not apply. Then I don’t see a reason why it shouldn’t be part of isPointInStroke.

@css-meeting-bot
Copy link
Member

The SVG Working Group just discussed SVGGeometry.prototype.isPointInStroke vs. vector-effects, and agreed to the following:

  • RESOLUTION: vector-effects property contributes to hit-testing and `isPointInStroke`.
The full IRC log of that discussion <AmeliaBR> Topic: SVGGeometry.prototype.isPointInStroke vs. vector-effects
<AmeliaBR> github: https://github.com//issues/559
<SairusPatel> "Sairus & Dean at Adobe" should be: "Myles & Dean at Apple". I may have misspoken
<AmeliaBR> s/Sairus & Dean at Adobe/Myles & Dean at Apple/
<AmeliaBR> s/"Sairus & Dean at Adobe" should be: "Myles & Dean at Apple". I may have misspoken//
<AmeliaBR> Dirk: Question is whether vector-effect is just a visual effect or whether it actually affects geometry, and therefore hit-testing and the API.
<AmeliaBR> ... I'd like to get a resolution to adopt fsoder's recommendation: vector-effect should affect these APIs.
<AmeliaBR> Amelia: Agree that's ideal. Only concern was implementation cost, but I think browsers are already doing all the calculations so there shouldn't be much overhead.
<AmeliaBR> Dirk: I don't think there is any implementation concern.
<AmeliaBR> RESOLUTION: vector-effects property contributes to hit-testing and `isPointInStroke`.

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

4 participants