Trade: add MeshData::findAttributeId().

doc/changelog.dox

@@ -433,6 +433,11 @@ See also:
     @ref Trade::PhongMaterialData::commonTextureCoordinates() exposing a
     common texture coordinate set as a complement to a per-texture property
     added in 2020.06
+-   Added @ref Trade::MeshData::findAttributeId() for an ability to check that
+    an attribute exists and retrieve its ID in a single step, avoiding a double
+    lookup compared to @relativeref{Trade::MeshData,hasAttribute()} +
+    @relativeref{Trade::MeshData,attributeId()}. This also makes the API
+    consistent with @ref Trade::SceneData::findFieldId().
 -   Added @ref Trade::TextureType::Texture1DArray,
     @relativeref{Trade::TextureType,Texture2DArray} and
     @relativeref{Trade::TextureType,CubeMapArray} in order to be able to

src/Magnum/Trade/MeshData.cpp

@@ -25,6 +25,7 @@
 
 #include "MeshData.h"
 
+#include <Corrade/Containers/Optional.h>
 #include <Corrade/Utility/Algorithms.h>
 #ifndef CORRADE_NO_ASSERT
 #include <Corrade/Utility/Format.h>
@@ -327,46 +328,46 @@ UnsignedInt MeshData::attributeCount(const MeshAttribute name) const {
     return count;
 }
 
-UnsignedInt MeshData::attributeFor(const MeshAttribute name, UnsignedInt id) const {
+UnsignedInt MeshData::findAttributeIdInternal(const MeshAttribute name, UnsignedInt id) const {
     for(std::size_t i = 0; i != _attributes.size(); ++i) {
         if(_attributes[i]._name != name) continue;
         if(id-- == 0) return i;
     }
-
-    #ifdef CORRADE_NO_ASSERT
-    CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
-    #else
     return ~UnsignedInt{};
-    #endif
+}
+
+Containers::Optional<UnsignedInt> MeshData::findAttributeId(const MeshAttribute name, UnsignedInt id) const {
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
+    return attributeId == ~UnsignedInt{} ? Containers::Optional<UnsignedInt>{} : attributeId;
 }
 
 UnsignedInt MeshData::attributeId(const MeshAttribute name, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attributeId(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return attributeId;
 }
 
 VertexFormat MeshData::attributeFormat(const MeshAttribute name, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attributeFormat(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return _attributes[attributeId]._format;
 }
 
 std::size_t MeshData::attributeOffset(const MeshAttribute name, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attributeOffset(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     /* Calculation is non-trivial, delegating */
     return attributeOffset(attributeId);
 }
 
 UnsignedInt MeshData::attributeStride(const MeshAttribute name, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attributeStride(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return _attributes[attributeId]._stride;
 }
 
 UnsignedShort MeshData::attributeArraySize(const MeshAttribute name, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attributeArraySize(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return _attributes[attributeId]._arraySize;
 }
@@ -404,7 +405,7 @@ Containers::StridedArrayView2D<char> MeshData::mutableAttribute(const UnsignedIn
 }
 
 Containers::StridedArrayView2D<const char> MeshData::attribute(const MeshAttribute name, UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::attribute(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return attribute(attributeId);
 }
@@ -412,7 +413,7 @@ Containers::StridedArrayView2D<const char> MeshData::attribute(const MeshAttribu
 Containers::StridedArrayView2D<char> MeshData::mutableAttribute(const MeshAttribute name, UnsignedInt id) {
     CORRADE_ASSERT(_vertexDataFlags & DataFlag::Mutable,
         "Trade::MeshData::mutableAttribute(): vertex data not mutable", {});
-    const UnsignedInt attributeId = attributeFor(name, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(name, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::mutableAttribute(): index" << id << "out of range for" << attributeCount(name) << name << "attributes", {});
     return mutableAttribute(attributeId);
 }
@@ -442,7 +443,7 @@ Containers::Array<UnsignedInt> MeshData::indicesAsArray() const {
 }
 
 void MeshData::positions2DInto(const Containers::StridedArrayView1D<Vector2>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Position, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Position, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::positions2DInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Position) << "position attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::positions2DInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -492,7 +493,7 @@ Containers::Array<Vector2> MeshData::positions2DAsArray(const UnsignedInt id) co
 }
 
 void MeshData::positions3DInto(const Containers::StridedArrayView1D<Vector3>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Position, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Position, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::positions3DInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Position) << "position attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::positions3DInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -591,7 +592,7 @@ void tangentsOrNormalsInto(const Containers::StridedArrayView1D<const void>& att
 }
 
 void MeshData::tangentsInto(const Containers::StridedArrayView1D<Vector3>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Tangent, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Tangent, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::tangentsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Tangent) << "tangent attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::tangentsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -620,7 +621,7 @@ Containers::Array<Vector3> MeshData::tangentsAsArray(const UnsignedInt id) const
 }
 
 void MeshData::bitangentSignsInto(const Containers::StridedArrayView1D<Float>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Tangent, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Tangent, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::bitangentSignsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Tangent) << "tangent attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::bitangentSignsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -647,7 +648,7 @@ Containers::Array<Float> MeshData::bitangentSignsAsArray(const UnsignedInt id) c
 }
 
 void MeshData::bitangentsInto(const Containers::StridedArrayView1D<Vector3>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Bitangent, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Bitangent, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::bitangentsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Bitangent) << "bitangent attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::bitangentsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -663,7 +664,7 @@ Containers::Array<Vector3> MeshData::bitangentsAsArray(const UnsignedInt id) con
 }
 
 void MeshData::normalsInto(const Containers::StridedArrayView1D<Vector3>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Normal, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Normal, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::normalsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Normal) << "normal attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::normalsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -679,7 +680,7 @@ Containers::Array<Vector3> MeshData::normalsAsArray(const UnsignedInt id) const
 }
 
 void MeshData::textureCoordinates2DInto(const Containers::StridedArrayView1D<Vector2>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::TextureCoordinates, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::TextureCoordinates, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::textureCoordinates2DInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::TextureCoordinates) << "texture coordinate attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::textureCoordinates2DInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -718,7 +719,7 @@ Containers::Array<Vector2> MeshData::textureCoordinates2DAsArray(const UnsignedI
 }
 
 void MeshData::colorsInto(const Containers::StridedArrayView1D<Color4>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::Color, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::Color, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::colorsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::Color) << "color attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::colorsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];
@@ -771,7 +772,7 @@ Containers::Array<Color4> MeshData::colorsAsArray(const UnsignedInt id) const {
 }
 
 void MeshData::objectIdsInto(const Containers::StridedArrayView1D<UnsignedInt>& destination, const UnsignedInt id) const {
-    const UnsignedInt attributeId = attributeFor(MeshAttribute::ObjectId, id);
+    const UnsignedInt attributeId = findAttributeIdInternal(MeshAttribute::ObjectId, id);
     CORRADE_ASSERT(attributeId != ~UnsignedInt{}, "Trade::MeshData::objectIdsInto(): index" << id << "out of range for" << attributeCount(MeshAttribute::ObjectId) << "object ID attributes", );
     CORRADE_ASSERT(destination.size() == _vertexCount, "Trade::MeshData::objectIdsInto(): expected a view with" << _vertexCount << "elements but got" << destination.size(), );
     const MeshAttributeData& attribute = _attributes[attributeId];

src/Magnum/Trade/MeshData.h

@@ -1276,11 +1276,24 @@ class MAGNUM_TRADE_EXPORT MeshData {
          */
         UnsignedInt attributeCount(MeshAttribute name) const;
 
+        /**
+         * @brief Find an absolute ID of a named attribute
+         * @m_since_latest
+         *
+         * If @p name isn't present or @p id is not smaller than
+         * @ref attributeCount(MeshAttribute) const, returns
+         * @ref Containers::NullOpt. The lookup is done in an
+         * @f$ \mathcal{O}(n) @f$ complexity with @f$ n @f$ being the attribute
+         * count.
+         * @see @ref hasAttribute(), @ref attributeId()
+         */
+        Containers::Optional<UnsignedInt> findAttributeId(MeshAttribute name, UnsignedInt id = 0) const;
+
         /**
          * @brief Absolute ID of a named attribute
          *
-         * The @p id is expected to be smaller than
-         * @ref attributeCount(MeshAttribute) const.
+         * Like @ref findAttributeId(), but the @p id is expected to be smaller
+         * than @ref attributeCount(MeshAttribute) const.
          */
         UnsignedInt attributeId(MeshAttribute name, UnsignedInt id = 0) const;
 
@@ -1580,7 +1593,8 @@ class MAGNUM_TRADE_EXPORT MeshData {
          *
          * Like @ref tangentsAsArray(), but puts the result into @p destination
          * instead of allocating a new array. Expects that @p destination is
-         * sized to contain exactly all data.
+         * sized to contain exactly all data. Use @ref bitangentSignsInto() to
+         * extract the fourth component wit bitangent direction, if present.
          * @see @ref vertexCount()
          */
         void tangentsInto(const Containers::StridedArrayView1D<Vector3>& destination, UnsignedInt id = 0) const;
@@ -1803,8 +1817,9 @@ class MAGNUM_TRADE_EXPORT MeshData {
         friend AbstractImporter;
         friend AbstractSceneConverter;
 
-        /* Internal helper that doesn't assert, unlike attributeId() */
-        UnsignedInt attributeFor(MeshAttribute name, UnsignedInt id) const;
+        /* Internal helper without the extra overhead from Optional, returns
+           ~UnsignedInt{} on failure */
+        UnsignedInt findAttributeIdInternal(MeshAttribute name, UnsignedInt id) const;
 
         /* Like attribute(), but returning just a 1D view */
         MAGNUM_TRADE_LOCAL Containers::StridedArrayView1D<const void> attributeDataViewInternal(const MeshAttributeData& attribute) const;
@@ -2222,7 +2237,7 @@ template<class T, class> Containers::StridedArrayView1D<const T> MeshData::attri
     if(!data.stride()[1]) return {};
     #endif
     #ifndef CORRADE_NO_ASSERT
-    if(!checkVertexFormatCompatibility<T>(_attributes[attributeFor(name, id)], "Trade::MeshData::attribute():")) return {};
+    if(!checkVertexFormatCompatibility<T>(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::attribute():")) return {};
     #endif
     return Containers::arrayCast<1, const T>(data);
 }
@@ -2233,7 +2248,7 @@ template<class T, class> Containers::StridedArrayView2D<const typename std::remo
     if(!data.stride()[1]) return {};
     #endif
     #ifndef CORRADE_NO_ASSERT
-    if(!checkVertexFormatCompatibility<T>(_attributes[attributeFor(name, id)], "Trade::MeshData::attribute():")) return {};
+    if(!checkVertexFormatCompatibility<T>(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::attribute():")) return {};
     #endif
     return Containers::arrayCast<2, const typename std::remove_extent<T>::type>(data);
 }
@@ -2244,7 +2259,7 @@ template<class T, class> Containers::StridedArrayView1D<T> MeshData::mutableAttr
     if(!data.stride()[1]) return {};
     #endif
     #ifndef CORRADE_NO_ASSERT
-    if(!checkVertexFormatCompatibility<T>(_attributes[attributeFor(name, id)], "Trade::MeshData::mutableAttribute():")) return {};
+    if(!checkVertexFormatCompatibility<T>(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::mutableAttribute():")) return {};
     #endif
     return Containers::arrayCast<1, T>(data);
 }
@@ -2255,7 +2270,7 @@ template<class T, class> Containers::StridedArrayView2D<typename std::remove_ext
     if(!data.stride()[1]) return {};
     #endif
     #ifndef CORRADE_NO_ASSERT
-    if(!checkVertexFormatCompatibility<T>(_attributes[attributeFor(name, id)], "Trade::MeshData::mutableAttribute():")) return {};
+    if(!checkVertexFormatCompatibility<T>(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::mutableAttribute():")) return {};
     #endif
     return Containers::arrayCast<2, typename std::remove_extent<T>::type>(data);
 }

src/Magnum/Trade/Test/MeshDataTest.cpp

@@ -24,6 +24,7 @@
 */
 
 #include <sstream>
+#include <Corrade/Containers/Optional.h>
 #include <Corrade/TestSuite/Tester.h>
 #include <Corrade/TestSuite/Compare/Container.h>
 #include <Corrade/Utility/DebugStl.h>
@@ -1169,11 +1170,18 @@ void MeshDataTest::construct() {
     CORRADE_COMPARE(data.attributeCount(meshAttributeCustom(13)), 1);
     CORRADE_COMPARE(data.attributeCount(MeshAttribute::Color), 0);
     CORRADE_COMPARE(data.attributeCount(meshAttributeCustom(23)), 0);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::Position), 0);
     CORRADE_COMPARE(data.attributeId(MeshAttribute::Position), 0);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::Normal), 2);
     CORRADE_COMPARE(data.attributeId(MeshAttribute::Normal), 2);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::TextureCoordinates), 1);
     CORRADE_COMPARE(data.attributeId(MeshAttribute::TextureCoordinates), 1);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::TextureCoordinates, 1), 3);
     CORRADE_COMPARE(data.attributeId(MeshAttribute::TextureCoordinates, 1), 3);
+    CORRADE_COMPARE(data.findAttributeId(meshAttributeCustom(13)), 4);
     CORRADE_COMPARE(data.attributeId(meshAttributeCustom(13)), 4);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::Color), Containers::NullOpt);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::TextureCoordinates, 2), Containers::NullOpt);
     CORRADE_COMPARE(data.attributeFormat(MeshAttribute::Position),
         VertexFormat::Vector3);
     CORRADE_COMPARE(data.attributeFormat(MeshAttribute::Normal),
@@ -2856,6 +2864,11 @@ void MeshDataTest::attributeNotFound() {
     MeshAttributeData colors2{MeshAttribute::Color, VertexFormat::Vector4, nullptr};
     MeshData data{MeshPrimitive::Points, nullptr, {colors1, colors2}};
 
+    /* This is fine */
+    CORRADE_COMPARE(data.attributeCount(MeshAttribute::Position), 0);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::Position), Containers::NullOpt);
+    CORRADE_COMPARE(data.findAttributeId(MeshAttribute::Color, 2), Containers::NullOpt);
+
     std::ostringstream out;
     Error redirectError{&out};
     data.attributeData(2);