[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

diff --git a/dali-toolkit/devel-api/controls/accessible-impl.cpp b/dali-toolkit/devel-api/controls/accessible-impl.cpp
index a962abf..e01bcb5 100644 (file)
--- a/dali-toolkit/devel-api/controls/accessible-impl.cpp
+++ b/dali-toolkit/devel-api/controls/accessible-impl.cpp
@@ -536,23 +536,16 @@ std::vector<Dali::Accessibility::Relation> AccessibleImpl::GetRelationSet()
 
   std::vector<Dali::Accessibility::Relation> ret;
 
 
   std::vector<Dali::Accessibility::Relation> 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
 
     // Map every Accessible* to its Address

-    std::vector<Accessibility::Address> 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();
     });
       return x->GetAddress();
     });

-
-    ret.emplace_back(Accessibility::Relation{static_cast<Accessibility::RelationType>(i), std::move(targets)});

   }
 
   return ret;
   }
 
   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 (file)
--- 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))
   {
 {
   if(auto controlDataImpl = GetControlImplementation(control))
   {

-    auto index = static_cast<Dali::Property::Array::SizeType>(relation);
-    if(index >= controlDataImpl->mAccessibilityRelations.size())
-    {
-      DALI_LOG_ERROR("Relation index exceeds vector size.");
-      return;
-    }
-

     auto object = controlDataImpl->GetAccessibilityObject(destination);
     if(object)
     {
     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))
   {
 {
   if(auto controlDataImpl = GetControlImplementation(control))
   {

-    auto index = static_cast<Dali::Property::Array::SizeType>(relation);
-    if(index >= controlDataImpl->mAccessibilityRelations.size())
-    {
-      DALI_LOG_ERROR("Relation index exceeds vector size.");
-      return;
-    }
-

     auto object = controlDataImpl->GetAccessibilityObject(destination);
     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<std::vector<Accessibility::Address>> GetAccessibilityRelations(Dali:
 {
   if(auto controlDataImpl = GetControlImplementation(control))
   {
 {
   if(auto controlDataImpl = GetControlImplementation(control))
   {

-    auto& relations = controlDataImpl->mAccessibilityRelations;
-
-    std::vector<std::vector<Accessibility::Address>> result(relations.size());
+    std::vector<std::vector<Accessibility::Address>> result(static_cast<std::size_t>(Accessibility::RelationType::MAX_COUNT));

 
     // Map every Accessible* to its Address
 
     // 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<std::size_t>(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();
       });
     }
         return x->GetAddress();
       });
     }


@@ -256,10 +237,7 @@ void ClearAccessibilityRelations(Dali::Actor control)
 {
   if(auto controlDataImpl = GetControlImplementation(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 (file)
--- 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<Dali::Accessibility::Accessible> {
     return std::unique_ptr<Dali::Accessibility::Accessible>(new DevelControl::AccessibleImpl(actor, Dali::Accessibility::Role::UNKNOWN));
   };
   mAccessibilityConstructor = [](Dali::Actor actor) -> std::unique_ptr<Dali::Accessibility::Accessible> {
     return std::unique_ptr<Dali::Accessibility::Accessible>(new DevelControl::AccessibleImpl(actor, Dali::Accessibility::Role::UNKNOWN));
   };

-
-  size_t length = static_cast<size_t>(Dali::Accessibility::RelationType::MAX_COUNT);
-  mAccessibilityRelations.reserve(length);
-  for(auto i = 0u; i < length; ++i)
-  {
-    mAccessibilityRelations.push_back({});
-  }

 }
 
 Control::Impl::~Impl()
 }
 
 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 (file)
--- 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 <dali-toolkit/public-api/visuals/visual-properties.h>
 #include <dali/devel-api/common/owner-container.h>
 #include <dali/integration-api/debug.h>
 #include <dali-toolkit/public-api/visuals/visual-properties.h>
 #include <dali/devel-api/common/owner-container.h>
 #include <dali/integration-api/debug.h>

+#include <map>

 #include <memory>
 #include <memory>

+#include <set>

 
 namespace Dali
 {
 
 namespace Dali
 {


@@ -546,9 +548,9 @@ public:
 
   Dali::Accessibility::Role mAccessibilityRole = Dali::Accessibility::Role::UNKNOWN;
 
 
   Dali::Accessibility::Role mAccessibilityRole = Dali::Accessibility::Role::UNKNOWN;
 

-  std::vector<std::vector<Accessibility::Accessible*>>                   mAccessibilityRelations;
-  std::function<std::unique_ptr<Dali::Accessibility::Accessible>(Actor)> mAccessibilityConstructor;
-  std::unique_ptr<Dali::Accessibility::Accessible>                       mAccessibilityObject;
+  std::map<Dali::Accessibility::RelationType, std::set<Accessibility::Accessible*>> mAccessibilityRelations;
+  std::function<std::unique_ptr<Dali::Accessibility::Accessible>(Actor)>            mAccessibilityConstructor;
+  std::unique_ptr<Dali::Accessibility::Accessible>                                  mAccessibilityObject;

 
   // Gesture Detection
   PinchGestureDetector     mPinchGestureDetector;
 
   // Gesture Detection
   PinchGestureDetector     mPinchGestureDetector;