Fix ellipseMode(CORNERS) and rectMode(CORNER) #7290
Conversation
|
🎉 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! |
|
Hi, Thanks for taking up this task. I believe I have tested this and it works well. I have a concern: are you planning to write tests for it in the If so, I don't think that approach would work for your case. Perhaps we could write unit or visual tests instead. What's your opinion? |
| rect(x1, y1, x2, y2); | ||
| } | ||
| } catch (e) { | ||
| console.warn(`Error drawing (${x1}, ${y1}, ${x2}, ${y2})`); |
There was a problem hiding this comment.
If you are trying to write tests for it? You could probably refer to this file, this could help you to write tests for it?
https://github.com/processing/p5.js/blob/main/contributor_docs/unit_testing.md
Let me know if this file is not related to testing purpose.
There was a problem hiding this comment.
Hi, thanks for getting back to me! Yes, I used this file to test the bug, and create the Screenshots above. Thanks for pointing me to the visual test docs, this will be a good approach for testing correct rendering. I'll make an attempt at these test and will be updating the PR.
Resolves #7289
Changes:
This mainly resolves a rendering/crashing issues with
ellipseMode(CORNERS)affectingellipse()andarc(); see issue above). It also fixes a related issue with usingrectMode(CORNER)and negative widths and/or heights.The bulk of the changes are in
modeAdjust()which should now properly handle the modes and return a normalized bounding box. As a result a few lines of handling negative widths and heights needed to be removed fromarc()and_renderEllipse().I've included a test sketch in test/shape-modes.html where you can cycle through all the different modes and shapes. I'm happy to update this, if somebody could direct me on how to properly test this case. I'm not sure, since it's mostly about the rendering output.
Screenshots of the change:
PR Checklist
npm run lintpasses