[Bugfix] qp_ellipse overflow

[elpekenin]

Description

Not really sure if i should add the Bugfix and/or Core tags, so I'm not doing it.

Noticed that qp_ellipse does some weird drawing when somewhat-big inputs are provided. I saw this while testing my custom code to expose Quantum Painter over XAP (there might be an issue on my part too 😅).

To fix it, I've refactored the function using the mid-point ellipse algorithm, I haven't compared performance but I optimized the best that I could (no floats , reduced expressions, pre-compute values that are used all over the code).

During tests I noticed that it draws significantly worse when sizey is smaller than sizex, so a check was added to flip values if needed and it draws pretty nice now.

Note: The "tips" of the ellipse get trimmed as the sizes become bigger and the ratio between them is rather small: a cutoff line appears.

Above text is now updated, ended up re-doing the algorithm from scratch based on the parametric equation for an ellipse, using lib8tion for the trigonometric functions.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[elpekenin]

Re-reading this many months later i feel like i should have just done return false; if the helper function returns false...

[tzarc]

There doesn't seem to be a copyright notice for the code in the linked site? Not sure we can accept it from a GPL perspective.

[elpekenin]

Oh, right.. Didnt think about it back in the day.

Just made minimal searching and found no info about its licensing, guess i will look for another impl that has a clear (and compatible) license and test with that.

[elpekenin]

This draws much better than what i remember my first reimplementation doing. But I must say I'm not usually drawing ellipses...

I dont have any metrics, but it definitely is slightly slower than the current code in the repo. I'm fine personally with the tradeoff, given the current code often draws very funny shapes, and this code is much simpler.

[elpekenin]

For illustration on how it becomes very worse with size (crappy test code here)

[daskygit]

out of interest why rewrite it rather than just fixing the overflow?

[daskygit]

old implementation with the obvious overflows fixed, might have missed an edge case

new algorithm

At least to me the fixed old one looks better than the new algorithm.

[elpekenin]

out of interest why rewrite it rather than just fixing the overflow?

Just because i didnt know it was an overflow 🤣😅