From 0c48d1ccb8b2e95ce606aeac0f39f3a0fbeae809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 4 Sep 2020 00:30:28 +0200 Subject: [PATCH] Shaders: repro case for a bug in Phong light direction calculation. Interestingly enough / sadly none of the tests showed a clear difference when removing the incorrect normalization, so here's a dedicated test case. Sigh. --- src/Magnum/Shaders/Test/CMakeLists.txt | 1 + src/Magnum/Shaders/Test/PhongGLTest.cpp | 45 +++++++++++++++++- .../Test/PhongTestFiles/low-light-angle.tga | Bin 0 -> 8325 bytes 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 src/Magnum/Shaders/Test/PhongTestFiles/low-light-angle.tga diff --git a/src/Magnum/Shaders/Test/CMakeLists.txt b/src/Magnum/Shaders/Test/CMakeLists.txt index 7919b6eea..805405e70 100644 --- a/src/Magnum/Shaders/Test/CMakeLists.txt +++ b/src/Magnum/Shaders/Test/CMakeLists.txt @@ -221,6 +221,7 @@ if(BUILD_GL_TESTS) PhongTestFiles/defaults.tga PhongTestFiles/instanced.tga PhongTestFiles/instanced-normal.tga + PhongTestFiles/low-light-angle.tga PhongTestFiles/shininess-black-specular.tga PhongTestFiles/shininess0-overflow.tga PhongTestFiles/shininess0.tga diff --git a/src/Magnum/Shaders/Test/PhongGLTest.cpp b/src/Magnum/Shaders/Test/PhongGLTest.cpp index 35390e929..522e75f0f 100644 --- a/src/Magnum/Shaders/Test/PhongGLTest.cpp +++ b/src/Magnum/Shaders/Test/PhongGLTest.cpp @@ -103,6 +103,7 @@ struct PhongGLTest: GL::OpenGLTester { void renderObjectId(); #endif + void renderLowLightAngle(); void renderZeroLights(); void renderInstanced(); @@ -379,7 +380,8 @@ PhongGLTest::PhongGLTest() { &PhongGLTest::renderObjectIdTeardown); #endif - addTests({&PhongGLTest::renderZeroLights}, + addTests({&PhongGLTest::renderLowLightAngle, + &PhongGLTest::renderZeroLights}, #ifndef MAGNUM_TARGET_GLES2 &PhongGLTest::renderObjectIdSetup, &PhongGLTest::renderObjectIdTeardown @@ -1327,6 +1329,47 @@ void PhongGLTest::renderObjectId() { } #endif +void PhongGLTest::renderLowLightAngle() { + GL::Mesh plane = MeshTools::compile(Primitives::planeSolid()); + + Matrix4 transformation = + Matrix4::translation({0.0f, 0.0f, -2.0f})* + Matrix4::rotationX(-75.0_degf)* + Matrix4::scaling(Vector3::yScale(10.0f)); + + /* The light position is at the camera location, so the most light should + be there and not at some other place. This is a repro case for a bug + where lightDirection = normalize(lightPosition - transformedPosition) + in the vertex shader, where the incorrect normalization caused the + fragment-interpolated light direction being incorrect, most visible with + long polygons and low light angles. */ + Phong{{}, 1} + .setLightPosition({0.0f, 0.1f, 0.0f}) + .setShininess(200) + .setTransformationMatrix(transformation) + .setNormalMatrix(transformation.normalMatrix()) + .setProjectionMatrix(Matrix4::perspectiveProjection(80.0_degf, 1.0f, 0.1f, 20.0f)) + .draw(plane); + + MAGNUM_VERIFY_NO_GL_ERROR(); + + if(!(_manager.loadState("AnyImageImporter") & PluginManager::LoadState::Loaded) || + !(_manager.loadState("TgaImporter") & PluginManager::LoadState::Loaded)) + CORRADE_SKIP("AnyImageImporter / TgaImageImporter plugins not found."); + + #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) + const Float maxThreshold = 0.0f, meanThreshold = 0.0f; + #else + /* WebGL 1 doesn't have 8bit renderbuffer storage, so it's way worse */ + const Float maxThreshold = 0.0f, meanThreshold = 0.0f; + #endif + CORRADE_COMPARE_WITH( + /* Dropping the alpha channel, as it's always 1.0 */ + Containers::arrayCast(_framebuffer.read(_framebuffer.viewport(), {PixelFormat::RGBA8Unorm}).pixels()), + Utility::Directory::join(_testDir, "PhongTestFiles/low-light-angle.tga"), + (DebugTools::CompareImageToFile{_manager, maxThreshold, meanThreshold})); +} + void PhongGLTest::renderZeroLights() { CORRADE_COMPARE(_framebuffer.checkStatus(GL::FramebufferTarget::Draw), GL::Framebuffer::Status::Complete); diff --git a/src/Magnum/Shaders/Test/PhongTestFiles/low-light-angle.tga b/src/Magnum/Shaders/Test/PhongTestFiles/low-light-angle.tga new file mode 100644 index 0000000000000000000000000000000000000000..e26497c00add67ef6f410eeb4e258501f5342741 GIT binary patch literal 8325 zcmc(h>9bVD8OEV1R;p6bgpyRjy|v_yuU1#oGA^DK5>gws`bU(lIylF>#xtAJ$vrl zx%21GU$}7Ln{U3kc=6)5-+p`P(xuCnFaNDtu3WkD-FM&py;{Ei{`-Ga%Rj5-U)6H; z>eWCkUAuNIqcsMNX=7nm9?+9)!A9DVtvP^WIE*7XnBx(F6a*m)aY#fcauJPm0Kh1m zJbCh?k3ORF$tRyUkZ8XA^2-|n5)=9;&J=Z5&>w!#Z%j4-z+%ev10Y5W4uD`}BOZiq z$B!RBapD9WStFR#n(Kj)L1>~N&JBv1Gl*JYh;5*6%r=UE)wsaJl=fY42pPfq76hV^ z4geU%4?q0y*s)`DWKF$T`%$1=#ED5*QG6MQKx+&clLkXq5!?_&8ci|-=>ULHc>n$P zKltDSIT(z^h2>iOe0f-VIS8Nw?V5NM4dW3oZ&+e~NepUY^ECEl|GAQ;(*2chuJJMX;v z?z@74%wf_?YI2oRtY z-hA`T!-o(5k5H*u&ZW;$lwCo7pf!e?lnkl<8}gzS0r@}_4;?!6#v5;34-R3PyR!+r zTW^ZzdEd#b+^r|S z94^XkiSogSZJ^&R#mG6*CJd&esjdvEg#iSd=6(D2z5MdaIk4AXf89gyt+(D{3%~o= zJ-z;jLbuG>bPwt;kq8aM5+H%r7&0b}Wexf()h35baTG)#g%lSG5(zB%A`~!yDel>` zXYbyNXEC;7Qdr^5Af4Dk`Mn-Fl8k0>5t@P%q^~CeUW6W(qq-+rk2!Mc7*sx*4#*G^{ zZQ3L-ndZ?bw$!xt*=pT0Bxi`vfQW6N-%X!%cjJ0U6l2O3k1^RI7!Uvfr?__Q+I8#J ztzW;M4x??FmdmtyzIj-3k?1ZFp^@EX^aomF$e3*Opi0x7xN(zA z<;s<-R;^mIW{p$HE>U@orHYYZ8aP09_{r@3A)Nf6+9 zqR^5dP67fT;QV^ovSrJcFJG}@1^v~lSBp({{U}>{Jz`qwo9;<{00g-N0^_GKXiPV{ z6Firi;i+DaLY80wVv&q+VA%&>Nk;auAspS9w3}Bi=2w9f3wzk$I!%m)e*ZlrS;nEutSrqt$ zs85_ByaYg?HHM5yV_8FbPo>F`X1XE^5gu9YhjRb`1~APpzWCxxFTG@DB5P@BNy(Nh zS)!0sp0R0~Y7WQowS2Nr5o!o80T5`7A!E`2XyEqoaH@q+o{K{e2qKi61G(Y=01RLX z^XJch;e{6#ELgB`;X?W;S!$b;`6c3TIrIEN5u%|tLw1Jz*arH>RFmZA!pS2lX9mYR z1hP^NL?aylFn}pO|NQfF=FFKpckaA-^XNNS&NheR{Rl3-Oca*9Pm1_-Tk2zAI9Wz{ zpf!e!$woi-3|WCxVIUp^K)@-^oH_HUr=FTMYu4=9 zvs1j(w07{Y-mOClMUbh70m&tlXS9ZJW3o|s!1~mLG?3Z^kQFHavJnpgAjQWYfBcCj zo_O-fCuhu*dpF@poGF=R{{%es0ddorhakcLq``G`h30ALg! zee}_3)22Q4*kjYDPrrVwTGu?`bX6)kg%~f*vSbHFs&|mFtgD234!ic+@JZi(Dj(5E z2LO!XLk~Ul@WT&3^2j4orc9YSb?Oa~rMc#CN;+MY^3M=kMs}bzhKxyLSyu&2D%?Z= zdbA@K(MSgX4C~aXQ@6>JCqMY$gY}^5IhH1w=b7kqnJ6v;QcH-=XpJFb(pc72>B^HX zTQOhz*8Roj#3LH%5KuB|mkARlOq@7z(xgdk64g58Vdx3wIcDUbp{xag8B${#`+@N; z63SX>^%!TrG)_2>lD=*!Mj$c~i{?OWrP{c0<9;=M{P-L~&X+XaJeibFVlk2}pr?$o z*arH>l(Fbmy+_5cgypp5EhR%950VjJQZQ^q1=`7afl>>xqx>90&Dax?;vff@?B7E#O8TStx@Icn6X(W6KI zwOW{!+FHR16-A3=8|P&lTS7o=1ASx4SY#}}8PZgMYtUI7%Fzfw$71Rd>Mw>38}^rK z89sdYh!G>0rAT2;q;wja#$d=esLXk>jr|bES#mQ(ew?4ko@~w$I*UU&JHC&l)J{W& z47vaQ`~O@mLx&D!mg1@;EY&7?f!!+{mC+hQ#v#Ts;{oGoW0RZMPWy5I$8cDMx}5s+ zL4yYUv04TX9{i_jVF87e6jx(xk+D^UEn^$#8?(kDW4ZC@PuN0xu_=2mqpqa>bl|{& zzpIwtSIZx&g*8SEJP~+s85_j5+K+QAGL{<;c4PzgSwUS*z3JY2@BK}+{I*&K3>aY4 zaNm9RWvs05L~LU}#5ooj%RBNIPp_h`rP`f$-Wl$?>#lJ3-FLU#bI&~$rehoXaXik) zb?d32PoF+n-@biY`t|Eq?%%(EY-2x;$N7!ajy-$!?A5DRpf2j&yLamycia)%jDDPA z#RlpoYIghWx3B5ZqepCGe+xBq@7{gYZMWSP+t}Ym?N}%jipAneYW&H_R_YGwiiU=U Z*zTfMI(P0I+db6MEw|hf+w=d2`fq1-1l|Au literal 0 HcmV?d00001