diff --git a/src/Magnum/Trade/MeshData.cpp b/src/Magnum/Trade/MeshData.cpp index c08ae6b2c..75e816293 100644 --- a/src/Magnum/Trade/MeshData.cpp +++ b/src/Magnum/Trade/MeshData.cpp @@ -32,7 +32,7 @@ namespace Magnum { namespace Trade { -MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayView data) noexcept: MeshIndexData{type, data, nullptr} { +MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayView 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&& 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(_indices.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast(_indices.end()) << Debug::nospace << "] are not contained in passed indexData array [" << Debug::nospace << static_cast(_indexData.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast(_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 diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h index 4ef07f9bb..cb3070760 100644 --- a/src/Magnum/Trade/MeshData.h +++ b/src/Magnum/Trade/MeshData.h @@ -173,17 +173,21 @@ class MAGNUM_TRADE_EXPORT MeshIndexData { * @ref MeshIndexData(Containers::ArrayView) or * @ref MeshIndexData(Containers::ArrayView) * 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 data) noexcept; /** @brief Construct with unsigned byte indices */ - constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: MeshIndexData{MeshIndexType::UnsignedByte, data, nullptr} {} + constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: _type{MeshIndexType::UnsignedByte}, _data{data} {} /** @brief Construct with unsigned short indices */ - constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: MeshIndexData{MeshIndexType::UnsignedShort, data, nullptr} {} + constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: _type{MeshIndexType::UnsignedShort}, _data{data} {} /** @brief Construct with unsigned int indices */ - constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: MeshIndexData{MeshIndexType::UnsignedInt, data, nullptr} {} + constexpr explicit MeshIndexData(Containers::ArrayView data) noexcept: _type{MeshIndexType::UnsignedInt}, _data{data} {} /** * @brief Constructor @@ -201,13 +205,6 @@ class MAGNUM_TRADE_EXPORT MeshIndexData { constexpr Containers::ArrayView 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 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&&, const MeshIndexData&, Containers::Array&&, Containers::Array&&, 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 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& data, std::nullptr_t) noexcept: _name{name}, _format{format}, _data{(CORRADE_CONSTEXPR_ASSERT( (name == MeshAttribute::Position && diff --git a/src/Magnum/Trade/Test/MeshDataTest.cpp b/src/Magnum/Trade/Test/MeshDataTest.cpp index 8d3b04fea..45e51067d 100644 --- a/src/Magnum/Trade/Test/MeshDataTest.cpp +++ b/src/Magnum/Trade/Test/MeshDataTest.cpp @@ -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(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 vertexData{3*sizeof(Vector3)}; + auto vertices = Containers::arrayCast(vertexData); + char i; + Containers::Array indexData{&i, 0, [](char*, std::size_t){}}; + auto indices = Containers::arrayCast(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 indexData{3*sizeof(UnsignedInt)}; + Containers::Array vertexData{3}; + auto indexView = Containers::arrayCast(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 indexData{3*sizeof(UnsignedInt)}; + auto indexView = Containers::arrayCast(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(0).size(), 0); + CORRADE_VERIFY(!data.vertexData()); + CORRADE_COMPARE(data.vertexCount(), 0); +} + void MeshDataTest::constructIndexless() { Containers::Array vertexData{3*sizeof(Vector2)}; auto vertexView = Containers::arrayCast(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 indexData{6}; - Containers::Array vertexData{6}; - - std::ostringstream out; - Error redirectError{&out}; - MeshData{MeshPrimitive::Points, std::move(indexData), MeshIndexData{Containers::arrayCast(indexData)}, std::move(vertexData), {}}; - CORRADE_COMPARE(out.str(), "Trade::MeshData: vertexData passed for an attribute-less mesh\n"); -} - -void MeshDataTest::constructVertexDataButNoVertices() { - Containers::Array 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};