search

linebender / kurbo

A Rust library for manipulating curves
Apache License 2.0
697 stars 67 forks source link

add triangle shape #350

Open aymey opened 3 months ago

aymey commented 3 months ago

Implements (at least starts to) a triangle shape, im not sure if this is useful enough to merge though.

Also while looking at the source i noticed that that places like here use Rect instead of Self unlike other parts of the codebase. And some parts of the documentation uses "[`Type`]" and other parts just use "Type". Wasnt sure to make an issue or not but should there be a cleanup of the codebase?

also there is alot of conversion between Vec2 and Point right now. is there a better way to handle these situations where i want to use Vec2 methods on a Point but will end up converting right back to a Point afterwards?

triangles have alot of opportunities for more specific methods so its worth noting that this can be heavily expanded upon which could come later. Aswell as this, i have made some decisions like using centroids as the origin of the triangle (https://web.archive.org/web/20131104015950/http://www.jimloy.com/geometry/centers.htm) which might not be the best? let me know.

ill continue working on this, specifically the TODOs (see inlined todos)

aymey commented 3 months ago

Should we add an Insets for triangle?

aymey commented 3 months ago

i realise that changes to other source files like vec2.rs should be another pr but right no this pr depends on them so ill keep it here for no. if this doesn't get merged then ill split my fork and make a pr just for those other source files.

aymey commented 3 months ago

and with 8b03193f6f932807b723c9d693a4010ba5e5076d this pr should be ready for a review!

aymey commented 3 months ago

on a side note i also think it would be useful to have from/to_triangle/circle/rect(), cause we do it for Ellipse already we may aswell do it for all other shapes which would also be super easy to implement. prob should be another pr tho

aymey commented 4 weeks ago

Ive double checked and it looks like my incircle and circumcircle implementations are correct yet when i check them in desmos.com the incircle radius is too small and the circumcircle radius is too large. Have i done something wrong?

also can the circumcenter function be more efficient?

aymey commented 2 weeks ago

do you think there should be an as_triangle in "shape.rs"?

raphlinus commented 2 weeks ago

do you think there should be an as_triangle in "shape.rs"?

Probably not needed. The main purpose of those methods is fast paths for renderers, and triangle is not (unlike circle, rect, etc) not a common specialized shape - for example, it's not in SVG or the Canvas API.