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

Conversation

@caroqliu
Copy link
Contributor

@caroqliu caroqliu commented Jul 11, 2023

This change also:

  • removes reliance from both tests on fuchsia.ui.scenic.Scenic protocols
  • migrates touch-input-test to get display info from fuchsia.ui.display.singleton.Info instead of fuchsia.ui.gfx.DisplayInfo
    • adds support for fuchsia.ui.display.singleton.Info in portable_ui_test to support the above
  • migrates flutter-embedder-test to capture screenshots using fuchsia.ui.composition.Screenshot instead of fuchsia.ui.scenic.Screenshot
    • updates screenshot tooling accordingly
  • removes the embedding view hit test disabled case from touch-input test, since disabling the hit region is not currently supported in Flutter's integration with Flatland afaict (this logic appears to set hit regions to the full view size by default, and Flatland appears to drop the value here)
    • removes hit test configurability on touch-input-test and flutter-embedder-test embedding views

Bug: fxbug.dev/125514

…t UI Stack

This change also:
- removes reliance on fuchsia.ui.scenic.Scenic protocols
- migrates touch-input-test from fuchsia.ui.gfx.DisplayInfo to fuchsia.ui.display.singleton.Info
- adds support for fuchsia.ui.display.singleton.Info in portable_ui_test
@chinmaygarde
Copy link
Member

cc @arbreng

@arbreng arbreng self-requested a review July 13, 2023 21:04
"fuchsia-pkg://fuchsia.com/parent-view#meta/parent-view.cm";
static constexpr auto kTestUiStackUrl =
"fuchsia-pkg://fuchsia.com/test-ui-stack#meta/test-ui-stack.cm";
"fuchsia-pkg://fuchsia.com/flatland-scene-manager-test-ui-stack#meta/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important to avoid encoding these implementation details in the name. This is in fact why alex chose to preserve the "test-ui-stack" name.

You can update the "test-ui-stack" package definition here to include the flatland-scene-manager config: https://source.corp.google.com/h/turquoise-internal/turquoise/+/main:src/ui/testing/test_ui_stack/BUILD.gn;l=82?q=test_ui_stack%2FBUILD.gn

Ideally, we could do that before landing this CL.

If that's not possible and we must wait until after this CL (due to soft transitions), then I insist that we file a bug and follow-up with another PR here to change the name to "test-ui-stack".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we are going to have to come back here anyways with a followup, so it's no big deal to do this now.

Test UI Stack will be replaced with the UI Test Realm Proxy, which will abstract these details. Additionally, we will use subpackages to run the "touch-input-test" from in-tree here and eliminate the duplicated code.

The naming issues can be fixed at that time, so it's OK to defer them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the heads up!

@arbreng
Copy link
Contributor

arbreng commented Jul 17, 2023

"removes the embedding view hit test disabled case from touch-input test, since disabling the hit region is not currently supported in Flutter's integration with Flatland afaict"

Please file a bug to re-add this support, via the in-tree touch-input-test, since it is possible to do so.

The plan of record is to use subpackages to replace the duplicated test code in this repository, and instead run the in-tree touch-input-test here. So, if that functionality is tested via fuchsia's "touch-input-test", it can eventually be tested and re-enabled here.

Copy link
Contributor

@arbreng arbreng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but please file the bug about re-enabling the "hit testable" support and CC me on it

@uysalere uysalere merged commit aa3239c into flutter:main Jul 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 17, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 17, 2023
…130735)

flutter/engine@09fe990...aa3239c

2023-07-17 [email protected] [fuchsia] flutter-embedder-test and touch-input-test use Flatland Test UI Stack (flutter/engine#43562)
2023-07-17 [email protected] Roll Skia from 4ec9f2497be1 to dc93f341ec38 (10 revisions) (flutter/engine#43739)
2023-07-17 [email protected] Roll Skia from 288c98d7ef0b to 4ec9f2497be1 (1 revision) (flutter/engine#43738)
2023-07-17 [email protected] Roll Fuchsia Mac SDK from VYjne_BEm9inQ5fnq... to jtvD_HgQVBqadF3jX... (flutter/engine#43736)
2023-07-17 [email protected] Roll Dart SDK from 827259dfffb9 to c1bfb2689f6f (1 revision) (flutter/engine#43735)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from VYjne_BEm9in to jtvD_HgQVBqa

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
…t UI Stack (flutter#43562)

This change also:
- removes reliance from both tests on `fuchsia.ui.scenic.Scenic`
protocols
- migrates `touch-input-test` to get display info from
`fuchsia.ui.display.singleton.Info` instead of
`fuchsia.ui.gfx.DisplayInfo`
- adds support for `fuchsia.ui.display.singleton.Info` in
`portable_ui_test` to support the above
- migrates `flutter-embedder-test` to capture screenshots using
`fuchsia.ui.composition.Screenshot` instead of
`fuchsia.ui.scenic.Screenshot`
  - updates screenshot tooling accordingly
- removes the embedding view hit test disabled case from `touch-input
test`, since disabling the hit region is not currently supported in
Flutter's integration with Flatland afaict ([this
logic](https://github.com/flutter/engine/blob/0f0436b28430bd79e854f2294a60aa33417068fa/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc#L411-L434)
appears to set hit regions to the full view size by default, and
Flatland appears to drop the value
[here](https://github.com/flutter/engine/blob/0f0436b28430bd79e854f2294a60aa33417068fa/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc#L578))
- removes hit test configurability on `touch-input-test` and
`flutter-embedder-test` embedding views

Bug: fxbug.dev/125514
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
…lutter#130735)

flutter/engine@09fe990...aa3239c

2023-07-17 [email protected] [fuchsia] flutter-embedder-test and touch-input-test use Flatland Test UI Stack (flutter/engine#43562)
2023-07-17 [email protected] Roll Skia from 4ec9f2497be1 to dc93f341ec38 (10 revisions) (flutter/engine#43739)
2023-07-17 [email protected] Roll Skia from 288c98d7ef0b to 4ec9f2497be1 (1 revision) (flutter/engine#43738)
2023-07-17 [email protected] Roll Fuchsia Mac SDK from VYjne_BEm9inQ5fnq... to jtvD_HgQVBqadF3jX... (flutter/engine#43736)
2023-07-17 [email protected] Roll Dart SDK from 827259dfffb9 to c1bfb2689f6f (1 revision) (flutter/engine#43735)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from VYjne_BEm9in to jtvD_HgQVBqa

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants