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

In Shape, have default impls for everything except path_elements #263

Open
derekdreery opened this issue Apr 6, 2023 · 2 comments · May be fixed by #293
Open

In Shape, have default impls for everything except path_elements #263

derekdreery opened this issue Apr 6, 2023 · 2 comments · May be fixed by #293
Labels
enhancement New feature or request needs discussion This change requires some more discussion before we decide we definitely want it

Comments

@derekdreery
Copy link
Collaborator

IIUC it should be possible to implement all other methods in terms of a Bézier path approximation.

@derekdreery derekdreery added enhancement New feature or request needs discussion This change requires some more discussion before we decide we definitely want it labels Apr 6, 2023
@raphlinus
Copy link
Contributor

raphlinus commented Apr 6, 2023

Generally I think this is a good idea, but there are challenges. My memory on this is not crisp, but I think one reason we didn't do this in the past is that the path_elements method had a tendency to allocate. This was fixed as of 0.8, gated by GATs landing in Rust.

One challenge is that some of the methods (area, winding, bounding_box) do not take an accuracy parameter, as they assume that analytical solutions are possible. That is true for some curve types (cubic Béziers) but not others (Euler spirals). I'm honestly not sure what's the best approach here, and think it deserves some consideration.

Adding an accuracy parameter degrades ergonomics and provides no value when the shape is a Bézier path. Yet, the trait is subtly wrong for curve types that do not admit analytical solutions. Is there a way to fix this without degrading the existing Bézier experience?

@george-steel
Copy link

I would say this would make Shape much more useful if all of the methods took accuracy parameters which could be ignored if an analytic solution exists.

I would also suggest exposing the internal versions of these methods on PathSeg (possibly adding their meanings for composable non-closed paths to the documentation) instead of the 0-returning placeholders. Having pub and non-pub methods on a type with the same name and different semantics is quite confusing.

@george-steel george-steel linked a pull request Jul 3, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion This change requires some more discussion before we decide we definitely want it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants