From 9742b8481555f1285ffe422b0c72fad22cae2d3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 19 May 2020 12:19:01 +0200 Subject: [PATCH] MeshTools: properly handle array attributes everywhere. Now the tests pass again. --- src/Magnum/MeshTools/Combine.cpp | 13 ++++++------ src/Magnum/MeshTools/CompressIndices.cpp | 2 +- src/Magnum/MeshTools/Concatenate.cpp | 5 ++++- src/Magnum/MeshTools/Duplicate.cpp | 4 +++- src/Magnum/MeshTools/GenerateIndices.cpp | 2 +- src/Magnum/MeshTools/Interleave.cpp | 25 +++++++++++++++--------- 6 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/Magnum/MeshTools/Combine.cpp b/src/Magnum/MeshTools/Combine.cpp index 9212f4a51..af28a3f31 100644 --- a/src/Magnum/MeshTools/Combine.cpp +++ b/src/Magnum/MeshTools/Combine.cpp @@ -46,7 +46,7 @@ Trade::MeshData combineIndexedImplementation(const MeshPrimitive primitive, Cont for(std::size_t i = 0; i != data.size(); ++i) { attributeCount += data[i]->attributeCount(); for(UnsignedInt j = 0; j != data[i]->attributeCount(); ++j) - vertexStride += vertexFormatSize(data[i]->attributeFormat(j)); + vertexStride += vertexFormatSize(data[i]->attributeFormat(j))*Math::max(data[i]->attributeArraySize(j), UnsignedShort{1}); } /* Make the combined index array unique */ @@ -74,15 +74,16 @@ Trade::MeshData combineIndexedImplementation(const MeshPrimitive primitive, Cont {std::ptrdiff_t(indexStride), 1}}; for(UnsignedInt i = 0; i != mesh.attributeCount(); ++i) { - const UnsignedInt attributeSize = vertexFormatSize(mesh.attributeFormat(i)); + Containers::StridedArrayView2D src = mesh.attribute(i); Containers::StridedArrayView2D dst{vertexData, vertexData.data() + vertexOffset, - {vertexCount, attributeSize}, + {vertexCount, src.size()[1]}, {std::ptrdiff_t(vertexStride), 1}}; - duplicateInto(indices, mesh.attribute(i), dst); - vertexOffset += attributeSize; + duplicateInto(indices, src, dst); + vertexOffset += src.size()[1]; attributeData[attributeOffset++] = Trade::MeshAttributeData{ - mesh.attributeName(i), mesh.attributeFormat(i), dst}; + mesh.attributeName(i), mesh.attributeFormat(i), + mesh.attributeArraySize(i), dst}; } indexOffset += indexSize; diff --git a/src/Magnum/MeshTools/CompressIndices.cpp b/src/Magnum/MeshTools/CompressIndices.cpp index 12ba8774c..689d1b788 100644 --- a/src/Magnum/MeshTools/CompressIndices.cpp +++ b/src/Magnum/MeshTools/CompressIndices.cpp @@ -155,7 +155,7 @@ Trade::MeshData compressIndices(Trade::MeshData&& data, MeshIndexType atLeast) { for(UnsignedInt i = 0, max = attributeData.size(); i != max; ++i) { const UnsignedInt stride = data.attributeStride(i); attributeData[i] = Trade::MeshAttributeData{data.attributeName(i), - data.attributeFormat(i), + data.attributeFormat(i), data.attributeArraySize(i), Containers::StridedArrayView1D{vertexData, vertexData.data() + data.attributeOffset(i) + offset*stride, newVertexCount, stride}}; } diff --git a/src/Magnum/MeshTools/Concatenate.cpp b/src/Magnum/MeshTools/Concatenate.cpp index 3ad883dc7..bf2831775 100644 --- a/src/Magnum/MeshTools/Concatenate.cpp +++ b/src/Magnum/MeshTools/Concatenate.cpp @@ -72,7 +72,7 @@ Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedI absolute, referencing the vertex data array */ for(Trade::MeshAttributeData& attribute: attributeData) { attribute = Trade::MeshAttributeData{ - attribute.name(), attribute.format(), + attribute.name(), attribute.format(), attribute.arraySize(), Containers::StridedArrayView1D{vertexData, vertexData + attribute.offset(vertexData), vertexCount, attribute.stride()}}; @@ -168,6 +168,9 @@ Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedI CORRADE_ASSERT(out.attributeFormat(dst) == mesh.attributeFormat(src), assertPrefix << "expected" << out.attributeFormat(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeFormat(src) << "in mesh" << i + meshIndexOffset << "attribute" << src, (Trade::MeshData{MeshPrimitive{}, 0})); + CORRADE_ASSERT(out.attributeArraySize(dst) == mesh.attributeArraySize(src), + assertPrefix << "expected array size" << out.attributeArraySize(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeArraySize(src) << "in mesh" << i + meshIndexOffset << "attribute" << src, + (Trade::MeshData{MeshPrimitive{}, 0})); /* Copy the data to a slice of the output, mark the attribute as copied */ diff --git a/src/Magnum/MeshTools/Duplicate.cpp b/src/Magnum/MeshTools/Duplicate.cpp index 3d6b49ed6..26b6051a8 100644 --- a/src/Magnum/MeshTools/Duplicate.cpp +++ b/src/Magnum/MeshTools/Duplicate.cpp @@ -28,6 +28,7 @@ #include #include +#include "Magnum/Math/Functions.h" #include "Magnum/MeshTools/Interleave.h" #include "Magnum/Trade/MeshData.h" @@ -106,7 +107,8 @@ Trade::MeshData duplicate(const Trade::MeshData& data, const Containers::ArrayVi "MeshTools::duplicate(): extra attribute" << i << "expected to have" << data.vertexCount() << "items but got" << extra[i].data().size(), (Trade::MeshData{MeshPrimitive::Triangles, 0})); const Containers::StridedArrayView2D attributeData = - Containers::arrayCast<2, const char>(extra[i].data(), vertexFormatSize(extra[i].format())); + Containers::arrayCast<2, const char>(extra[i].data(), + vertexFormatSize(extra[i].format())*Math::max(extra[i].arraySize(), UnsignedShort{1})); duplicateInto(data.indices(), attributeData, layout.mutableAttribute(attributeIndex)); } diff --git a/src/Magnum/MeshTools/GenerateIndices.cpp b/src/Magnum/MeshTools/GenerateIndices.cpp index 23786cbb1..b0ed280e2 100644 --- a/src/Magnum/MeshTools/GenerateIndices.cpp +++ b/src/Magnum/MeshTools/GenerateIndices.cpp @@ -190,7 +190,7 @@ Trade::MeshData generateIndices(Trade::MeshData&& data) { Containers::Array attributeData{data.attributeCount()}; for(UnsignedInt i = 0, max = attributeData.size(); i != max; ++i) { attributeData[i] = Trade::MeshAttributeData{data.attributeName(i), - data.attributeFormat(i), + data.attributeFormat(i), data.attributeArraySize(i), Containers::StridedArrayView1D{vertexData, vertexData.data() + data.attributeOffset(i), vertexCount, data.attributeStride(i)}}; } diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index f8965fe99..97a734be3 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -35,6 +35,13 @@ namespace Magnum { namespace MeshTools { namespace { +inline std::size_t attributeSize(const Trade::MeshData& data, UnsignedInt i) { + return vertexFormatSize(data.attributeFormat(i))*Math::max(data.attributeArraySize(i), UnsignedShort{1}); +} + +inline std::size_t attributeSize(const Trade::MeshAttributeData& data) { + return vertexFormatSize(data.format())*Math::max(data.arraySize(), UnsignedShort{1}); +} Containers::Optional> interleavedDataInternal(const Trade::MeshData& data) { /* There is no attributes, return a zero-sized view to indicate a success */ if(!data.attributeCount()) @@ -42,13 +49,13 @@ Containers::Optional> interleavedData const UnsignedInt stride = data.attributeStride(0); std::size_t minOffset = data.attributeOffset(0); - std::size_t maxOffset = minOffset + vertexFormatSize(data.attributeFormat(0)); + std::size_t maxOffset = minOffset + attributeSize(data, 0); for(UnsignedInt i = 0; i != data.attributeCount(); ++i) { if(data.attributeStride(i) != stride) return Containers::NullOpt; const std::size_t offset = data.attributeOffset(i); minOffset = Math::min(minOffset, offset); - maxOffset = Math::max(maxOffset, offset + vertexFormatSize(data.attributeFormat(i))); + maxOffset = Math::max(maxOffset, offset + attributeSize(data, i)); } /* The offsets can't fit into the stride, report failure */ @@ -104,7 +111,7 @@ Containers::Array interleavedLayout(Trade::MeshData&& stride = 0; minOffset = 0; for(UnsignedInt i = 0, max = data.attributeCount(); i != max; ++i) - stride += vertexFormatSize(data.attributeFormat(i)); + stride += attributeSize(data, i); } /* Add the extra attributes and explicit padding */ @@ -115,7 +122,7 @@ Containers::Array interleavedLayout(Trade::MeshData&& "MeshTools::interleavedLayout(): negative padding" << extra[i].stride() << "in extra attribute" << i << "too large for stride" << stride, {}); stride += extra[i].stride(); } else { - stride += vertexFormatSize(extra[i].format()); + stride += attributeSize(extra[i]); ++extraAttributeCount; } } @@ -146,9 +153,9 @@ Containers::Array interleavedLayout(Trade::MeshData&& attributeData[i] = Trade::MeshAttributeData{ attributeData[i].name(), attributeData[i].format(), - offset, 0, std::ptrdiff_t(stride)}; + offset, 0, std::ptrdiff_t(stride), attributeData[i].arraySize()}; - if(!interleaved) offset += vertexFormatSize(attributeData[i].format()); + if(!interleaved) offset += attributeSize(attributeData[i]); } /* In case the original is already interleaved, set the offset for extra @@ -168,9 +175,9 @@ Containers::Array interleavedLayout(Trade::MeshData&& attributeData[attributeIndex++] = Trade::MeshAttributeData{ extra[i].name(), extra[i].format(), - offset, 0, std::ptrdiff_t(stride)}; + offset, 0, std::ptrdiff_t(stride), extra[i].arraySize()}; - offset += vertexFormatSize(extra[i].format()); + offset += attributeSize(extra[i]); } return attributeData; @@ -193,7 +200,7 @@ Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vert absolute, referencing the above-allocated data array */ for(Trade::MeshAttributeData& attribute: attributeData) { attribute = Trade::MeshAttributeData{ - attribute.name(), attribute.format(), + attribute.name(), attribute.format(), attribute.arraySize(), Containers::StridedArrayView1D{vertexData, vertexData + attribute.offset(vertexData), vertexCount, attribute.stride()}};