MeshTools: disallow impl-spec index types in concatenate().

src/Magnum/MeshTools/Concatenate.cpp

@@ -120,6 +120,10 @@ Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedI
 
         /* If the mesh is indexed, copy the indices over, expanded to 32bit */
         if(mesh.isIndexed()) {
+            CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(mesh.indexType()),
+                assertPrefix << "mesh" << i << "has an implementation-specific index type" << reinterpret_cast<void*>(meshIndexTypeUnwrap(mesh.indexType())),
+                (Trade::MeshData{MeshPrimitive{}, 0}));
+
             Containers::ArrayView<UnsignedInt> dst = indices.slice(indexOffset, indexOffset + mesh.indexCount());
             mesh.indicesInto(dst);
             indexOffset += mesh.indexCount();

src/Magnum/MeshTools/Concatenate.h

@@ -50,9 +50,10 @@ namespace Implementation {
 @m_since{2020,06}
 
 The returned mesh contains vertices from all meshes concatenated together. If
-any mesh is indexed, the resulting mesh is indexed as well, with indices
+any mesh is indexed (expected to not have an implementation-specific index
-adjusted for vertex offsets of particular meshes. The behavior is undefined if
+type), the resulting mesh is indexed as well, with indices adjusted for vertex
-any mesh has indices out of bounds for its particular vertex count. Meshes with
+offsets of particular meshes. The behavior is undefined if any mesh has indices
+out of bounds for its particular vertex count. Meshes with
 @ref MeshPrimitive::LineStrip, @ref MeshPrimitive::LineLoop,
 @ref MeshPrimitive::TriangleStrip and @ref MeshPrimitive::TriangleFan can't be
 concatenated --- use @ref generateIndices() to turn them into
@@ -76,7 +77,8 @@ description.
 If an index buffer is needed, @ref MeshIndexType::UnsignedInt is always used.
 Call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result
 to compress it to a smaller type, if desired.
-@see @ref concatenateInto(), @ref isVertexFormatImplementationSpecific(),
+@see @ref concatenateInto(), @ref isMeshIndexTypeImplementationSpecific(),
+    @ref isVertexFormatImplementationSpecific(),
     @ref SceneTools::flattenMeshHierarchy2D(),
     @ref SceneTools::flattenMeshHierarchy3D()
 */

src/Magnum/MeshTools/Test/ConcatenateTest.cpp

@@ -51,6 +51,7 @@ struct ConcatenateTest: TestSuite::Tester {
     void concatenateInconsistentPrimitive();
     void concatenateInconsistentAttributeFormat();
     void concatenateInconsistentAttributeArraySize();
+    void concatenateImplementationSpecificIndexType();
     void concatenateImplementationSpecificVertexFormat();
     void concatenateIntoNoMeshes();
 };
@@ -81,6 +82,7 @@ ConcatenateTest::ConcatenateTest() {
               &ConcatenateTest::concatenateInconsistentPrimitive,
               &ConcatenateTest::concatenateInconsistentAttributeFormat,
               &ConcatenateTest::concatenateInconsistentAttributeArraySize,
+              &ConcatenateTest::concatenateImplementationSpecificIndexType,
               &ConcatenateTest::concatenateImplementationSpecificVertexFormat,
               &ConcatenateTest::concatenateIntoNoMeshes});
 }
@@ -668,6 +670,42 @@ void ConcatenateTest::concatenateInconsistentAttributeArraySize() {
         "MeshTools::concatenateInto(): expected array size 5 for attribute 2 (Trade::MeshAttribute::Custom(42)) but got 4 in mesh 3 attribute 1\n");
 }
 
+void ConcatenateTest::concatenateImplementationSpecificIndexType() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    /* Things are a bit duplicated to test correct numbering */
+    Trade::MeshData a{MeshPrimitive::Lines, nullptr, {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position,
+            VertexFormat::Vector3, nullptr},
+    }};
+    const Trade::MeshData b{MeshPrimitive::Lines,
+        nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D<const void>{}},
+        nullptr, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position,
+                VertexFormat::Vector3, nullptr},
+        }};
+    Trade::MeshData bDestination{MeshPrimitive::Lines,
+        nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D<const void>{}},
+        nullptr, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position,
+                VertexFormat::Vector3, nullptr},
+        }};
+
+    /* This is fine, as the mesh index buffer is cleared and replaced with
+       a tightly-packed 32bit buffer */
+    MeshTools::concatenateInto(bDestination, {a});
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MeshTools::concatenate({a, b});
+    MeshTools::concatenateInto(a, {b});
+    CORRADE_COMPARE(out.str(),
+        "MeshTools::concatenate(): mesh 1 has an implementation-specific index type 0xcaca\n"
+        "MeshTools::concatenateInto(): mesh 0 has an implementation-specific index type 0xcaca\n");
+}
+
 void ConcatenateTest::concatenateImplementationSpecificVertexFormat() {
     #ifdef CORRADE_NO_ASSERT
     CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");