diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index 55d7ce29a..803e4c840 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -265,15 +265,41 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView indexData; Trade::MeshIndexData indices; if(data.isIndexed()) { - /* If we can steal the data, do it */ - if(data.indexDataFlags() & Trade::DataFlag::Owned) { - indices = Trade::MeshIndexData{data.indices()}; + 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))) { + indices = Trade::MeshIndexData{indexType, + Containers::StridedArrayView1D{ + data.indexData(), + data.indexData().data() + data.indexOffset(), + data.indexCount(), + data.indexStride()}}; indexData = data.releaseIndexData(); - } else { - indexData = Containers::Array{data.indexData().size()}; + + /* Otherwise, if we can't steal the data but we're told to preserve + strided indices, make a full copy including any extra offsets and + paddings */ + } else if(flags & InterleaveFlag::PreserveStridedIndices) { + indexData = Containers::Array{NoInit, data.indexData().size()}; + indices = Trade::MeshIndexData{indexType, + Containers::StridedArrayView1D{ + indexData, + indexData.data() + data.indexOffset(), + data.indexCount(), + data.indexStride()}}; Utility::copy(data.indexData(), indexData); - indices = Trade::MeshIndexData{data.indexType(), - Containers::ArrayView{indexData + data.indexOffset(), data.indices().size()[0]*data.indices().size()[1]}}; + + /* Otherwise, make a tightly packed copy */ + } else { + indexData = Containers::Array{NoInit, data.indexCount()*indexTypeSize}; + Containers::StridedArrayView2D out{indexData, + {data.indexCount(), indexTypeSize}, + {std::ptrdiff_t(indexTypeSize), 1}}; + indices = Trade::MeshIndexData{out}; + Utility::copy(data.indices(), out); } } diff --git a/src/Magnum/MeshTools/Interleave.h b/src/Magnum/MeshTools/Interleave.h index 60c2e13f7..4420aabc1 100644 --- a/src/Magnum/MeshTools/Interleave.h +++ b/src/Magnum/MeshTools/Interleave.h @@ -209,7 +209,24 @@ enum class InterleaveFlag: UnsignedInt { * removing all padding. In that case an implementation-specific * @ref VertexFormat can't be used for any attribute. */ - PreserveInterleavedAttributes = 1 << 0 + PreserveInterleavedAttributes = 1 << 0, + + /** + * If a mesh is indexed, makes @ref interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags) + * preserve the index buffer even if it's not tightly packed. Since such + * data layouts are not commonly supported by GPU APIs, this flag is not + * set by default. + * + * 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. + * + * Has no effect when passed to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags) "interleavedLayout()" + * as that function doesn't preserve the index buffer. Has no effect when + * passed to @ref concatenate(Containers::ArrayView>, InterleaveFlags) "concatenate()" + * as that function allocates a new combined index buffer anyway. + */ + PreserveStridedIndices = 1 << 1 }; /** @@ -335,14 +352,17 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data @brief Interleave mesh data @m_since{2020,06} -Returns a copy of @p data with all attributes interleaved. Indices (if any) are -kept as-is. The @p extra attributes, if any, are interleaved together with -existing attributes (or, in case the attribute view is empty, only the -corresponding space for given attribute type is reserved, with memory left -uninitialized). The data layouting is done by @ref interleavedLayout() with the -@p flags parameter propagated to it, see its documentation for detailed -behavior description. Note that offset-only @ref Trade::MeshAttributeData -instances are not supported in the @p extra array. +Returns a copy of @p data with all attributes interleaved. The @p extra +attributes, if any, are interleaved together with existing attributes (or, in +case the attribute view is empty, only the corresponding space for given +attribute type is reserved, with memory left uninitialized). The data layouting +is done by @ref interleavedLayout() with the @p flags parameter propagated to +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. 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 diff --git a/src/Magnum/MeshTools/Test/InterleaveTest.cpp b/src/Magnum/MeshTools/Test/InterleaveTest.cpp index fe22891bd..834b54917 100644 --- a/src/Magnum/MeshTools/Test/InterleaveTest.cpp +++ b/src/Magnum/MeshTools/Test/InterleaveTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -93,6 +94,7 @@ struct InterleaveTest: Corrade::TestSuite::Tester { void interleaveMeshDataExtraOffsetOnly(); void interleaveMeshDataExtraImplementationSpecificVertexFormat(); void interleaveMeshDataAlreadyInterleavedMove(); + void interleaveMeshDataAlreadyInterleavedMoveIndices(); void interleaveMeshDataAlreadyInterleavedMoveNonOwned(); void interleaveMeshDataNothing(); }; @@ -108,6 +110,17 @@ const struct { {"don't preserve layout", VertexFormat::Vector3, InterleaveFlags{}, false} }; +const struct { + const char* name; + Containers::Optional 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} +}; + InterleaveTest::InterleaveTest() { addTests({&InterleaveTest::attributeCount, &InterleaveTest::attributeCountGaps, @@ -153,9 +166,12 @@ InterleaveTest::InterleaveTest() { addTests({&InterleaveTest::interleavedLayoutNothing, &InterleaveTest::interleavedLayoutRvalue, - &InterleaveTest::interleaveMeshData, - &InterleaveTest::interleaveMeshDataIndexed, - &InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat, + &InterleaveTest::interleaveMeshData}); + + addInstancedTests({&InterleaveTest::interleaveMeshDataIndexed}, + Containers::arraySize(StridedIndicesData)); + + addTests({&InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat, &InterleaveTest::interleaveMeshDataExtra, &InterleaveTest::interleaveMeshDataExtraEmpty, &InterleaveTest::interleaveMeshDataExtraOriginalEmpty, @@ -166,6 +182,9 @@ InterleaveTest::InterleaveTest() { addInstancedTests({&InterleaveTest::interleaveMeshDataAlreadyInterleavedMove}, Containers::arraySize(AlreadyInterleavedData)); + addInstancedTests({&InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveIndices}, + Containers::arraySize(StridedIndicesData)); + addTests({&InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveNonOwned, &InterleaveTest::interleaveMeshDataNothing}); } @@ -1162,27 +1181,46 @@ void InterleaveTest::interleaveMeshData() { } void InterleaveTest::interleaveMeshDataIndexed() { + auto&& data = StridedIndicesData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + /* Testing also offset */ UnsignedShort indexData[50 + 3]; - indexData[50] = 0; - indexData[51] = 2; - indexData[52] = 1; + Containers::StridedArrayView1D indices = Containers::arrayView(indexData).suffix(50); + if(data.flip) indices = indices.flipped<0>(); + Utility::copy({0, 2, 1}, indices); + Vector2 positions[]{{1.3f, 0.3f}, {0.87f, 1.1f}, {1.0f, -0.5f}}; - Trade::MeshData data{MeshPrimitive::TriangleFan, - {}, Containers::arrayView(indexData), Trade::MeshIndexData{Containers::arrayView(indexData).suffix(50)}, + Trade::MeshData mesh{MeshPrimitive::TriangleFan, + {}, Containers::arrayView(indexData), Trade::MeshIndexData{indices}, {}, Containers::arrayView(positions), { Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::arrayView(positions)} }}; - Trade::MeshData interleaved = MeshTools::interleave(data); + Trade::MeshData interleaved = data.flags ? + MeshTools::interleave(mesh, {}, *data.flags) : + MeshTools::interleave(mesh); CORRADE_VERIFY(MeshTools::isInterleaved(interleaved)); CORRADE_COMPARE(interleaved.primitive(), MeshPrimitive::TriangleFan); + CORRADE_VERIFY(interleaved.isIndexed()); CORRADE_COMPARE(interleaved.indexType(), MeshIndexType::UnsignedShort); - CORRADE_COMPARE(interleaved.indexData().size(), 106); CORRADE_COMPARE_AS(interleaved.indices(), - Containers::arrayView(indexData).suffix(50), + Containers::arrayView({0, 2, 1}), TestSuite::Compare::Container); + + if(data.shouldPreserveLayoutInCopy) { + CORRADE_COMPARE(interleaved.indexData().size(), sizeof(indexData)); + CORRADE_COMPARE(interleaved.indexOffset(), static_cast(indices.data()) - reinterpret_cast(indexData)); + CORRADE_COMPARE(interleaved.indexStride(), indices.stride()); + } else { + /* Only the actually used part of the index buffer gets transferred and + is tightly packed */ + CORRADE_COMPARE(interleaved.indexData().size(), 3*sizeof(UnsignedShort)); + CORRADE_COMPARE(interleaved.indexOffset(), 0); + CORRADE_COMPARE(interleaved.indexStride(), sizeof(UnsignedShort)); + } + CORRADE_COMPARE(interleaved.attributeCount(), 1); CORRADE_COMPARE_AS(interleaved.attribute(Trade::MeshAttribute::Position), Containers::stridedArrayView(positions), @@ -1381,6 +1419,64 @@ void InterleaveTest::interleaveMeshDataAlreadyInterleavedMove() { } } +void InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveIndices() { + auto&& data = StridedIndicesData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + /* Testing also offset */ + Containers::Array indexData{(50 + 3)*sizeof(UnsignedShort)}; + Containers::StridedArrayView1D indices = Containers::arrayCast(indexData).suffix(50); + if(data.flip) indices = indices.flipped<0>(); + Utility::copy({0, 2, 1}, indices); + + Containers::Array vertexData{3*8}; + Containers::StridedArrayView1D positionView = Containers::arrayCast(vertexData); + auto attributeData = Containers::array({ + Trade::MeshAttributeData{Trade::MeshAttribute::Position, positionView}, + }); + const void* indexPointer = indexData; + const Trade::MeshAttributeData* attributePointer = attributeData; + + Trade::MeshData mesh{MeshPrimitive::TriangleFan, + std::move(indexData), Trade::MeshIndexData{indices}, + std::move(vertexData), std::move(attributeData)}; + CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); + + /* To catch when the default argument becomes different */ + Trade::MeshData interleaved = data.flags ? + MeshTools::interleave(std::move(mesh), {}, *data.flags) : + MeshTools::interleave(std::move(mesh)); + + CORRADE_VERIFY(MeshTools::isInterleaved(interleaved)); + CORRADE_COMPARE(interleaved.indexType(), MeshIndexType::UnsignedShort); + CORRADE_COMPARE_AS(interleaved.indices(), + Containers::arrayView({0, 2, 1}), + TestSuite::Compare::Container); + + if(data.shouldPreserveLayoutInMove) { + /* Indices got just moved without copying, with all metadata + preserved */ + CORRADE_VERIFY(interleaved.indexData().data() == indexPointer); + CORRADE_COMPARE(interleaved.indexData().size(), (50 + 3)*sizeof(UnsignedShort)); + CORRADE_COMPARE(interleaved.indexOffset(), static_cast(indices.data()) - reinterpret_cast(indexPointer)); + CORRADE_COMPARE(interleaved.indexStride(), indices.stride()); + } else { + /* Only the actually used part of the index buffer gets transferred and + is tightly packed */ + CORRADE_VERIFY(interleaved.indexData().data() != indexData); + CORRADE_COMPARE(interleaved.indexData().size(), 3*sizeof(UnsignedShort)); + CORRADE_COMPARE(interleaved.indexOffset(), 0); + CORRADE_COMPARE(interleaved.indexStride(), sizeof(UnsignedShort)); + } + + CORRADE_COMPARE(interleaved.attributeCount(), 1); + CORRADE_COMPARE(interleaved.attributeStride(0), 8); + CORRADE_VERIFY(interleaved.attributeData().data() == attributePointer); + + CORRADE_COMPARE(interleaved.vertexCount(), 3); + CORRADE_VERIFY(interleaved.vertexData().data() == positionView.data()); +} + void InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveNonOwned() { Containers::Array indexData{4}; auto indexView = Containers::arrayCast(indexData);