Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

nturgut
Copy link
Contributor

@nturgut nturgut commented Oct 15, 2020

Changing the css matrix transformation to only add 2d styling values (top, left, width, height). Calculating bounding rectangle for the transform so that if the semantics boundary is not an ordinary 2d translate, still focus is around the semantics area. This is similar to the Flutter native behaviour.

Fixes: flutter/flutter#68225

@chinmaygarde chinmaygarde added the platform-web Code specifically for the web engine label Oct 15, 2020
@nturgut nturgut requested a review from goderbauer October 16, 2020 00:38
@nturgut
Copy link
Contributor Author

nturgut commented Oct 16, 2020

@goderbauer can you confirm that the semantics focus node's border can be represented with 2d translation only? Or is it possible to receive a matrix we can't represent with top/left.

I tested this transform on a native Flutter app:
transform: Matrix4.skewY(0.3)..rotateZ(-pi / 12.0),

The screen reader focus node's border was a rectangle (not a parallelogram).

@nturgut nturgut requested a review from ferhatb October 16, 2020 00:45
@nturgut nturgut marked this pull request as ready for review October 16, 2020 00:45
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@goderbauer
Copy link
Member

@goderbauer can you confirm that the semantics focus node's border can be represented with 2d translation only? Or is it possible to receive a matrix we can't represent with top/left.

It's really up to the embedder to decide how to and where to draw the focus rect. The flutter framework provides you with a Rect and an arbitrary transform Matrix4 for each semantics node. It's upto the embedder to translate that into a focus rect. As far as I know, most (all?) platforms we support just draw the focus rect as the bounding box around the transformed rect, see Android for example:

However, in theory, if the platform supports it, you could draw a parallelogram as a focus rect. Flutter provides you all the data for this. Example:

Align(
      alignment: Alignment.topLeft,
      child: Transform(
        transform: Matrix4.skewY(0.3)..rotateZ(-pi / 12.0),
        child: Semantics(
          container: true,
          label: 'hello',
          child: Container(
            height: 100,
            width: 100,
            color: Colors.green,
          ),
        ),
      )
    );

This creates the following semantics node:

I/flutter (18226):        └─SemanticsNode#4
I/flutter (18226):            Rect.fromLTRB(0.0, 0.0, 100.0, 100.0) with transform
I/flutter (18226):            [0.9659258262890683,0.25881904510252074,0.0,0.0;
I/flutter (18226):            0.039976827402816106,1.045987939028626,0.0,0.0; 0.0,0.0,1.0,0.0;
I/flutter (18226):            0.0,0.0,0.0,1.0]
I/flutter (18226):            label: "hello"
I/flutter (18226):            textDirection: ltr

and if you apply that matrix to the rect you should get a parallelogram.

I believe just reading the translation values of the transform matrix is not enough, though. In the example above, if you replace the transform parameter with something like Matrix4.identity()..scale(2.0) wouldn't you draw a focus rect that's too small since it's missing the scale? Or is that applied elsewhere in web land?

@nturgut
Copy link
Contributor Author

nturgut commented Oct 16, 2020

@goderbauer can you confirm that the semantics focus node's border can be represented with 2d translation only? Or is it possible to receive a matrix we can't represent with top/left.

It's really up to the embedder to decide how to and where to draw the focus rect. The flutter framework provides you with a Rect and an arbitrary transform Matrix4 for each semantics node. It's upto the embedder to translate that into a focus rect. As far as I know, most (all?) platforms we support just draw the focus rect as the bounding box around the transformed rect, see Android for example:

However, in theory, if the platform supports it, you could draw a parallelogram as a focus rect. Flutter provides you all the data for this. Example:

Align(
      alignment: Alignment.topLeft,
      child: Transform(
        transform: Matrix4.skewY(0.3)..rotateZ(-pi / 12.0),
        child: Semantics(
          container: true,
          label: 'hello',
          child: Container(
            height: 100,
            width: 100,
            color: Colors.green,
          ),
        ),
      )
    );

This creates the following semantics node:

I/flutter (18226):        └─SemanticsNode#4
I/flutter (18226):            Rect.fromLTRB(0.0, 0.0, 100.0, 100.0) with transform
I/flutter (18226):            [0.9659258262890683,0.25881904510252074,0.0,0.0;
I/flutter (18226):            0.039976827402816106,1.045987939028626,0.0,0.0; 0.0,0.0,1.0,0.0;
I/flutter (18226):            0.0,0.0,0.0,1.0]
I/flutter (18226):            label: "hello"
I/flutter (18226):            textDirection: ltr

and if you apply that matrix to the rect you should get a parallelogram.

I believe just reading the translation values of the transform matrix is not enough, though. In the example above, if you replace the transform parameter with something like Matrix4.identity()..scale(2.0) wouldn't you draw a focus rect that's too small since it's missing the scale? Or is that applied elsewhere in web land?

Thanks a lot for the insight. I can see that we are loosing the scale information here, so that's important to address.

Let me rephrase what I understand from the parallelogram example. Other embedder's still does not have a parallelogram shaped focus node. The focus node is a rectangle bounding the parallelogram. I think this is also another area to address on the PR.

@nturgut nturgut requested a review from ferhatb October 30, 2020 18:00
@nturgut nturgut force-pushed the semantics_fix_for_borders branch from 608dc11 to 5e024c4 Compare October 30, 2020 18:56
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 3, 2020
cbracken pushed a commit to flutter/flutter that referenced this pull request Nov 3, 2020
* c597333 Roll Skia from f548a028ce70 to c21902c0d3cc (46 revisions) (flutter/engine#22224)

* 37d766c Fix includes to start with shell (flutter/engine#22227)

* 1ad6765 [web] Fixes canvas pixelation and overallocation due to transforms.  (flutter/engine#22160)

* 9f9fc1f Roll Skia from c21902c0d3cc to 9615bcf71f2a (1 revision) (flutter/engine#22226)

* 0faa72e Roll Dart SDK from fed66f60a3bc to 25ef5dc559cf (1 revision) (flutter/engine#22225)

* 172a393 Report image diff status for iOS scenario golden tests (flutter/engine#22230)

* fddabca updating integration tests version. (flutter/engine#22235)

* a36bcdc Roll Fuchsia Mac SDK from mhak7e_o6... to 8SkbMXJJ9... (flutter/engine#22231)

* 6a331d3 Roll Skia from 9615bcf71f2a to d5e6368fffd0 (8 revisions) (flutter/engine#22234)

* 9b34207 Fixing semantics borders on mobile web (flutter/engine#21856)

* 4e9459e Refactored the FlutterEngine to make it easier to implement spawn functionality (flutter/engine#21890)

* fa77e68 disable AppLifecycleTests (flutter/engine#22236)

* 153775b update golden (flutter/engine#22247)

* 50dbe85 [web] fix hot restart type error (flutter/engine#22248)

* c8cf09a Roll Skia from d5e6368fffd0 to 7585a65ac709 (7 revisions) (flutter/engine#22237)

* 68e2e46 Roll Fuchsia Mac SDK from 8SkbMXJJ9... to Pz4ZHZrUp... (flutter/engine#22246)

* d3182bc Roll Dart SDK from 25ef5dc559cf to 5acb5fcf84cb (4 revisions) (flutter/engine#22243)

* 9b4bb20 makes android semanticsnode to ignore hittest if it is not focusable (flutter/engine#22205)

* 3c7a54e Roll Fuchsia Linux SDK from sNx8qabBn... to QqGvMWaYk... (flutter/engine#22244)

* eea98b2 Roll Skia from 7585a65ac709 to dffd20efe95c (14 revisions) (flutter/engine#22250)

* 46e3bba Defer Windows arrow key and delete handling (flutter/engine#22207)

* 14437d6 fix _getArrayBuffer signature (flutter/engine#22251)

* 8defec6 Fix nullability issue with Image.network (flutter/engine#22252)

* 67d55ed Roll Dart SDK from 5acb5fcf84cb to a9d583383410 (4 revisions) (flutter/engine#22255)

* 7c8f57c Report error when instantiating CanvasKit network image (flutter/engine#22159)

* 9945db3 Remove the metrics task from cirrus. (flutter/engine#22240)

* bd19181 Add braces on if statements to match linter style (flutter/engine#22130)

* e9c62e7 do not print in _computePixelDensity (flutter/engine#22257)

* 2617101 Roll Dart SDK from a9d583383410 to d2577410a501 (1 revision) (flutter/engine#22258)

* 3d194fa Switch macOS embedding to proc table embedder API (flutter/engine#21811)

* 31b6f0b Roll Fuchsia Mac SDK from Pz4ZHZrUp... to 6yEx5GNGG... (flutter/engine#22262)

* 78a0181 Roll Fuchsia Linux SDK from QqGvMWaYk... to oLF1FW-gC... (flutter/engine#22264)

* ccdb681 WeakPersistentHandle migration (flutter/engine#19843)

* ce0a30c Roll Dart SDK from 52783837369d to b43baaaa477d (723 revisions) (flutter/engine#22265)

* 59b01e0 [web] Fix repaint logic for cullrect,transform changes (flutter/engine#22273)
chaselatta pushed a commit to chaselatta/engine that referenced this pull request Nov 30, 2020
* logging

* fixing positions with wrong a11y borders screenreader-on/mobile browsers

* remove logs from the window class

* work on unit tests

* using reviewer suggestion for translations

* compute bounding matrix

* compute bounding matrix

* addding more comments

* reenable failing test case
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[web] [a11y] The a11y border in incorrectly positioned for mobile browsers

6 participants