From b7de0916ab8d9a039024e5c7630d000bafaacc55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 11 May 2023 19:05:16 +0200 Subject: [PATCH] Trade: make SceneData::releaseData() and releaseFieldData() independent. In some cases it's needed to release (or copy) the data first and only then access the field properties through the SceneData to (optionally) re-route the field views to a new data location. But since releaseData() was implicitly erasing field data as well, this wasn't possible and the only other option was to release the field data first and then access them through the low-level SceneFieldData API with all convenience lost. This makes the release*Data() APIs a bit dangerous to use, but that should be fine -- those aren't meant to be used by regular code anyway. Similar caveat is with MaterialData already. --- src/Magnum/Trade/SceneData.cpp | 1 - src/Magnum/Trade/SceneData.h | 17 +++++++++-------- src/Magnum/Trade/Test/SceneDataTest.cpp | 9 ++++++--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Magnum/Trade/SceneData.cpp b/src/Magnum/Trade/SceneData.cpp index b1881c0ea..cfdd266e7 100644 --- a/src/Magnum/Trade/SceneData.cpp +++ b/src/Magnum/Trade/SceneData.cpp @@ -2927,7 +2927,6 @@ Containers::Array SceneData::releaseFieldData() { } Containers::Array SceneData::releaseData() { - _fields = {}; Containers::Array out = std::move(_data); _data = {}; return out; diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index ca2d2adef..34939e27f 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/src/Magnum/Trade/SceneData.h @@ -3684,14 +3684,15 @@ class MAGNUM_TRADE_EXPORT SceneData { * @brief Release data storage * @m_since_latest * - * Releases the ownership of the data array and resets internal - * field-related state to default. The scene then behaves like it has - * no fields and no data. If you want to release field data as well, - * first call @ref releaseFieldData() and then this function. - * - * Note that the returned array has a custom no-op deleter when the - * data are not owned by the scene, and while the returned array type - * is mutable, the actual memory might be not. + * Releases the ownership of the data array. Note that the returned + * array has a custom no-op deleter when the data are not owned by the + * scene, and while the returned array type is mutable, the actual + * memory might be not. + * + * @attention Querying any field after calling @ref releaseData() has + * undefined behavior and might lead to crashes. This is done + * intentionally in order to simplify the interaction between this + * function and @ref releaseFieldData(). * @see @ref data(), @ref dataFlags() */ Containers::Array releaseData(); diff --git a/src/Magnum/Trade/Test/SceneDataTest.cpp b/src/Magnum/Trade/Test/SceneDataTest.cpp index 3d86211e1..855c9cb3f 100644 --- a/src/Magnum/Trade/Test/SceneDataTest.cpp +++ b/src/Magnum/Trade/Test/SceneDataTest.cpp @@ -7639,11 +7639,14 @@ void SceneDataTest::releaseData() { CORRADE_COMPARE(released.data(), view.data()); CORRADE_COMPARE(released.size(), 3*sizeof(Field)); - /* Both fields and data are all gone */ - CORRADE_COMPARE(static_cast(scene.fieldData()), nullptr); - CORRADE_COMPARE(scene.fieldCount(), 0); + /* Data are gone */ CORRADE_COMPARE(static_cast(scene.data()), nullptr); + /* Fields stay untouched so it's possible to release them separately + without being forced to order these two releases in any way */ + CORRADE_VERIFY(scene.fieldData().data()); + CORRADE_COMPARE(scene.fieldCount(), 2); + /* Object count and type stays untouched, as it con't result in any dangling data access */ CORRADE_COMPARE(scene.mappingBound(), 50);