joakin / elm-canvas

A canvas drawing library for Elm
https://package.elm-lang.org/packages/joakin/elm-canvas/latest/
BSD 3-Clause "New" or "Revised" License
163 stars 30 forks source link

arc function: glitches #17

Closed CatrielD closed 4 years ago

CatrielD commented 4 years ago

I was attempting to replicate p5 examples, namely pie chart.

Found a glitch with clockwise=True. The center of the arc seems a little off the provided point.

using clockwise induces a glitch. Using anti-clockwise doesn't, but it's really confusing.

joakin commented 4 years ago

Oh wow, it seems like the center is off and the background is showing through, weird!

With white background it is easier to see https://ellie-app.com/88GPvsYsQ5ya1

I also can't reproduce with just JS (https://jsbin.com/qujazixiqo/edit?js,output) so the library is doing something wrong 😕

joakin commented 4 years ago

I've looked at the commands produced by that ellie example and they are

["moveTo 361,200", "moveTo 360,201", "moveTo 359,200"]

Which is very strange, the center seems to keep moving around 😕

joakin commented 4 years ago

I think I've found what is happening.

For some reason, I did this (I'm not sure what I was trying to do):

https://github.com/joakin/elm-canvas/blob/8dba3dea98ed777e3ca15e19f33771f0edbc2d85/src/Canvas.elm#L559

Where it seems to me that just

 :: CE.moveTo x y

Is what it should do...

This got me thinking, that with the current arc shape you can't really do everything you could (see the mdn example 2).

Arc by moving to the center to be nice isn't really behaving as it should for fill. It is really a partial circle, more than an arc.

What do you think if we fixing Arc to not move to the center by default? That way we make arc behave like the JS version and enable creating more shapes.

For accomplishing your use case then you would have to move to the point manually though by using a path first, like this:

        shapes
            [ fill Color.red ]
            [ path pos []
            , arc pos radius { sides | startAngle = 0, endAngle = degrees 90 }
            ]

This would also be a breaking change but not change the API so I'm not sure how we handle that.

I would appreciate your thoughts.

joakin commented 4 years ago

I've published 4.1.0 with the fixed arc function. Before the starting point calculation was broken because it wasn't multiplying by the radius. Now arc behaves like the primitive arc (see MDN).

I also added an example rendering pie slices to verify the behaviour and ensure your use case was doable with the library. See:

https://github.com/joakin/elm-canvas/blob/master/examples/PieChart.elm

And the live example https://chimeces.com/elm-canvas/pie-chart.html which for some reason github is refusing to show for me.

Hope this helps, let me know if you find any other isssues, and thank you for your help!