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

Add GRRLIB_matrix and transformation functions #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HTV04
Copy link
Contributor

@HTV04 HTV04 commented Feb 27, 2022

These functions make it possible to manage GRRLIB's 2D matrix and transform it directly and safely from GRRLIB.

@HTV04 HTV04 marked this pull request as draft March 1, 2022 03:27
@HTV04
Copy link
Contributor Author

HTV04 commented Mar 1, 2022

Drafted this PR for now because I am working on adding set/get matrix functionality to GRRLIB, as well as a GRRLIB_matrix type.

@HTV04 HTV04 changed the title Add 2D coordinate system transformation functions Add GRRLIB_matrix and transformation functions Mar 3, 2022
@HTV04 HTV04 marked this pull request as ready for review March 3, 2022 02:59
@HTV04 HTV04 marked this pull request as draft March 3, 2022 14:03
@HTV04
Copy link
Contributor Author

HTV04 commented Mar 3, 2022

I need to rename GRRLIB_transform to GRRLIB_matrix, but I am not home right now, so I drafted this PR again in the meantime

@HTV04 HTV04 marked this pull request as ready for review March 3, 2022 20:56
@HTV04
Copy link
Contributor Author

HTV04 commented Mar 3, 2022

Okay, I don't think I have any more changes to make. If there's anything you think I should change, let me know.

guMtxIdentity(GXmodelView2D);
guMtxTransApply(GXmodelView2D, GXmodelView2D, 0.0F, 0.0F, -100.0F);
guMtxTransApply(GXmodelView2D, GXmodelView2D, 0.0, 0.0, -100.0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: this function expects floats, double precision is not needed. So f suffix was correct (lowercase though).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to sound lazy, but doesn't the compiler handle that automatically, since the function takes float arguments? Kind of like with signed and unsigned integers, where adding a U is unnecessary in most cases

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this specific case it works, since the double value will be cut off to represent a float. (See question 2 in the linked quiz). Generally this leads to subtle differences, since the representation of both are quite different.

https://frama-c.com/2011/11/08/Floating-point-quiz.html

Copy link
Contributor Author

@HTV04 HTV04 Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I added f suffixes, even if it's just for clarity. I also squashed and rebased my PR's commits.

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.

None yet

2 participants