ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Trade: relax index/attribute/vertex count restrictions in MeshData. 

Allow to make:

 - an indexed mesh with zero indices
 - a mesh with non-zero attribute count but zero vertices
 - a mesh with non-zero vertex count but zero attributes

All of these are valid use cases as explained in the tests, and will
also make the release*() behavior defined better.
pull/371/head
Vladimír Vondruš 6 years ago
parent
87d16bc627
commit
270e93e134
3 changed files with 80 additions and 57 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 10
      src/Magnum/Trade/MeshData.cpp
  2. 28
      src/Magnum/Trade/MeshData.h
  3. 99
      src/Magnum/Trade/Test/MeshDataTest.cpp

10
src/Magnum/Trade/MeshData.cpp
Unescape View File

@ -32,7 +32,7 @@
namespace Magnum { namespace Trade {
MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayView<const void> data) noexcept: MeshIndexData{type, data, nullptr} {
MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayView<const void> data) noexcept: _type{type}, _data{data} {
/* Yes, this calls into a constexpr function defined in the header --
because I feel that makes more sense than duplicating the full assert
logic */
@ -84,14 +84,10 @@ MeshData::MeshData(const MeshPrimitive primitive, Containers::Array<char>&& inde
_vertexCount = 0;
} else _vertexCount = _attributes[0]._data.size();
CORRADE_ASSERT(!_indices.empty() || !_indexData,
CORRADE_ASSERT(!_indices.empty() || _indexData.empty(),
"Trade::MeshData: indexData passed for a non-indexed mesh", );
CORRADE_ASSERT(_indices.empty() || (_indices.begin() >= _indexData.begin() && _indices.end() <= _indexData.end()),
CORRADE_ASSERT(!_indices || (_indices.begin() >= _indexData.begin() && _indices.end() <= _indexData.end()),
"Trade::MeshData: indices [" << Debug::nospace << static_cast<const void*>(_indices.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_indices.end()) << Debug::nospace << "] are not contained in passed indexData array [" << Debug::nospace << static_cast<const void*>(_indexData.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_indexData.end()) << Debug::nospace << "]", );
CORRADE_ASSERT(!_attributes.empty() || !_vertexData,
"Trade::MeshData: vertexData passed for an attribute-less mesh", );
CORRADE_ASSERT(_vertexCount || !_vertexData,
"Trade::MeshData: vertexData passed for a mesh with zero vertices", );
#ifndef CORRADE_NO_ASSERT
/* Not checking what's already checked in MeshIndexData / MeshAttributeData

28
src/Magnum/Trade/MeshData.h
Unescape View File

@ -173,17 +173,21 @@ class MAGNUM_TRADE_EXPORT MeshIndexData {
* @ref MeshIndexData(Containers::ArrayView<const UnsignedShort>) or
* @ref MeshIndexData(Containers::ArrayView<const UnsignedInt>)
* constructors, which infer the index type automatically.
*
* If @p data is empty, the mesh will be treated as indexed but with
* zero indices. To create a non-indexed mesh, use the
* @ref MeshIndexData(std::nullptr_t) constructor.
*/
explicit MeshIndexData(MeshIndexType type, Containers::ArrayView<const void> data) noexcept;
/** @brief Construct with unsigned byte indices */
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedByte> data) noexcept: MeshIndexData{MeshIndexType::UnsignedByte, data, nullptr} {}
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedByte> data) noexcept: _type{MeshIndexType::UnsignedByte}, _data{data} {}
/** @brief Construct with unsigned short indices */
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedShort> data) noexcept: MeshIndexData{MeshIndexType::UnsignedShort, data, nullptr} {}
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedShort> data) noexcept: _type{MeshIndexType::UnsignedShort}, _data{data} {}
/** @brief Construct with unsigned int indices */
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedInt> data) noexcept: MeshIndexData{MeshIndexType::UnsignedInt, data, nullptr} {}
constexpr explicit MeshIndexData(Containers::ArrayView<const UnsignedInt> data) noexcept: _type{MeshIndexType::UnsignedInt}, _data{data} {}
/**
* @brief Constructor
@ -201,13 +205,6 @@ class MAGNUM_TRADE_EXPORT MeshIndexData {
constexpr Containers::ArrayView<const void> data() const { return _data; }
private:
/* Contains an assert common for all constexpr constructor, nullptr_t
to disambiguate from the public constructor of the same signature --
can't delegate into that one, because it checks against
meshIndexTypeSize() that's not constexpr, and since we come from a
template, we don't need that check anyway */
constexpr explicit MeshIndexData(MeshIndexType type, Containers::ArrayView<const void> data, std::nullptr_t);
friend MeshData;
MeshIndexType _type;
/* Void so the constructors can be constexpr */
@ -371,8 +368,8 @@ class MAGNUM_TRADE_EXPORT MeshData {
* The @p indices are expected to point to a sub-range of @p indexData.
* The @p attributes are expected to reference (sparse) sub-ranges of
* @p vertexData. If the mesh has no attributes, the @p indices are
* expected to be valid and non-empty. If you want to create an
* index-less attribute-less mesh, use
* expected to be valid (but can be empty). If you want to create an
* attribute-less non-indexed mesh, use
* @ref MeshData(MeshPrimitive, UnsignedInt, const void*) to specify
* desired vertex count.
*
@ -515,8 +512,8 @@ class MAGNUM_TRADE_EXPORT MeshData {
*
* Same as calling @ref MeshData(MeshPrimitive, Containers::Array<char>&&, const MeshIndexData&, Containers::Array<char>&&, Containers::Array<MeshAttributeData>&&, const void*)
* with default-constructed @p vertexData and @p attributes arguments.
* The @p indices are expected to be valid and non-empty. If you want
* to create an index-less attribute-less mesh, use
* The @p indices are expected to be valid (but can be empty). If you
* want to create an attribute-less non-indexed mesh, use
* @ref MeshData(MeshPrimitive, UnsignedInt, const void*) to specify
* desired vertex count.
*
@ -1159,9 +1156,6 @@ namespace Implementation {
}
#endif
constexpr MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayView<const void> data, std::nullptr_t):
_type{type}, _data{(CORRADE_CONSTEXPR_ASSERT(!data.empty(), "Trade::MeshIndexData: index array can't be empty, create a non-indexed mesh instead"), data)} {}
constexpr MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexFormat format, const Containers::StridedArrayView1D<const void>& data, std::nullptr_t) noexcept:
_name{name}, _format{format}, _data{(CORRADE_CONSTEXPR_ASSERT(
(name == MeshAttribute::Position &&

99
src/Magnum/Trade/Test/MeshDataTest.cpp
Unescape View File

@ -42,7 +42,6 @@ struct MeshDataTest: TestSuite::Tester {
void debugAttributeName();
void constructIndex();
void constructIndexZeroCount();
void constructIndexTypeErased();
void constructIndexTypeErasedWrongSize();
void constructIndex2D();
@ -63,6 +62,9 @@ struct MeshDataTest: TestSuite::Tester {
void constructAttributeNonOwningArray();
void construct();
void constructZeroIndices();
void constructZeroAttributes();
void constructZeroVertices();
void constructIndexless();
void constructIndexlessZeroVertices();
void constructAttributeless();
@ -76,8 +78,6 @@ struct MeshDataTest: TestSuite::Tester {
void constructAttributelessNotOwned();
void constructIndexDataButNotIndexed();
void constructVertexDataButNoAttributes();
void constructVertexDataButNoVertices();
void constructAttributelessInvalidIndices();
void constructIndicesNotContained();
void constructAttributeNotContained();
@ -143,7 +143,6 @@ MeshDataTest::MeshDataTest() {
&MeshDataTest::debugAttributeName,
&MeshDataTest::constructIndex,
&MeshDataTest::constructIndexZeroCount,
&MeshDataTest::constructIndexTypeErased,
&MeshDataTest::constructIndexTypeErasedWrongSize,
&MeshDataTest::constructIndex2D,
@ -164,6 +163,9 @@ MeshDataTest::MeshDataTest() {
&MeshDataTest::constructAttributeNonOwningArray,
&MeshDataTest::construct,
&MeshDataTest::constructZeroIndices,
&MeshDataTest::constructZeroAttributes,
&MeshDataTest::constructZeroVertices,
&MeshDataTest::constructIndexless,
&MeshDataTest::constructIndexlessZeroVertices,
&MeshDataTest::constructAttributeless,
@ -179,8 +181,6 @@ MeshDataTest::MeshDataTest() {
Containers::arraySize(SingleNotOwnedData));
addTests({&MeshDataTest::constructIndexDataButNotIndexed,
&MeshDataTest::constructVertexDataButNoAttributes,
&MeshDataTest::constructVertexDataButNoVertices,
&MeshDataTest::constructAttributelessInvalidIndices,
&MeshDataTest::constructIndicesNotContained,
&MeshDataTest::constructAttributeNotContained,
@ -311,13 +311,6 @@ void MeshDataTest::constructIndex() {
}
}
void MeshDataTest::constructIndexZeroCount() {
std::ostringstream out;
Error redirectError{&out};
MeshIndexData{MeshIndexType::UnsignedInt, nullptr};
CORRADE_COMPARE(out.str(), "Trade::MeshIndexData: index array can't be empty, create a non-indexed mesh instead\n");
}
void MeshDataTest::constructIndexTypeErased() {
const char indexData[3*2]{};
MeshIndexData indices{MeshIndexType::UnsignedShort, indexData};
@ -688,6 +681,66 @@ void MeshDataTest::construct() {
CORRADE_COMPARE(data.attribute<Short>(meshAttributeCustom(13))[2], 22);
}
void MeshDataTest::constructZeroIndices() {
/* This is a valid use case because this could be an empty slice of a
well-defined indexed mesh. Explicitly use a non-null zero-sized array
to check the importer is checking size and not pointer. */
Containers::Array<char> vertexData{3*sizeof(Vector3)};
auto vertices = Containers::arrayCast<Vector3>(vertexData);
char i;
Containers::Array<char> indexData{&i, 0, [](char*, std::size_t){}};
auto indices = Containers::arrayCast<UnsignedInt>(indexData);
MeshAttributeData positions{MeshAttribute::Position, vertices};
MeshData data{MeshPrimitive::Triangles,
std::move(indexData), MeshIndexData{indices},
std::move(vertexData), {positions}};
CORRADE_COMPARE(data.indexDataFlags(), DataFlag::Owned|DataFlag::Mutable);
CORRADE_VERIFY(data.isIndexed());
CORRADE_COMPARE(data.indexType(), MeshIndexType::UnsignedInt);
CORRADE_COMPARE(data.indexCount(), 0);
CORRADE_COMPARE(data.vertexCount(), 3);
}
void MeshDataTest::constructZeroAttributes() {
/* This is a valid use case because e.g. the index/vertex data can be
shared by multiple meshes and this particular one is just a plain
index array */
Containers::Array<char> indexData{3*sizeof(UnsignedInt)};
Containers::Array<char> vertexData{3};
auto indexView = Containers::arrayCast<UnsignedInt>(indexData);
MeshData data{MeshPrimitive::Triangles,
std::move(indexData), MeshIndexData{indexView},
std::move(vertexData), {}};
CORRADE_COMPARE(data.indexCount(), 3);
CORRADE_COMPARE(data.vertexDataFlags(), DataFlag::Owned|DataFlag::Mutable);
CORRADE_COMPARE(data.attributeCount(), 0);
CORRADE_VERIFY(!data.attributeData());
CORRADE_COMPARE(data.vertexData().size(), 3);
CORRADE_COMPARE(data.vertexCount(), 0);
}
void MeshDataTest::constructZeroVertices() {
/* This is a valid use case because this could be an empty slice of a
well-defined indexed mesh */
Containers::Array<char> indexData{3*sizeof(UnsignedInt)};
auto indexView = Containers::arrayCast<UnsignedInt>(indexData);
MeshAttributeData positions{MeshAttribute::Position, VertexFormat::Vector3, nullptr};
MeshData data{MeshPrimitive::Triangles,
std::move(indexData), MeshIndexData{indexView},
nullptr, {positions}};
CORRADE_COMPARE(data.indexCount(), 3);
CORRADE_COMPARE(data.vertexDataFlags(), DataFlag::Owned|DataFlag::Mutable);
CORRADE_COMPARE(data.attributeCount(), 1);
CORRADE_COMPARE(data.attributeName(0), MeshAttribute::Position);
CORRADE_COMPARE(data.attributeFormat(0), VertexFormat::Vector3);
CORRADE_COMPARE(data.attribute<Vector3>(0).size(), 0);
CORRADE_VERIFY(!data.vertexData());
CORRADE_COMPARE(data.vertexCount(), 0);
}
void MeshDataTest::constructIndexless() {
Containers::Array<char> vertexData{3*sizeof(Vector2)};
auto vertexView = Containers::arrayCast<Vector2>(vertexData);
@ -1004,26 +1057,6 @@ void MeshDataTest::constructIndexDataButNotIndexed() {
CORRADE_COMPARE(out.str(), "Trade::MeshData: indexData passed for a non-indexed mesh\n");
}
void MeshDataTest::constructVertexDataButNoAttributes() {
Containers::Array<char> indexData{6};
Containers::Array<char> vertexData{6};
std::ostringstream out;
Error redirectError{&out};
MeshData{MeshPrimitive::Points, std::move(indexData), MeshIndexData{Containers::arrayCast<UnsignedShort>(indexData)}, std::move(vertexData), {}};
CORRADE_COMPARE(out.str(), "Trade::MeshData: vertexData passed for an attribute-less mesh\n");
}
void MeshDataTest::constructVertexDataButNoVertices() {
Containers::Array<char> vertexData{6};
std::ostringstream out;
Error redirectError{&out};
MeshAttributeData positions{MeshAttribute::Position, VertexFormat::Vector2, nullptr};
MeshData{MeshPrimitive::LineLoop, std::move(vertexData), {positions}};
CORRADE_COMPARE(out.str(), "Trade::MeshData: vertexData passed for a mesh with zero vertices\n");
}
void MeshDataTest::constructAttributelessInvalidIndices() {
std::ostringstream out;
Error redirectError{&out};

Write Preview
Loading…
Cancel
Save