From 0c7e5c53e7dcb08ebf1273c7bd8775967ff5b84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 6 Oct 2024 17:02:54 +0200 Subject: [PATCH] Shaders: 1.0/0.0 doesn't produce +inf on NVidia, but a NaN. This caused MeshToolsCompileGLTest to fail in a strange way, and PhongGLTest::renderLowLightAngle() as well. Which looked rare enough that I first suspected some random driver bug, but apparently it was all caused by these using the default infinity range instead of explicitly calling setLightRanges() on the shader. The test is now updated to explicitly verify the default value when a setter isn't called, to catch this problem better in case it reappears in a different form elsewhere. --- src/Magnum/Shaders/PhongGL.cpp | 7 ++++- src/Magnum/Shaders/Test/PhongGLTest.cpp | 41 +++++++++++++++++++------ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/Magnum/Shaders/PhongGL.cpp b/src/Magnum/Shaders/PhongGL.cpp index 99df66dc6..7ab6bde8f 100644 --- a/src/Magnum/Shaders/PhongGL.cpp +++ b/src/Magnum/Shaders/PhongGL.cpp @@ -224,7 +224,12 @@ PhongGL::CompileState PhongGL::compile(const Configuration& configuration) { "#define LIGHT_RANGE_INITIALIZER {}\n", ("vec4(0.0, 0.0, 1.0, 0.0), "_s*configuration.lightCount()).exceptSuffix(2), ("vec3(1.0), "_s*configuration.lightCount()).exceptSuffix(2), - ("1.0/0.0, "_s*configuration.lightCount()).exceptSuffix(2)); + /* While 1.0/0.0 works on certain drivers such as Mesa and produces + +inf, on NVidia it produces a NaN. Working around it by making + the divisor non-zero. The PhongGLTest::renderLights() then + verifies that both the default and explicitly passed + Constants::pi() give the same result. */ + ("1.0/0.000000000000000000000001, "_s*configuration.lightCount()).exceptSuffix(2)); #endif GL::Shader vert{version, GL::Shader::Type::Vertex}; diff --git a/src/Magnum/Shaders/Test/PhongGLTest.cpp b/src/Magnum/Shaders/Test/PhongGLTest.cpp index f0723eee5..7d10b8724 100644 --- a/src/Magnum/Shaders/Test/PhongGLTest.cpp +++ b/src/Magnum/Shaders/Test/PhongGLTest.cpp @@ -724,7 +724,7 @@ const struct { Vector4 position; Color3 specularColor, lightSpecularColor; Float intensity; - Float range; + Containers::Optional range; /* Constants::inf() if not set */ Containers::Array> picks; } RenderLightsData[] { {"directional", "light-directional.tga", @@ -746,7 +746,13 @@ const struct { {10.0f, -15.0f, 5.0f, 0.0f}, Color3{1.0f}, Color3{1.0f}, 1.0f, Constants::inf(), {}}, /* Range should have no effect either, especially zero range should not - cause any NaNs */ + cause any NaNs. Default or explicit infinity shouldn't either. */ + {"directional, range left at (infinity) default", "light-directional.tga", + {1.0f, -1.5f, 0.5f, 0.0f}, Color3{1.0f}, Color3{1.0f}, + 1.0f, {}, {}}, + {"directional, range=inf", "light-directional.tga", + {1.0f, -1.5f, 0.5f, 0.0f}, Color3{1.0f}, Color3{1.0f}, + 1.0f, Constants::inf(), {}}, {"directional, range=0.1", "light-directional.tga", {1.0f, -1.5f, 0.5f, 0.0f}, Color3{1.0f}, Color3{1.0f}, 1.0f, 1.0f, {}}, @@ -818,6 +824,13 @@ const struct { {"point, intensity=10, range=1.0", "light-point-intensity10-range1.0.tga", {0.75f, -0.75f, -0.75f, 1.0f}, Color3{1.0f}, Color3{1.0f}, 10.0f, 1.0f, {}}, + /* These two should produce the same result */ + {"point, range left at (infinity) default", "light-point.tga", + {0.75f, -0.75f, -0.75f, 1.0f}, Color3{1.0f}, Color3{1.0f}, + 1.0f, {}, {}}, + {"point, range=inf", "light-point.tga", + {0.75f, -0.75f, -0.75f, 1.0f}, Color3{1.0f}, Color3{1.0f}, + 1.0f, Constants::inf(), {}}, /* Range ends right at the surface, so no contribution */ {"point, range=0.75", "light-none.tga", {0.75f, -0.75f, -0.75f, 1.0f}, Color3{1.0f}, Color3{1.0f}, @@ -3987,12 +4000,17 @@ template void PhongGLTest::renderLights() { .setLightPositions({data.position}) .setLightColors({0xff8080_rgbf*data.intensity}) .setLightSpecularColors({data.lightSpecularColor}) - .setLightRanges({data.range}) .setShininess(60.0f) .setTransformationMatrix(transformation) .setNormalMatrix(transformation.normalMatrix()) - .setProjectionMatrix(Matrix4::perspectiveProjection(80.0_degf, 1.0f, 0.1f, 20.0f)) - .draw(plane); + .setProjectionMatrix(Matrix4::perspectiveProjection(80.0_degf, 1.0f, 0.1f, 20.0f)); + /* Also testing a case where it's left at the default infinity value + embedded in the shader code or passed directly during construction + --- it should not cause any difference compared to passing + Constants::inf(). */ + if(data.range) + shader.setLightRanges({*data.range}); + shader.draw(plane); } #ifndef MAGNUM_TARGET_GLES2 else if(flag == PhongGL::Flag::UniformBuffers @@ -4012,12 +4030,15 @@ template void PhongGLTest::renderLights() { GL::Buffer drawUniform{GL::Buffer::TargetHint::Uniform, { PhongDrawUniform{}.setNormalMatrix(transformation.normalMatrix()) }}; + PhongLightUniform lightUniformData; + lightUniformData + .setPosition({data.position}) + .setColor(0xff8080_rgbf*data.intensity) + .setSpecularColor(data.lightSpecularColor); + if(data.range) + lightUniformData.setRange(*data.range); GL::Buffer lightUniform{GL::Buffer::TargetHint::Uniform, { - PhongLightUniform{} - .setPosition({data.position}) - .setColor(0xff8080_rgbf*data.intensity) - .setSpecularColor(data.lightSpecularColor) - .setRange(data.range), + lightUniformData }}; GL::Buffer materialUniform{GL::Buffer::TargetHint::Uniform, { PhongMaterialUniform{}