split children into two ordered lists: traversal and hit test (flut…

…ter#5091)

* split `children` into two ordered lists: traversal and hit test

* address comments

* reduce node object byte size

* link to DebugSemanticsDumpOrder

lib/ui/semantics.dart

@@ -434,11 +434,18 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 {
   /// Update the information associated with the node with the given `id`.
   ///
   /// The semantics nodes form a tree, with the root of the tree always having
-  /// an id of zero. The `children` are the ids of the nodes that are immediate
-  /// children of this node. The system retains the nodes that are currently
-  /// reachable from the root. A given update need not contain information for
-  /// nodes that do not change in the update. If a node is not reachable from
-  /// the root after an update, the node will be discarded from the tree.
+  /// an id of zero. The `childrenInTraversalOrder` and `childrenInHitTestOrder`
+  /// are the ids of the nodes that are immediate children of this node. The
+  /// former enumerates children in traversal order, and the latter enumerates
+  /// the same children in the hit test order. The two lists must have the same
+  /// length and contain the same ids. They may only differ in the order the
+  /// ids are listed in. For more information about different child orders, see
+  /// [DebugSemanticsDumpOrder].
+  /// 
+  /// The system retains the nodes that are currently reachable from the root.
+  /// A given update need not contain information for nodes that do not change
+  /// in the update. If a node is not reachable from the root after an update,
+  /// the node will be discarded from the tree.
   ///
   /// The `flags` are a bit field of [SemanticsFlag]s that apply to this node.
   ///
@@ -488,33 +495,39 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 {
     String increasedValue,
     String decreasedValue,
     TextDirection textDirection,
-    int hitTestPosition,
     Float64List transform,
+    // TODO(yjbanov): remove after moving the framework to the new param names.
     Int32List children,
+    Int32List childrenInTraversalOrder,
+    Int32List childrenInHitTestOrder,
   }) {
+    childrenInTraversalOrder ??= children;
+    childrenInHitTestOrder ??= children;
     if (transform.length != 16)
       throw new ArgumentError('transform argument must have 16 entries.');
-    _updateNode(id,
-                flags,
-                actions,
-                textSelectionBase,
-                textSelectionExtent,
-                scrollPosition,
-                scrollExtentMax,
-                scrollExtentMin,
-                rect.left,
-                rect.top,
-                rect.right,
-                rect.bottom,
-                label,
-                hint,
-                value,
-                increasedValue,
-                decreasedValue,
-                textDirection != null ? textDirection.index + 1 : 0,
-                hitTestPosition,
-                transform,
-                children,);
+    _updateNode(
+      id,
+      flags,
+      actions,
+      textSelectionBase,
+      textSelectionExtent,
+      scrollPosition,
+      scrollExtentMax,
+      scrollExtentMin,
+      rect.left,
+      rect.top,
+      rect.right,
+      rect.bottom,
+      label,
+      hint,
+      value,
+      increasedValue,
+      decreasedValue,
+      textDirection != null ? textDirection.index + 1 : 0,
+      transform,
+      childrenInTraversalOrder,
+      childrenInHitTestOrder,
+    );
   }
   void _updateNode(
     int id,
@@ -535,9 +548,9 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 {
     String increasedValue,
     String decreasedValue,
     int textDirection,
-    int hitTestPosition,
     Float64List transform,
-    Int32List children,
+    Int32List childrenInTraversalOrder,
+    Int32List childrenInHitTestOrder,
   ) native 'SemanticsUpdateBuilder_updateNode';
 
   /// Creates a [SemanticsUpdate] object that encapsulates the updates recorded

lib/ui/semantics/semantics_node.h

@@ -82,11 +82,11 @@ struct SemanticsNode {
   std::string increasedValue;
   std::string decreasedValue;
   int32_t textDirection = 0;  // 0=unknown, 1=rtl, 2=ltr
-  int32_t hitTestPosition = -1;
 
   SkRect rect = SkRect::MakeEmpty();
   SkMatrix44 transform = SkMatrix44(SkMatrix44::kIdentity_Constructor);
-  std::vector<int32_t> children;
+  std::vector<int32_t> childrenInTraversalOrder;
+  std::vector<int32_t> childrenInHitTestOrder;
 };
 
 // Contains semantic nodes that need to be updated.

lib/ui/semantics/semantics_update_builder.cc

@@ -52,9 +52,9 @@ void SemanticsUpdateBuilder::updateNode(int id,
                                         std::string increasedValue,
                                         std::string decreasedValue,
                                         int textDirection,
-                                        int hitTestPosition,
                                         const tonic::Float64List& transform,
-                                        const tonic::Int32List& children) {
+                                        const tonic::Int32List& childrenInTraversalOrder,
+                                        const tonic::Int32List& childrenInHitTestOrder) {
   SemanticsNode node;
   node.id = id;
   node.flags = flags;
@@ -71,10 +71,11 @@ void SemanticsUpdateBuilder::updateNode(int id,
   node.increasedValue = increasedValue;
   node.decreasedValue = decreasedValue;
   node.textDirection = textDirection;
-  node.hitTestPosition = hitTestPosition;
   node.transform.setColMajord(transform.data());
-  node.children = std::vector<int32_t>(
-      children.data(), children.data() + children.num_elements());
+  node.childrenInTraversalOrder = std::vector<int32_t>(
+      childrenInTraversalOrder.data(), childrenInTraversalOrder.data() + childrenInTraversalOrder.num_elements());
+  node.childrenInHitTestOrder = std::vector<int32_t>(
+      childrenInHitTestOrder.data(), childrenInHitTestOrder.data() + childrenInHitTestOrder.num_elements());
   nodes_[id] = node;
 }
 

lib/ui/semantics/semantics_update_builder.h

@@ -43,9 +43,9 @@ class SemanticsUpdateBuilder
                   std::string increasedValue,
                   std::string decreasedValue,
                   int textDirection,
-                  int hitTestPosition,
                   const tonic::Float64List& transform,
-                  const tonic::Int32List& children);
+                  const tonic::Int32List& childrenInTraversalOrder,
+                  const tonic::Int32List& childrenInHitTestOrder);
 
   fxl::RefPtr<SemanticsUpdate> build();
 

shell/platform/android/io/flutter/view/AccessibilityBridge.java

@@ -259,8 +259,8 @@ public AccessibilityNodeInfo createAccessibilityNodeInfo(int virtualViewId) {
             result.addAction(AccessibilityNodeInfo.ACTION_ACCESSIBILITY_FOCUS);
         }
 
-        if (object.children != null) {
-            for (SemanticsObject child : object.children) {
+        if (object.childrenInTraversalOrder != null) {
+            for (SemanticsObject child : object.childrenInTraversalOrder) {
                 if (!child.hasFlag(Flag.IS_HIDDEN)) {
                     result.addChild(mOwner, child.id);
                 }
@@ -760,7 +760,6 @@ private class SemanticsObject {
         String decreasedValue;
         String hint;
         TextDirection textDirection;
-        int hitTestPosition;
 
         boolean hadPreviousConfig = false;
         int previousFlags;
@@ -779,7 +778,8 @@ private class SemanticsObject {
         private float[] transform;
 
         SemanticsObject parent;
-        List<SemanticsObject> children;  // In inverse hit test order (i.e. paint order).
+        List<SemanticsObject> childrenInTraversalOrder;
+        List<SemanticsObject> childrenInHitTestOrder;
 
         private boolean inverseTransformDirty = true;
         private float[] inverseTransform;
@@ -813,12 +813,11 @@ boolean didScroll() {
         void log(String indent, boolean recursive) {
           Log.i(TAG, indent + "SemanticsObject id=" + id + " label=" + label + " actions=" +  actions + " flags=" + flags + "\n" +
                      indent + "  +-- textDirection=" + textDirection  + "\n"+
-                     indent + "  +-- hitTestPosition=" + hitTestPosition  + "\n"+
                      indent + "  +-- rect.ltrb=(" + left + ", " + top + ", " + right + ", " + bottom + ")\n" +
                      indent + "  +-- transform=" + Arrays.toString(transform) + "\n");
-          if (children != null && recursive) {
+          if (childrenInTraversalOrder != null && recursive) {
               String childIndent = indent + "  ";
-              for (SemanticsObject child : children) {
+              for (SemanticsObject child : childrenInTraversalOrder) {
                   child.log(childIndent, recursive);
               }
           }
@@ -860,8 +859,6 @@ void updateWith(ByteBuffer buffer, String[] strings) {
 
             textDirection = TextDirection.fromInt(buffer.getInt());
 
-            hitTestPosition = buffer.getInt();
-
             left = buffer.getFloat();
             top = buffer.getFloat();
             right = buffer.getFloat();
@@ -876,17 +873,29 @@ void updateWith(ByteBuffer buffer, String[] strings) {
 
             final int childCount = buffer.getInt();
             if (childCount == 0) {
-                children = null;
+                childrenInTraversalOrder = null;
+                childrenInHitTestOrder = null;
             } else {
-                if (children == null)
-                    children = new ArrayList<SemanticsObject>(childCount);
+                if (childrenInTraversalOrder == null)
+                    childrenInTraversalOrder = new ArrayList<SemanticsObject>(childCount);
+                else
+                    childrenInTraversalOrder.clear();
+
+                for (int i = 0; i < childCount; ++i) {
+                    SemanticsObject child = getOrCreateObject(buffer.getInt());
+                    child.parent = this;
+                    childrenInTraversalOrder.add(child);
+                }
+
+                if (childrenInHitTestOrder == null)
+                    childrenInHitTestOrder = new ArrayList<SemanticsObject>(childCount);
                 else
-                    children.clear();
+                    childrenInHitTestOrder.clear();
 
                 for (int i = 0; i < childCount; ++i) {
                     SemanticsObject child = getOrCreateObject(buffer.getInt());
                     child.parent = this;
-                    children.add(child);
+                    childrenInHitTestOrder.add(child);
                 }
             }
         }
@@ -912,10 +921,10 @@ SemanticsObject hitTest(float[] point) {
             final float y = point[1] / w;
             if (x < left || x >= right || y < top || y >= bottom)
                 return null;
-            if (children != null) {
+            if (childrenInHitTestOrder != null) {
                 final float[] transformedPoint = new float[4];
-                for (int i = children.size() - 1; i >= 0; i -= 1) {
-                    final SemanticsObject child = children.get(i);
+                for (int i = 0; i < childrenInHitTestOrder.size(); i += 1) {
+                    final SemanticsObject child = childrenInHitTestOrder.get(i);
                     if (child.hasFlag(Flag.IS_HIDDEN)) {
                         continue;
                     }
@@ -951,9 +960,9 @@ void collectRoutes(List<SemanticsObject> edges) {
             if (hasFlag(Flag.SCOPES_ROUTE)) {
                 edges.add(this);
             }
-            if (children != null) {
-                for (int i = 0; i < children.size(); ++i) {
-                    children.get(i).collectRoutes(edges);
+            if (childrenInTraversalOrder != null) {
+                for (int i = 0; i < childrenInTraversalOrder.size(); ++i) {
+                    childrenInTraversalOrder.get(i).collectRoutes(edges);
                 }
             }
         }
@@ -966,9 +975,9 @@ String getRouteName() {
                     return label;
                 }
             }
-            if (children != null) {
-                for (int i = 0; i < children.size(); ++i) {
-                    String newName = children.get(i).getRouteName();
+            if (childrenInTraversalOrder != null) {
+                for (int i = 0; i < childrenInTraversalOrder.size(); ++i) {
+                    String newName = childrenInTraversalOrder.get(i).getRouteName();
                     if (newName != null && !newName.isEmpty()) {
                         return newName;
                     }
@@ -1029,9 +1038,9 @@ void updateRecursively(float[] ancestorTransform, Set<SemanticsObject> visitedOb
             assert globalTransform != null;
             assert globalRect != null;
 
-            if (children != null) {
-                for (int i = 0; i < children.size(); ++i) {
-                    children.get(i).updateRecursively(globalTransform, visitedObjects, forceUpdate);
+            if (childrenInTraversalOrder != null) {
+                for (int i = 0; i < childrenInTraversalOrder.size(); ++i) {
+                    childrenInTraversalOrder.get(i).updateRecursively(globalTransform, visitedObjects, forceUpdate);
                 }
             }
         }

shell/platform/android/platform_view_android.cc

@@ -178,7 +178,7 @@ void PlatformViewAndroid::DispatchSemanticsAction(JNIEnv* env,
 
 // |shell::PlatformView|
 void PlatformViewAndroid::UpdateSemantics(blink::SemanticsNodeUpdates update) {
-  constexpr size_t kBytesPerNode = 36 * sizeof(int32_t);
+  constexpr size_t kBytesPerNode = 35 * sizeof(int32_t);
   constexpr size_t kBytesPerChild = sizeof(int32_t);
 
   JNIEnv* env = fml::jni::AttachCurrentThread();
@@ -190,7 +190,8 @@ void PlatformViewAndroid::UpdateSemantics(blink::SemanticsNodeUpdates update) {
     size_t num_bytes = 0;
     for (const auto& value : update) {
       num_bytes += kBytesPerNode;
-      num_bytes += value.second.children.size() * kBytesPerChild;
+      num_bytes += value.second.childrenInTraversalOrder.size() * kBytesPerChild;
+      num_bytes += value.second.childrenInHitTestOrder.size() * kBytesPerChild;
     }
 
     std::vector<uint8_t> buffer(num_bytes);
@@ -243,15 +244,18 @@ void PlatformViewAndroid::UpdateSemantics(blink::SemanticsNodeUpdates update) {
         strings.push_back(node.hint);
       }
       buffer_int32[position++] = node.textDirection;
-      buffer_int32[position++] = node.hitTestPosition;
       buffer_float32[position++] = node.rect.left();
       buffer_float32[position++] = node.rect.top();
       buffer_float32[position++] = node.rect.right();
       buffer_float32[position++] = node.rect.bottom();
       node.transform.asColMajorf(&buffer_float32[position]);
       position += 16;
-      buffer_int32[position++] = node.children.size();
-      for (int32_t child : node.children)
+
+      buffer_int32[position++] = node.childrenInTraversalOrder.size();
+      for (int32_t child : node.childrenInTraversalOrder)
+        buffer_int32[position++] = child;
+
+      for (int32_t child : node.childrenInHitTestOrder)
         buffer_int32[position++] = child;
     }
 

shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm

@@ -490,11 +490,11 @@ - (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction {
     layoutChanged = layoutChanged || [object nodeWillCauseLayoutChange:&node];
     scrollOccured = scrollOccured || [object nodeWillCauseScroll:&node];
     [object setSemanticsNode:&node];
-    const NSUInteger newChildCount = node.children.size();
+    const NSUInteger newChildCount = node.childrenInTraversalOrder.size();
     NSMutableArray* newChildren =
         [[[NSMutableArray alloc] initWithCapacity:newChildCount] autorelease];
     for (NSUInteger i = 0; i < newChildCount; ++i) {
-      SemanticsObject* child = GetOrCreateObject(node.children[i], nodes);
+      SemanticsObject* child = GetOrCreateObject(node.childrenInTraversalOrder[i], nodes);
       child.parent = object;
       [newChildren addObject:child];
     }