diff --git a/src/Magnum/MeshTools/Combine.cpp b/src/Magnum/MeshTools/Combine.cpp index 7f0792e5e..d7194421a 100644 --- a/src/Magnum/MeshTools/Combine.cpp +++ b/src/Magnum/MeshTools/Combine.cpp @@ -40,14 +40,25 @@ namespace Magnum { namespace MeshTools { namespace { -Trade::MeshData combineIndexedImplementation(const MeshPrimitive primitive, Containers::Array& combinedIndices, const UnsignedInt indexCount, const UnsignedInt indexStride, const Containers::ArrayView> data) { +Trade::MeshData combineIndexedImplementation( + #ifndef CORRADE_NO_ASSERT + const char* assertPrefix, + #endif + const MeshPrimitive primitive, Containers::Array& combinedIndices, const UnsignedInt indexCount, const UnsignedInt indexStride, const Containers::ArrayView> data) +{ /* Calculate attribute count and vertex stride */ UnsignedInt attributeCount = 0; UnsignedInt vertexStride = 0; for(std::size_t i = 0; i != data.size(); ++i) { attributeCount += data[i]->attributeCount(); - for(UnsignedInt j = 0; j != data[i]->attributeCount(); ++j) - vertexStride += vertexFormatSize(data[i]->attributeFormat(j))*Math::max(data[i]->attributeArraySize(j), UnsignedShort{1}); + for(UnsignedInt j = 0; j != data[i]->attributeCount(); ++j) { + const VertexFormat format = data[i]->attributeFormat(j); + CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), + assertPrefix << "attribute" << j << "of mesh" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), + (Trade::MeshData{MeshPrimitive::Points, 0})); + + vertexStride += vertexFormatSize(format)*Math::max(data[i]->attributeArraySize(j), UnsignedShort{1}); + } } /* Make the combined index array unique */ @@ -149,7 +160,11 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data) { @@ -203,8 +218,11 @@ Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade:: Utility::copy(combinedFaceIndices[0], combinedFaceIndices[2]); /* Then combine the two into a single buffer */ - return combineIndexedImplementation(mesh.primitive(), combinedIndices, - meshIndexCount, indexStride, + return combineIndexedImplementation( + #ifndef CORRADE_NO_ASSERT + "MeshTools::combineFaceAttributes():", + #endif + mesh.primitive(), combinedIndices, meshIndexCount, indexStride, Containers::arrayView>({ mesh, faceAttributes })); diff --git a/src/Magnum/MeshTools/Combine.h b/src/Magnum/MeshTools/Combine.h index 3e2dc0a34..d6d0484bc 100644 --- a/src/Magnum/MeshTools/Combine.h +++ b/src/Magnum/MeshTools/Combine.h @@ -83,7 +83,9 @@ index count. All inputs have to be indexed, although the particular @ref MeshIndexType doesn't matter. For non-indexed attributes combining can be done much more efficiently using @ref duplicate(const Trade::MeshData&, Containers::ArrayView), alternatively you can turn a non-indexed attribute to an indexed one first -using @ref removeDuplicatesInPlace() and then call this function. +using @ref removeDuplicatesInPlace() and then call this function. Attributes in +all meshes are expected to not have an implementation-specific format. +@see @ref isVertexFormatImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data); @@ -107,8 +109,9 @@ Expects that @p mesh is indexed @ref MeshPrimitive::Triangles and latter corresponding to triangle count of the former. If @p faceAttributes is indexed, it's assumed to have the data unique; if it's not indexed, it's first made unique using @ref removeDuplicates() and in that case it's expected to -be interleaved. -@see @ref isInterleaved() +be interleaved. Attributes in both meshes are expected to not have an +implementation-specific format. +@see @ref isInterleaved(), @ref isVertexFormatImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade::MeshData& faceAttributes); diff --git a/src/Magnum/MeshTools/Test/CombineTest.cpp b/src/Magnum/MeshTools/Test/CombineTest.cpp index bd7773618..432637cce 100644 --- a/src/Magnum/MeshTools/Test/CombineTest.cpp +++ b/src/Magnum/MeshTools/Test/CombineTest.cpp @@ -47,6 +47,7 @@ struct CombineTest: TestSuite::Tester { void combineIndexedAttributesNotIndexed(); void combineIndexedAttributesDifferentPrimitive(); void combineIndexedAttributesDifferentIndexCount(); + void combineIndexedAttributesImplementationSpecificVertexFormat(); void combineFaceAttributes(); void combineFaceAttributesMeshNotIndexed(); @@ -54,6 +55,7 @@ struct CombineTest: TestSuite::Tester { void combineFaceAttributesUnexpectedFaceCount(); void combineFaceAttributesFacesNotInterleaved(); void combineFaceAttributesFaceAttributeOffsetOnly(); + void combineFaceAttributesImplementationSpecificVertexFormat(); }; constexpr struct { @@ -72,7 +74,8 @@ CombineTest::CombineTest() { &CombineTest::combineIndexedAttributesNoMeshes, &CombineTest::combineIndexedAttributesNotIndexed, &CombineTest::combineIndexedAttributesDifferentPrimitive, - &CombineTest::combineIndexedAttributesDifferentIndexCount}); + &CombineTest::combineIndexedAttributesDifferentIndexCount, + &CombineTest::combineIndexedAttributesImplementationSpecificVertexFormat}); addInstancedTests({&CombineTest::combineFaceAttributes}, Containers::arraySize(CombineFaceAttributesData)); @@ -81,7 +84,8 @@ CombineTest::CombineTest() { &CombineTest::combineFaceAttributesUnexpectedPrimitive, &CombineTest::combineFaceAttributesUnexpectedFaceCount, &CombineTest::combineFaceAttributesFacesNotInterleaved, - &CombineTest::combineFaceAttributesFaceAttributeOffsetOnly}); + &CombineTest::combineFaceAttributesFaceAttributeOffsetOnly, + &CombineTest::combineFaceAttributesImplementationSpecificVertexFormat}); } void CombineTest::combineIndexedAttributes() { @@ -243,6 +247,31 @@ void CombineTest::combineIndexedAttributesDifferentIndexCount() { CORRADE_COMPARE(out.str(), "MeshTools::combineIndexedAttributes(): data 2 has 4 indices but expected 5\n"); } +void CombineTest::combineIndexedAttributesImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData a{MeshPrimitive::Points, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + Trade::MeshData b{MeshPrimitive::Points, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + Trade::MeshAttributeData{Trade::meshAttributeCustom(3), vertexFormatWrap(0xcaca), nullptr} + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::combineIndexedAttributes({a, b}); + CORRADE_COMPARE(out.str(), "MeshTools::combineIndexedAttributes(): attribute 2 of mesh 1 has an implementation-specific format 0xcaca\n"); +} + void CombineTest::combineFaceAttributes() { auto&& data = CombineFaceAttributesData[testCaseInstanceId()]; setTestCaseDescription(data.name); @@ -418,6 +447,48 @@ void CombineTest::combineFaceAttributesUnexpectedFaceCount() { "MeshTools::combineFaceAttributes(): expected 1 face entries for 3 indices but got 2\n"); } +void CombineTest::combineFaceAttributesImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData triangles{MeshPrimitive::Triangles, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + Trade::MeshData trianglesImplementationSpecific{MeshPrimitive::Triangles, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Color, VertexFormat::Vector4, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Tangent, vertexFormatWrap(0xcaca), nullptr} + }}; + + Trade::MeshData faces{MeshPrimitive::Faces, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + Trade::MeshData facesImplementationSpecific{MeshPrimitive::Faces, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Color, VertexFormat::Vector4, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Tangent, vertexFormatWrap(0xcaca), nullptr} + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::combineFaceAttributes(triangles, facesImplementationSpecific); + MeshTools::combineFaceAttributes(trianglesImplementationSpecific, faces); + CORRADE_COMPARE(out.str(), + "MeshTools::combineFaceAttributes(): attribute 2 of mesh 1 has an implementation-specific format 0xcaca\n" + "MeshTools::combineFaceAttributes(): attribute 2 of mesh 0 has an implementation-specific format 0xcaca\n"); +} + void CombineTest::combineFaceAttributesFacesNotInterleaved() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");