From 3869e7616a31a0517d776027311d20aa7fa357e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 19 Dec 2021 20:24:14 +0100 Subject: [PATCH] Trade: add MeshData::findAttributeId(). --- doc/changelog.dox | 5 +++ src/Magnum/Trade/MeshData.cpp | 45 +++++++++++++------------- src/Magnum/Trade/MeshData.h | 33 +++++++++++++------ src/Magnum/Trade/Test/MeshDataTest.cpp | 13 ++++++++ 4 files changed, 65 insertions(+), 31 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 973f68847..2a2ecf21f 100644 --- a/doc/changelog.dox +++ b/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 diff --git a/src/Magnum/Trade/MeshData.cpp b/src/Magnum/Trade/MeshData.cpp index 54f1d0165..1b493b210 100644 --- a/src/Magnum/Trade/MeshData.cpp +++ b/src/Magnum/Trade/MeshData.cpp @@ -25,6 +25,7 @@ #include "MeshData.h" +#include #include #ifndef CORRADE_NO_ASSERT #include @@ -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 MeshData::findAttributeId(const MeshAttribute name, UnsignedInt id) const { + const UnsignedInt attributeId = findAttributeIdInternal(name, id); + return attributeId == ~UnsignedInt{} ? Containers::Optional{} : 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 MeshData::mutableAttribute(const UnsignedIn } Containers::StridedArrayView2D 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 MeshData::attribute(const MeshAttribu Containers::StridedArrayView2D 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 MeshData::indicesAsArray() const { } void MeshData::positions2DInto(const Containers::StridedArrayView1D& 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 MeshData::positions2DAsArray(const UnsignedInt id) co } void MeshData::positions3DInto(const Containers::StridedArrayView1D& 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& att } void MeshData::tangentsInto(const Containers::StridedArrayView1D& 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 MeshData::tangentsAsArray(const UnsignedInt id) const } void MeshData::bitangentSignsInto(const Containers::StridedArrayView1D& 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 MeshData::bitangentSignsAsArray(const UnsignedInt id) c } void MeshData::bitangentsInto(const Containers::StridedArrayView1D& 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 MeshData::bitangentsAsArray(const UnsignedInt id) con } void MeshData::normalsInto(const Containers::StridedArrayView1D& 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 MeshData::normalsAsArray(const UnsignedInt id) const } void MeshData::textureCoordinates2DInto(const Containers::StridedArrayView1D& 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 MeshData::textureCoordinates2DAsArray(const UnsignedI } void MeshData::colorsInto(const Containers::StridedArrayView1D& 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 MeshData::colorsAsArray(const UnsignedInt id) const { } void MeshData::objectIdsInto(const Containers::StridedArrayView1D& 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]; diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h index f32b39369..2dd1e1990 100644 --- a/src/Magnum/Trade/MeshData.h +++ b/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 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& 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 attributeDataViewInternal(const MeshAttributeData& attribute) const; @@ -2222,7 +2237,7 @@ template Containers::StridedArrayView1D MeshData::attri if(!data.stride()[1]) return {}; #endif #ifndef CORRADE_NO_ASSERT - if(!checkVertexFormatCompatibility(_attributes[attributeFor(name, id)], "Trade::MeshData::attribute():")) return {}; + if(!checkVertexFormatCompatibility(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::attribute():")) return {}; #endif return Containers::arrayCast<1, const T>(data); } @@ -2233,7 +2248,7 @@ template Containers::StridedArrayView2D(_attributes[attributeFor(name, id)], "Trade::MeshData::attribute():")) return {}; + if(!checkVertexFormatCompatibility(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::attribute():")) return {}; #endif return Containers::arrayCast<2, const typename std::remove_extent::type>(data); } @@ -2244,7 +2259,7 @@ template Containers::StridedArrayView1D MeshData::mutableAttr if(!data.stride()[1]) return {}; #endif #ifndef CORRADE_NO_ASSERT - if(!checkVertexFormatCompatibility(_attributes[attributeFor(name, id)], "Trade::MeshData::mutableAttribute():")) return {}; + if(!checkVertexFormatCompatibility(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::mutableAttribute():")) return {}; #endif return Containers::arrayCast<1, T>(data); } @@ -2255,7 +2270,7 @@ template Containers::StridedArrayView2D(_attributes[attributeFor(name, id)], "Trade::MeshData::mutableAttribute():")) return {}; + if(!checkVertexFormatCompatibility(_attributes[findAttributeIdInternal(name, id)], "Trade::MeshData::mutableAttribute():")) return {}; #endif return Containers::arrayCast<2, typename std::remove_extent::type>(data); } diff --git a/src/Magnum/Trade/Test/MeshDataTest.cpp b/src/Magnum/Trade/Test/MeshDataTest.cpp index 049980f11..c595dd258 100644 --- a/src/Magnum/Trade/Test/MeshDataTest.cpp +++ b/src/Magnum/Trade/Test/MeshDataTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include #include @@ -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);