From ac03278602d5f3e20943acca636a8bf0812dc7db Mon Sep 17 00:00:00 2001 From: nturgut Date: Thu, 15 Oct 2020 11:58:05 -0700 Subject: [PATCH 1/9] logging --- lib/web_ui/lib/src/engine/window.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index f7140140a74b5..33f7477976994 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -185,6 +185,10 @@ class EngineFlutterWindow extends ui.Window { width = html.window.innerWidth! * devicePixelRatio; } + final int height2 = html.window.innerHeight ?? 0; + final int width2 = html.window.innerWidth ?? 0; + print('height width: $height2 $width2 semantics windowww $devicePixelRatio'); + // This method compares the new dimensions with the previous ones. // Return false if the previous dimensions are not set. if (_physicalSize != null) { From 1e2b8756a069f0b79b7b4f296e88d1b9226b86b2 Mon Sep 17 00:00:00 2001 From: nturgut Date: Thu, 15 Oct 2020 17:20:45 -0700 Subject: [PATCH 2/9] fixing positions with wrong a11y borders screenreader-on/mobile browsers --- .../lib/src/engine/semantics/semantics.dart | 6 ++-- lib/web_ui/lib/src/engine/util.dart | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 67fc749e901e4..93e0af5a8b01f 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -882,9 +882,11 @@ class SemanticsObject { } if (!effectiveTransformIsIdentity) { + final Position2D position2d = + matrix4ToCssTransformForSemantics(effectiveTransform); element.style - ..transformOrigin = '0 0 0' - ..transform = matrix4ToCssTransform(effectiveTransform); + ..top = position2d.top + ..left = position2d.left; } else { element.style ..removeProperty('transform-origin') diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 0338a6f2d7ea3..850d802bedb51 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -55,6 +55,11 @@ String matrix4ToCssTransform(Matrix4 matrix) { return float64ListToCssTransform(matrix.storage); } +/// Converts [matrix] to CSS top/left values. +Position2D matrix4ToCssTransformForSemantics(Matrix4 matrix) { + return float64ListToCss2dPositionSemantics(matrix.storage); +} + /// Applies a transform to the [element]. /// /// See [float64ListToCssTransform] for details on how the CSS value is chosen. @@ -86,6 +91,19 @@ String float64ListToCssTransform(Float32List matrix) { } } +/// Converts [matrix] to coordinates on 2 dimensional space. +/// +/// Only used by semantics node styling calculations, since semantics +/// node does not receive 3D transforms, only tx and ty values of the matrix +/// is used. +Position2D float64ListToCss2dPositionSemantics(Float32List matrix) { + assert(transformKindOf(matrix) != TransformKind.transform2d); + assert(matrix.length == 16); + final Float32List m = matrix; + print('top left: ${matrix[13]} ${matrix[12]} semantics'); + return Position2D('${matrix[13]}px', '${matrix[12]}px'); +} + /// The kind of effect a transform matrix performs. enum TransformKind { /// No effect. @@ -596,3 +614,15 @@ int clampInt(int value, int min, int max) { return value; } } + +/// Position of a node in 2 dimensional space. +class Position2D { + /// The position on y-axis. + final String top; + + /// The position on x-axis. + final String left; + + /// Position2D default constructor. + Position2D(this.top, this.left); +} From d7fb95ee2054c5e1fda15d7f6490d05a891a6a64 Mon Sep 17 00:00:00 2001 From: nturgut Date: Thu, 15 Oct 2020 17:38:13 -0700 Subject: [PATCH 3/9] remove logs from the window class --- lib/web_ui/lib/src/engine/window.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 33f7477976994..f7140140a74b5 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -185,10 +185,6 @@ class EngineFlutterWindow extends ui.Window { width = html.window.innerWidth! * devicePixelRatio; } - final int height2 = html.window.innerHeight ?? 0; - final int width2 = html.window.innerWidth ?? 0; - print('height width: $height2 $width2 semantics windowww $devicePixelRatio'); - // This method compares the new dimensions with the previous ones. // Return false if the previous dimensions are not set. if (_physicalSize != null) { From 7defdfc1c0f2ba1970589ac005cf64ae6788240d Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 16 Oct 2020 16:05:55 -0700 Subject: [PATCH 4/9] work on unit tests --- .../test/engine/semantics/semantics_test.dart | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index d3b0e52eb0180..e78e54e077102 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -366,22 +366,21 @@ void _testContainer() { ); semantics().updateSemantics(builder.build()); - expectSemanticsTree(''' - - - - -'''); - - final html.Element parentElement = - html.document.querySelector('flt-semantics'); - final html.Element container = - html.document.querySelector('flt-semantics-container'); - - expect(parentElement.style.transform, 'matrix(1, 0, 0, 1, 10, 10)'); - expect(parentElement.style.transformOrigin, '0px 0px 0px'); - expect(container.style.transform, 'translate(-10px, -10px)'); - expect(container.style.transformOrigin, '0px 0px 0px'); +// expectSemanticsTree(''' +// +// +// +// +// '''); + + // final html.Element parentElement = + // html.document.querySelector('flt-semantics'); + // final html.Element container = + // html.document.querySelector('flt-semantics-container'); + + // expect(container.style.top, '10px'); + // expect(container.style.left, '10px'); + // expect(container.style.transform, 'translate(-10px, -10px)'); semantics().semanticsEnabled = false; }, From 5c518335137f3eacc372a7980ea5263a93c18e32 Mon Sep 17 00:00:00 2001 From: nturgut Date: Thu, 29 Oct 2020 14:59:05 -0700 Subject: [PATCH 5/9] using reviewer suggestion for translations --- .../lib/src/engine/semantics/semantics.dart | 27 +++++++++---------- lib/web_ui/lib/src/engine/util.dart | 18 ------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 93e0af5a8b01f..aaed4aa0a4aac 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -852,12 +852,12 @@ class SemanticsObject { verticalContainerAdjustment == 0.0 && horizontalContainerAdjustment == 0.0) { element.style - ..removeProperty('transform-origin') - ..removeProperty('transform'); + ..removeProperty('top') + ..removeProperty('left'); if (containerElement != null) { containerElement.style - ..removeProperty('transform-origin') - ..removeProperty('transform'); + ..removeProperty('top') + ..removeProperty('left'); } return; } @@ -882,15 +882,14 @@ class SemanticsObject { } if (!effectiveTransformIsIdentity) { - final Position2D position2d = - matrix4ToCssTransformForSemantics(effectiveTransform); + final Vector3 translation = effectiveTransform.getTranslation(); element.style - ..top = position2d.top - ..left = position2d.left; + ..top = '${translation.y}px' + ..left = '${translation.x}px'; } else { element.style - ..removeProperty('transform-origin') - ..removeProperty('transform'); + ..removeProperty('top') + ..removeProperty('left'); } if (containerElement != null) { @@ -900,12 +899,12 @@ class SemanticsObject { final double translateX = -_rect!.left + horizontalContainerAdjustment; final double translateY = -_rect!.top + verticalContainerAdjustment; containerElement.style - ..transformOrigin = '0 0 0' - ..transform = 'translate(${translateX}px, ${translateY}px)'; + ..top = '${translateY}px' + ..left = '${translateX}px'; } else { containerElement.style - ..removeProperty('transform-origin') - ..removeProperty('transform'); + ..removeProperty('top') + ..removeProperty('left'); } } } diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 850d802bedb51..c4b781bd98fee 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -55,11 +55,6 @@ String matrix4ToCssTransform(Matrix4 matrix) { return float64ListToCssTransform(matrix.storage); } -/// Converts [matrix] to CSS top/left values. -Position2D matrix4ToCssTransformForSemantics(Matrix4 matrix) { - return float64ListToCss2dPositionSemantics(matrix.storage); -} - /// Applies a transform to the [element]. /// /// See [float64ListToCssTransform] for details on how the CSS value is chosen. @@ -91,19 +86,6 @@ String float64ListToCssTransform(Float32List matrix) { } } -/// Converts [matrix] to coordinates on 2 dimensional space. -/// -/// Only used by semantics node styling calculations, since semantics -/// node does not receive 3D transforms, only tx and ty values of the matrix -/// is used. -Position2D float64ListToCss2dPositionSemantics(Float32List matrix) { - assert(transformKindOf(matrix) != TransformKind.transform2d); - assert(matrix.length == 16); - final Float32List m = matrix; - print('top left: ${matrix[13]} ${matrix[12]} semantics'); - return Position2D('${matrix[13]}px', '${matrix[12]}px'); -} - /// The kind of effect a transform matrix performs. enum TransformKind { /// No effect. From 28f74c00e20b811f5f524fef80571c9cc2ebfc33 Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 30 Oct 2020 10:54:46 -0700 Subject: [PATCH 6/9] compute bounding matrix --- .../lib/src/engine/semantics/semantics.dart | 78 +++++++++++++++---- lib/web_ui/lib/src/engine/util.dart | 54 ++++++++++--- 2 files changed, 104 insertions(+), 28 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index aaed4aa0a4aac..b63c8832955f3 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -851,13 +851,25 @@ class SemanticsObject { hasIdentityTransform && verticalContainerAdjustment == 0.0 && horizontalContainerAdjustment == 0.0) { - element.style + if (isDesktop) { + element.style + ..removeProperty('transform-origin') + ..removeProperty('transform'); + } else { + element.style ..removeProperty('top') ..removeProperty('left'); + } if (containerElement != null) { - containerElement.style - ..removeProperty('top') - ..removeProperty('left'); + if (isDesktop) { + containerElement.style + ..removeProperty('transform-origin') + ..removeProperty('transform'); + } else { + containerElement.style + ..removeProperty('top') + ..removeProperty('left'); + } } return; } @@ -882,14 +894,34 @@ class SemanticsObject { } if (!effectiveTransformIsIdentity) { - final Vector3 translation = effectiveTransform.getTranslation(); - element.style - ..top = '${translation.y}px' - ..left = '${translation.x}px'; + if (isDesktop) { + element.style + ..transformOrigin = '0 0 0' + ..transform = matrix4ToCssTransform(effectiveTransform); + } else { + // Mobile screen readers observed have errors reading the semantics + // borders when css `transform` properties are used. + // See: https://github.com/flutter/flutter/issues/68225 + final ui.Rect rect = + computeBoundingRectangleFromMatrix(effectiveTransform, _rect!); + element.style + ..top = '${rect.top}px' + ..left = '${rect.left}px' + ..width = '${rect.width}px' + ..height = '${rect.height}px'; + print( + '** rectangle: ${rect.top}, ${rect.left}, ${rect.width}, ${rect.height}'); + } } else { - element.style - ..removeProperty('top') - ..removeProperty('left'); + if (isDesktop) { + element.style + ..removeProperty('transform-origin') + ..removeProperty('transform'); + } else { + element.style + ..removeProperty('top') + ..removeProperty('left'); + } } if (containerElement != null) { @@ -898,13 +930,25 @@ class SemanticsObject { horizontalContainerAdjustment != 0.0) { final double translateX = -_rect!.left + horizontalContainerAdjustment; final double translateY = -_rect!.top + verticalContainerAdjustment; - containerElement.style - ..top = '${translateY}px' - ..left = '${translateX}px'; + if (isDesktop) { + containerElement.style + ..transformOrigin = '0 0 0' + ..transform = 'translate(${translateX}px, ${translateY}px)'; + } else { + containerElement.style + ..top = '${translateY}px' + ..left = '${translateX}px'; + } } else { - containerElement.style - ..removeProperty('top') - ..removeProperty('left'); + if (isDesktop) { + containerElement.style + ..removeProperty('transform-origin') + ..removeProperty('transform'); + } else { + containerElement.style + ..removeProperty('top') + ..removeProperty('left'); + } } } } diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index c4b781bd98fee..413eb153ebce5 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -597,14 +597,46 @@ int clampInt(int value, int min, int max) { } } -/// Position of a node in 2 dimensional space. -class Position2D { - /// The position on y-axis. - final String top; - - /// The position on x-axis. - final String left; - - /// Position2D default constructor. - Position2D(this.top, this.left); -} +ui.Rect computeBoundingRectangleFromMatrix(Matrix4 transform, ui.Rect rect) { + final Float32List m = transform.storage; + // Apply perspective transform to all 4 corners. Can't use left,top, bottom, + // right since for example rotating 45 degrees would yield inaccurate size. + double x = rect.left; + double y = rect.top; + double wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + double xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + double yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + double minX = xp, maxX = xp; + double minY =yp, maxY = yp; + x = rect.right; + y = rect.bottom; + wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); + + x = rect.left; + y = rect.bottom; + wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); + + x = rect.right; + y = rect.top; + wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); + return ui.Rect.fromLTWH(minX, minY, maxX-minX, maxY-minY); + } From 6d4c26e63c65a6d3ad27d1f9ce07bec47bf75321 Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 30 Oct 2020 10:56:16 -0700 Subject: [PATCH 7/9] compute bounding matrix --- lib/web_ui/lib/src/engine/semantics/semantics.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index b63c8832955f3..9619b6782c0f1 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -909,8 +909,6 @@ class SemanticsObject { ..left = '${rect.left}px' ..width = '${rect.width}px' ..height = '${rect.height}px'; - print( - '** rectangle: ${rect.top}, ${rect.left}, ${rect.width}, ${rect.height}'); } } else { if (isDesktop) { From 5e024c43a8b09aec701d70022be946fcc0aa3ee1 Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 30 Oct 2020 10:59:20 -0700 Subject: [PATCH 8/9] addding more comments --- lib/web_ui/lib/src/engine/semantics/semantics.dart | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 9619b6782c0f1..791caed14cfb2 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -899,9 +899,13 @@ class SemanticsObject { ..transformOrigin = '0 0 0' ..transform = matrix4ToCssTransform(effectiveTransform); } else { - // Mobile screen readers observed have errors reading the semantics - // borders when css `transform` properties are used. + // Mobile screen readers observed to have errors while calculating the + // semantics focus borders if css `transform` properties are used. // See: https://github.com/flutter/flutter/issues/68225 + // Therefore we are calculating a bounding rectangle for the + // effective transform and use that rectangle to set TLWH css style + // properties. + // Note: Identity matrix is not using this code path. final ui.Rect rect = computeBoundingRectangleFromMatrix(effectiveTransform, _rect!); element.style From f181f2ac5a683623583aa5c4f63108ba8633e317 Mon Sep 17 00:00:00 2001 From: nturgut Date: Fri, 30 Oct 2020 12:27:56 -0700 Subject: [PATCH 9/9] reenable failing test case --- .../test/engine/semantics/semantics_test.dart | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index e78e54e077102..d3b0e52eb0180 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -366,21 +366,22 @@ void _testContainer() { ); semantics().updateSemantics(builder.build()); -// expectSemanticsTree(''' -// -// -// -// -// '''); - - // final html.Element parentElement = - // html.document.querySelector('flt-semantics'); - // final html.Element container = - // html.document.querySelector('flt-semantics-container'); - - // expect(container.style.top, '10px'); - // expect(container.style.left, '10px'); - // expect(container.style.transform, 'translate(-10px, -10px)'); + expectSemanticsTree(''' + + + + +'''); + + final html.Element parentElement = + html.document.querySelector('flt-semantics'); + final html.Element container = + html.document.querySelector('flt-semantics-container'); + + expect(parentElement.style.transform, 'matrix(1, 0, 0, 1, 10, 10)'); + expect(parentElement.style.transformOrigin, '0px 0px 0px'); + expect(container.style.transform, 'translate(-10px, -10px)'); + expect(container.style.transformOrigin, '0px 0px 0px'); semantics().semanticsEnabled = false; },