From 903153026c530787ad26ebea1a3796407b52cb5a Mon Sep 17 00:00:00 2001 From: Alex Bilger Date: Wed, 18 Mar 2026 16:55:27 +0100 Subject: [PATCH 1/3] [Core] Introduce WriteAccessor move constructor --- .../Core/src/sofa/core/objectmodel/Data.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sofa/framework/Core/src/sofa/core/objectmodel/Data.h b/Sofa/framework/Core/src/sofa/core/objectmodel/Data.h index ff947990e93..797c76f4cea 100644 --- a/Sofa/framework/Core/src/sofa/core/objectmodel/Data.h +++ b/Sofa/framework/Core/src/sofa/core/objectmodel/Data.h @@ -409,14 +409,27 @@ class WriteAccessor< core::objectmodel::Data > : public WriteAccessor protected: data_container_type& data; + bool m_moved = false; /// @internal used by WriteOnlyAccessor WriteAccessor( container_type* c, data_container_type& d) : Inherit(*c), data(d) {} public: + WriteAccessor(WriteAccessor&& other) noexcept : Inherit(std::move(other)), data(other.data) + { + other.m_moved = true; + } + WriteAccessor(data_container_type& d) : Inherit(*d.beginEdit()), data(d) {} WriteAccessor(data_container_type* d) : Inherit(*d->beginEdit()), data(*d) {} - ~WriteAccessor() { data.endEdit(); } + + ~WriteAccessor() + { + if (!m_moved) + { + data.endEdit(); + } + } }; From 8f47c338e3b3e6bf2f60073560436b91b9670fe5 Mon Sep 17 00:00:00 2001 From: Alex Bilger Date: Thu, 19 Mar 2026 14:44:43 +0100 Subject: [PATCH 2/3] rename test files --- Sofa/framework/Core/test/CMakeLists.txt | 4 ++-- .../test/accessor/{ReadAccessor.cpp => ReadAccessor_test.cpp} | 0 .../accessor/{WriteAccessor.cpp => WriteAccessor_test.cpp} | 0 Sofa/framework/Helper/test/CMakeLists.txt | 4 ++-- .../test/accessor/{ReadAccessor.cpp => ReadAccessor_test.cpp} | 0 .../accessor/{WriteAccessor.cpp => WriteAccessor_test.cpp} | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename Sofa/framework/Core/test/accessor/{ReadAccessor.cpp => ReadAccessor_test.cpp} (100%) rename Sofa/framework/Core/test/accessor/{WriteAccessor.cpp => WriteAccessor_test.cpp} (100%) rename Sofa/framework/Helper/test/accessor/{ReadAccessor.cpp => ReadAccessor_test.cpp} (100%) rename Sofa/framework/Helper/test/accessor/{WriteAccessor.cpp => WriteAccessor_test.cpp} (100%) diff --git a/Sofa/framework/Core/test/CMakeLists.txt b/Sofa/framework/Core/test/CMakeLists.txt index a9766161d92..951a6e0701a 100644 --- a/Sofa/framework/Core/test/CMakeLists.txt +++ b/Sofa/framework/Core/test/CMakeLists.txt @@ -3,8 +3,8 @@ cmake_minimum_required(VERSION 3.22) project(Sofa.Core_test) set(SOURCE_FILES - accessor/ReadAccessor.cpp - accessor/WriteAccessor.cpp + accessor/ReadAccessor_test.cpp + accessor/WriteAccessor_test.cpp collision/NarrowPhaseDetection_test.cpp loader/MeshLoader_test.cpp objectmodel/AspectPool_test.cpp diff --git a/Sofa/framework/Core/test/accessor/ReadAccessor.cpp b/Sofa/framework/Core/test/accessor/ReadAccessor_test.cpp similarity index 100% rename from Sofa/framework/Core/test/accessor/ReadAccessor.cpp rename to Sofa/framework/Core/test/accessor/ReadAccessor_test.cpp diff --git a/Sofa/framework/Core/test/accessor/WriteAccessor.cpp b/Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp similarity index 100% rename from Sofa/framework/Core/test/accessor/WriteAccessor.cpp rename to Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp diff --git a/Sofa/framework/Helper/test/CMakeLists.txt b/Sofa/framework/Helper/test/CMakeLists.txt index 67aa4c3fe8b..b0ba94b2111 100644 --- a/Sofa/framework/Helper/test/CMakeLists.txt +++ b/Sofa/framework/Helper/test/CMakeLists.txt @@ -13,8 +13,8 @@ set(SOURCE_FILES StringUtils_test.cpp TagFactory_test.cpp Utils_test.cpp - accessor/ReadAccessor.cpp - accessor/WriteAccessor.cpp + accessor/ReadAccessor_test.cpp + accessor/WriteAccessor_test.cpp io/MeshOBJ_test.cpp io/STBImage_test.cpp io/XspLoader_test.cpp diff --git a/Sofa/framework/Helper/test/accessor/ReadAccessor.cpp b/Sofa/framework/Helper/test/accessor/ReadAccessor_test.cpp similarity index 100% rename from Sofa/framework/Helper/test/accessor/ReadAccessor.cpp rename to Sofa/framework/Helper/test/accessor/ReadAccessor_test.cpp diff --git a/Sofa/framework/Helper/test/accessor/WriteAccessor.cpp b/Sofa/framework/Helper/test/accessor/WriteAccessor_test.cpp similarity index 100% rename from Sofa/framework/Helper/test/accessor/WriteAccessor.cpp rename to Sofa/framework/Helper/test/accessor/WriteAccessor_test.cpp From 428fa861e19a8209e983fea8c48fb64689952392 Mon Sep 17 00:00:00 2001 From: Alex Bilger Date: Thu, 19 Mar 2026 15:02:21 +0100 Subject: [PATCH 3/3] add tests on move constructor --- .../Core/test/accessor/WriteAccessor_test.cpp | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp b/Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp index 39c77ba571b..c5a2c38567c 100644 --- a/Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp +++ b/Sofa/framework/Core/test/accessor/WriteAccessor_test.cpp @@ -22,6 +22,7 @@ #include #include +#include #include namespace sofa @@ -79,5 +80,74 @@ TEST(WriteAccessor, VectorTypes) EXPECT_FLOAT_EQ(vector.getValue()[3], 6.f); } +TEST(WriteAccessor, MoveConstructor) +{ + Data floatValue { 12.f }; + int initialCounter = floatValue.getCounter(); + + { + sofa::helper::WriteAccessor floatAccessor(floatValue); + EXPECT_EQ(floatValue.getCounter(), initialCounter + 1); + EXPECT_FLOAT_EQ(floatAccessor.ref(), 12.f); + + sofa::helper::WriteAccessor floatAccessorMoved(std::move(floatAccessor)); + EXPECT_EQ(floatValue.getCounter(), initialCounter + 1); + EXPECT_FLOAT_EQ(floatAccessorMoved.ref(), 12.f); + + // Even if we moved from it, we can still call ref() and wref() + // but it is not recommended as it doesn't hold the responsibility of endEdit anymore. + EXPECT_FLOAT_EQ(floatAccessor.ref(), 12.f); + + floatAccessorMoved.wref() = 14.f; + EXPECT_FLOAT_EQ(floatValue.getValue(), 14.f); + } + + Data data { 1.f }; + int endEditCount = 0; + sofa::core::objectmodel::DataCallback cb; + cb.addInput(&data); + cb.addCallback([&endEditCount](){ + endEditCount++; + }); + + { + sofa::helper::WriteAccessor acc1(data); + EXPECT_EQ(endEditCount, 0); // beginEdit doesn't trigger callback + { + sofa::helper::WriteAccessor acc2(std::move(acc1)); + EXPECT_EQ(endEditCount, 0); + } + // acc2 out of scope, endEdit called + EXPECT_EQ(endEditCount, 1); + } + // acc1 out of scope, if move constructor worked correctly, endEdit should NOT be called again. + EXPECT_EQ(endEditCount, 1); +} + +TEST(WriteAccessor, MoveConstructorVector) +{ + Data> vector { sofa::type::vector { 0.f, 1.f, 2.f, 3.f, 4.f} }; + int endEditCount = 0; + sofa::core::objectmodel::DataCallback cb; + cb.addInput(&vector); + cb.addCallback([&endEditCount](){ + endEditCount++; + }); + + { + sofa::helper::WriteAccessor acc1(vector); + EXPECT_EQ(acc1.size(), 5); + { + sofa::helper::WriteAccessor acc2(std::move(acc1)); + EXPECT_EQ(acc2.size(), 5); + EXPECT_EQ(acc1.size(), 5); // Still valid but no longer responsible for endEdit + acc2[0] = 10.f; + } + EXPECT_EQ(endEditCount, 1); + } + EXPECT_EQ(endEditCount, 1); + EXPECT_FLOAT_EQ(vector.getValue()[0], 10.f); +} + }