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

Fix bug in normal map rendering #1418

Merged
merged 3 commits into from Jul 23, 2017

Conversation

Projects
None yet
4 participants
@The-E
Member

The-E commented Jul 20, 2017

This reapplies the fix from 8e0304a, which got overwritten at some point.

@The-E The-E added this to the Release 3.8 milestone Jul 20, 2017

@The-E The-E added the bugfix label Jul 20, 2017

@MageKing17

This comment has been minimized.

Show comment
Hide comment
@MageKing17

MageKing17 Jul 20, 2017

Member

Download of prebuilt binaries failed: "SSL connect error"!

...Are you kidding? Argh. So much for prebuilt download failures being a thing of the past.

Member

MageKing17 commented Jul 20, 2017

Download of prebuilt binaries failed: "SSL connect error"!

...Are you kidding? Argh. So much for prebuilt download failures being a thing of the past.

@asarium

This comment has been minimized.

Show comment
Hide comment
@asarium

asarium Jul 21, 2017

Member

I have no idea why that suddenly stopped working. We could try disabling the TLS verification but that is not really a good idea, especially if it's used to transfer code that gets executed.

Member

asarium commented Jul 21, 2017

I have no idea why that suddenly stopped working. We could try disabling the TLS verification but that is not really a good idea, especially if it's used to transfer code that gets executed.

@@ -291,7 +291,7 @@ void main()
// Normal map - convert from DXT5nm
vec2 normalSample;
normal.rg = normalSample = (texture(sNormalmap, texCoord).ag * 2.0) - 1.0;
normal.b = sqrt(1.0 - dot(normal.rg, normal.rg));

This comment has been minimized.

@MageKing17

MageKing17 Jul 22, 2017

Member

After conversation with Kobrar and DahBlount on Discord, the old line was the correct behavior and the fix should probably be just to clamp it to avoid values of 0 (clamping to between, say, 0.0001 and 1.0 should be safe enough).

@MageKing17

MageKing17 Jul 22, 2017

Member

After conversation with Kobrar and DahBlount on Discord, the old line was the correct behavior and the fix should probably be just to clamp it to avoid values of 0 (clamping to between, say, 0.0001 and 1.0 should be safe enough).

This comment has been minimized.

@The-E

The-E Jul 22, 2017

Member

I've made that change. @ln-zookeeper, could you test this please?

@The-E

The-E Jul 22, 2017

Member

I've made that change. @ln-zookeeper, could you test this please?

This comment has been minimized.

@ln-zookeeper

ln-zookeeper Jul 22, 2017

Member

Yes, the clamping fixes the bug as well.

@ln-zookeeper

ln-zookeeper Jul 22, 2017

Member

Yes, the clamping fixes the bug as well.

@MageKing17 MageKing17 merged commit ed3f072 into scp-fs2open:master Jul 23, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment