abey79 / vsketch

Generative plotter art environment for Python
https://vsketch.readthedocs.io/en/latest/
Other
486 stars 50 forks source link

Ellipse mode radius changes behavior of radius labelled argument #352

Open R4chel opened 1 year ago

R4chel commented 1 year ago

I would expect that these two lines of code would produce the same output vsk.circle(0,0,radius=5) vsk.circle(0,0,radius=5, mode="radius") but was suprised to find that vsk.circle(0,0,radius=5, mode="radius") draws a circle with radius 10!

I created a repo to write a test and explore this outside the context of the complex code where I had come across it: https://github.com/R4chel/vsketch-circle-mode-debugging And here is an output image from that showing the problem. The lines are drawn from the center of the circle to a point distance radius away. image

R4chel commented 1 year ago

Another issue with vsk.circle if the radius / diameter is a negative number it draws a triangle! vsk.circle(0,0,-1)

Screenshot 2023-02-09 at 9 54 02 PM
R4chel commented 1 year ago

vsk.rect() with negative values also does strange things

R4chel commented 1 year ago

I found the line responsible for drawing triangles when the radius is negative.

https://github.com/abey79/vpype/blob/15ae8110eac724226d1050ea88f4f9301a39cbec/vpype/primitives.py#L131 n = max(3, math.ceil((stop - start) / 180 * math.pi * max(rw, rh) / quantization))

if radius (rw and rh) are negative the whole right side will be negative and n will be 3.

R4chel commented 1 year ago

I think it's not immediately obvious what vsk.cricle(0,0,-1) should draw. Here is a start of potential answers

abey79 commented 1 year ago

I think the clean way would be to draw with the absolute value and invert the circle rotation direction. This latter point would be completely pointless in the current situation IMO, so I'd drop it. I'm actually thinking of the future of vpype/vsketch data model, with first class support for arcs, bezier, etc. When/if that happens, it'll be easy (and more meaningful) to do the "right thing".