Drop invisible SemanticsNodes from tree (#12358)

* Drop invisible SemanticsNodes from tree

A node is invisible if it is outside of the bounds of the screen and if it is not merged into its (partially) visible parent.

Also in this PR: only set `wasAffectedByClip` to true if the nodes has actually been clipped.

* Fix other failing tests

* renaming

* review feedback

* more doc

packages/flutter/lib/src/rendering/object.dart

@@ -603,12 +603,14 @@ class _SemanticsGeometry {
     assert(parentSemantics != null);
     assert(parentSemantics.wasAffectedByClip != null);
     semantics.transform = _transform;
+    final Rect semanticBounds = rendering.semanticBounds;
     if (_clipRect != null) {
-      semantics.rect = _clipRect.intersect(rendering.semanticBounds);
-      semantics.wasAffectedByClip = true;
+      final Rect rect = _clipRect.intersect(semanticBounds);
+      semantics.rect = rect;
+      semantics.wasAffectedByClip = rect != semanticBounds;
     } else {
-      semantics.rect = rendering.semanticBounds;
-      semantics.wasAffectedByClip = parentSemantics?.wasAffectedByClip ?? false;
+      semantics.rect = semanticBounds;
+      semantics.wasAffectedByClip = false;
     }
   }
 }
@@ -697,6 +699,8 @@ class _CleanSemanticsFragment extends _SemanticsFragment {
     if (geometry != null) {
       geometry.applyAncestorChain(_ancestorChain);
       geometry.updateSemanticsNode(rendering: renderObjectOwner, semantics: node, parentSemantics: parentSemantics);
+      if (node.isInvisible)
+        return; // drop the node
     } else {
       assert(_ancestorChain.length == 1);
     }
@@ -722,6 +726,8 @@ abstract class _InterestingSemanticsFragment extends _SemanticsFragment {
     assert(!_debugCompiled);
     assert(() { _debugCompiled = true; return true; }());
     final SemanticsNode node = establishSemanticsNode(geometry, currentSemantics, parentSemantics);
+    if (node.isInvisible)
+      return; // drop the node
     final List<SemanticsNode> children = <SemanticsNode>[];
     for (_SemanticsFragment child in _children) {
       assert(child._ancestorChain.last == renderObjectOwner);
@@ -2714,6 +2720,7 @@ abstract class RenderObject extends AbstractNode with DiagnosticableTreeMixin im
       assert(fragment is _InterestingSemanticsFragment);
       final SemanticsNode node = fragment.compile(parentSemantics: _semantics?.parent).single;
       assert(node != null);
+      assert(!node.isInvisible);
       assert(node == _semantics);
     } catch (e, stack) {
       _debugReportException('_updateSemantics', e, stack);

packages/flutter/lib/src/rendering/semantics.dart

@@ -285,9 +285,24 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
     }
   }
 
-  /// Whether [rect] might have been influenced by clips applied by ancestors.
+  /// Whether [rect] was clipped by ancestors.
+  ///
+  /// This is only true if the [rect] of this [SemanticsNode] has been altered
+  /// due to clipping by an ancestor. If ancestors have been clipped, but the
+  /// [rect] of this node was unaffected it will be false.
   bool wasAffectedByClip = false;
 
+  /// Whether the node is invisible.
+  ///
+  /// A node whose [rect] is outside of the bounds of the screen and hence not
+  /// reachable for users is considered invisible if its semantic information
+  /// is not merged into a (partially) visible parent as indicated by
+  /// [isMergedIntoParent].
+  ///
+  /// An invisible node can be safely dropped from the semantic tree without
+  /// loosing semantic information that is relevant for describing the content
+  /// currently shown on screen.
+  bool get isInvisible => !isMergedIntoParent && rect.isEmpty;
 
   // FLAGS AND LABELS
   // These are supposed to be set by SemanticsAnnotator obtained from getSemanticsAnnotators
@@ -519,8 +534,10 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
         child._dead = true;
     }
     if (_newChildren != null) {
-      for (SemanticsNode child in _newChildren)
+      for (SemanticsNode child in _newChildren) {
+        assert(!child.isInvisible, 'Child with id ${child.id} is invisible and should not be added to tree.');
         child._dead = false;
+      }
     }
     bool sawChange = false;
     if (_children != null) {

packages/flutter/test/material/scaffold_test.dart

@@ -501,11 +501,11 @@ void main() {
 
     final SemanticsTester semantics = new SemanticsTester(tester);
     await tester.pumpWidget(new MaterialApp(home: new Scaffold(
-      body: new Semantics(label: bodyLabel, child: new Container()),
-      persistentFooterButtons: <Widget>[new Semantics(label: persistentFooterButtonLabel, child: new Container())],
-      bottomNavigationBar: new Semantics(label: bottomNavigationBarLabel, child: new Container()),
-      floatingActionButton: new Semantics(label: floatingActionButtonLabel, child: new Container()),
-      drawer: new Drawer(child:new Semantics(label: drawerLabel, child: new Container())),
+      body: const Text(bodyLabel),
+      persistentFooterButtons: <Widget>[const Text(persistentFooterButtonLabel)],
+      bottomNavigationBar: const Text(bottomNavigationBarLabel),
+      floatingActionButton: const Text(floatingActionButtonLabel),
+      drawer: const Drawer(child:const Text(drawerLabel)),
     )));
 
     expect(semantics, includesNodeWith(label: bodyLabel));

packages/flutter/test/rendering/semantics_test.dart

@@ -32,6 +32,7 @@ void main() {
 
     test('getSemanticsData includes tags', () {
       final SemanticsNode node = new SemanticsNode()
+        ..rect = new Rect.fromLTRB(0.0, 0.0, 10.0, 10.0)
         ..addTag(tag1)
         ..addTag(tag2);
 
@@ -43,7 +44,9 @@ void main() {
 
       node.mergeAllDescendantsIntoThisNode = true;
       node.addChildren(<SemanticsNode>[
-        new SemanticsNode()..addTag(tag3)
+        new SemanticsNode()
+          ..rect = new Rect.fromLTRB(5.0, 5.0, 10.0, 10.0)
+          ..addTag(tag3),
       ]);
       node.finalizeChildren();
 
@@ -95,9 +98,12 @@ void main() {
   });
 
   test('toStringDeep() does not throw with transform == null', () {
-    final SemanticsNode child1 = new SemanticsNode();
-    final SemanticsNode child2 = new SemanticsNode();
-    final SemanticsNode root = new SemanticsNode();
+    final SemanticsNode child1 = new SemanticsNode()
+      ..rect = new Rect.fromLTRB(0.0, 0.0, 5.0, 5.0);
+    final SemanticsNode child2 = new SemanticsNode()
+      ..rect = new Rect.fromLTRB(5.0, 0.0, 10.0, 5.0);
+    final SemanticsNode root = new SemanticsNode()
+      ..rect = new Rect.fromLTRB(0.0, 0.0, 10.0, 5.0);
     root.addChildren(<SemanticsNode>[child1, child2]);
     root.finalizeChildren();
 
@@ -107,64 +113,68 @@ void main() {
 
     expect(
       root.toStringDeep(childOrder: DebugSemanticsDumpOrder.traversal),
-      'SemanticsNode#8(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '├SemanticsNode#6(Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '└SemanticsNode#7(Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n',
+      'SemanticsNode#8(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 10.0, 5.0))\n'
+      '├SemanticsNode#6(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 5.0, 5.0))\n'
+      '└SemanticsNode#7(STALE, owner: null, Rect.fromLTRB(5.0, 0.0, 10.0, 5.0))\n',
     );
   });
 
   test('toStringDeep respects childOrder parameter', () {
     final SemanticsNode child1 = new SemanticsNode()
-      ..rect = new Rect.fromLTRB(20.0, 20.0, 20.0, 20.0);
+      ..rect = new Rect.fromLTRB(15.0, 0.0, 20.0, 5.0);
     final SemanticsNode child2 = new SemanticsNode()
-      ..rect = new Rect.fromLTRB(10.0, 10.0, 10.0, 10.0);
-    final SemanticsNode root = new SemanticsNode();
+      ..rect = new Rect.fromLTRB(10.0, 0.0, 15.0, 5.0);
+    final SemanticsNode root = new SemanticsNode()
+      ..rect = new Rect.fromLTRB(0.0, 0.0, 20.0, 5.0);
     root.addChildren(<SemanticsNode>[child1, child2]);
     root.finalizeChildren();
     expect(
       root.toStringDeep(childOrder: DebugSemanticsDumpOrder.traversal),
-      'SemanticsNode#11(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 10.0, 10.0, 10.0))\n'
-      '└SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(20.0, 20.0, 20.0, 20.0))\n',
+      'SemanticsNode#11(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 20.0, 5.0))\n'
+      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 0.0, 15.0, 5.0))\n'
+      '└SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(15.0, 0.0, 20.0, 5.0))\n',
     );
 
     expect(
       root.toStringDeep(childOrder: DebugSemanticsDumpOrder.inverseHitTest),
-      'SemanticsNode#11(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '├SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(20.0, 20.0, 20.0, 20.0))\n'
-      '└SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 10.0, 10.0, 10.0))\n',
+      'SemanticsNode#11(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 20.0, 5.0))\n'
+      '├SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(15.0, 0.0, 20.0, 5.0))\n'
+      '└SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 0.0, 15.0, 5.0))\n',
     );
 
     final SemanticsNode child3 = new SemanticsNode()
-      ..rect = new Rect.fromLTRB(0.0, 0.0, 0.0, 0.0);
+      ..rect = new Rect.fromLTRB(0.0, 0.0, 10.0, 5.0);
     child3.addChildren(<SemanticsNode>[
-      new SemanticsNode()..rect = new Rect.fromLTRB(20.0, 0.0, 20.0, 0.0),
-      new SemanticsNode(),
+      new SemanticsNode()
+        ..rect = new Rect.fromLTRB(5.0, 0.0, 10.0, 5.0),
+      new SemanticsNode()
+        ..rect = new Rect.fromLTRB(0.0, 0.0, 5.0, 5.0),
     ]);
     child3.finalizeChildren();
 
-    final SemanticsNode rootComplex = new SemanticsNode();
+    final SemanticsNode rootComplex = new SemanticsNode()
+      ..rect = new Rect.fromLTRB(0.0, 0.0, 25.0, 5.0);
     rootComplex.addChildren(<SemanticsNode>[child1, child2, child3]);
     rootComplex.finalizeChildren();
 
     expect(
       rootComplex.toStringDeep(childOrder: DebugSemanticsDumpOrder.traversal),
-      'SemanticsNode#15(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '├SemanticsNode#12(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '│├SemanticsNode#14(Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '│└SemanticsNode#13(STALE, owner: null, Rect.fromLTRB(20.0, 0.0, 20.0, 0.0))\n'
-      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 10.0, 10.0, 10.0))\n'
-      '└SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(20.0, 20.0, 20.0, 20.0))\n',
+      'SemanticsNode#15(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 25.0, 5.0))\n'
+      '├SemanticsNode#12(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 10.0, 5.0))\n'
+      '│├SemanticsNode#14(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 5.0, 5.0))\n'
+      '│└SemanticsNode#13(STALE, owner: null, Rect.fromLTRB(5.0, 0.0, 10.0, 5.0))\n'
+      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 0.0, 15.0, 5.0))\n'
+      '└SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(15.0, 0.0, 20.0, 5.0))\n',
     );
 
     expect(
       rootComplex.toStringDeep(childOrder: DebugSemanticsDumpOrder.inverseHitTest),
-      'SemanticsNode#15(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      '├SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(20.0, 20.0, 20.0, 20.0))\n'
-      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 10.0, 10.0, 10.0))\n'
-      '└SemanticsNode#12(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n'
-      ' ├SemanticsNode#13(STALE, owner: null, Rect.fromLTRB(20.0, 0.0, 20.0, 0.0))\n'
-      ' └SemanticsNode#14(Rect.fromLTRB(0.0, 0.0, 0.0, 0.0))\n',
+      'SemanticsNode#15(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 25.0, 5.0))\n'
+      '├SemanticsNode#9(STALE, owner: null, Rect.fromLTRB(15.0, 0.0, 20.0, 5.0))\n'
+      '├SemanticsNode#10(STALE, owner: null, Rect.fromLTRB(10.0, 0.0, 15.0, 5.0))\n'
+      '└SemanticsNode#12(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 10.0, 5.0))\n'
+      ' ├SemanticsNode#13(STALE, owner: null, Rect.fromLTRB(5.0, 0.0, 10.0, 5.0))\n'
+      ' └SemanticsNode#14(STALE, owner: null, Rect.fromLTRB(0.0, 0.0, 5.0, 5.0))\n',
     );
   });
 

packages/flutter/test/widgets/semantics_clipping_test.dart

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+import 'semantics_tester.dart';
+
+void main() {
+  testWidgets('SemanticNode.rect is clipped', (WidgetTester tester) async {
+    final SemanticsTester semantics = new SemanticsTester(tester);
+
+    await tester.pumpWidget(new Directionality(
+      textDirection: TextDirection.ltr,
+      child: new Center(
+        child: new Container(
+          width: 100.0,
+          child: new Flex(
+            direction: Axis.horizontal,
+            children: <Widget>[
+              new Container(
+                width: 75.0,
+                child: const Text('1'),
+              ),
+              new Container(
+                width: 75.0,
+                child: const Text('2'),
+              ),
+              new Container(
+                width: 75.0,
+                child: const Text('3'),
+              ),
+            ],
+          ),
+        ),
+      ),
+    ));
+
+    expect(semantics, hasSemantics(
+      new TestSemantics.root(
+        children: <TestSemantics>[
+          new TestSemantics(
+            id: 1,
+            label: '1',
+            rect: new Rect.fromLTRB(0.0, 0.0, 75.0, 14.0),
+          ),
+          new TestSemantics(
+            id: 2,
+            label: '2',
+            rect: new Rect.fromLTRB(0.0, 0.0, 25.0, 14.0), // clipped form original 75.0 to 25.0
+          ),
+          // node with Text 3 not present.
+        ],
+      ),
+      ignoreTransform: true,
+    ));
+
+    final SemanticsNode node1 = tester.renderObject(find.byWidget(const Text('1'))).debugSemantics;
+    final SemanticsNode node2 = tester.renderObject(find.byWidget(const Text('2'))).debugSemantics;
+    final SemanticsNode node3 = tester.renderObject(find.byWidget(const Text('3'))).debugSemantics;
+
+    expect(node1.wasAffectedByClip, false);
+    expect(node2.wasAffectedByClip, true);
+    expect(node3.wasAffectedByClip, true);
+
+    expect(node1.isInvisible, isFalse);
+    expect(node2.isInvisible, isFalse);
+    expect(node3.isInvisible, isTrue);
+
+    semantics.dispose();
+  });
+
+  testWidgets('SemanticsNode is not removed if out of bounds and merged into something within bounds', (WidgetTester tester) async {
+    final SemanticsTester semantics = new SemanticsTester(tester);
+
+    await tester.pumpWidget(new Directionality(
+      textDirection: TextDirection.ltr,
+      child: new Center(
+        child: new Container(
+          width: 100.0,
+          child: new Flex(
+            direction: Axis.horizontal,
+            children: <Widget>[
+              new Container(
+                width: 75.0,
+                child: const Text('1'),
+              ),
+              new MergeSemantics(
+                child: new Flex(
+                  direction: Axis.horizontal,
+                  children: <Widget>[
+                    new Container(
+                      width: 75.0,
+                      child: const Text('2'),
+                    ),
+                    new Container(
+                      width: 75.0,
+                      child: const Text('3'),
+                    ),
+                  ]
+                )
+              ),
+            ],
+          ),
+        ),
+      ),
+    ));
+
+    expect(semantics, hasSemantics(
+      new TestSemantics.root(
+        children: <TestSemantics>[
+          new TestSemantics(
+            id: 4,
+            label: '1',
+            rect: new Rect.fromLTRB(0.0, 0.0, 75.0, 14.0),
+          ),
+          new TestSemantics(
+            id: 5,
+            label: '2\n3',
+            rect: new Rect.fromLTRB(0.0, 0.0, 25.0, 14.0), // clipped form original 75.0 to 25.0
+          ),
+        ],
+      ),
+      ignoreTransform: true,
+    ));
+
+    semantics.dispose();
+  });
+}

packages/flutter/test/widgets/semantics_merge_test.dart

@@ -19,16 +19,8 @@ void main() {
         textDirection: TextDirection.ltr,
         child: new Row(
           children: <Widget>[
-            new Semantics(
-                label: 'test1',
-                textDirection: TextDirection.ltr,
-                child: new Container()
-            ),
-            new Semantics(
-                label: 'test2',
-                textDirection: TextDirection.ltr,
-                child: new Container()
-            )
+            const Text('test1'),
+            const Text('test2'),
           ],
         ),
       ),
@@ -52,16 +44,8 @@ void main() {
         child: new MergeSemantics(
           child: new Row(
             children: <Widget>[
-              new Semantics(
-                  label: 'test1',
-                  textDirection: TextDirection.ltr,
-                  child: new Container()
-              ),
-              new Semantics(
-                  label: 'test2',
-                  textDirection: TextDirection.ltr,
-                  child: new Container()
-              )
+              const Text('test1'),
+              const Text('test2'),
             ],
           ),
         ),
@@ -80,16 +64,8 @@ void main() {
         textDirection: TextDirection.ltr,
         child: new Row(
           children: <Widget>[
-            new Semantics(
-                label: 'test1',
-                textDirection: TextDirection.ltr,
-                child: new Container()
-            ),
-            new Semantics(
-                label: 'test2',
-                textDirection: TextDirection.ltr,
-                child: new Container()
-            )
+            const Text('test1'),
+            const Text('test2'),
           ],
         ),
       ),