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

Addresses #6485 Refactor ellipse drawing logic in p5.Renderer2D.js #6499

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

tuminzee
Copy link
Contributor

Addresses #6485

Changes:

Screenshots of the change:

image image

PR Checklist

@welcome
Copy link

welcome bot commented Oct 26, 2023

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

@limzykenneth
Copy link
Member

@tuminzee Basic manual tests I've done with the fork looks fine to me. Let me know if you are ready for this to be merged or you want to implement roundedRect here as well.

@tuminzee
Copy link
Contributor Author

tuminzee commented Nov 1, 2023

ellipse is ready to be merged, but I am still working on arc, rect
What do you recommend should be the approach? Should this PR be merged, and I should make different PR for individual functions?
or should I finish all of it in this PR

@tuminzee
Copy link
Contributor Author

@GregStanton it is taking some time for me finish the other functions, should we merge this one function first, and I will complete the other functions later?

@GregStanton
Copy link
Collaborator

Hi @tuminzee! Sorry I missed your question before. Yeah, let's go ahead and merge this if you're okay with it. I just added a task list to #6485, so that others can start to work on the remaining two functions in separate pull requests. Does that work for you?

@limzykenneth limzykenneth merged commit b2c29b6 into processing:main Jan 21, 2024
2 checks passed
@limzykenneth
Copy link
Member

Looks good. Thanks!

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

Successfully merging this pull request may close these issues.

4 participants