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

GPU: Handle bad fog params as large signed vals #11380

Merged
merged 1 commit into from
Sep 10, 2018

Conversation

unknownbrackets
Copy link
Collaborator

From tests, it seems they're just treated as valid exponents.

Using 65535 since that's the range of depth, can't think of a game using a larger value for a fog parameter, probably never even this large.

Did some more fog tests, I think I had a mistake before when I thought softgpu fog was reversed - but since I was there, I validated how inf/nan behave. It seems like they act just like vfpu min/max - the exponent is just not treated as special.

This could use validation against MediEvil and any other affected games, though. But this matches tests.

-[Unknown]

From tests, it seems they're just treated as valid exponents.

Using 65535 since that's the range of depth, can't think of a game using a
larger value for a fog parameter, probably never even this large.
@unknownbrackets unknownbrackets added this to the v1.7.0 milestone Sep 10, 2018
@hrydgard
Copy link
Owner

This is probably just as fine as the previous value of 100000 for coef 0, but it's conceivable that games would not want the fog to fully fade in at the end of depth. That would be pretty strange usage though, so.... let's try it.

@hrydgard hrydgard merged commit 6ffa5bd into hrydgard:master Sep 10, 2018
@unknownbrackets unknownbrackets deleted the softgpu branch September 10, 2018 14:28
@unknownbrackets
Copy link
Collaborator Author

Right. I'm not opposed to a different constant - was just extending it to handle -NAN and INF for both registers. I actually didn't notice that 100000 wasn't just 10000 used in the INF branch of the prev code (note: both were only for coef 1.)

-[Unknown]

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