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
diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index b3dafd4..3b0a5ec 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart
@@ -603,12 +603,14 @@ 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 @@ 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 @@ 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 @@ 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);
diff --git a/packages/flutter/lib/src/rendering/semantics.dart b/packages/flutter/lib/src/rendering/semantics.dart index 96fa649..e120896 100644 --- a/packages/flutter/lib/src/rendering/semantics.dart +++ b/packages/flutter/lib/src/rendering/semantics.dart
@@ -285,9 +285,24 @@ } } - /// 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 @@ 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) {
diff --git a/packages/flutter/test/material/scaffold_test.dart b/packages/flutter/test/material/scaffold_test.dart index fe21c6b..eae2660 100644 --- a/packages/flutter/test/material/scaffold_test.dart +++ b/packages/flutter/test/material/scaffold_test.dart
@@ -501,11 +501,11 @@ 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));
diff --git a/packages/flutter/test/rendering/semantics_test.dart b/packages/flutter/test/rendering/semantics_test.dart index 9e15f1e..6052249 100644 --- a/packages/flutter/test/rendering/semantics_test.dart +++ b/packages/flutter/test/rendering/semantics_test.dart
@@ -32,6 +32,7 @@ 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 @@ 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 @@ }); 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 @@ 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', ); });
diff --git a/packages/flutter/test/widgets/semantics_clipping_test.dart b/packages/flutter/test/widgets/semantics_clipping_test.dart new file mode 100644 index 0000000..1cccef3 --- /dev/null +++ b/packages/flutter/test/widgets/semantics_clipping_test.dart
@@ -0,0 +1,132 @@ +// 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(); + }); +}
diff --git a/packages/flutter/test/widgets/semantics_merge_test.dart b/packages/flutter/test/widgets/semantics_merge_test.dart index 15e6920..4cd35c2 100644 --- a/packages/flutter/test/widgets/semantics_merge_test.dart +++ b/packages/flutter/test/widgets/semantics_merge_test.dart
@@ -19,16 +19,8 @@ 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 @@ 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 @@ 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'), ], ), ),