Unverified Commit 7be0bd1c authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Move geometric sort for semantics to the framework side. (#14539)

This adds geometric sort ordering back in for semantics nodes that don't have a sort order defined.

With this change, widgets that either have no sort order, or have an equivalent sort order, will be compared geometrically. The comparison is between the two starting corners, so it is TextDirection-aware: parent nodes that are set to have LTR text will compare upper left corners of their children, and upper right when set to RTL.

Also fixed a bug in the Transform widget that didn't mark modified nodes as needing a semantics update.
parent c54f0bcd
...@@ -1895,7 +1895,7 @@ class RenderTransform extends RenderProxyBox { ...@@ -1895,7 +1895,7 @@ class RenderTransform extends RenderProxyBox {
this.origin = origin; this.origin = origin;
} }
/// The origin of the coordinate system (relative to the upper left corder of /// The origin of the coordinate system (relative to the upper left corner of
/// this render object) in which to apply the matrix. /// this render object) in which to apply the matrix.
/// ///
/// Setting an origin is equivalent to conjugating the transform matrix by a /// Setting an origin is equivalent to conjugating the transform matrix by a
...@@ -1907,6 +1907,7 @@ class RenderTransform extends RenderProxyBox { ...@@ -1907,6 +1907,7 @@ class RenderTransform extends RenderProxyBox {
return; return;
_origin = value; _origin = value;
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// The alignment of the origin, relative to the size of the box. /// The alignment of the origin, relative to the size of the box.
...@@ -1927,6 +1928,7 @@ class RenderTransform extends RenderProxyBox { ...@@ -1927,6 +1928,7 @@ class RenderTransform extends RenderProxyBox {
return; return;
_alignment = value; _alignment = value;
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// The text direction with which to resolve [alignment]. /// The text direction with which to resolve [alignment].
...@@ -1940,6 +1942,7 @@ class RenderTransform extends RenderProxyBox { ...@@ -1940,6 +1942,7 @@ class RenderTransform extends RenderProxyBox {
return; return;
_textDirection = value; _textDirection = value;
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// When set to true, hit tests are performed based on the position of the /// When set to true, hit tests are performed based on the position of the
...@@ -1960,42 +1963,49 @@ class RenderTransform extends RenderProxyBox { ...@@ -1960,42 +1963,49 @@ class RenderTransform extends RenderProxyBox {
return; return;
_transform = new Matrix4.copy(value); _transform = new Matrix4.copy(value);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Sets the transform to the identity matrix. /// Sets the transform to the identity matrix.
void setIdentity() { void setIdentity() {
_transform.setIdentity(); _transform.setIdentity();
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Concatenates a rotation about the x axis into the transform. /// Concatenates a rotation about the x axis into the transform.
void rotateX(double radians) { void rotateX(double radians) {
_transform.rotateX(radians); _transform.rotateX(radians);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Concatenates a rotation about the y axis into the transform. /// Concatenates a rotation about the y axis into the transform.
void rotateY(double radians) { void rotateY(double radians) {
_transform.rotateY(radians); _transform.rotateY(radians);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Concatenates a rotation about the z axis into the transform. /// Concatenates a rotation about the z axis into the transform.
void rotateZ(double radians) { void rotateZ(double radians) {
_transform.rotateZ(radians); _transform.rotateZ(radians);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Concatenates a translation by (x, y, z) into the transform. /// Concatenates a translation by (x, y, z) into the transform.
void translate(double x, [double y = 0.0, double z = 0.0]) { void translate(double x, [double y = 0.0, double z = 0.0]) {
_transform.translate(x, y, z); _transform.translate(x, y, z);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
/// Concatenates a scale into the transform. /// Concatenates a scale into the transform.
void scale(double x, [double y, double z]) { void scale(double x, [double y, double z]) {
_transform.scale(x, y, z); _transform.scale(x, y, z);
markNeedsPaint(); markNeedsPaint();
markNeedsSemanticsUpdate();
} }
Matrix4 get _effectiveTransform { Matrix4 get _effectiveTransform {
......
...@@ -1431,18 +1431,74 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin { ...@@ -1431,18 +1431,74 @@ class SemanticsNode extends AbstractNode with DiagnosticableTreeMixin {
} }
} }
/// A helper class to contain a semantics node and the effective /// This class defines the comparison that is used to sort [SemanticsNode]s
/// sort order of that node during the traversal of the tree. This is /// before sending them to the platform side.
/// what is actually sorted when semantics nodes are sorted. ///
/// This is a helper class used to contain a [node], the effective
/// [order], the globally transformed starting corner [globalStartCorner],
/// and the containing node's [containerTextDirection] during the traversal of
/// the semantics node tree. A null value is allowed for [containerTextDirection],
/// because in that case we want to fall back to ordering by child insertion
/// order for nodes that are equal after sorting from top to bottom.
class _TraversalSortNode implements Comparable<_TraversalSortNode> { class _TraversalSortNode implements Comparable<_TraversalSortNode> {
_TraversalSortNode(this.node, this.order); _TraversalSortNode(this.node, this.order, this.containerTextDirection, Matrix4 transform)
: assert(node != null) {
// When containerTextDirection is null, this is set to topLeft, but the x
// coordinate is also ignored when doing the comparison in that case, so
// this isn't actually expressing a directionality opinion.
globalStartCorner = _transformPoint(
containerTextDirection == TextDirection.rtl ? node.rect.topRight : node.rect.topLeft,
transform,
);
}
/// The node that this sort node represents.
SemanticsNode node; SemanticsNode node;
/// The effective text direction for this node is the directionality that
/// its container has.
TextDirection containerTextDirection;
/// This is the effective sort order for this node, taking into account its
/// parents.
SemanticsSortOrder order; SemanticsSortOrder order;
/// The is the starting corner for the rectangle on this semantics node in
/// global coordinates. When the container has the directionality
/// [TextDirection.ltr], this is the upper left corner. When the container
/// has the directionality [TextDirection.rtl], this is the upper right
/// corner. When the container has no directionality, this is set, but the
/// x coordinate is ignored.
Offset globalStartCorner;
static Offset _transformPoint(Offset point, Matrix4 matrix) {
final Vector3 result = matrix.transform3(new Vector3(point.dx, point.dy, 0.0));
return new Offset(result.x, result.y);
}
/// Compares the node's start corner with that of `other`.
///
/// Sorts top to bottom, and then start to end.
///
/// This takes into account the container text direction, since the
/// coordinate system has zero on the left, and we need to compare
/// differently for different text directions.
///
/// If no text direction is available (i.e. [containerTextDirection] is
/// null), then we sort by vertical position first, and then by child
/// insertion order.
int _compareGeometry(_TraversalSortNode other) { int _compareGeometry(_TraversalSortNode other) {
// TODO(gspencer): Move the geometric comparison from the platform side to here. final int verticalDiff = globalStartCorner.dy.compareTo(other.globalStartCorner.dy);
// This involves calculating the globally-transformed quad for the semantics node rect if (verticalDiff != 0) {
// and then sorting by its bounding box, based on the container's directionality. return verticalDiff;
}
switch (containerTextDirection) {
case TextDirection.rtl:
return other.globalStartCorner.dx.compareTo(globalStartCorner.dx);
case TextDirection.ltr:
return globalStartCorner.dx.compareTo(other.globalStartCorner.dx);
}
// In case containerTextDirection is null we fall back to child insertion order.
return 0; return 0;
} }
...@@ -1490,17 +1546,34 @@ class SemanticsOwner extends ChangeNotifier { ...@@ -1490,17 +1546,34 @@ class SemanticsOwner extends ChangeNotifier {
void _updateTraversalOrder() { void _updateTraversalOrder() {
final List<_TraversalSortNode> nodesInSemanticsTraversalOrder = <_TraversalSortNode>[]; final List<_TraversalSortNode> nodesInSemanticsTraversalOrder = <_TraversalSortNode>[];
SemanticsSortOrder currentSortOrder = new SemanticsSortOrder(keys: <SemanticsSortKey>[]); SemanticsSortOrder currentSortOrder = new SemanticsSortOrder(keys: <SemanticsSortKey>[]);
Matrix4 currentTransform = new Matrix4.identity();
TextDirection currentTextDirection = rootSemanticsNode.textDirection;
bool visitor(SemanticsNode node) { bool visitor(SemanticsNode node) {
final SemanticsSortOrder previousOrder = currentSortOrder; final SemanticsSortOrder previousOrder = currentSortOrder;
final Matrix4 previousTransform = currentTransform.clone();
if (node.sortOrder != null) { if (node.sortOrder != null) {
currentSortOrder = currentSortOrder.merge(node.sortOrder); currentSortOrder = currentSortOrder.merge(node.sortOrder);
} }
final _TraversalSortNode traversalNode = new _TraversalSortNode(node, currentSortOrder); if (node.transform != null) {
currentTransform.multiply(node.transform);
}
final _TraversalSortNode traversalNode = new _TraversalSortNode(
node,
currentSortOrder,
currentTextDirection,
currentTransform,
);
// The text direction in force here is the parent's text direction.
nodesInSemanticsTraversalOrder.add(traversalNode); nodesInSemanticsTraversalOrder.add(traversalNode);
if (node.hasChildren) { if (node.hasChildren) {
final TextDirection previousTextDirection = currentTextDirection;
currentTextDirection = node.textDirection;
// Now visit the children with this node's text direction in force.
node.visitChildren(visitor); node.visitChildren(visitor);
currentTextDirection = previousTextDirection;
} }
currentSortOrder = previousOrder; currentSortOrder = previousOrder;
currentTransform = previousTransform;
return true; return true;
} }
rootSemanticsNode.visitChildren(visitor); rootSemanticsNode.visitChildren(visitor);
...@@ -2522,7 +2595,9 @@ String _concatStrings({ ...@@ -2522,7 +2595,9 @@ String _concatStrings({
/// of keys can co-exist at the same level and not interfere with each other, /// of keys can co-exist at the same level and not interfere with each other,
/// allowing for sorting into groups. Keys that evaluate as equal, or when /// allowing for sorting into groups. Keys that evaluate as equal, or when
/// compared with Widgets that don't have [Semantics], fall back to the default /// compared with Widgets that don't have [Semantics], fall back to the default
/// upper-start-to-lower-end geometric ordering. /// upper-start-to-lower-end geometric ordering if a text directionality
/// exists, and they sort from top to bottom followed by child insertion order
/// when no directionality is present.
/// ///
/// Since widgets are globally sorted by their sort key, the order does not have /// Since widgets are globally sorted by their sort key, the order does not have
/// to conform to the widget hierarchy. /// to conform to the widget hierarchy.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment