From 7df12426912a43d7a66bd63dcb470944bbb0f1fa Mon Sep 17 00:00:00 2001 From: Yegor Date: Mon, 21 May 2018 17:44:23 -0700 Subject: [PATCH] split `children` into two ordered lists: traversal and hit test (flutter/engine#5091) * split `children` into two ordered lists: traversal and hit test * address comments * reduce node object byte size * link to DebugSemanticsDumpOrder --- engine/src/flutter/lib/ui/semantics.dart | 71 +++++++++++-------- .../flutter/lib/ui/semantics/semantics_node.h | 4 +- .../ui/semantics/semantics_update_builder.cc | 11 +-- .../ui/semantics/semantics_update_builder.h | 4 +- .../io/flutter/view/AccessibilityBridge.java | 61 +++++++++------- .../platform/android/platform_view_android.cc | 14 ++-- .../framework/Source/accessibility_bridge.mm | 4 +- 7 files changed, 98 insertions(+), 71 deletions(-) diff --git a/engine/src/flutter/lib/ui/semantics.dart b/engine/src/flutter/lib/ui/semantics.dart index 09436ed7fa..14a94147ea 100644 --- a/engine/src/flutter/lib/ui/semantics.dart +++ b/engine/src/flutter/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 diff --git a/engine/src/flutter/lib/ui/semantics/semantics_node.h b/engine/src/flutter/lib/ui/semantics/semantics_node.h index 55f2771c90..3ddb431e90 100644 --- a/engine/src/flutter/lib/ui/semantics/semantics_node.h +++ b/engine/src/flutter/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 children; + std::vector childrenInTraversalOrder; + std::vector childrenInHitTestOrder; }; // Contains semantic nodes that need to be updated. diff --git a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc b/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc index 2ff5568ec1..576967525b 100644 --- a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc +++ b/engine/src/flutter/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( - children.data(), children.data() + children.num_elements()); + node.childrenInTraversalOrder = std::vector( + childrenInTraversalOrder.data(), childrenInTraversalOrder.data() + childrenInTraversalOrder.num_elements()); + node.childrenInHitTestOrder = std::vector( + childrenInHitTestOrder.data(), childrenInHitTestOrder.data() + childrenInHitTestOrder.num_elements()); nodes_[id] = node; } diff --git a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.h b/engine/src/flutter/lib/ui/semantics/semantics_update_builder.h index 6f33330eaa..c4d4b27781 100644 --- a/engine/src/flutter/lib/ui/semantics/semantics_update_builder.h +++ b/engine/src/flutter/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 build(); diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java index 57b171994d..a969b1bb6b 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -259,8 +259,8 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess String decreasedValue; String hint; TextDirection textDirection; - int hitTestPosition; boolean hadPreviousConfig = false; int previousFlags; @@ -779,7 +778,8 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess private float[] transform; SemanticsObject parent; - List children; // In inverse hit test order (i.e. paint order). + List childrenInTraversalOrder; + List childrenInHitTestOrder; private boolean inverseTransformDirty = true; private float[] inverseTransform; @@ -813,12 +813,11 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess textDirection = TextDirection.fromInt(buffer.getInt()); - hitTestPosition = buffer.getInt(); - left = buffer.getFloat(); top = buffer.getFloat(); right = buffer.getFloat(); @@ -876,17 +873,29 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess final int childCount = buffer.getInt(); if (childCount == 0) { - children = null; + childrenInTraversalOrder = null; + childrenInHitTestOrder = null; } else { - if (children == null) - children = new ArrayList(childCount); + if (childrenInTraversalOrder == null) + childrenInTraversalOrder = new ArrayList(childCount); else - children.clear(); + childrenInTraversalOrder.clear(); for (int i = 0; i < childCount; ++i) { SemanticsObject child = getOrCreateObject(buffer.getInt()); child.parent = this; - children.add(child); + childrenInTraversalOrder.add(child); + } + + if (childrenInHitTestOrder == null) + childrenInHitTestOrder = new ArrayList(childCount); + else + childrenInHitTestOrder.clear(); + + for (int i = 0; i < childCount; ++i) { + SemanticsObject child = getOrCreateObject(buffer.getInt()); + child.parent = this; + childrenInHitTestOrder.add(child); } } } @@ -912,10 +921,10 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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 @@ class AccessibilityBridge extends AccessibilityNodeProvider implements BasicMess 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); } } } diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.cc b/engine/src/flutter/shell/platform/android/platform_view_android.cc index 8a241b0bfd..f5ab79542a 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android.cc +++ b/engine/src/flutter/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 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; } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm index cf086608c6..4a6d70655e 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge.mm @@ -490,11 +490,11 @@ void AccessibilityBridge::UpdateSemantics(blink::SemanticsNodeUpdates nodes) { 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]; }