MeshTools: disallow impl-spec index types in combine*Attributes().

src/Magnum/MeshTools/Combine.cpp

@@ -127,6 +127,10 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayView<const Conta
         CORRADE_ASSERT(data[i]->isIndexed(),
             "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<void*>(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::ArrayView<const Conta
             CORRADE_ASSERT(data[i]->indexCount() == 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<void*>(meshIndexTypeUnwrap(meshIndexType)),
+        (Trade::MeshData{MeshPrimitive{}, 0}));
     const UnsignedInt meshIndexSize = meshIndexTypeSize(mesh.indexType());
-    const UnsignedInt faceIndexSize = faceAttributes.isIndexed() ?
+    UnsignedInt faceIndexSize;
-        meshIndexTypeSize(faceAttributes.indexType()) : 4;
+    if(faceAttributes.isIndexed()) {
+        const MeshIndexType faceIndexType = faceAttributes.indexType();
+        CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(faceIndexType),
+            "MeshTools::combineFaceAttributes(): face mesh has an implementation-specific index type" << reinterpret_cast<void*>(meshIndexTypeUnwrap(faceIndexType)),
+            (Trade::MeshData{MeshPrimitive{}, 0}));
+        faceIndexSize = meshIndexTypeSize(faceAttributes.indexType());
+    } else faceIndexSize = 4;
     const UnsignedInt indexStride = meshIndexSize + faceIndexSize;
     Containers::Array<char> combinedIndices{NoInit, meshIndexCount*indexStride};
     Utility::copy(mesh.indices(),

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
+index count. All inputs have to be indexed. For non-indexed attributes
-@ref MeshIndexType doesn't matter. For non-indexed attributes combining can be
+combining can be done much more efficiently using @ref duplicate(const Trade::MeshData&, Containers::ArrayView<const Trade::MeshAttributeData>),
-done much more efficiently using @ref duplicate(const Trade::MeshData&, Containers::ArrayView<const Trade::MeshAttributeData>),
 alternatively you can turn a non-indexed attribute to an indexed one first
-using @ref removeDuplicatesInPlace() and then call this function. Attributes in
+using @ref removeDuplicatesInPlace() and then call this function. Index buffers
-all meshes are expected to not have an implementation-specific format.
+and attributes in all meshes are expected to not have an
-@see @ref isVertexFormatImplementationSpecific()
+implementation-specific format.
+@see @ref isMeshIndexTypeImplementationSpecific(),
+    @ref isVertexFormatImplementationSpecific()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> 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
+be interleaved. Index buffers and attributes in both meshes are expected to not
-implementation-specific format.
+have an implementation-specific format.
-@see @ref isInterleaved(), @ref isVertexFormatImplementationSpecific()
+@see @ref isInterleaved(), @ref isMeshIndexTypeImplementationSpecific(),
+    @ref isVertexFormatImplementationSpecific()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade::MeshData& faceAttributes);
 

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<const void>{}},
+        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<const void>{}},
+        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<const void>{}},
+        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");