diff --git a/src/Magnum/MeshTools/Combine.cpp b/src/Magnum/MeshTools/Combine.cpp index d7194421a..e61b7eaa6 100644 --- a/src/Magnum/MeshTools/Combine.cpp +++ b/src/Magnum/MeshTools/Combine.cpp @@ -127,6 +127,10 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayViewisIndexed(), "MeshTools::combineIndexedAttributes(): data" << i << "is not indexed", (Trade::MeshData{MeshPrimitive{}, 0})); + const MeshIndexType indexType = data[i]->indexType(); + CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(indexType), + "MeshTools::combineIndexedAttributes(): data" << i << "has an implementation-specific index type" << reinterpret_cast(meshIndexTypeUnwrap(indexType)), + (Trade::MeshData{MeshPrimitive{}, 0})); if(i == 0) { primitive = data[i]->primitive(); indexCount = data[i]->indexCount(); @@ -136,7 +140,7 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayViewindexCount() == indexCount, "MeshTools::combineIndexedAttributes(): data" << i << "has" << data[i]->indexCount() << "indices but expected" << indexCount, (Trade::MeshData{MeshPrimitive{}, 0})); } - indexStride += meshIndexTypeSize(data[i]->indexType()); + indexStride += meshIndexTypeSize(indexType); } /** @todo handle alignment in the above somehow (duplicate() will fail when @@ -186,9 +190,19 @@ Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade:: (Trade::MeshData{MeshPrimitive{}, 0})); /* Make a combined index array. First copy the mesh indices as-is. */ + const MeshIndexType meshIndexType = mesh.indexType(); + CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(meshIndexType), + "MeshTools::combineFaceAttributes(): vertex mesh has an implementation-specific index type" << reinterpret_cast(meshIndexTypeUnwrap(meshIndexType)), + (Trade::MeshData{MeshPrimitive{}, 0})); const UnsignedInt meshIndexSize = meshIndexTypeSize(mesh.indexType()); - const UnsignedInt faceIndexSize = faceAttributes.isIndexed() ? - meshIndexTypeSize(faceAttributes.indexType()) : 4; + UnsignedInt faceIndexSize; + if(faceAttributes.isIndexed()) { + const MeshIndexType faceIndexType = faceAttributes.indexType(); + CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(faceIndexType), + "MeshTools::combineFaceAttributes(): face mesh has an implementation-specific index type" << reinterpret_cast(meshIndexTypeUnwrap(faceIndexType)), + (Trade::MeshData{MeshPrimitive{}, 0})); + faceIndexSize = meshIndexTypeSize(faceAttributes.indexType()); + } else faceIndexSize = 4; const UnsignedInt indexStride = meshIndexSize + faceIndexSize; Containers::Array combinedIndices{NoInit, meshIndexCount*indexStride}; Utility::copy(mesh.indices(), diff --git a/src/Magnum/MeshTools/Combine.h b/src/Magnum/MeshTools/Combine.h index d6d0484bc..98b08a1eb 100644 --- a/src/Magnum/MeshTools/Combine.h +++ b/src/Magnum/MeshTools/Combine.h @@ -79,13 +79,14 @@ function can be also called with just a single argument to compact a mesh with a sparse index buffer. Expects that @p data is non-empty and all data have the same primitive and -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), +index count. All inputs have to be indexed. 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. Attributes in -all meshes are expected to not have an implementation-specific format. -@see @ref isVertexFormatImplementationSpecific() +using @ref removeDuplicatesInPlace() and then call this function. Index buffers +and attributes in all meshes are expected to not have an +implementation-specific format. +@see @ref isMeshIndexTypeImplementationSpecific(), + @ref isVertexFormatImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data); @@ -109,9 +110,10 @@ 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. Attributes in both meshes are expected to not have an -implementation-specific format. -@see @ref isInterleaved(), @ref isVertexFormatImplementationSpecific() +be interleaved. Index buffers and attributes in both meshes are expected to not +have an implementation-specific format. +@see @ref isInterleaved(), @ref isMeshIndexTypeImplementationSpecific(), + @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 432637cce..f0b138a2d 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 combineIndexedAttributesImplementationSpecificIndexType(); void combineIndexedAttributesImplementationSpecificVertexFormat(); void combineFaceAttributes(); @@ -55,6 +56,7 @@ struct CombineTest: TestSuite::Tester { void combineFaceAttributesUnexpectedFaceCount(); void combineFaceAttributesFacesNotInterleaved(); void combineFaceAttributesFaceAttributeOffsetOnly(); + void combineFaceAttributesImplementationSpecificIndexType(); void combineFaceAttributesImplementationSpecificVertexFormat(); }; @@ -75,6 +77,7 @@ CombineTest::CombineTest() { &CombineTest::combineIndexedAttributesNotIndexed, &CombineTest::combineIndexedAttributesDifferentPrimitive, &CombineTest::combineIndexedAttributesDifferentIndexCount, + &CombineTest::combineIndexedAttributesImplementationSpecificIndexType, &CombineTest::combineIndexedAttributesImplementationSpecificVertexFormat}); addInstancedTests({&CombineTest::combineFaceAttributes}, @@ -85,6 +88,7 @@ CombineTest::CombineTest() { &CombineTest::combineFaceAttributesUnexpectedFaceCount, &CombineTest::combineFaceAttributesFacesNotInterleaved, &CombineTest::combineFaceAttributesFaceAttributeOffsetOnly, + &CombineTest::combineFaceAttributesImplementationSpecificIndexType, &CombineTest::combineFaceAttributesImplementationSpecificVertexFormat}); } @@ -247,6 +251,28 @@ void CombineTest::combineIndexedAttributesDifferentIndexCount() { CORRADE_COMPARE(out.str(), "MeshTools::combineIndexedAttributes(): data 2 has 4 indices but expected 5\n"); } +void CombineTest::combineIndexedAttributesImplementationSpecificIndexType() { + #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::MeshData b{MeshPrimitive::Points, + nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D{}}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::combineIndexedAttributes({a, b}); + CORRADE_COMPARE(out.str(), "MeshTools::combineIndexedAttributes(): data 1 has an implementation-specific index type 0xcaca\n"); +} + void CombineTest::combineIndexedAttributesImplementationSpecificVertexFormat() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); @@ -447,6 +473,42 @@ void CombineTest::combineFaceAttributesUnexpectedFaceCount() { "MeshTools::combineFaceAttributes(): expected 1 face entries for 3 indices but got 2\n"); } +void CombineTest::combineFaceAttributesImplementationSpecificIndexType() { + #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::MeshData trianglesImplementationSpecific{MeshPrimitive::Triangles, + nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D{}}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + + Trade::MeshData faces{MeshPrimitive::Faces, + nullptr, Trade::MeshIndexData{MeshIndexType::UnsignedShort, nullptr}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector3, nullptr}, + }}; + Trade::MeshData facesImplementationSpecific{MeshPrimitive::Faces, + nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D{}}, + nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::combineFaceAttributes(triangles, facesImplementationSpecific); + MeshTools::combineFaceAttributes(trianglesImplementationSpecific, faces); + CORRADE_COMPARE(out.str(), + "MeshTools::combineFaceAttributes(): face mesh has an implementation-specific index type 0xcaca\n" + "MeshTools::combineFaceAttributes(): vertex mesh has an implementation-specific index type 0xcaca\n"); +} + void CombineTest::combineFaceAttributesImplementationSpecificVertexFormat() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");