Browse Source

MeshTools: assert on impl-specific vert format in interleave*().

Haha, I even had a TODO here. An exception in this case is when the mesh
is already interleaved -- then the layout is kept intact and thus it's
not needed to know what vertex format sizes to repack.
pull/549/head
Vladimír Vondruš 4 years ago
parent
commit
8ae8603d7a
  1. 20
      src/Magnum/MeshTools/Interleave.cpp
  2. 12
      src/Magnum/MeshTools/Interleave.h
  3. 139
      src/Magnum/MeshTools/Test/InterleaveTest.cpp

20
src/Magnum/MeshTools/Interleave.cpp

@ -128,11 +128,12 @@ Containers::Array<Trade::MeshAttributeData> 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<void*>(vertexFormatUnwrap(format)), {});
stride += attributeSize(data, i);
}
}
/* Add the extra attributes and explicit padding */
@ -143,7 +144,9 @@ Containers::Array<Trade::MeshAttributeData> 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<void*>(vertexFormatUnwrap(format)), {});
stride += attributeSize(extra[i]);
++extraAttributeCount;
}
@ -281,6 +284,13 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView<c
/* Calculate the layout. Can't std::move() the data in to avoid copying
the attribute array as we need the original attributes below. */
Trade::MeshData layout = interleavedLayout(data, vertexCount, extra);
#ifdef CORRADE_GRACEFUL_ASSERT
/* If interleavedLayout() gracefully asserted and returned no
attributes (but the original had some), exit right away to not blow
up on something else later. Sorry, yes, this is shitty. */
if(!layout.attributeCount() && (data.attributeCount() || extra.size()))
return Trade::MeshData{MeshPrimitive::Points, 0};
#endif
/* Copy existing attributes to new locations */
for(UnsignedInt i = 0; i != data.attributeCount(); ++i)

12
src/Magnum/MeshTools/Interleave.h

@ -255,6 +255,11 @@ instance with attribute and vertex data transferred from the returned instance:
This function will unconditionally allocate a new array to store all
@ref Trade::MeshAttributeData, use @ref interleavedLayout(Trade::MeshData&&, UnsignedInt, Containers::ArrayView<const Trade::MeshAttributeData>)
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<const Trade::MeshAttributeData> 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<const Trade::MeshAttributeData>)
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<const Trade::MeshAttributeData> extra = {});

139
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<char> 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<char> indexData{6};
/* Test also removing the initial offset */
Containers::Array<char> vertexData{100 + 3*24};
Trade::MeshAttributeData positions{Trade::MeshAttribute::Position,
Containers::StridedArrayView1D<Vector2>{vertexData, reinterpret_cast<Vector2*>(vertexData.data() + 100), 3, 24}};
Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal,
data.vertexFormat,
Containers::StridedArrayView1D<Vector3>{vertexData, reinterpret_cast<Vector3*>(vertexData.data() + 100 + 10), 3, 24}};
Trade::MeshIndexData indices{Containers::arrayCast<UnsignedShort>(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<char> indexData{6};
Containers::Array<char> vertexData{3*12};
Trade::MeshAttributeData positions{Trade::MeshAttribute::Position,
Containers::StridedArrayView1D<Vector2>{vertexData, reinterpret_cast<Vector2*>(vertexData.data()), 3, 12}};
Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal,
data.vertexFormat,
Containers::StridedArrayView1D<Vector3>{vertexData, reinterpret_cast<Vector3*>(vertexData.data()), 3, 12}};
Trade::MeshIndexData indices{Containers::arrayCast<UnsignedShort>(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<char> vertexData{100 + 3*24};
Trade::MeshAttributeData positions{Trade::MeshAttribute::Position,
Containers::StridedArrayView1D<Vector2>{vertexData, reinterpret_cast<Vector2*>(vertexData.data() + 100), 3, 24}};
Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal,
data.vertexFormat,
Containers::StridedArrayView1D<Vector3>{vertexData, reinterpret_cast<Vector3*>(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<char> indexData{4};
auto indexView = Containers::arrayCast<UnsignedShort>(indexData);

Loading…
Cancel
Save