Arcs being drawn asymmetrically

[ecridge]

Arcs seem to be drawn asymmetrically if their start and end angles are not both multiples of QUARTER_PI. The arc is drawn correctly for the first half but deviates quite significantly from a circle between the middle and the end.

[workergnome]

Paging @christinedt!

[workergnome]

@joecridge—I know that we've been trying to get the arc drawing code right, and it's turning out to be harder than we thought. @christinedt took a pass through it at the p5jscon, and fixed a problem with it, but apparently we've still got issues.

Do you have any interest in looking into this yourself? I think @christinedt is currently busy on other projects, so I don't think that she'd have an issue with you tackling it yourself. If so, let me know, and I can dump what I know about the issue on you. If not, no worries—just wanted to know that we've acknowledged this, and we're looking for someone to work on it.

[ecridge]

@workergnome Okay, yes I'd be up for giving it a go. I've been using Processing for a long time but I've only recently started to look into its code and p5.js, so any pointers you could give me would be great.

[workergnome]

Wonderful!

The arc drawing code is in the /src/core/2d_primitives.js file. There are also comments there referencing the sources for the math there—a couple papers.

I know that @christinedt found a revised paper fixing some of the existing math, and it seemed to help with some previous problems, and she listed the revision in a comment, but it might not work.

It might also be worth looking at the Java processing code, seeing if their implementation is somehow different and better. I haven't done that.

If it turns our that our current technique is totally untenable, we're open to suggestions and complete revisions, as long as there aren't API changes.

Thank you again for looking into this, and if you have any questions feel free to ping me in a comment here!

[workergnome]

Also—Have you done the github pull request dance yet? If you have questions about that, let me know as well. I'm supposed to have written up instructions, but I haven't quite finished them.

[ecridge]

Okay great, I'll see what I can do.

I haven't had a go at the dance yet, any tips?

[workergnome]

Pull Requests:

We love pull requests! To learn move about how this works, read GitHub's excellent documentation. We also have some notes of our own on our wiki. All changes to the code need to be submitted as pull requests against the master branch of the p5 repo. DO NOT merge your code directly into the main repo. That's just rude.

We recommend typical GitHub best practices for pull requests. This means submitting a:
* single pull request for each feature or bug,
* related to an existing issue,
* from a branch on your fork of the repo,
* preferably with a single commit,
* with a good title and a descriptive comment,
* mentioning all related issues,
* that merges cleanly,
* and that passes all tests.

With your pull request, along with the code, add tests, documentation, and examples. If this is a new feature or an enhancement, these are required. For bugs, or fixes to existing code, this is important, but we will not refuse a fix just because the documentation is bad. However, for both bugs and features, make sure the all tests pass. This means getting set up with grunt and running the grunt default task from within our repo. See the Development Setup for more information on how to do this.

If you can't figure out how to do any part of this, don't let that stop you from submitting a pull request. We aren't code snobs, and we'd rather have your input and help than have you do this perfectly. You can also always ask us for help—we'll do our best to walk you through the process, and explain as well as we can. Teaching you how to do this makes our lives easier, and means that the world has another person who helps with open source, which is even more important than p5.js!

[ecridge]

Great, thanks. I'm working on a fix at the mo so I'll hopefully send a PR in the next day or two.

[workergnome]

Fantastic. If you have questions, let me know, and thank you so much.

[ecridge]

Hey, I'm pretty much done with this now, but before I submit a pull request there are a couple of minor API changes that I'd like to raise for discussion – let me know what you think.

1. Currently, arcs that cross the x-axis are drawn with their start and stop angles swapped relative to what the user specifies. E.g., using arc(x, y, w, h, radians(315), radians(45), PIE) to draw a 90 degree arc centred on the x-axis yields the below result.
   
   If it's okay with you, I'd like to correct this so that the arc always starts at angle start and ends at stop, regardless of axes crossed or angular convention used (p5 already differs from Processing here in that Processing wouldn't render such an arc at all).
2. The documentation describes start and stop as the beginning and end angles of an arc, but currently this is only true only when w=h. For example, the ‘45 degree’ angles that these arcs start and finish at are clearly not parallel to the 45 degree red lines (this also makes it difficult to rotate elliptical arcs at constant speeds).
   
   This is fairly trivial to fix (and I think it would be useful for anyone using p5 to draw scientific or mathematical diagrams) but it would introduce an inconsistency between Processing and p5, and obviously mean that some existing sketches would be displayed differently.

[lmccart]

interesting... i think both changes makes sense to go forward with

[workergnome]

I agree—we should note in a change log that this happened, but both of those changes seem to make arc drawing more intuitive than what we have right now.