From 06e51bb1b127122783024601956ccf7188144e4b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Artur=20=C5=9Awigo=C5=84?= Date: Fri, 26 Nov 2021 12:47:53 +0100 Subject: [PATCH] [AT-SPI] Follow-up on mAccessibilityRelations The previous implementation used an `std::vector` (the outer one) as a map, with the `RelationType` key working as the index to that vector. However, relations are rare in practice, and every `Control::Impl` having a 23-element vector or vectors, most, if not all of which are empty, feels like a waste of memory. This patch also changes the inner vector to a set, which resolves the previously unaddressed issue of duplicate elements. Change-Id: I639082fe49e0f6107213cda5b142a09882a3be39 --- .../devel-api/controls/accessible-impl.cpp | 15 ++----- dali-toolkit/devel-api/controls/control-devel.cpp | 48 ++++++---------------- .../controls/control/control-data-impl.cpp | 7 ---- .../internal/controls/control/control-data-impl.h | 8 ++-- 4 files changed, 22 insertions(+), 56 deletions(-) diff --git a/dali-toolkit/devel-api/controls/accessible-impl.cpp b/dali-toolkit/devel-api/controls/accessible-impl.cpp index a962abf..e01bcb5 100644 --- a/dali-toolkit/devel-api/controls/accessible-impl.cpp +++ b/dali-toolkit/devel-api/controls/accessible-impl.cpp @@ -536,23 +536,16 @@ std::vector AccessibleImpl::GetRelationSet() std::vector ret; - auto& relations = controlImpl.mAccessibilityRelations; - for(auto i = 0u; i < relations.size(); ++i) + for(auto& relation : controlImpl.mAccessibilityRelations) { - auto& relation = relations[i]; + auto& targets = relation.second; - if(relation.empty()) - { - continue; - } + ret.emplace_back(Accessibility::Relation{relation.first, {}}); // Map every Accessible* to its Address - std::vector targets; - std::transform(relation.begin(), relation.end(), std::back_inserter(targets), [](auto* x) { + std::transform(targets.begin(), targets.end(), std::back_inserter(ret.back().targets), [](auto* x) { return x->GetAddress(); }); - - ret.emplace_back(Accessibility::Relation{static_cast(i), std::move(targets)}); } return ret; diff --git a/dali-toolkit/devel-api/controls/control-devel.cpp b/dali-toolkit/devel-api/controls/control-devel.cpp index ccf624b..7b497c9 100644 --- a/dali-toolkit/devel-api/controls/control-devel.cpp +++ b/dali-toolkit/devel-api/controls/control-devel.cpp @@ -183,17 +183,10 @@ void AppendAccessibilityRelation(Dali::Actor control, Actor destination, Dali::A { if(auto controlDataImpl = GetControlImplementation(control)) { - auto index = static_cast(relation); - if(index >= controlDataImpl->mAccessibilityRelations.size()) - { - DALI_LOG_ERROR("Relation index exceeds vector size."); - return; - } - auto object = controlDataImpl->GetAccessibilityObject(destination); if(object) { - controlDataImpl->mAccessibilityRelations[index].push_back(object); + controlDataImpl->mAccessibilityRelations[relation].insert(object); } } } @@ -202,27 +195,16 @@ void RemoveAccessibilityRelation(Dali::Actor control, Actor destination, Dali::A { if(auto controlDataImpl = GetControlImplementation(control)) { - auto index = static_cast(relation); - if(index >= controlDataImpl->mAccessibilityRelations.size()) - { - DALI_LOG_ERROR("Relation index exceeds vector size."); - return; - } - auto object = controlDataImpl->GetAccessibilityObject(destination); - if(!object) + if(object) { - return; - } + auto& relations = controlDataImpl->mAccessibilityRelations; - auto& targets = controlDataImpl->mAccessibilityRelations[index]; - for(auto i = 0u; i < targets.size(); ++i) - { - if(targets[i] == object) + relations[relation].erase(object); + + if(relations[relation].empty()) { - std::swap(targets[i], targets.back()); - targets.pop_back(); - --i; + relations.erase(relation); } } } @@ -232,16 +214,15 @@ std::vector> GetAccessibilityRelations(Dali: { if(auto controlDataImpl = GetControlImplementation(control)) { - auto& relations = controlDataImpl->mAccessibilityRelations; - - std::vector> result(relations.size()); + std::vector> result(static_cast(Accessibility::RelationType::MAX_COUNT)); // Map every Accessible* to its Address - for(std::size_t i = 0; i < relations.size(); ++i) + for(auto& relation : controlDataImpl->mAccessibilityRelations) { - auto& relation = relations[i]; + auto index = static_cast(relation.first); + auto& targets = relation.second; - std::transform(relation.begin(), relation.end(), std::back_inserter(result[i]), [](auto* x) { + std::transform(targets.begin(), targets.end(), std::back_inserter(result[index]), [](auto* x) { return x->GetAddress(); }); } @@ -256,10 +237,7 @@ void ClearAccessibilityRelations(Dali::Actor control) { if(auto controlDataImpl = GetControlImplementation(control)) { - for(auto& it : controlDataImpl->mAccessibilityRelations) - { - it.clear(); - } + controlDataImpl->mAccessibilityRelations.clear(); } } diff --git a/dali-toolkit/internal/controls/control/control-data-impl.cpp b/dali-toolkit/internal/controls/control/control-data-impl.cpp index d9a554e..4aecc29 100644 --- a/dali-toolkit/internal/controls/control/control-data-impl.cpp +++ b/dali-toolkit/internal/controls/control/control-data-impl.cpp @@ -515,13 +515,6 @@ Control::Impl::Impl(Control& controlImpl) mAccessibilityConstructor = [](Dali::Actor actor) -> std::unique_ptr { return std::unique_ptr(new DevelControl::AccessibleImpl(actor, Dali::Accessibility::Role::UNKNOWN)); }; - - size_t length = static_cast(Dali::Accessibility::RelationType::MAX_COUNT); - mAccessibilityRelations.reserve(length); - for(auto i = 0u; i < length; ++i) - { - mAccessibilityRelations.push_back({}); - } } Control::Impl::~Impl() diff --git a/dali-toolkit/internal/controls/control/control-data-impl.h b/dali-toolkit/internal/controls/control/control-data-impl.h index 5b2cee5..c81be41 100644 --- a/dali-toolkit/internal/controls/control/control-data-impl.h +++ b/dali-toolkit/internal/controls/control/control-data-impl.h @@ -35,7 +35,9 @@ #include #include #include +#include #include +#include namespace Dali { @@ -546,9 +548,9 @@ public: Dali::Accessibility::Role mAccessibilityRole = Dali::Accessibility::Role::UNKNOWN; - std::vector> mAccessibilityRelations; - std::function(Actor)> mAccessibilityConstructor; - std::unique_ptr mAccessibilityObject; + std::map> mAccessibilityRelations; + std::function(Actor)> mAccessibilityConstructor; + std::unique_ptr mAccessibilityObject; // Gesture Detection PinchGestureDetector mPinchGestureDetector; -- 2.7.4