From 2895339b1a0d2ba6d837c5aa62258c4a815d3a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9renger=20Dalle-Cort?= Date: Mon, 2 Dec 2024 16:56:42 -0500 Subject: [PATCH 1/2] refactor(tools::UniqueOrderedList): renamed UniqueList, ElemType => Element, add ConstIterator/Iterator begin/end vbegin/cend --- CMakeLists.txt | 4 +- src/ndbl/gui/GraphView.cpp | 6 +- src/ndbl/gui/GraphView.h | 6 +- src/ndbl/gui/Nodable.cpp | 4 +- .../{UniqueOrderedList.h => UniqueList.h} | 40 ++++++++++---- src/tools/core/UniqueList.specs.cpp | 10 ++++ src/tools/core/UniqueOrderedList.specs.cpp | 12 ---- ...deredVariantList.h => UniqueVariantList.h} | 55 +++++++++++-------- ....specs.cpp => UniqueVariantList.specs.cpp} | 11 ++-- 9 files changed, 85 insertions(+), 63 deletions(-) rename src/tools/core/{UniqueOrderedList.h => UniqueList.h} (51%) create mode 100644 src/tools/core/UniqueList.specs.cpp delete mode 100644 src/tools/core/UniqueOrderedList.specs.cpp rename src/tools/core/{UniqueOrderedVariantList.h => UniqueVariantList.h} (69%) rename src/tools/core/{UniqueOrderedVariantList.specs.cpp => UniqueVariantList.specs.cpp} (71%) diff --git a/CMakeLists.txt b/CMakeLists.txt index f98a5d018..a240001b7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -210,9 +210,9 @@ target_link_libraries( add_executable( test-tools - src/tools/core/UniqueOrderedList.specs.cpp + src/tools/core/UniqueList.specs.cpp src/tools/core/Variant.specs.cpp - src/tools/core/UniqueOrderedVariantList.specs.cpp + src/tools/core/UniqueVariantList.specs.cpp src/tools/core/Delegate.specs.cpp src/tools/core/reflection/reflection.specs.cpp src/tools/core/reflection/Type.specs.cpp diff --git a/src/ndbl/gui/GraphView.cpp b/src/ndbl/gui/GraphView.cpp index a76bed484..e53553b90 100644 --- a/src/ndbl/gui/GraphView.cpp +++ b/src/ndbl/gui/GraphView.cpp @@ -646,7 +646,7 @@ void GraphView::_on_graph_change() m_physics_dirty = true; } -void GraphView::_on_selection_change(Selection::EventType type, Selection::ElemType elem) +void GraphView::_on_selection_change(Selection::EventType type, Selection::Element elem) { bool selected = type == Selection::EventType::Append; @@ -732,7 +732,7 @@ void GraphView::draw_create_node_context_menu(CreateNodeCtxMenu& menu, SlotView* void GraphView::drag_state_enter() { - for( const Selectable& elem : m_selection.data() ) + for( const Selectable& elem : m_selection ) { if ( auto* nodeview = elem.get_if() ) nodeview->state().set_pinned(); @@ -746,7 +746,7 @@ void GraphView::drag_state_tick() const Vec2 delta = ImGui::GetMouseDragDelta(); ImGui::ResetMouseDragDelta(); - for ( const Selectable& elem : m_selection.data() ) + for ( const Selectable& elem : m_selection ) { if ( auto* nodeview = elem.get_if() ) nodeview->spatial_node().translate(delta); diff --git a/src/ndbl/gui/GraphView.h b/src/ndbl/gui/GraphView.h index 6fcb015a9..f9edb2426 100644 --- a/src/ndbl/gui/GraphView.h +++ b/src/ndbl/gui/GraphView.h @@ -7,7 +7,7 @@ #include "tools/core/reflection/reflection" #include "tools/core/Variant.h" -#include "tools/core/UniqueOrderedVariantList.h" +#include "tools/core/UniqueVariantList.h" #include "tools/gui/ViewState.h" #include "tools/gui/geometry/Pivots.h" @@ -38,7 +38,7 @@ namespace ndbl }; using Selectable = tools::Variant ; - using Selection = tools::UniqueOrderedVariantList ; + using Selection = tools::UniqueVariantList ; class GraphView { @@ -80,7 +80,7 @@ namespace ndbl void _update(float dt, u16_t iterations); void _update(float dt); void _on_graph_change(); - void _on_selection_change(Selection::EventType, Selection::ElemType ); + void _on_selection_change(Selection::EventType, Selection::Element ); void frame_views(const std::vector&, const Vec2& pivot ); void draw_create_node_context_menu(CreateNodeCtxMenu& menu, SlotView* dragged_slotview = nullptr ); void create_constraints__align_top_recursively(const std::vector& unfiltered_follower, ndbl::Node *leader); diff --git a/src/ndbl/gui/Nodable.cpp b/src/ndbl/gui/Nodable.cpp index 5b77ec22c..12e12c52c 100644 --- a/src/ndbl/gui/Nodable.cpp +++ b/src/ndbl/gui/Nodable.cpp @@ -201,7 +201,7 @@ void Nodable::update() { if ( graph_view ) { - for(const Selectable& elem : graph_view->selection().data() ) + for(const Selectable& elem : graph_view->selection() ) { if ( auto nodeview = elem.get_if() ) graph_view->graph()->destroy_next_frame( nodeview->node() ); @@ -217,7 +217,7 @@ void Nodable::update() { if ( graph_view ) { - for(const Selectable& elem : graph_view->selection().data() ) + for( const Selectable& elem : graph_view->selection() ) { switch ( elem.index() ) { diff --git a/src/tools/core/UniqueOrderedList.h b/src/tools/core/UniqueList.h similarity index 51% rename from src/tools/core/UniqueOrderedList.h rename to src/tools/core/UniqueList.h index 0fefd7bef..28a5ded46 100644 --- a/src/tools/core/UniqueOrderedList.h +++ b/src/tools/core/UniqueList.h @@ -11,23 +11,39 @@ namespace tools // Ordered list of unique elements // Uses a hashset (rely on std::hash ) and a list to guarantee uniqueness and order. // - template - struct UniqueOrderedList + template + struct UniqueList { - using ElemType = T; - static_assert( std::is_default_constructible_v>, "std::hash must be implemented"); + static_assert( std::is_default_constructible_v>, "std::hash must be implemented"); + + using Element = ElementT; + using Set = std::unordered_set; // std::unordered, because we need uniqueness, furthermore unordered_set::contains is faster than std::find on a list + using List = std::list; // std::list, because it is ordered, and it supports constant time insertion and removal of elements from anywhere in the container." (see https://devdocs.io/cpp/container/list) + using Iterator = typename List::iterator; + using ConstIterator = typename List::const_iterator; + + Iterator begin() { return _ordered_elem.begin(); } + Iterator end() { return _ordered_elem.end(); } + ConstIterator cbegin() const { return _ordered_elem.cbegin(); } + ConstIterator cend() const { return _ordered_elem.cend(); } + + Element& front() + { return _ordered_elem.front(); } + + Element& back() + { return _ordered_elem.back(); } bool empty() const { return _unique_elem.empty(); } - bool contains(const ElemType& elem) const // Constant on average, worst case linear in the size of the container. + bool contains(const Element& elem) const // Constant on average, worst case linear in the size of the container. { return _unique_elem.contains( _hash(elem) ); } - const std::list& data() const + const std::list& data() const { return _ordered_elem; } @@ -58,7 +74,7 @@ namespace tools return _unique_elem.size(); } - bool append(ElemType& elem) // Constant on average, worst case linear in the size of the container. + bool append(Element& elem) // Constant on average, worst case linear in the size of the container. { const auto& [_, inserted] = _unique_elem.insert( _hash(elem) ); if ( inserted ) @@ -69,7 +85,7 @@ namespace tools return false; } - bool remove(const ElemType& elem)// Constant on average, worst case linear in the size of the container. + bool remove(const Element& elem)// Constant on average, worst case linear in the size of the container. { if ( _unique_elem.erase( _hash(elem) ) ) { @@ -81,10 +97,10 @@ namespace tools } private: - u64_t _hash(const ElemType& elem) const - { return std::hash{}(elem); } + u64_t _hash(const Element& elem) const + { return std::hash{}(elem); } - std::unordered_set _unique_elem{}; // unordered_set, because we need uniqueness, furthermore unordered_set::contains is faster than std::find on a list - std::list _ordered_elem{}; // list, because it " supports constant time insertion and removal of elements from anywhere in the container." (see https://devdocs.io/cpp/container/list) + Set _unique_elem{}; + List _ordered_elem{}; }; } diff --git a/src/tools/core/UniqueList.specs.cpp b/src/tools/core/UniqueList.specs.cpp new file mode 100644 index 000000000..23d220799 --- /dev/null +++ b/src/tools/core/UniqueList.specs.cpp @@ -0,0 +1,10 @@ +#include "UniqueList.h" +#include + +using namespace tools; + +TEST(UniqueList, is_constructible ) +{ + EXPECT_TRUE( std::is_constructible_v> ); +} + diff --git a/src/tools/core/UniqueOrderedList.specs.cpp b/src/tools/core/UniqueOrderedList.specs.cpp deleted file mode 100644 index c8b47c490..000000000 --- a/src/tools/core/UniqueOrderedList.specs.cpp +++ /dev/null @@ -1,12 +0,0 @@ -#include "UniqueOrderedList.h" -#include "tools/core/log.h" -#include - -using namespace tools; - -TEST(UniqueOrderedList, is_constructible ) -{ - using MyVec = UniqueOrderedList; - EXPECT_TRUE( std::is_constructible_v ); -} - diff --git a/src/tools/core/UniqueOrderedVariantList.h b/src/tools/core/UniqueVariantList.h similarity index 69% rename from src/tools/core/UniqueOrderedVariantList.h rename to src/tools/core/UniqueVariantList.h index 13b7afe54..c4e31361d 100644 --- a/src/tools/core/UniqueOrderedVariantList.h +++ b/src/tools/core/UniqueVariantList.h @@ -7,21 +7,29 @@ #include "Signals.h" #include "Hash.h" #include "Variant.h" -#include "UniqueOrderedList.h" +#include "UniqueList.h" namespace tools { template - struct UniqueOrderedVariantList; + struct UniqueVariantList; // // Extend UniqueOrderedList specifically for Variant // Also provide a signal on_change to listen to append/remove events // template - struct UniqueOrderedVariantList> + struct UniqueVariantList> { - using ElemType = Variant; + using Element = Variant; + using WrappedList = UniqueList; + using Iterator = typename WrappedList::Iterator ; + using ConstIterator = typename WrappedList::ConstIterator; + + Iterator begin() { return _wrapped_list.begin(); } + Iterator end() { return _wrapped_list.end(); } + ConstIterator cbegin() const { return _wrapped_list.cbegin(); } + ConstIterator cend() const { return _wrapped_list.cend(); } enum class EventType { @@ -29,33 +37,35 @@ namespace tools Remove, }; - SIGNAL(on_change, EventType, ElemType); + SIGNAL(on_change, EventType, Element); + + Element& front() + { return _wrapped_list.front(); } + + Element& back() + { return _wrapped_list.back(); } bool empty() const { return _wrapped_list.empty(); } void clear() { - for( const ElemType& elem : _wrapped_list.data() ) + for( const Element& elem : _wrapped_list.data() ) { on_change.emit( EventType::Remove, elem ); } _wrapped_list.clear(); _count_by_index.clear(); } - - bool contains(const ElemType& elem ) const - { return _wrapped_list.contains(elem); } - template - bool append(T* ptr) - { ElemType elem{ptr}; return append(elem); } + bool contains(const Element& elem ) const + { return _wrapped_list.contains(elem); } template - bool append(AlternativeT& data) - { return append(ElemType{data}); } + bool append(AlternativeT data) + { return append(Element{data}); } // we use add_lvalue_reference to handle pointers - bool append(ElemType& elem ) + bool append(Element& elem ) { const bool ok = _wrapped_list.append(elem); if ( ok ) @@ -77,7 +87,7 @@ namespace tools return count; } - bool remove(const ElemType& elem) + bool remove(const Element& elem) { const bool ok = _wrapped_list.remove(elem); { @@ -89,13 +99,13 @@ namespace tools template bool contains() const // O(1), read from cache. { - constexpr size_t index = ElemType::template index_of(); + constexpr size_t index = Element::template index_of(); return _count_by_index.contains(index ); } template size_t count() const // O(1), read from cache. { - constexpr size_t index = ElemType::template index_of(); + constexpr size_t index = Element::template index_of(); if ( _count_by_index.contains(index ) ) { return _count_by_index.at(index ); @@ -110,7 +120,7 @@ namespace tools if ( _count == 0 ) return {}; - for ( const ElemType& elem : _wrapped_list.data() ) + for ( const Element& elem : _wrapped_list.data() ) if ( AlternativeT ptr = elem.template get_if() ) return ptr; @@ -129,18 +139,15 @@ namespace tools result.reserve( _count ); // 1 allocation max :) // OPTIM: we could use a cache per type_index if necessary ( type_index => list ) - for ( const ElemType& elem : _wrapped_list.data() ) + for ( const Element& elem : _wrapped_list.data() ) if ( AlternativeT data = elem.template get_if() ) result.push_back( data ); return result; } - const std::list& data() const - { return _wrapped_list.data(); } - private: - UniqueOrderedList> _wrapped_list{}; + WrappedList _wrapped_list{}; std::unordered_map _count_by_index{}; }; } \ No newline at end of file diff --git a/src/tools/core/UniqueOrderedVariantList.specs.cpp b/src/tools/core/UniqueVariantList.specs.cpp similarity index 71% rename from src/tools/core/UniqueOrderedVariantList.specs.cpp rename to src/tools/core/UniqueVariantList.specs.cpp index 3255be4d6..44329d3c4 100644 --- a/src/tools/core/UniqueOrderedVariantList.specs.cpp +++ b/src/tools/core/UniqueVariantList.specs.cpp @@ -1,13 +1,14 @@ -#include "UniqueOrderedVariantList.h" +#include "Variant.h" +#include "UniqueVariantList.h" #include "tools/core/log.h" #include using namespace tools; -TEST(UniqueOrderedVectorList, with_Variant ) +TEST(UniqueVectorList, with_Variant ) { using MyVariant = Variant; - using MyVec = UniqueOrderedVariantList; + using MyVec = UniqueVariantList; MyVec vec; @@ -24,8 +25,8 @@ TEST(UniqueOrderedVectorList, with_Variant ) EXPECT_TRUE( vec.contains(a) ); EXPECT_TRUE( vec.contains(b) ); - EXPECT_TRUE( vec.data().front() == a ); - EXPECT_TRUE( vec.data().back() == b ); + EXPECT_TRUE( vec.front() == a ); + EXPECT_TRUE( vec.back() == b ); vec.clear(); From b04396b872ca385c5528e132921ecc9d55fe62ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9renger=20Dalle-Cort?= Date: Mon, 2 Dec 2024 16:59:15 -0500 Subject: [PATCH 2/2] refactor(tools::Variant): add default constructors & assignment operators --- src/tools/core/Variant.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/core/Variant.h b/src/tools/core/Variant.h index 8518baf0f..831af5af8 100644 --- a/src/tools/core/Variant.h +++ b/src/tools/core/Variant.h @@ -33,6 +33,8 @@ namespace tools static constexpr size_t index_null = 0; // mono state's Variant() = default; + Variant(const Variant&) = default; + Variant(Variant&&) = default; template Variant(T data) @@ -47,6 +49,7 @@ namespace tools } Variant& operator=(const Variant& other) = default; + Variant& operator=(Variant&& other) = default; template T get() const