MeshTools: make interleave() aware of impl-spec index types.

As implementation-specific index types cause the index buffer to be
strided. which is not allowed by default, these work only if
InterleaveFlag::PreserveStridedIndices is set. Originally I thought
about straight-out disallowing implementation-specific types here,
unfortunately that broke some valid use cases so I backtracked on that
decision.

In the future I might change the default to preserve
implementation-specific indices with sane strides (i.e., positive powers
of 2), but that will only be done once similar treatment is finished for
attributes (currently zero and negative strides are just disallowed
completely).

src/Magnum/MeshTools/Interleave.cpp

@@ -265,11 +265,10 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView<c
     Trade::MeshIndexData indices;
     if(data.isIndexed()) {
         const MeshIndexType indexType = data.indexType();
-        const std::size_t indexTypeSize = meshIndexTypeSize(indexType);
 
         /* If we can steal the data and we're allowed to preserve a strided
            layout or it's tightly packed, do the steal */
-        if((data.indexDataFlags() & Trade::DataFlag::Owned) && ((flags & InterleaveFlag::PreserveStridedIndices) || data.indexStride() == Int(indexTypeSize))) {
+        if((data.indexDataFlags() & Trade::DataFlag::Owned) && ((flags & InterleaveFlag::PreserveStridedIndices) || (!isMeshIndexTypeImplementationSpecific(indexType) && data.indexStride() == Int(meshIndexTypeSize(indexType))))) {
             indices = Trade::MeshIndexData{indexType,
                 Containers::StridedArrayView1D<const void>{
                     data.indexData(),
@@ -291,8 +290,14 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView<c
                     data.indexStride()}};
             Utility::copy(data.indexData(), indexData);
 
-        /* Otherwise, make a tightly packed copy */
+        /* Otherwise, make a tightly packed copy, in which case we can't have
+           an implementation-specific index type */
         } else {
+            CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(indexType),
+                "MeshTools::interleave(): mesh has an implementation-specific index type" << reinterpret_cast<void*>(meshIndexTypeUnwrap(indexType)) << Debug::nospace << ", enable MeshTools::InterleaveFlag::PreserveStridedIndices to pass the array through unchanged",
+                (Trade::MeshData{MeshPrimitive{}, 0}));
+
+            const std::size_t indexTypeSize = meshIndexTypeSize(indexType);
             indexData = Containers::Array<char>{NoInit, data.indexCount()*indexTypeSize};
             Containers::StridedArrayView2D<char> out{indexData,
                 {data.indexCount(), indexTypeSize},

src/Magnum/MeshTools/Interleave.h

@@ -219,12 +219,14 @@ enum class InterleaveFlag: UnsignedInt {
      *
      * If not set and the index buffer is strided, a tightly packed copy with
      * the same index type is allocated for the output, dropping also any
-     * padding before or after the original index view.
+     * padding before or after the original index view. In such case however,
+     * the index type is not allowed to be implementation-specific.
      *
      * Has no effect when passed to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView<const Trade::MeshAttributeData>, InterleaveFlags) "interleavedLayout()"
      * as that function doesn't preserve the index buffer. Has no effect when
      * passed to @ref concatenate(Containers::ArrayView<const Containers::Reference<const Trade::MeshData>>, InterleaveFlags) "concatenate()"
      * as that function allocates a new combined index buffer anyway.
+     * @see @ref isMeshIndexTypeImplementationSpecific()
      */
     PreserveStridedIndices = 1 << 1
 };
@@ -361,8 +363,9 @@ it, see its documentation for detailed behavior description. Note that
 offset-only @ref Trade::MeshAttributeData instances are not supported in the
 @p extra array.
 
-Indices (if any) are kept as-is only if they're tightly packed. Otherwise the
-behavior depends on presence of @ref InterleaveFlag::PreserveStridedIndices.
+Indices (if any) are kept as-is only if they're tightly packed and not with an
+implementation-specific type. Otherwise the behavior depends on presence of
+@ref InterleaveFlag::PreserveStridedIndices.
 
 Expects that each attribute in @p extra has either the same amount of elements
 as @p data vertex count or has none. This function will unconditionally make a
@@ -373,7 +376,8 @@ to avoid that copy.
 All attributes in both @p data and @p extra are expected to not have an
 implementation-specific format, except for @p data attributes in case @p data
 is already interleaved, then the layout is untouched.
-@see @ref isInterleaved(), @ref isVertexFormatImplementationSpecific(),
+@see @ref isInterleaved(), @ref isMeshIndexTypeImplementationSpecific(),
+    @ref isVertexFormatImplementationSpecific(),
     @ref Trade::MeshData::attributeData()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, Containers::ArrayView<const Trade::MeshAttributeData> extra = {}, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes);

src/Magnum/MeshTools/Test/InterleaveTest.cpp

@@ -86,6 +86,7 @@ struct InterleaveTest: Corrade::TestSuite::Tester {
 
     void interleaveMeshData();
     void interleaveMeshDataIndexed();
+    void interleaveMeshDataImplementationSpecificIndexType();
     void interleaveMeshDataImplementationSpecificVertexFormat();
     void interleaveMeshDataExtra();
     void interleaveMeshDataExtraEmpty();
@@ -112,13 +113,15 @@ const struct {
 
 const struct {
     const char* name;
+    MeshIndexType indexType;
     Containers::Optional<InterleaveFlags> flags;
     bool flip;
     bool shouldPreserveLayoutInCopy, shouldPreserveLayoutInMove;
 } StridedIndicesData[]{
-    {"", {}, false, false, true},
-    {"strided indices", {}, true, false, false},
-    {"strided indices, preserved", InterleaveFlag::PreserveInterleavedAttributes|InterleaveFlag::PreserveStridedIndices, true, true, true}
+    {"", MeshIndexType::UnsignedShort, {}, false, false, true},
+    {"strided indices", MeshIndexType::UnsignedShort, {}, true, false, false},
+    {"strided indices, preserved", MeshIndexType::UnsignedShort, InterleaveFlag::PreserveInterleavedAttributes|InterleaveFlag::PreserveStridedIndices, true, true, true},
+    {"strided indices, implementation-specific index type, preserved", meshIndexTypeWrap(0xbaf), InterleaveFlag::PreserveInterleavedAttributes|InterleaveFlag::PreserveStridedIndices, true, true, true}
 };
 
 InterleaveTest::InterleaveTest() {
@@ -171,7 +174,8 @@ InterleaveTest::InterleaveTest() {
     addInstancedTests({&InterleaveTest::interleaveMeshDataIndexed},
         Containers::arraySize(StridedIndicesData));
 
-    addTests({&InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat,
+    addTests({&InterleaveTest::interleaveMeshDataImplementationSpecificIndexType,
+              &InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat,
               &InterleaveTest::interleaveMeshDataExtra,
               &InterleaveTest::interleaveMeshDataExtraEmpty,
               &InterleaveTest::interleaveMeshDataExtraOriginalEmpty,
@@ -1192,7 +1196,7 @@ void InterleaveTest::interleaveMeshDataIndexed() {
 
     Vector2 positions[]{{1.3f, 0.3f}, {0.87f, 1.1f}, {1.0f, -0.5f}};
     Trade::MeshData mesh{MeshPrimitive::TriangleFan,
-        {}, Containers::arrayView(indexData), Trade::MeshIndexData{indices},
+        {}, Containers::arrayView(indexData), Trade::MeshIndexData{data.indexType, indices},
         {}, Containers::arrayView(positions), {
             Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::arrayView(positions)}
         }};
@@ -1204,8 +1208,8 @@ void InterleaveTest::interleaveMeshDataIndexed() {
     CORRADE_COMPARE(interleaved.primitive(), MeshPrimitive::TriangleFan);
 
     CORRADE_VERIFY(interleaved.isIndexed());
-    CORRADE_COMPARE(interleaved.indexType(), MeshIndexType::UnsignedShort);
-    CORRADE_COMPARE_AS(interleaved.indices<UnsignedShort>(),
+    CORRADE_COMPARE(interleaved.indexType(), data.indexType);
+    CORRADE_COMPARE_AS((Containers::arrayCast<1, const UnsignedShort>(interleaved.indices())),
         Containers::arrayView<UnsignedShort>({0, 2, 1}),
         TestSuite::Compare::Container);
 
@@ -1227,6 +1231,23 @@ void InterleaveTest::interleaveMeshDataIndexed() {
         TestSuite::Compare::Container);
 }
 
+void InterleaveTest::interleaveMeshDataImplementationSpecificIndexType() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    Trade::MeshData data{MeshPrimitive::Points,
+        nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D<const void>{}},
+        nullptr, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector2, nullptr}
+        }};
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MeshTools::interleave(data);
+    CORRADE_COMPARE(out.str(), "MeshTools::interleave(): mesh has an implementation-specific index type 0xcaca, enable MeshTools::InterleaveFlag::PreserveStridedIndices to pass the array through unchanged\n");
+}
+
 void InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat() {
     #ifdef CORRADE_NO_ASSERT
     CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
@@ -1438,7 +1459,7 @@ void InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveIndices() {
     const Trade::MeshAttributeData* attributePointer = attributeData;
 
     Trade::MeshData mesh{MeshPrimitive::TriangleFan,
-        std::move(indexData), Trade::MeshIndexData{indices},
+        std::move(indexData), Trade::MeshIndexData{data.indexType,indices},
         std::move(vertexData), std::move(attributeData)};
     CORRADE_VERIFY(MeshTools::isInterleaved(mesh));
 
@@ -1448,8 +1469,8 @@ void InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveIndices() {
         MeshTools::interleave(std::move(mesh));
 
     CORRADE_VERIFY(MeshTools::isInterleaved(interleaved));
-    CORRADE_COMPARE(interleaved.indexType(), MeshIndexType::UnsignedShort);
-    CORRADE_COMPARE_AS(interleaved.indices<UnsignedShort>(),
+    CORRADE_COMPARE(interleaved.indexType(), data.indexType);
+    CORRADE_COMPARE_AS((Containers::arrayCast<1, const UnsignedShort>(interleaved.indices())),
         Containers::arrayView<UnsignedShort>({0, 2, 1}),
         TestSuite::Compare::Container);