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

arc function: glitches #17

Closed
CatrielD opened this issue Feb 23, 2020 · 4 comments
Closed

arc function: glitches #17

CatrielD opened this issue Feb 23, 2020 · 4 comments

Comments

@CatrielD
Copy link
Contributor

CatrielD commented Feb 23, 2020

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
Copy link
Owner

joakin commented Feb 26, 2020

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
Copy link
Owner

joakin commented Feb 26, 2020

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
Copy link
Owner

joakin commented Feb 26, 2020

I think I've found what is happening.

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

:: CE.moveTo (x + cos startAngle) (y + sin startAngle)

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 joakin closed this as completed in 79eb62e Feb 29, 2020
joakin added a commit that referenced this issue Feb 29, 2020
@joakin
Copy link
Owner

joakin commented Feb 29, 2020

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants