Trade: make creation of non-owning MeshData references easier.

While 27f6cc309d made it easier to create
references to attribute-less meshes by avoiding a branch on
attributeCount() (and then using a constructor with explicit vertex
count), code still had to branch on isIndexed() because indices() could
be called only on indexed meshes. This change allows code like

    Trade::MeshData reference{data.primitive(),
        {}, data.indexData(), Trade::MeshIndexData{data.indices()},
        {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()),
        data.vertexCount()};

which works correctly for all cases and doesn't introduce extra branches
and code paths that would need to be tested. While definitely better, I
might still give up at some point and introduce some
Trade::MeshData::nonOwningImmutableReference() helper for this.

src/Magnum/MeshTools/CompressIndices.cpp

@@ -165,8 +165,6 @@ Trade::MeshData compressIndices(Trade::MeshData&& data, MeshIndexType atLeast) {
 }
 
 Trade::MeshData compressIndices(const Trade::MeshData& data, MeshIndexType atLeast) {
-    CORRADE_ASSERT(data.isIndexed(), "MeshTools::compressIndices(): mesh data not indexed", (Trade::MeshData{MeshPrimitive::Triangles, 0}));
-
     return compressIndices(Trade::MeshData{data.primitive(),
         {}, data.indexData(), Trade::MeshIndexData{data.indices()},
         {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()),

src/Magnum/MeshTools/Concatenate.cpp

@@ -226,15 +226,9 @@ Trade::MeshData concatenate(Trade::MeshData&& first, std::initializer_list<Conta
 }
 
 Trade::MeshData concatenate(const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next) {
-    Containers::ArrayView<const char> indexData;
-    Trade::MeshIndexData indices;
-    if(first.isIndexed()) {
-        indexData = first.indexData();
-        indices = Trade::MeshIndexData{first.indices()};
-    }
-
     return concatenate(Trade::MeshData{first.primitive(),
-        {}, indexData, indices,
+        /* If data is not indexed, the reference will be also non-indexed */
+        {}, first.indexData(), Trade::MeshIndexData{first.indices()},
         {}, first.vertexData(), Trade::meshAttributeDataNonOwningArray(first.attributeData()),
         first.vertexCount(),
     }, next);

src/Magnum/MeshTools/GenerateIndices.cpp

@@ -222,11 +222,11 @@ Trade::MeshData generateIndices(Trade::MeshData&& data) {
 }
 
 Trade::MeshData generateIndices(const Trade::MeshData& data) {
-    CORRADE_ASSERT(!data.isIndexed(),
-        "MeshTools::generateIndices(): mesh data already indexed",
-        (Trade::MeshData{MeshPrimitive::Triangles, 0}));
-
     return generateIndices(Trade::MeshData{data.primitive(),
+        /* Passing the indices through. If the mesh isn't indexed, this makes
+           the reference non-indexed also, if the mesh is indexed, it causes an
+           assert inside the delegated generateIndices(). */
+        {}, data.indexData(), Trade::MeshIndexData{data.indices()},
         {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()),
         data.vertexCount()});
 }

src/Magnum/MeshTools/Interleave.cpp

@@ -301,15 +301,9 @@ Trade::MeshData interleave(Trade::MeshData&& data, const std::initializer_list<T
 }
 
 Trade::MeshData interleave(const Trade::MeshData& data, const Containers::ArrayView<const Trade::MeshAttributeData> extra) {
-    Containers::ArrayView<const char> indexData;
-    Trade::MeshIndexData indices;
-    if(data.isIndexed()) {
-        indexData = data.indexData();
-        indices = Trade::MeshIndexData{data.indices()};
-    }
-
     return interleave(Trade::MeshData{data.primitive(),
-        {}, indexData, indices,
+        /* If data is not indexed, the reference will be also non-indexed */
+        {}, data.indexData(), Trade::MeshIndexData{data.indices()},
         {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()),
         data.vertexCount()
     }, extra);

src/Magnum/Trade/MeshData.cpp

@@ -42,6 +42,12 @@ MeshIndexData::MeshIndexData(const MeshIndexType type, const Containers::ArrayVi
 }
 
 MeshIndexData::MeshIndexData(const Containers::StridedArrayView2D<const char>& data) noexcept {
+    /* Second dimension being zero indicates a non-indexed mesh */
+    if(data.size()[1] == 0) {
+        _type = MeshIndexType{};
+        return;
+    }
+
     if(data.size()[1] == 4) _type = MeshIndexType::UnsignedInt;
     else if(data.size()[1] == 2) _type = MeshIndexType::UnsignedShort;
     else if(data.size()[1] == 1) _type = MeshIndexType::UnsignedByte;
@@ -230,8 +236,9 @@ std::size_t MeshData::indexOffset() const {
 }
 
 Containers::StridedArrayView2D<const char> MeshData::indices() const {
-    CORRADE_ASSERT(isIndexed(),
+    /* For a non-indexed mesh returning zero size in both dimensions, indexed
-        "Trade::MeshData::indices(): the mesh is not indexed", {});
+       mesh with zero indices sill has the second dimension non-zero */
+    if(!isIndexed()) return {};
     const std::size_t indexTypeSize = meshIndexTypeSize(_indexType);
     /* Build a 2D view using information about attribute type size */
     return {{_indices, _indexCount*indexTypeSize}, {_indexCount, indexTypeSize}};
@@ -240,8 +247,9 @@ Containers::StridedArrayView2D<const char> MeshData::indices() const {
 Containers::StridedArrayView2D<char> MeshData::mutableIndices() {
     CORRADE_ASSERT(_indexDataFlags & DataFlag::Mutable,
         "Trade::MeshData::mutableIndices(): index data not mutable", {});
-    CORRADE_ASSERT(isIndexed(),
+    /* For a non-indexed mesh returning zero size in both dimensions, indexed
-        "Trade::MeshData::mutableIndices(): the mesh is not indexed", {});
+       mesh with zero indices sill has the second dimension non-zero */
+    if(!isIndexed()) return {};
     const std::size_t indexTypeSize = meshIndexTypeSize(_indexType);
     /* Build a 2D view using information about index type size */
     Containers::StridedArrayView2D<const char> out{{_indices, _indexCount*indexTypeSize}, {_indexCount, indexTypeSize}};

src/Magnum/Trade/MeshData.h

@@ -265,7 +265,8 @@ class MAGNUM_TRADE_EXPORT MeshIndexData {
          *
          * Expects that @p data is contiguous and size of the second dimension
          * is either 1, 2 or 4, corresponding to one of the @ref MeshIndexType
-         * values.
+         * values. As a special case, if second dimension size is 0, the
+         * constructor is equivalent to @ref MeshIndexData(std::nullptr_t).
          */
         explicit MeshIndexData(const Containers::StridedArrayView2D<const char>& data) noexcept;
 
@@ -1093,8 +1094,11 @@ class MAGNUM_TRADE_EXPORT MeshData {
         /**
          * @brief Mesh indices
          *
-         * The view is guaranteed to be contiguous and its second dimension
+         * For an indexed mesh, the view is guaranteed to be contiguous and its
-         * represents the actual data type (its size is equal to type size).
+         * second dimension represents the actual data type (its size is equal
+         * to type size, even in case there's zero indices). For a non-indexed
+         * mesh, the returned view has a zero size in both dimensions.
+         *
          * Use the templated overload below to get the indices in a concrete
          * type.
          * @see @ref Corrade::Containers::StridedArrayView::isContiguous()
@@ -2107,6 +2111,8 @@ template<class T> constexpr MeshAttributeData::MeshAttributeData(MeshAttribute n
 template<class T> constexpr MeshAttributeData::MeshAttributeData(MeshAttribute name, const Containers::StridedArrayView2D<T>& data) noexcept: MeshAttributeData{name, Implementation::vertexFormatFor<typename std::remove_const<T>::type>(), UnsignedShort(data.size()[1]), Containers::StridedArrayView1D<const void>{{data.data(), ~std::size_t{}}, data.size()[0], data.stride()[0]}, (CORRADE_CONSTEXPR_ASSERT(data.stride()[1] == sizeof(T), "Trade::MeshAttributeData: second view dimension is not contiguous"), nullptr)} {}
 
 template<class T> Containers::ArrayView<const T> MeshData::indices() const {
+    CORRADE_ASSERT(isIndexed(),
+        "Trade::MeshData::indices(): the mesh is not indexed", {});
     Containers::StridedArrayView2D<const char> data = indices();
     #ifdef CORRADE_GRACEFUL_ASSERT /* Sigh. Brittle. Better idea? */
     if(!data.stride()[1]) return {};
@@ -2117,6 +2123,8 @@ template<class T> Containers::ArrayView<const T> MeshData::indices() const {
 }
 
 template<class T> Containers::ArrayView<T> MeshData::mutableIndices() {
+    CORRADE_ASSERT(isIndexed(),
+        "Trade::MeshData::mutableIndices(): the mesh is not indexed", {});
     Containers::StridedArrayView2D<char> data = mutableIndices();
     #ifdef CORRADE_GRACEFUL_ASSERT /* Sigh. Brittle. Better idea? */
     if(!data.stride()[1]) return {};

src/Magnum/Trade/Test/MeshDataTest.cpp

@@ -46,6 +46,7 @@ struct MeshDataTest: TestSuite::Tester {
     void constructIndexTypeErased();
     void constructIndexTypeErasedWrongSize();
     void constructIndex2D();
+    void constructIndex2DNotIndexed();
     void constructIndex2DWrongSize();
     void constructIndex2DNonContiguous();
     void constructIndexNullptr();
@@ -204,6 +205,7 @@ MeshDataTest::MeshDataTest() {
               &MeshDataTest::constructIndexTypeErased,
               &MeshDataTest::constructIndexTypeErasedWrongSize,
               &MeshDataTest::constructIndex2D,
+              &MeshDataTest::constructIndex2DNotIndexed,
               &MeshDataTest::constructIndex2DWrongSize,
               &MeshDataTest::constructIndex2DNonContiguous,
               &MeshDataTest::constructIndexNullptr,
@@ -516,6 +518,12 @@ void MeshDataTest::constructIndex2D() {
     }
 }
 
+void MeshDataTest::constructIndex2DNotIndexed() {
+    MeshIndexData indices{Containers::StridedArrayView2D<const char>{}};
+    CORRADE_COMPARE(indices.type(), MeshIndexType{});
+    CORRADE_COMPARE(indices.data().data(), nullptr);
+}
+
 void MeshDataTest::constructIndex2DWrongSize() {
     #ifdef CORRADE_NO_ASSERT
     CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
@@ -1192,6 +1200,8 @@ void MeshDataTest::constructZeroIndices() {
     CORRADE_VERIFY(data.isIndexed());
     CORRADE_COMPARE(data.indexType(), MeshIndexType::UnsignedInt);
     CORRADE_COMPARE(data.indexCount(), 0);
+    CORRADE_COMPARE(data.indices().size(), (Containers::StridedArrayView2D<std::size_t>::Size{0, 4}));
+    CORRADE_COMPARE(data.mutableIndices().size(), (Containers::StridedArrayView2D<std::size_t>::Size{0, 4}));
     CORRADE_COMPARE(data.vertexCount(), 3);
 }
 
@@ -1249,10 +1259,18 @@ void MeshDataTest::constructIndexless() {
        default */
     CORRADE_COMPARE(data.vertexDataFlags(), DataFlag::Owned|DataFlag::Mutable);
     CORRADE_COMPARE(data.primitive(), MeshPrimitive::LineLoop);
-    CORRADE_COMPARE(data.indexData(), nullptr);
     CORRADE_COMPARE(data.importerState(), &importerState);
 
+    /* Access to indexData() and typeless access to (mutable)indices() is
+       allowed, to allow creation of MeshData instances referencing other
+       MeshData without having to branch on isIndexed(). */
     CORRADE_VERIFY(!data.isIndexed());
+    CORRADE_COMPARE(data.indexData(), nullptr);
+    CORRADE_COMPARE(data.indices().data(), nullptr);
+    CORRADE_COMPARE(data.indices().size(), (Containers::StridedArrayView2D<std::size_t>::Size{0, 0}));
+    CORRADE_COMPARE(data.mutableIndices().data(), nullptr);
+    CORRADE_COMPARE(data.mutableIndices().size(), (Containers::StridedArrayView2D<std::size_t>::Size{0, 0}));
+
     CORRADE_COMPARE(data.vertexCount(), 3);
     CORRADE_COMPARE(data.attributeCount(), 1);
     CORRADE_COMPARE(data.attributeFormat(MeshAttribute::Position), VertexFormat::Vector2);
@@ -2777,8 +2795,8 @@ void MeshDataTest::indicesNotIndexed() {
     data.indexCount();
     data.indexType();
     data.indexOffset();
-    data.indices();
     data.indices<UnsignedInt>();
+    data.mutableIndices<UnsignedShort>();
     data.indicesAsArray();
     UnsignedInt a[1];
     data.indicesInto(a);
@@ -2787,7 +2805,7 @@ void MeshDataTest::indicesNotIndexed() {
         "Trade::MeshData::indexType(): the mesh is not indexed\n"
         "Trade::MeshData::indexOffset(): the mesh is not indexed\n"
         "Trade::MeshData::indices(): the mesh is not indexed\n"
-        "Trade::MeshData::indices(): the mesh is not indexed\n"
+        "Trade::MeshData::mutableIndices(): the mesh is not indexed\n"
         "Trade::MeshData::indicesAsArray(): the mesh is not indexed\n"
         "Trade::MeshData::indicesInto(): the mesh is not indexed\n");
 }