Skip to content

Implement a proper unit test for RectUtil and make it actually work right.#166

Merged
VirxEC merged 2 commits into
RLBot:masterfrom
Veeno:RectUtil-fix
Jun 3, 2026
Merged

Implement a proper unit test for RectUtil and make it actually work right.#166
VirxEC merged 2 commits into
RLBot:masterfrom
Veeno:RectUtil-fix

Conversation

@Veeno
Copy link
Copy Markdown
Contributor

@Veeno Veeno commented Jun 3, 2026

I said:

I think it's incredibly unlikely that this approach fails to either find an exact solution, or one which rounds to the exact pixel count. However, this would need to be more thoroughly tested (I can't guarantee it for certain at this point).

Given that I've already implemented a unit test, you'd think it'd have crossed my mind to implement another one to actually test this properly. Alas, it didn't, so I only did that now.

It turned out that I was wrong - the lookup table actually failed to accurately produce many rectangles (even below 1000px range). So I made ApproximateRect just calculate the exact solution instead. Out of curiosity, I tried to not even use the lookup table as a backup strategy. It turns out that all rectangles from 1×1 up to 7680×7680 (and probably beyond) do have an exact solution (for Rendering.FontWidthPixels and Rendering.FontHeightPixels) that fits in the 30000 string length limit.

So I just dropped the lookup table entirely and renamed ApproximateRect to RectSolve.

To my credit, I did optimize the GCD function. So at least we've got that from this whole adventure. 👍

@Veeno
Copy link
Copy Markdown
Contributor Author

Veeno commented Jun 3, 2026

Sorry, forgot to change test names, so decided to do that as well.

@VirxEC VirxEC merged commit 4368da6 into RLBot:master Jun 3, 2026
3 checks passed
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.

3 participants