From 4036c32c6b4ebaee701c0072281a1e34ce46e749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 6 Jun 2022 18:47:43 +0200 Subject: [PATCH] Trade: verify *MaterialData::hasTextureCoordinates() does what docs say. In particular, returning true only if some coordinate set index is non-zero -- the main use case is to know whether we need a complex shader with multiple coordinate attributes or not. Only PhongMaterialData do that, others not. --- .../Trade/Test/FlatMaterialDataTest.cpp | 30 +++++++++++++++++++ .../Test/PbrClearCoatMaterialDataTest.cpp | 23 +++++++++++++- .../PbrMetallicRoughnessMaterialDataTest.cpp | 22 +++++++++++++- .../PbrSpecularGlossinessMaterialDataTest.cpp | 22 +++++++++++++- .../Trade/Test/PhongMaterialDataTest.cpp | 22 +++++++++++++- 5 files changed, 115 insertions(+), 4 deletions(-) diff --git a/src/Magnum/Trade/Test/FlatMaterialDataTest.cpp b/src/Magnum/Trade/Test/FlatMaterialDataTest.cpp index 824db621c..999b9b9d3 100644 --- a/src/Magnum/Trade/Test/FlatMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/FlatMaterialDataTest.cpp @@ -46,6 +46,7 @@ class FlatMaterialDataTest: public TestSuite::Tester { void texturedBaseColorSingleMatrixCoordinates(); void texturedDiffuseColorSingleMatrixCoordinates(); void texturedMismatchedMatrixCoordinates(); + void texturedImplicitCoordinates(); void invalidTextures(); }; @@ -59,6 +60,7 @@ FlatMaterialDataTest::FlatMaterialDataTest() { &FlatMaterialDataTest::texturedBaseColorSingleMatrixCoordinates, &FlatMaterialDataTest::texturedDiffuseColorSingleMatrixCoordinates, &FlatMaterialDataTest::texturedMismatchedMatrixCoordinates, + &FlatMaterialDataTest::texturedImplicitCoordinates, &FlatMaterialDataTest::invalidTextures}); } @@ -243,6 +245,34 @@ void FlatMaterialDataTest::texturedMismatchedMatrixCoordinates() { } } +void FlatMaterialDataTest::texturedImplicitCoordinates() { + { + FlatMaterialData data{{}, { + {MaterialAttribute::BaseColorTexture, 5u}, + {MaterialAttribute::BaseColorTextureCoordinates, 0u}, + + /* This is ignored because it doesn't match the texture */ + {MaterialAttribute::DiffuseTextureCoordinates, 2u}, + }}; + + CORRADE_VERIFY(data.hasTexture()); + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_COMPARE(data.textureCoordinates(), 0); + } { + FlatMaterialData data{{}, { + {MaterialAttribute::DiffuseTexture, 5u}, + {MaterialAttribute::DiffuseTextureCoordinates, 0u}, + + /* This is ignored because it doesn't match the texture */ + {MaterialAttribute::BaseColorTextureCoordinates, 2u}, + }}; + + CORRADE_VERIFY(data.hasTexture()); + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_COMPARE(data.textureCoordinates(), 0); + } +} + void FlatMaterialDataTest::invalidTextures() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); diff --git a/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp b/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp index d9297eac0..a6c0d3c36 100644 --- a/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp @@ -47,6 +47,7 @@ class PbrClearCoatMaterialDataTest: public TestSuite::Tester { void commonTransformationCoordinatesNoTextures(); void commonTransformationCoordinatesOneTexture(); void commonTransformationCoordinatesOneDifferentTexture(); + void commonCoordinatesImplicit(); void noCommonTransformationCoordinates(); }; @@ -69,7 +70,8 @@ PbrClearCoatMaterialDataTest::PbrClearCoatMaterialDataTest() { addInstancedTests({ &PbrClearCoatMaterialDataTest::commonTransformationCoordinatesOneTexture, - &PbrClearCoatMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture}, + &PbrClearCoatMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture, + &PbrClearCoatMaterialDataTest::commonCoordinatesImplicit}, Containers::arraySize(PbrClearCoatTextureData)); addTests({&PbrClearCoatMaterialDataTest::noCommonTransformationCoordinates}); @@ -398,6 +400,25 @@ void PbrClearCoatMaterialDataTest::commonTransformationCoordinatesOneDifferentTe CORRADE_VERIFY(!data.hasCommonTextureCoordinates()); } +void PbrClearCoatMaterialDataTest::commonCoordinatesImplicit() { + Containers::StringView textureName = PbrClearCoatTextureData[testCaseInstanceId()]; + setTestCaseDescription(textureName); + + /* The transformation doesn't have this behavior, because there checking an + identity is rather expensive */ + + PbrClearCoatMaterialData data{{}, { + {MaterialLayer::ClearCoat}, + {textureName, 5u}, + {std::string{textureName} + "Coordinates", 0u} + }, {0, 3}}; + + /* Zero is treated same as if there would be no attribute at all */ + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_VERIFY(data.hasCommonTextureCoordinates()); + CORRADE_COMPARE(data.commonTextureCoordinates(), 0u); +} + void PbrClearCoatMaterialDataTest::noCommonTransformationCoordinates() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); diff --git a/src/Magnum/Trade/Test/PbrMetallicRoughnessMaterialDataTest.cpp b/src/Magnum/Trade/Test/PbrMetallicRoughnessMaterialDataTest.cpp index 35a598f9c..ea03bd3dc 100644 --- a/src/Magnum/Trade/Test/PbrMetallicRoughnessMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/PbrMetallicRoughnessMaterialDataTest.cpp @@ -52,6 +52,7 @@ class PbrMetallicRoughnessMaterialDataTest: public TestSuite::Tester { void commonTransformationCoordinatesNoTextures(); void commonTransformationCoordinatesOneTexture(); void commonTransformationCoordinatesOneDifferentTexture(); + void commonCoordinatesImplicit(); void noCommonTransformationCoordinates(); }; @@ -80,7 +81,8 @@ PbrMetallicRoughnessMaterialDataTest::PbrMetallicRoughnessMaterialDataTest() { addInstancedTests({ &PbrMetallicRoughnessMaterialDataTest::commonTransformationCoordinatesOneTexture, - &PbrMetallicRoughnessMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture}, + &PbrMetallicRoughnessMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture, + &PbrMetallicRoughnessMaterialDataTest::commonCoordinatesImplicit}, Containers::arraySize(PbrMetallicRoughnessTextureData)); addTests({&PbrMetallicRoughnessMaterialDataTest::noCommonTransformationCoordinates}); @@ -819,6 +821,24 @@ void PbrMetallicRoughnessMaterialDataTest::commonTransformationCoordinatesOneDif CORRADE_VERIFY(!data.hasCommonTextureCoordinates()); } +void PbrMetallicRoughnessMaterialDataTest::commonCoordinatesImplicit() { + Containers::StringView textureName = PbrMetallicRoughnessTextureData[testCaseInstanceId()]; + setTestCaseDescription(textureName); + + /* The transformation doesn't have this behavior, because there checking an + identity is rather expensive */ + + PbrMetallicRoughnessMaterialData data{{}, { + {textureName, 5u}, + {std::string{textureName} + "Coordinates", 0u} + }}; + + /* Zero is treated same as if there would be no attribute at all */ + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_VERIFY(data.hasCommonTextureCoordinates()); + CORRADE_COMPARE(data.commonTextureCoordinates(), 0u); +} + void PbrMetallicRoughnessMaterialDataTest::noCommonTransformationCoordinates() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); diff --git a/src/Magnum/Trade/Test/PbrSpecularGlossinessMaterialDataTest.cpp b/src/Magnum/Trade/Test/PbrSpecularGlossinessMaterialDataTest.cpp index b63293439..528d90791 100644 --- a/src/Magnum/Trade/Test/PbrSpecularGlossinessMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/PbrSpecularGlossinessMaterialDataTest.cpp @@ -49,6 +49,7 @@ class PbrSpecularGlossinessMaterialDataTest: public TestSuite::Tester { void commonTransformationCoordinatesNoTextures(); void commonTransformationCoordinatesOneTexture(); void commonTransformationCoordinatesOneDifferentTexture(); + void commonCoordinatesImplicit(); void noCommonTransformationCoordinates(); }; @@ -74,7 +75,8 @@ PbrSpecularGlossinessMaterialDataTest::PbrSpecularGlossinessMaterialDataTest() { addInstancedTests({ &PbrSpecularGlossinessMaterialDataTest::commonTransformationCoordinatesOneTexture, - &PbrSpecularGlossinessMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture}, + &PbrSpecularGlossinessMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture, + &PbrSpecularGlossinessMaterialDataTest::commonCoordinatesImplicit}, Containers::arraySize(PbrSpecularGlossinessTextureData)); addTests({&PbrSpecularGlossinessMaterialDataTest::noCommonTransformationCoordinates}); @@ -535,6 +537,24 @@ void PbrSpecularGlossinessMaterialDataTest::commonTransformationCoordinatesOneDi CORRADE_VERIFY(!data.hasCommonTextureCoordinates()); } +void PbrSpecularGlossinessMaterialDataTest::commonCoordinatesImplicit() { + Containers::StringView textureName = PbrSpecularGlossinessTextureData[testCaseInstanceId()]; + setTestCaseDescription(textureName); + + /* The transformation doesn't have this behavior, because there checking an + identity is rather expensive */ + + PbrSpecularGlossinessMaterialData data{{}, { + {textureName, 5u}, + {std::string{textureName} + "Coordinates", 0u} + }}; + + /* Zero is treated same as if there would be no attribute at all */ + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_VERIFY(data.hasCommonTextureCoordinates()); + CORRADE_COMPARE(data.commonTextureCoordinates(), 0u); +} + void PbrSpecularGlossinessMaterialDataTest::noCommonTransformationCoordinates() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); diff --git a/src/Magnum/Trade/Test/PhongMaterialDataTest.cpp b/src/Magnum/Trade/Test/PhongMaterialDataTest.cpp index f27822df5..9f37b3e12 100644 --- a/src/Magnum/Trade/Test/PhongMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/PhongMaterialDataTest.cpp @@ -58,6 +58,7 @@ class PhongMaterialDataTest: public TestSuite::Tester { void commonTransformationCoordinatesNoTextures(); void commonTransformationCoordinatesOneTexture(); void commonTransformationCoordinatesOneDifferentTexture(); + void commonCoordinatesImplicit(); void noCommonTransformationCoordinates(); #ifdef MAGNUM_BUILD_DEPRECATED @@ -96,7 +97,8 @@ PhongMaterialDataTest::PhongMaterialDataTest() { addInstancedTests({ &PhongMaterialDataTest::commonTransformationCoordinatesOneTexture, - &PhongMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture}, + &PhongMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture, + &PhongMaterialDataTest::commonCoordinatesImplicit}, Containers::arraySize(PhongTextureData)); addTests({ @@ -583,6 +585,24 @@ void PhongMaterialDataTest::commonTransformationCoordinatesOneDifferentTexture() #endif } +void PhongMaterialDataTest::commonCoordinatesImplicit() { + Containers::StringView textureName = PhongTextureData[testCaseInstanceId()]; + setTestCaseDescription(textureName); + + /* The transformation doesn't have this behavior, because there checking an + identity is rather expensive */ + + PhongMaterialData data{{}, { + {textureName, 5u}, + {std::string{textureName} + "Coordinates", 0u} + }}; + + /* Zero is treated same as if there would be no attribute at all */ + CORRADE_VERIFY(!data.hasTextureCoordinates()); + CORRADE_VERIFY(data.hasCommonTextureCoordinates()); + CORRADE_COMPARE(data.commonTextureCoordinates(), 0u); +} + void PhongMaterialDataTest::noCommonTransformationCoordinates() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");