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

Rename struct/aabb2d.h functions to match aabb2d.h, add tests #442

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

duarm
Copy link
Contributor

@duarm duarm commented Jan 20, 2025

closes #429

Problem

Missing _sizev and _diag from structs/aabb2d

Solution

Rename _size to _diag, add missing _sizev, I've also added a test file for the aabb2d api

Discussion

The struct API usually returns the struct instead of a dest parameter, aabb2d and box however, still have a dest parameter on some functions, as we do not have an aabb2d struct, just an array. Since the _sizev is a new function, I've opted for returning the vec2s instead of a dest parameter. Should I keep it this way or make it a dest parameter for consistency?

void glms_aabb2d_sizev(vec2s aabb[2], vec2s *dest);
# or
vec2s glms_aabb2d_sizev(vec2s aabb[2]);

@recp
Copy link
Owner

recp commented Jan 20, 2025

Hi @duarm,

No dest param in structure api, it looks good!

Since it seems Draft, can we merge it?

@duarm duarm marked this pull request as ready for review January 20, 2025 21:35
@duarm
Copy link
Contributor Author

duarm commented Jan 20, 2025

I was planning on adding more tests, but that can be done later, let's merge

@recp recp merged commit 26e174d into recp:master Jan 21, 2025
49 of 74 checks passed
@recp
Copy link
Owner

recp commented Jan 21, 2025

@duarm the PR is merged, thanks for your contributions 🚀

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.

aabb2d Struct API missing diag and sizev functions from Array API
2 participants