From 9ba9c406aa36e2be672ce2cc2e1e4b4d0e16c368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 30 Nov 2022 11:27:34 +0100 Subject: [PATCH] Trade: deinline SceneFieldData::*Data() accessors. These APIs are mostly just for debugging purposes, not widely used, so it doesn't make sense to have them as constexpr in the header. (Plus the returned void view is useless in a constexpr context anyway.) This header size is getting out of hand, so every stripped bit counts. Also now that they're no longer constexpr, I can go back to using regular assertions. The reinterpret_cast<> wasn't needed either. --- src/Magnum/Trade/SceneData.cpp | 32 +++++++++++++++++++++++++ src/Magnum/Trade/SceneData.h | 30 ++++------------------- src/Magnum/Trade/Test/SceneDataTest.cpp | 17 ++++++------- 3 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/Magnum/Trade/SceneData.cpp b/src/Magnum/Trade/SceneData.cpp index 3ebbac9ca..29b4ad022 100644 --- a/src/Magnum/Trade/SceneData.cpp +++ b/src/Magnum/Trade/SceneData.cpp @@ -531,6 +531,38 @@ SceneFieldData::SceneFieldData(const SceneField name, const Containers::StridedA CORRADE_ASSERT(fieldData.isContiguous<1>(), "Trade::SceneFieldData: second field view dimension is not contiguous", ); } +Containers::StridedArrayView1D SceneFieldData::mappingData() const { + CORRADE_ASSERT(!(_flags & SceneFieldFlag::OffsetOnly), + "Trade::SceneFieldData::mappingData(): the field is offset-only, supply a data array", {}); + return Containers::StridedArrayView1D{ + /* We're *sure* the view is correct, so faking the view size */ + /** @todo better ideas for the StridedArrayView API? */ + {_mappingData.pointer, ~std::size_t{}}, _size, _mappingStride}; +} + +Containers::StridedArrayView1D SceneFieldData::mappingData(const Containers::ArrayView data) const { + return Containers::StridedArrayView1D{ + /* We're *sure* the view is correct, so faking the view size */ + /** @todo better ideas for the StridedArrayView API? */ + data, _flags & SceneFieldFlag::OffsetOnly ? static_cast(data.data()) + _mappingData.offset : _mappingData.pointer, _size, _mappingStride}; +} + +Containers::StridedArrayView1D SceneFieldData::fieldData() const { + CORRADE_ASSERT(!(_flags & SceneFieldFlag::OffsetOnly), + "Trade::SceneFieldData::fieldData(): the field is offset-only, supply a data array", {}); + return Containers::StridedArrayView1D{ + /* We're *sure* the view is correct, so faking the view size */ + /** @todo better ideas for the StridedArrayView API? */ + {_fieldData.pointer, ~std::size_t{}}, _size, _fieldStride}; +} + +Containers::StridedArrayView1D SceneFieldData::fieldData(const Containers::ArrayView data) const { + return Containers::StridedArrayView1D{ + /* We're *sure* the view is correct, so faking the view size */ + /** @todo better ideas for the StridedArrayView API? */ + data, _flags & SceneFieldFlag::OffsetOnly ? static_cast(data.data()) + _fieldData.offset : _fieldData.pointer, _size, _fieldStride}; +} + Containers::Array sceneFieldDataNonOwningArray(const Containers::ArrayView view) { return Containers::Array{const_cast(view.data()), view.size(), Implementation::nonOwnedArrayDeleter}; } diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index 9778c830a..536388b72 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/src/Magnum/Trade/SceneData.h @@ -812,13 +812,7 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { * overload instead. * @see @ref flags() */ - constexpr Containers::StridedArrayView1D mappingData() const { - return Containers::StridedArrayView1D{ - /* We're *sure* the view is correct, so faking the view size */ - /** @todo better ideas for the StridedArrayView API? */ - {_mappingData.pointer, ~std::size_t{}}, _size, - (CORRADE_CONSTEXPR_ASSERT(!(_flags & SceneFieldFlag::OffsetOnly), "Trade::SceneFieldData::mappingData(): the field is offset-only, supply a data array"), _mappingStride)}; - } + Containers::StridedArrayView1D mappingData() const; /** * @brief Type-erased object mapping data for an offset-only field @@ -827,12 +821,7 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { * @p data parameter is ignored. * @see @ref flags(), @ref mappingData() const */ - Containers::StridedArrayView1D mappingData(Containers::ArrayView data) const { - return Containers::StridedArrayView1D{ - /* We're *sure* the view is correct, so faking the view size */ - /** @todo better ideas for the StridedArrayView API? */ - data, _flags & SceneFieldFlag::OffsetOnly ? reinterpret_cast(data.data()) + _mappingData.offset : _mappingData.pointer, _size, _mappingStride}; - } + Containers::StridedArrayView1D mappingData(Containers::ArrayView data) const; /** @brief Field type */ constexpr SceneFieldType fieldType() const { return _fieldType; } @@ -848,13 +837,7 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { * overload instead. * @see @ref flags() */ - constexpr Containers::StridedArrayView1D fieldData() const { - return Containers::StridedArrayView1D{ - /* We're *sure* the view is correct, so faking the view size */ - /** @todo better ideas for the StridedArrayView API? */ - {_fieldData.pointer, ~std::size_t{}}, _size, - (CORRADE_CONSTEXPR_ASSERT(!(_flags & SceneFieldFlag::OffsetOnly), "Trade::SceneFieldData::fieldData(): the field is offset-only, supply a data array"), _fieldStride)}; - } + Containers::StridedArrayView1D fieldData() const; /** * @brief Type-erased field data for an offset-only field @@ -863,12 +846,7 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { * @p data parameter is ignored. * @see @ref flags(), @ref fieldData() const */ - Containers::StridedArrayView1D fieldData(Containers::ArrayView data) const { - return Containers::StridedArrayView1D{ - /* We're *sure* the view is correct, so faking the view size */ - /** @todo better ideas for the StridedArrayView API? */ - data, _flags & SceneFieldFlag::OffsetOnly ? reinterpret_cast(data.data()) + _fieldData.offset : _fieldData.pointer, _size, _fieldStride}; - } + Containers::StridedArrayView1D fieldData(Containers::ArrayView data) const; private: friend SceneData; diff --git a/src/Magnum/Trade/Test/SceneDataTest.cpp b/src/Magnum/Trade/Test/SceneDataTest.cpp index bf760404b..dc41b7be2 100644 --- a/src/Magnum/Trade/Test/SceneDataTest.cpp +++ b/src/Magnum/Trade/Test/SceneDataTest.cpp @@ -816,21 +816,22 @@ void SceneDataTest::constructField() { constexpr SceneField name = crotations.name(); constexpr SceneFieldFlags flags = crotations.flags(); constexpr SceneMappingType mappingType = crotations.mappingType(); - constexpr Containers::StridedArrayView1D mappingData = crotations.mappingData(); constexpr SceneFieldType fieldType = crotations.fieldType(); constexpr UnsignedShort fieldArraySize = crotations.fieldArraySize(); - constexpr Containers::StridedArrayView1D fieldData = crotations.fieldData(); CORRADE_COMPARE(name, SceneField::Rotation); CORRADE_COMPARE(flags, SceneFieldFlag::ImplicitMapping); CORRADE_COMPARE(mappingType, SceneMappingType::UnsignedShort); - CORRADE_COMPARE(mappingData.size(), 3); - CORRADE_COMPARE(mappingData.stride(), sizeof(UnsignedShort)); - CORRADE_COMPARE(mappingData.data(), RotationMapping2D); CORRADE_COMPARE(fieldType, SceneFieldType::Complexd); CORRADE_COMPARE(fieldArraySize, 0); - CORRADE_COMPARE(fieldData.size(), 3); - CORRADE_COMPARE(fieldData.stride(), sizeof(Complexd)); - CORRADE_COMPARE(fieldData.data(), RotationField2D); + /* These are deliberately not constexpr to save header size a bit -- + compared to SceneField APIs they get used very little and it's mostly + useless in a constexpr context anyway */ + CORRADE_COMPARE(crotations.mappingData().size(), 3); + CORRADE_COMPARE(crotations.mappingData().stride(), sizeof(UnsignedShort)); + CORRADE_COMPARE(crotations.mappingData().data(), RotationMapping2D); + CORRADE_COMPARE(crotations.fieldData().size(), 3); + CORRADE_COMPARE(crotations.fieldData().stride(), sizeof(Complexd)); + CORRADE_COMPARE(crotations.fieldData().data(), RotationField2D); #endif }