diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index e089a5283..843b79d7c 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -128,11 +128,12 @@ Containers::Array interleavedLayout(Trade::MeshData&& } else { stride = 0; minOffset = 0; - /** @todo explitily assert on impl-specific vertex formats here -- - however it should work when the original is already interleaved and - nothing in extras is impl-specific */ - for(UnsignedInt i = 0, max = data.attributeCount(); i != max; ++i) + for(UnsignedInt i = 0, max = data.attributeCount(); i != max; ++i) { + const VertexFormat format = data.attributeFormat(i); + CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), + "MeshTools::interleavedLayout(): attribute" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), {}); stride += attributeSize(data, i); + } } /* Add the extra attributes and explicit padding */ @@ -143,7 +144,9 @@ 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 { - /** @todo explitily assert on impl-specific vertex formats here */ + const VertexFormat format = extra[i].format(); + CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), + "MeshTools::interleavedLayout(): extra attribute" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), {}); stride += attributeSize(extra[i]); ++extraAttributeCount; } @@ -281,6 +284,13 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView) to avoid that allocation. + +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 isVertexFormatImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}); @@ -299,7 +304,12 @@ as @p data vertex count or has none. This function will unconditionally make a copy of all data even if @p data is already interleaved and needs no change, use @ref interleave(Trade::MeshData&&, Containers::ArrayView) to avoid that copy. -@see @ref isInterleaved(), @ref Trade::MeshData::attributeData() + +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(), + @ref Trade::MeshData::attributeData() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, Containers::ArrayView extra = {}); diff --git a/src/Magnum/MeshTools/Test/InterleaveTest.cpp b/src/Magnum/MeshTools/Test/InterleaveTest.cpp index 77bf86d21..97d117baa 100644 --- a/src/Magnum/MeshTools/Test/InterleaveTest.cpp +++ b/src/Magnum/MeshTools/Test/InterleaveTest.cpp @@ -72,10 +72,12 @@ struct InterleaveTest: Corrade::TestSuite::Tester { void interleavedDataImplementationSpecificVertexFormat(); void interleavedLayout(); + void interleavedLayoutImplementationSpecificVertexFormat(); void interleavedLayoutExtra(); void interleavedLayoutExtraAliased(); void interleavedLayoutExtraTooNegativePadding(); void interleavedLayoutExtraOnly(); + void interleavedLayoutExtraImplementationSpecificVertexFormat(); void interleavedLayoutAlreadyInterleaved(); void interleavedLayoutAlreadyInterleavedAliased(); void interleavedLayoutAlreadyInterleavedExtra(); @@ -84,16 +86,26 @@ struct InterleaveTest: Corrade::TestSuite::Tester { void interleaveMeshData(); void interleaveMeshDataIndexed(); + void interleaveMeshDataImplementationSpecificVertexFormat(); void interleaveMeshDataExtra(); void interleaveMeshDataExtraEmpty(); void interleaveMeshDataExtraOriginalEmpty(); void interleaveMeshDataExtraWrongCount(); void interleaveMeshDataExtraOffsetOnly(); + void interleaveMeshDataExtraImplementationSpecificVertexFormat(); void interleaveMeshDataAlreadyInterleavedMove(); void interleaveMeshDataAlreadyInterleavedMoveNonOwned(); void interleaveMeshDataNothing(); }; +const struct { + const char* name; + VertexFormat vertexFormat; +} AlreadyInterleavedData[]{ + {"", VertexFormat::Vector3}, + {"implementation-specific vertex format", vertexFormatWrap(0xcaca)} +}; + InterleaveTest::InterleaveTest() { addTests({&InterleaveTest::attributeCount, &InterleaveTest::attributeCountGaps, @@ -126,23 +138,30 @@ InterleaveTest::InterleaveTest() { &InterleaveTest::interleavedDataImplementationSpecificVertexFormat, &InterleaveTest::interleavedLayout, + &InterleaveTest::interleavedLayoutImplementationSpecificVertexFormat, &InterleaveTest::interleavedLayoutExtra, &InterleaveTest::interleavedLayoutExtraAliased, &InterleaveTest::interleavedLayoutExtraTooNegativePadding, &InterleaveTest::interleavedLayoutExtraOnly, - &InterleaveTest::interleavedLayoutAlreadyInterleaved, - &InterleaveTest::interleavedLayoutAlreadyInterleavedAliased, - &InterleaveTest::interleavedLayoutAlreadyInterleavedExtra, - &InterleaveTest::interleavedLayoutNothing, + &InterleaveTest::interleavedLayoutExtraImplementationSpecificVertexFormat}); + + addInstancedTests({&InterleaveTest::interleavedLayoutAlreadyInterleaved, + &InterleaveTest::interleavedLayoutAlreadyInterleavedAliased, + &InterleaveTest::interleavedLayoutAlreadyInterleavedExtra}, + Containers::arraySize(AlreadyInterleavedData)); + + addTests({&InterleaveTest::interleavedLayoutNothing, &InterleaveTest::interleavedLayoutRvalue, &InterleaveTest::interleaveMeshData, &InterleaveTest::interleaveMeshDataIndexed, + &InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat, &InterleaveTest::interleaveMeshDataExtra, &InterleaveTest::interleaveMeshDataExtraEmpty, &InterleaveTest::interleaveMeshDataExtraOriginalEmpty, &InterleaveTest::interleaveMeshDataExtraWrongCount, &InterleaveTest::interleaveMeshDataExtraOffsetOnly, + &InterleaveTest::interleaveMeshDataExtraImplementationSpecificVertexFormat, &InterleaveTest::interleaveMeshDataAlreadyInterleavedMove, &InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveNonOwned, &InterleaveTest::interleaveMeshDataNothing}); @@ -687,6 +706,22 @@ void InterleaveTest::interleavedLayout() { CORRADE_COMPARE(layout.vertexData().size(), 10*24); } +void InterleaveTest::interleavedLayoutImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData data{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector2, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::interleavedLayout(data, 5); + CORRADE_COMPARE(out.str(), "MeshTools::interleavedLayout(): attribute 1 has an implementation-specific format 0xcaca\n"); +} + void InterleaveTest::interleavedLayoutExtra() { Containers::Array vertexData{3*20}; Trade::MeshAttributeData positions{Trade::MeshAttribute::Position, @@ -806,30 +841,52 @@ void InterleaveTest::interleavedLayoutExtraOnly() { CORRADE_COMPARE(layout.vertexData().size(), 10*24); } +void InterleaveTest::interleavedLayoutExtraImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData data{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector2, nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::interleavedLayout(data, 5, { + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr}, + }); + CORRADE_COMPARE(out.str(), "MeshTools::interleavedLayout(): extra attribute 1 has an implementation-specific format 0xcaca\n"); +} + void InterleaveTest::interleavedLayoutAlreadyInterleaved() { + auto&& data = AlreadyInterleavedData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + Containers::Array indexData{6}; /* Test also removing the initial offset */ Containers::Array vertexData{100 + 3*24}; Trade::MeshAttributeData positions{Trade::MeshAttribute::Position, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data() + 100), 3, 24}}; Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal, + data.vertexFormat, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data() + 100 + 10), 3, 24}}; Trade::MeshIndexData indices{Containers::arrayCast(indexData)}; - Trade::MeshData data{MeshPrimitive::Triangles, + Trade::MeshData mesh{MeshPrimitive::Triangles, std::move(indexData), indices, std::move(vertexData), {positions, normals}}; - CORRADE_VERIFY(MeshTools::isInterleaved(data)); + CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(data, 10); + Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10); CORRADE_VERIFY(MeshTools::isInterleaved(layout)); CORRADE_VERIFY(!layout.isIndexed()); /* Indices are not preserved */ CORRADE_COMPARE(layout.attributeCount(), 2); CORRADE_COMPARE(layout.attributeName(0), Trade::MeshAttribute::Position); CORRADE_COMPARE(layout.attributeName(1), Trade::MeshAttribute::Normal); CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); - CORRADE_COMPARE(layout.attributeFormat(1), VertexFormat::Vector3); - /* Original stride should be preserved */ + CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); + /* Original stride should be preserved no matter what the formats are */ CORRADE_COMPARE(layout.attributeStride(0), 24); CORRADE_COMPARE(layout.attributeStride(1), 24); /* Relative offsets should be preserved, but the initial one removed */ @@ -840,27 +897,31 @@ void InterleaveTest::interleavedLayoutAlreadyInterleaved() { } void InterleaveTest::interleavedLayoutAlreadyInterleavedAliased() { + auto&& data = AlreadyInterleavedData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + Containers::Array indexData{6}; Containers::Array vertexData{3*12}; Trade::MeshAttributeData positions{Trade::MeshAttribute::Position, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data()), 3, 12}}; Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal, + data.vertexFormat, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data()), 3, 12}}; Trade::MeshIndexData indices{Containers::arrayCast(indexData)}; - Trade::MeshData data{MeshPrimitive::Triangles, + Trade::MeshData mesh{MeshPrimitive::Triangles, std::move(indexData), indices, std::move(vertexData), {positions, normals}}; - CORRADE_VERIFY(MeshTools::isInterleaved(data)); + CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(data, 10); + Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10); CORRADE_VERIFY(MeshTools::isInterleaved(layout)); CORRADE_VERIFY(!layout.isIndexed()); /* Indices are not preserved */ CORRADE_COMPARE(layout.attributeCount(), 2); CORRADE_COMPARE(layout.attributeName(0), Trade::MeshAttribute::Position); CORRADE_COMPARE(layout.attributeName(1), Trade::MeshAttribute::Normal); CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); - CORRADE_COMPARE(layout.attributeFormat(1), VertexFormat::Vector3); + CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); CORRADE_COMPARE(layout.attributeStride(0), 12); CORRADE_COMPARE(layout.attributeStride(1), 12); CORRADE_COMPARE(layout.attributeOffset(0), 0); @@ -870,17 +931,21 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedAliased() { } void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { + auto&& data = AlreadyInterleavedData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + Containers::Array vertexData{100 + 3*24}; Trade::MeshAttributeData positions{Trade::MeshAttribute::Position, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data() + 100), 3, 24}}; Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal, + data.vertexFormat, Containers::StridedArrayView1D{vertexData, reinterpret_cast(vertexData.data() + 100 + 10), 3, 24}}; - Trade::MeshData data{MeshPrimitive::Triangles, + Trade::MeshData mesh{MeshPrimitive::Triangles, std::move(vertexData), {positions, normals}}; - CORRADE_VERIFY(MeshTools::isInterleaved(data)); + CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(data, 10, { + Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10, { Trade::MeshAttributeData{1}, Trade::MeshAttributeData{Trade::meshAttributeCustom(15), VertexFormat::UnsignedShort, nullptr}, @@ -896,11 +961,11 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { CORRADE_COMPARE(layout.attributeName(2), Trade::meshAttributeCustom(15)); CORRADE_COMPARE(layout.attributeName(3), Trade::MeshAttribute::Color); CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); - CORRADE_COMPARE(layout.attributeFormat(1), VertexFormat::Vector3); + CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); CORRADE_COMPARE(layout.attributeFormat(2), VertexFormat::UnsignedShort); CORRADE_COMPARE(layout.attributeFormat(3), VertexFormat::Vector3); - /* Original stride should be preserved, with stride from extra attribs - added */ + /* Original stride should be preserved no matter what the formats, with + stride from extra attribs added */ CORRADE_COMPARE(layout.attributeStride(0), 24 + 20); CORRADE_COMPARE(layout.attributeStride(1), 24 + 20); CORRADE_COMPARE(layout.attributeStride(2), 24 + 20); @@ -1023,6 +1088,23 @@ void InterleaveTest::interleaveMeshDataIndexed() { TestSuite::Compare::Container); } +void InterleaveTest::interleaveMeshDataImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData data{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector2, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::interleave(data); + /* Assert is coming from interleavedLayout() because... that's easier */ + CORRADE_COMPARE(out.str(), "MeshTools::interleavedLayout(): attribute 1 has an implementation-specific format 0xcaca\n"); +} + void InterleaveTest::interleaveMeshDataExtra() { Vector2 positions[]{{1.3f, 0.3f}, {0.87f, 1.1f}, {1.0f, -0.5f}}; Trade::MeshData data{MeshPrimitive::TriangleFan, @@ -1131,6 +1213,25 @@ void InterleaveTest::interleaveMeshDataExtraOffsetOnly() { CORRADE_COMPARE(out.str(), "MeshTools::interleave(): extra attribute 1 is offset-only, which is not supported\n"); } +void InterleaveTest::interleaveMeshDataExtraImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + Trade::MeshData data{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, VertexFormat::Vector2, nullptr}, + }}; + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::interleave(data, { + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, VertexFormat::Vector2, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr}, + }); + /* Assert is coming from interleavedLayout() because... that's easier */ + CORRADE_COMPARE(out.str(), "MeshTools::interleavedLayout(): extra attribute 1 has an implementation-specific format 0xcaca\n"); +} + void InterleaveTest::interleaveMeshDataAlreadyInterleavedMove() { Containers::Array indexData{4}; auto indexView = Containers::arrayCast(indexData);