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

Improve precision for vp computation #212

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

Conversation

toxieainc
Copy link
Contributor

...to fix regression for at least SRally2 and HarleyD

fixes/improves #201

@dukeeeey
Copy link
Collaborator

dukeeeey commented Oct 13, 2024

Are these functionally the same?

float x = 2.0F / (right - left);
float y = 2.0F / (top - bottom);
float a = (right + left) / (right - left);
float b = (top + bottom) / (top - bottom);

float x = 2.0f / (right - left);
float y = 2.0f / (top - bottom);
float a = left*x + 1.0f;
float b = bottom*y + 1.0f;

Some games use off-axis projection, harley etc. Also a small request :) If you could put less changes in each pull request lol. It's hard to know what has really changed with all the factoring.

Also c++ has automatic type promotion. It's really not necessary to cast an int to a float when multiplying by another float. I mean, you can, but it happens automatically anyway. But maybe this is just style bitching :)

@toxieainc
Copy link
Contributor Author

I personally prefer it like that, so that its clear whats happening. And in theory you can loose precision, depending on the integer value.

And the code is the same, yes. This way you can share more of the same potential error.

@toxieainc
Copy link
Contributor Author

As for putting additional stuff into such PRs: Sorry, i will try to be more careful then.
I have a lot of local changes piled up, so if some are in the same file as my 'actual fix', and these are rather harmless, then i tend to just squeeze them in, too.

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