Skip to content

mat2: add tables to docs & fix comments #469

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

Merged
merged 3 commits into from
Apr 11, 2025

Conversation

EasyIP2023
Copy link
Contributor

No description provided.

@EasyIP2023 EasyIP2023 changed the title Feature/mat2 tables mat2: add tables to docs & fix comments Apr 6, 2025
@EasyIP2023
Copy link
Contributor Author

EasyIP2023 commented Apr 6, 2025

@recp Rearranged some functions and changed parameter names for API consistency.

Question with

glms_mat2_(transpose)(mat2s m)

Do we need to break this up into

mat2s
glms_mat2_(transpose_to)(mat2s m);

void
glms_mat2_(transpose)(mat2s m);

@EasyIP2023 EasyIP2023 force-pushed the feature/mat2-tables branch from c462cc6 to 0f97854 Compare April 6, 2025 02:15
@recp
Copy link
Owner

recp commented Apr 6, 2025

There is no need to have transpose_to and any other _to versions since we always return a new struct [mat2s] from func ( for structure api ).

@EasyIP2023
Copy link
Contributor Author

@recp Okay good to know. Left that the same. Lastly wanted to update the docs format. So, that it's hopefully easier for new comers to understand the mathematics

  1. Start with function definition, parameters, etc...
  2. Given mathematical definition in table format
  3. Provide an example in table format of a code snippet

Good? Or is it a bit too much?

Screenshot from 2025-04-06 10-56-11
Screenshot from 2025-04-06 10-57-22
Screenshot from 2025-04-06 11-00-03

@recp
Copy link
Owner

recp commented Apr 8, 2025

@EasyIP2023 the docs looks good many thanks. But I'm not sure about naming about src.

m|mat -> src

for instance

glms_mat2_inv(mat2 src, mat2 dest)

we are inverting src okay but also inverting given matrix which is m. I would prefer m/mat and similar over src even it writes to dest param.

Also just realized that glms_mat2_inv() should not have dest since dest is the returned value, see glms_mat4_inv() and glms_mat3_inv()

@EasyIP2023
Copy link
Contributor Author

EasyIP2023 commented Apr 9, 2025

@recp Okay cool cool. Will change functions param names when I find time and let you know.

@EasyIP2023 EasyIP2023 force-pushed the feature/mat2-tables branch from 0f97854 to 27a3ddc Compare April 11, 2025 04:14
@EasyIP2023
Copy link
Contributor Author

@recp Believe to be ready for another review.

@recp recp merged commit 9da4e78 into recp:master Apr 11, 2025
175 of 188 checks passed
@recp
Copy link
Owner

recp commented Apr 11, 2025

@EasyIP2023 the PR is merged, I may update param names later e.g src to m...

Thanks for your contributions 🚀

@EasyIP2023 EasyIP2023 deleted the feature/mat2-tables branch April 12, 2025 02:43
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.

2 participants