diff --git a/src/Magnum/MaterialTools/Test/PhongToPbrMetallicRoughnessTest.cpp b/src/Magnum/MaterialTools/Test/PhongToPbrMetallicRoughnessTest.cpp index 400072c30..47be5af1d 100644 --- a/src/Magnum/MaterialTools/Test/PhongToPbrMetallicRoughnessTest.cpp +++ b/src/Magnum/MaterialTools/Test/PhongToPbrMetallicRoughnessTest.cpp @@ -152,7 +152,7 @@ const struct { {Trade::MaterialAttribute::AlphaMask, 0.7f}, {Trade::MaterialLayer::ClearCoat}, {Trade::MaterialAttribute::LayerFactor, 0.35f}, - }, {2, 4}}, + }, {3, 5}}, Trade::MaterialData{Trade::MaterialType::PbrMetallicRoughness, { {Trade::MaterialAttribute::BaseColor, 0xff3366ff_rgbaf}, {Trade::MaterialAttribute::BaseColorTexture, 0u}, @@ -161,7 +161,7 @@ const struct { {Trade::MaterialAttribute::AlphaMask, 0.7f}, {Trade::MaterialLayer::ClearCoat}, {Trade::MaterialAttribute::LayerFactor, 0.35f}, - }, {4, 6}}}, + }, {5, 7}}}, {"diffuse texture properties without texture", {}, Trade::MaterialData{{}, { {Trade::MaterialAttribute::DiffuseColor, 0xff3366cc_rgbaf}, diff --git a/src/Magnum/Trade/MaterialData.cpp b/src/Magnum/Trade/MaterialData.cpp index 68e839878..94b982bb1 100644 --- a/src/Magnum/Trade/MaterialData.cpp +++ b/src/Magnum/Trade/MaterialData.cpp @@ -321,6 +321,9 @@ MaterialData::MaterialData(const MaterialTypes types, Containers::Array attributeData, const std::initializer_list layerData, const void* const importerState): MaterialData{types, Implementation::initializerListToArrayWithDefaultDeleter(attributeData), Implementation::initializerListToArrayWithDefaultDeleter(layerData), importerState} {} @@ -358,6 +361,9 @@ MaterialData::MaterialData(const MaterialTypes types, const DataFlags attributeD begin = end; } + + CORRADE_ASSERT(layerOffsets.back() == _data.size(), + "Trade::MaterialData: last layer offset" << layerOffsets.back() << "too short for" << _data.size() << "attributes in total", ); #endif } diff --git a/src/Magnum/Trade/MaterialData.h b/src/Magnum/Trade/MaterialData.h index 662c4db3a..f2b61c362 100644 --- a/src/Magnum/Trade/MaterialData.h +++ b/src/Magnum/Trade/MaterialData.h @@ -2095,7 +2095,7 @@ class MAGNUM_TRADE_EXPORT MaterialData { * The @p attributeData gets sorted by name internally, expecting no * duplicates inside each layer. The @p layerData is expected to be * either empty or a monotonically non-decreasing sequence of offsets - * not larger than @p attributeData size, with *i*-th item specifying + * counting up to @p attributeData size, with *i*-th item specifying * end offset of *i*-th layer. * * The @ref attributeDataFlags() / @ref layerDataFlags() are implicitly @@ -2123,7 +2123,7 @@ class MAGNUM_TRADE_EXPORT MaterialData { * The @p data is expected to be already sorted by name, without * duplicates inside each layer. The @p layerData is expected to be * either empty or a monotonically non-decreasing sequence of offsets - * not larger than @p attributeData size, with *i*-th item specifying + * counting up to @p attributeData size, with *i*-th item specifying * end offset of *i*-th layer. * * The @p attributeDataFlags / @p layerDataFlags parameters can contain diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 23dd7fc25..e08414e73 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -6515,7 +6515,7 @@ void AbstractImporterTest::materialNonOwningDeleters() { return MaterialData{{}, {}, attributeData, {}, layerData}; } - UnsignedInt layerData[1]{}; + UnsignedInt layerData[1]{1}; MaterialAttributeData attributeData[1]{ {MaterialAttribute::DiffuseColor, Color4{}} }; diff --git a/src/Magnum/Trade/Test/MaterialDataTest.cpp b/src/Magnum/Trade/Test/MaterialDataTest.cpp index f73a53d3d..0124385c9 100644 --- a/src/Magnum/Trade/Test/MaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/MaterialDataTest.cpp @@ -100,6 +100,7 @@ struct MaterialDataTest: TestSuite::Tester { void constructLayers(); void constructLayersNotMonotonic(); void constructLayersOffsetOutOfBounds(); + void constructLayersLastOffsetTooShort(); void constructNonOwned(); void constructNonOwnedLayers(); @@ -108,6 +109,7 @@ struct MaterialDataTest: TestSuite::Tester { void constructNonOwnedDuplicateAttribute(); void constructNonOwnedLayersNotMonotonic(); void constructNonOwnedLayersOffsetOutOfBounds(); + void constructNonOwnedLayersLastOffsetTooShort(); void constructNonOwnedAttributeFlagOwned(); void constructNonOwnedLayerFlagOwned(); @@ -267,6 +269,7 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::constructLayers, &MaterialDataTest::constructLayersNotMonotonic, &MaterialDataTest::constructLayersOffsetOutOfBounds, + &MaterialDataTest::constructLayersLastOffsetTooShort, &MaterialDataTest::constructNonOwned, &MaterialDataTest::constructNonOwnedLayers, @@ -275,6 +278,7 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::constructNonOwnedDuplicateAttribute, &MaterialDataTest::constructNonOwnedLayersNotMonotonic, &MaterialDataTest::constructNonOwnedLayersOffsetOutOfBounds, + &MaterialDataTest::constructNonOwnedLayersLastOffsetTooShort, &MaterialDataTest::constructNonOwnedAttributeFlagOwned, &MaterialDataTest::constructNonOwnedLayerFlagOwned, @@ -1511,6 +1515,20 @@ void MaterialDataTest::constructLayersOffsetOutOfBounds() { CORRADE_COMPARE(out.str(), "Trade::MaterialData: invalid range (2, 6) for layer 1 with 5 attributes in total\n"); } +void MaterialDataTest::constructLayersLastOffsetTooShort() { + CORRADE_SKIP_IF_NO_ASSERT(); + + std::ostringstream out; + Error redirectError{&out}; + MaterialData data{MaterialType::Phong, { + {MaterialAttribute::DoubleSided, true}, + {MaterialLayer::ClearCoat}, + {MaterialAttribute::LayerFactor, 0.5f}, + {MaterialAttribute::NormalTexture, 3u} + }, {1, 3}}; + CORRADE_COMPARE(out.str(), "Trade::MaterialData: last layer offset 3 too short for 4 attributes in total\n"); +} + void MaterialDataTest::constructNonOwned() { constexpr MaterialAttributeData attributes[]{ {"AmbientTextureMatrix"_s, Matrix3{{0.5f, 0.0f, 0.0f}, @@ -1700,6 +1718,28 @@ void MaterialDataTest::constructNonOwnedLayersOffsetOutOfBounds() { CORRADE_COMPARE(out.str(), "Trade::MaterialData: invalid range (2, 6) for layer 1 with 5 attributes in total\n"); } +void MaterialDataTest::constructNonOwnedLayersLastOffsetTooShort() { + CORRADE_SKIP_IF_NO_ASSERT(); + + MaterialAttributeData attributes[]{ + {MaterialAttribute::DoubleSided, true}, + {MaterialLayer::ClearCoat}, + {MaterialAttribute::LayerFactor, 0.5f}, + {MaterialAttribute::NormalTexture, 3u} + }; + + UnsignedInt layers[]{ + 1, 3 + }; + + std::ostringstream out; + Error redirectError{&out}; + MaterialData data{MaterialType::Phong, + {}, attributes, + {}, layers}; + CORRADE_COMPARE(out.str(), "Trade::MaterialData: last layer offset 3 too short for 4 attributes in total\n"); +} + void MaterialDataTest::constructNonOwnedAttributeFlagOwned() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -2953,7 +2993,7 @@ void MaterialDataTest::accessNotFoundInLayerString() { MaterialData data{{}, { {MaterialAttribute::LayerName, "ClearCoat"}, {"DiffuseColor", 0xff3366aa_rgbaf} - }, {0, 1}}; + }, {0, 2}}; /* These are fine */ CORRADE_VERIFY(!data.hasAttribute("ClearCoat", "DiffuseColour")); diff --git a/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp b/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp index bdf8434d3..bf35ae2f5 100644 --- a/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp @@ -464,7 +464,7 @@ void PbrClearCoatMaterialDataTest::commonCoordinatesLayerImplicit() { {textureName, 5u}, {textureName + "Coordinates", 0u}, {textureName + "Layer", 0u}, - }, {0, 3}}; + }, {0, 4}}; /* Zero is treated same as if there would be no attribute at all */ CORRADE_VERIFY(!data.hasTextureCoordinates());