From 23cada281aeb0d6acc47c362c5e7fcc2db00e88e Mon Sep 17 00:00:00 2001 From: Xilai Zhang Date: Mon, 5 Feb 2024 17:31:54 -0800 Subject: [PATCH 01/18] [github actions] add cherry pick workflow for engine repo (#50265) Follow up from sync with @itsjustkevin : Add cherry pick github actions to flutter/engine repository. Would have the same functionalities as https://github.com/flutter/flutter/blob/master/.github/workflows/easy-cp.yml. @godofredoc would be great if we could add [actions bot token](https://valentine.corp.google.com/#/show/1702960394753966?tab=metadata) as a secret under the name FLUTTERACTIONSBOT_CP_TOKEN to the engine repository too. Thank you! --- .github/workflows/engine-cp.yml | 86 +++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 .github/workflows/engine-cp.yml diff --git a/.github/workflows/engine-cp.yml b/.github/workflows/engine-cp.yml new file mode 100644 index 0000000000000..fc71aad818434 --- /dev/null +++ b/.github/workflows/engine-cp.yml @@ -0,0 +1,86 @@ +# Copyright 2023 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +name: Cherry-pick Labeled Engine PR to Release Branch + +on: + pull_request_target: + branches: main + types: [labeled] + +permissions: write-all + +jobs: + cherrypick_to_release: + name: cherrypick_to_release + runs-on: ubuntu-latest + if: | + (github.event.label.name == format('cp{0} beta', ':') || github.event.label.name == format('cp{0} stable', ':')) && + (github.event.pull_request.merged == true) + steps: + - name: Get Release Channel + run: | + echo "CHANNEL=$(echo ${{ github.event.label.name }} | cut -d ':' -f 2 | xargs)" >> $GITHUB_ENV + - name: Get Release Candidate Branch + run: | + RELEASE_BRANCH=$(curl https://raw.githubusercontent.com/flutter/flutter/$CHANNEL/bin/internal/release-candidate-branch.version) + echo "RELEASE_BRANCH=$(echo $RELEASE_BRANCH | tr -d '\n')" >> $GITHUB_ENV + - name: Get Cherry Pick PR + run: | + echo "COMMIT_SHA=$(echo ${{ github.event.pull_request.merge_commit_sha }})" >> $GITHUB_ENV + - name: Checkout Flutter Engine Repo + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + with: + repository: flutteractionsbot/engine + path: engine + ref: main + persist-credentials: false + # Checkout all history commits on main branch, so that the cp commit is a known object + fetch-depth: 0 + - name: Attempt CP + id: attempt-cp + working-directory: ./engine + run: | + git config user.name "GitHub Actions Bot" + git config user.email "<>" + git remote add upstream https://github.com/flutter/engine.git + git fetch upstream $RELEASE_BRANCH + git fetch upstream main + git checkout -b cp-engine-${CHANNEL}-${COMMIT_SHA} --track upstream/$RELEASE_BRANCH + git cherry-pick $COMMIT_SHA + # TODO(xilaizhang): remove this step once the template is available on release branches. + - name: Get CP Template + run: | + curl -o PULL_REQUEST_CP_TEMPLATE.md https://raw.githubusercontent.com/flutter/flutter/master/.github/PR_TEMPLATE/PULL_REQUEST_CP_TEMPLATE.md + - name: Create PR on CP success + if: ${{ steps.attempt-cp.conclusion == 'success' }} + working-directory: ./engine + id: create-pr + run: | + git push https://${{ env.GITHUB_TOKEN }}@github.com/flutteractionsbot/engine cp-engine-${CHANNEL}-${COMMIT_SHA} + { + echo 'PR_URL<> "$GITHUB_ENV" + env: + GITHUB_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_CP_TOKEN }} + PR_TITLE: ${{ github.event.pull_request.title }} + - name: Leave Comment on CP success + if: ${{ steps.create-pr.conclusion == 'success' }} + run: | + echo $PR_URL + NEW_PR_NUMBER="${PR_URL##*/}" + SUCCESS_MSG=" @${{ github.actor }} please fill out the PR description above, afterwards the release team will review this request." + gh pr comment $NEW_PR_NUMBER -R flutter/engine -b "${SUCCESS_MSG}" + env: + GITHUB_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_CP_TOKEN }} + - name: Leave Comment on CP failure + if: ${{ failure() && steps.attempt-cp.conclusion == 'failure' }} + run: | + FAILURE_MSG="Failed to create CP due to merge conflicts.
" + FAILURE_MSG+="You will need to create the PR manually. See [the cherrypick wiki](https://github.com/flutter/flutter/wiki/Flutter-Cherrypick-Process) for more info." + gh pr comment ${{ github.event.pull_request.number }} -R flutter/engine -b "${FAILURE_MSG}" + env: + GITHUB_TOKEN: ${{ secrets.FLUTTERACTIONSBOT_CP_TOKEN }} From 880bf52e6c42a363684c17be9223ace0761ee6ce Mon Sep 17 00:00:00 2001 From: David Iglesias Date: Mon, 5 Feb 2024 17:48:00 -0800 Subject: [PATCH 02/18] [web] Fix Scene clip bounds. Trigger resize on DPR Change. (#50161) The Scene of the HTML renderer is passing incorrect size information to the engine, and when DPR<1, it can result in elements being culled off of the viewport. In addition to that, when an app is embedded, not all changes in DPR cause a Resize event (only those some of the dimensions fails by a rounding error!), so this PR ensures that all DPR events in embedded trigger a resize event. ### Issues Fixes https://github.com/flutter/flutter/issues/129182 ### Testing Looking good at: https://dit-astral-test.web.app [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- ci/licenses_golden/licenses_flutter | 2 + lib/web_ui/lib/src/engine.dart | 1 + lib/web_ui/lib/src/engine/html/scene.dart | 12 ++- .../custom_element_dimensions_provider.dart | 40 +++++--- .../dimensions_provider.dart | 25 +++-- .../full_page_dimensions_provider.dart | 5 +- .../view_embedder/display_dpr_stream.dart | 93 +++++++++++++++++++ ...stom_element_dimensions_provider_test.dart | 34 +++++-- .../dimensions_provider_test.dart | 14 --- .../full_page_dimensions_provider_test.dart | 3 - .../display_dpr_stream_test.dart | 45 +++++++++ 11 files changed, 226 insertions(+), 48 deletions(-) create mode 100644 lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart create mode 100644 lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 26dfb762b3930..6419812c072a5 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -6134,6 +6134,7 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart + ../../../f ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart + ../../../flutter/LICENSE @@ -8997,6 +8998,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart +FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 57c7b35fafb8a..14bda2390fdcc 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -190,6 +190,7 @@ export 'engine/vector_math.dart'; export 'engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart'; +export 'engine/view_embedder/display_dpr_stream.dart'; export 'engine/view_embedder/dom_manager.dart'; export 'engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart'; export 'engine/view_embedder/embedding_strategy/embedding_strategy.dart'; diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index c9703038b6103..b9f69996b1097 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -45,9 +45,15 @@ class PersistedScene extends PersistedContainerSurface { @override void recomputeTransformAndClip() { - // The scene clip is the size of the entire window. - final ui.Size screen = window.physicalSize; - localClipBounds = ui.Rect.fromLTRB(0, 0, screen.width, screen.height); + // The scene clip is the size of the entire window **in Logical pixels**. + // + // Even though the majority of the engine uses `physicalSize`, there are some + // bits (like the HTML renderer, or dynamic view sizing) that are implemented + // using CSS, and CSS operates in logical pixels. + // + // See also: [EngineFlutterView.resize]. + final ui.Size bounds = window.physicalSize / window.devicePixelRatio; + localClipBounds = ui.Rect.fromLTRB(0, 0, bounds.width, bounds.height); projectedClip = null; } diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart index 7b39ca908e576..25f77afdd5038 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -12,21 +13,36 @@ import 'dimensions_provider.dart'; /// This class provides observable, real-time dimensions of a host element. /// +/// This class needs a `Stream` of `devicePixelRatio` changes, like the one +/// provided by [DisplayDprStream], because html resize observers do not report +/// DPR changes. +/// /// All the measurements returned from this class are potentially *expensive*, /// and should be cached as needed. Every call to every method on this class /// WILL perform actual DOM measurements. +/// +/// This broadcasts `null` size events, to match the implementation of the +/// FullPageDimensionsProvider, but it could broadcast the size coming from the +/// DomResizeObserverEntry. Further changes in the engine are required for this +/// to be effective. class CustomElementDimensionsProvider extends DimensionsProvider { /// Creates a [CustomElementDimensionsProvider] from a [_hostElement]. - CustomElementDimensionsProvider(this._hostElement) { + CustomElementDimensionsProvider(this._hostElement, { + Stream? onDprChange, + }) { + // Send a resize event when the page DPR changes. + _dprChangeStreamSubscription = onDprChange?.listen((_) { + _broadcastSize(null); + }); + // Hook up a resize observer on the hostElement (if supported!). _hostElementResizeObserver = createDomResizeObserver(( List entries, DomResizeObserver _, ) { - entries - .map((DomResizeObserverEntry entry) => - ui.Size(entry.contentRect.width, entry.contentRect.height)) - .forEach(_broadcastSize); + for (final DomResizeObserverEntry _ in entries) { + _broadcastSize(null); + } }); assert(() { @@ -45,11 +61,12 @@ class CustomElementDimensionsProvider extends DimensionsProvider { // Handle resize events late DomResizeObserver? _hostElementResizeObserver; - final StreamController _onResizeStreamController = - StreamController.broadcast(); + late StreamSubscription? _dprChangeStreamSubscription; + final StreamController _onResizeStreamController = + StreamController.broadcast(); // Broadcasts the last seen `Size`. - void _broadcastSize(ui.Size size) { + void _broadcastSize(ui.Size? size) { _onResizeStreamController.add(size); } @@ -58,16 +75,17 @@ class CustomElementDimensionsProvider extends DimensionsProvider { super.close(); _hostElementResizeObserver?.disconnect(); // ignore:unawaited_futures + _dprChangeStreamSubscription?.cancel(); + // ignore:unawaited_futures _onResizeStreamController.close(); } @override - Stream get onResize => _onResizeStreamController.stream; + Stream get onResize => _onResizeStreamController.stream; @override ui.Size computePhysicalSize() { - final double devicePixelRatio = getDevicePixelRatio(); - + final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; return ui.Size( _hostElement.clientWidth * devicePixelRatio, _hostElement.clientHeight * devicePixelRatio, diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart index 2fcf44834395e..469f3a197d25b 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart @@ -5,11 +5,11 @@ import 'dart:async'; import 'package:meta/meta.dart'; +import 'package:ui/src/engine/dom.dart'; +import 'package:ui/src/engine/view_embedder/display_dpr_stream.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; -import '../../display.dart'; -import '../../dom.dart'; import 'custom_element_dimensions_provider.dart'; import 'full_page_dimensions_provider.dart'; @@ -32,18 +32,15 @@ abstract class DimensionsProvider { /// Creates the appropriate DimensionsProvider depending on the incoming [hostElement]. factory DimensionsProvider.create({DomElement? hostElement}) { if (hostElement != null) { - return CustomElementDimensionsProvider(hostElement); + return CustomElementDimensionsProvider( + hostElement, + onDprChange: DisplayDprStream.instance.dprChanged, + ); } else { return FullPageDimensionsProvider(); } } - /// Returns the DPI reported by the browser. - double getDevicePixelRatio() { - // This is overridable in tests. - return EngineFlutterDisplay.instance.devicePixelRatio; - } - /// Returns the [ui.Size] of the "viewport". /// /// This function is expensive. It triggers browser layout if there are @@ -57,6 +54,16 @@ abstract class DimensionsProvider { ); /// Returns a Stream with the changes to [ui.Size] (when cheap to get). + /// + /// Currently this Stream always returns `null` measurements because the + /// resize event that we use for [FullPageDimensionsProvider] does not contain + /// the new size, so users of this Stream everywhere immediately retrieve the + /// new `physicalSize` from the window. + /// + /// The [CustomElementDimensionsProvider] *could* broadcast the new size, but + /// to keep both implementations consistent (and their consumers), for now all + /// events from this Stream are going to be `null` (until we find a performant + /// way to retrieve the dimensions in full-page mode). Stream get onResize; /// Whether the [DimensionsProvider] instance has been closed or not. diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart index 8221d95457aef..bcacba4c869f3 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:ui/src/engine/browser_detection.dart'; +import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -67,7 +68,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { late double windowInnerWidth; late double windowInnerHeight; final DomVisualViewport? viewport = domWindow.visualViewport; - final double devicePixelRatio = getDevicePixelRatio(); + final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; if (viewport != null) { if (operatingSystem == OperatingSystem.iOs) { @@ -102,7 +103,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { double physicalHeight, bool isEditingOnMobile, ) { - final double devicePixelRatio = getDevicePixelRatio(); + final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; final DomVisualViewport? viewport = domWindow.visualViewport; late double windowInnerHeight; diff --git a/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart b/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart new file mode 100644 index 0000000000000..d73eeb5aec244 --- /dev/null +++ b/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart @@ -0,0 +1,93 @@ +// Copyright 2013 The Flutter 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 'dart:async'; +import 'dart:js_interop'; + +import 'package:meta/meta.dart'; +import 'package:ui/src/engine/display.dart'; +import 'package:ui/src/engine/dom.dart'; +import 'package:ui/ui.dart' as ui show Display; + +/// Provides a stream of `devicePixelRatio` changes for the given display. +/// +/// Note that until the Window Management API is generally available, this class +/// only monitors the global `devicePixelRatio` property, provided by the default +/// [EngineFlutterDisplay.instance]. +/// +/// See: https://developer.mozilla.org/en-US/docs/Web/API/Window_Management_API +class DisplayDprStream { + DisplayDprStream( + this._display, { + @visibleForTesting DebugDisplayDprStreamOverrides? overrides, + }) : _currentDpr = _display.devicePixelRatio, + _debugOverrides = overrides { + // Start listening to DPR changes. + _subscribeToMediaQuery(); + } + + /// A singleton instance of DisplayDprStream. + static DisplayDprStream instance = + DisplayDprStream(EngineFlutterDisplay.instance); + + // The display object that will provide the DPR information. + final ui.Display _display; + + // Last reported value of DPR. + double _currentDpr; + + // Controls the [dprChanged] broadcast Stream. + final StreamController _dprStreamController = + StreamController.broadcast(); + + // Object that fires a `change` event for the `_currentDpr`. + late DomEventTarget _dprMediaQuery; + + // Creates the media query for the latest known DPR value, and adds a change listener to it. + void _subscribeToMediaQuery() { + if (_debugOverrides?.getMediaQuery != null) { + _dprMediaQuery = _debugOverrides!.getMediaQuery!(_currentDpr); + } else { + _dprMediaQuery = domWindow.matchMedia('(resolution: ${_currentDpr}dppx)'); + } + _dprMediaQuery.addEventListenerWithOptions( + 'change', + createDomEventListener(_onDprMediaQueryChange), + { + // We only listen `once` because this event only triggers once when the + // DPR changes from `_currentDpr`. Once that happens, we need a new + // `_dprMediaQuery` that is watching the new `_currentDpr`. + // + // By using `once`, we don't need to worry about detaching the event + // listener from the old mediaQuery after we're done with it. + 'once': true, + 'passive': true, + }, + ); + } + + // Handler of the _dprMediaQuery 'change' event. + // + // This calls subscribe again because events are listened to with `once: true`. + JSVoid _onDprMediaQueryChange(DomEvent _) { + _currentDpr = _display.devicePixelRatio; + _dprStreamController.add(_currentDpr); + // Re-subscribe... + _subscribeToMediaQuery(); + } + + /// A stream that emits the latest value of [EngineFlutterDisplay.instance.devicePixelRatio]. + Stream get dprChanged => _dprStreamController.stream; + + // The overrides object that is used for testing. + final DebugDisplayDprStreamOverrides? _debugOverrides; +} + +@visibleForTesting +class DebugDisplayDprStreamOverrides { + DebugDisplayDprStreamOverrides({ + this.getMediaQuery, + }); + final DomEventTarget Function(double currentValue)? getMediaQuery; +} diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart index 9ee7a95083c05..4416aa59bc56c 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -@TestOn('browser') -library; - import 'dart:async'; import 'package:test/bootstrap/browser.dart'; @@ -109,23 +106,48 @@ void doTests() { }); test('funnels resize events on sizeSource', () async { + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.7); + sizeSource ..style.width = '100px' ..style.height = '100px'; - expect(await provider.onResize.first, const ui.Size(100, 100)); + expect(provider.onResize.first, completes); + expect(provider.computePhysicalSize(), const ui.Size(270, 270)); sizeSource ..style.width = '200px' ..style.height = '200px'; - expect(await provider.onResize.first, const ui.Size(200, 200)); + expect(provider.onResize.first, completes); + expect(provider.computePhysicalSize(), const ui.Size(540, 540)); sizeSource ..style.width = '300px' ..style.height = '300px'; - expect(await provider.onResize.first, const ui.Size(300, 300)); + expect(provider.onResize.first, completes); + expect(provider.computePhysicalSize(), const ui.Size(810, 810)); + }); + + test('funnels DPR change events too', () async { + // Override the source of DPR events... + final StreamController dprController = + StreamController.broadcast(); + + // Inject the dprController stream into the CustomElementDimensionsProvider. + final CustomElementDimensionsProvider provider = + CustomElementDimensionsProvider( + sizeSource, + onDprChange: dprController.stream, + ); + + // Set and broadcast the mock DPR value + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(3.2); + dprController.add(3.2); + + expect(provider.onResize.first, completes); + expect(provider.computePhysicalSize(), const ui.Size(32, 32)); }); test('closed by onHotRestart', () async { diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart index ba81aa32f01e3..7c355f94fdd17 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -@TestOn('browser') -library; - import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; @@ -31,15 +28,4 @@ void doTests() { expect(provider, isA()); }); }); - - group('getDevicePixelRatio', () { - test('Returns the correct pixelRatio', () async { - // Override the DPI to something known, but weird... - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(33930); - - final DimensionsProvider provider = DimensionsProvider.create(); - - expect(provider.getDevicePixelRatio(), 33930); - }); - }); } diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart index 88a037c053e45..cd633da01526d 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -@TestOn('browser') -library; - import 'dart:async'; import 'package:test/bootstrap/browser.dart'; diff --git a/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart b/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart new file mode 100644 index 0000000000000..a32ff78620fcf --- /dev/null +++ b/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart @@ -0,0 +1,45 @@ +// Copyright 2013 The Flutter 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 'dart:async'; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; +import 'package:ui/src/engine.dart'; + +void main() { + internalBootstrapBrowserTest(() => doTests); +} + +void doTests() { + final DomEventTarget eventTarget = createDomElement('div'); + + group('dprChanged Stream', () { + late DisplayDprStream dprStream; + + setUp(() async { + dprStream = DisplayDprStream(EngineFlutterDisplay.instance, + overrides: DebugDisplayDprStreamOverrides( + getMediaQuery: (_) => eventTarget, + )); + }); + + test('funnels display DPR on every mediaQuery "change" event.', () async { + final Future> dprs = dprStream.dprChanged + .take(3) + .timeout(const Duration(seconds: 1)) + .toList(); + + // Simulate the events + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(6.9); + eventTarget.dispatchEvent(createDomEvent('Event', 'change')); + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(4.2); + eventTarget.dispatchEvent(createDomEvent('Event', 'change')); + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(0.71); + eventTarget.dispatchEvent(createDomEvent('Event', 'change')); + + expect(await dprs, [6.9, 4.2, 0.71]); + }); + }); +} From 67f1c2bbb300be16257ed8ca959b3b93138b8a50 Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Mon, 5 Feb 2024 21:09:20 -0500 Subject: [PATCH 03/18] Roll Skia from cdf214adfb4d to c29a20702356 (54 revisions) (#50382) Roll Skia from cdf214adfb4d to c29a20702356 (54 revisions) https://skia.googlesource.com/skia.git/+log/cdf214adfb4d..c29a20702356 2024-02-06 brianosman@google.com Bring back DISABLE_LOWP 2024-02-06 skia-autoroll@skia-public.iam.gserviceaccount.com Roll ANGLE from f8c06f103a1f to 5a0615588a1a (10 revisions) 2024-02-05 lokokung@google.com Migrates graphite's main wait mechanism from using Tick to ProcessEvents. 2024-02-05 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from 3a55a3d5ee47 to 83223ec30684 (1 revision) 2024-02-05 fmalita@chromium.org [skottie] Fix supernormal spatial interpolation 2024-02-05 robertphillips@google.com [graphite] Add new Graphite Precompilation DMSink 2024-02-05 skia-autoroll@skia-public.iam.gserviceaccount.com Roll Dawn from dd852a700405 to c9c20468546f (26 revisions) 2024-02-05 skia-autoroll@skia-public.iam.gserviceaccount.com Roll SK Tool from 9a1a69ed50c4 to 5c8f3c7d6467 2024-02-05 kjlubick@google.com Fix skottie rules for G3 2024-02-05 kjlubick@google.com Temporarily disable animated_gif.cpp from Bazel test 2024-02-05 kjlubick@google.com Add bazel modules for Skottie and sksg 2024-02-03 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from a1def468c6b4 to 3a55a3d5ee47 (9 revisions) 2024-02-03 skia-autoroll@skia-public.iam.gserviceaccount.com Roll SK Tool from 8af152c5f7ce to 9a1a69ed50c4 2024-02-02 kjlubick@google.com Fix copypasta in public.bzl 2024-02-02 kjlubick@google.com Guard some gms/tests that require Skottie code 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll jsfiddle-base from 969399a89649 to b575daa903da 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll shaders-base from 9054c5988daf to 89826f3f6f5d 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skottie-base from 9ce4d2efff0c to 906e4c4c9b17 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll debugger-app-base from 54bf3755c837 to d2ed43ed5bb9 2024-02-02 bungeman@google.com [unicode] Fix SkBreakIterator_libgrapheme 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from 07589c29ccba to a1def468c6b4 (1 revision) 2024-02-02 kjlubick@google.com Make SVG backend explicitly depend on JPEG+PNG for OpenType 2024-02-02 kjlubick@google.com Add Bazel target for modular build of SkResources 2024-02-02 kjlubick@google.com Remove SkAnimCodecPlayer from the public API 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skottie-base from 5b5661dc98c7 to 9ce4d2efff0c 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll shaders-base from e7695f0fdff5 to 9054c5988daf 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll jsfiddle-base from f6c22747cda4 to 969399a89649 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll debugger-app-base from 067d72e0dddf to 54bf3755c837 2024-02-02 johnstiles@google.com Fix fuzzer-discovered timeout by caching uniform information. 2024-02-02 bungeman@google.com [pdf] Properly handle drawing not on a page 2024-02-02 johnstiles@google.com Add method Type::isAllowedInUniform. 2024-02-02 brianosman@google.com Remove SK_DISABLE_LOWP_RASTER_PIPELINE 2024-02-02 drott@chromium.org [Fontations-backend] Minor: Remove return statement 2024-02-02 49699333+dependabot[bot]@users.noreply.github.com Bump follow-redirects from 1.14.8 to 1.15.5 2024-02-02 49699333+dependabot[bot]@users.noreply.github.com Bump socket.io-parser from 4.2.2 to 4.2.4 in /modules/canvaskit 2024-02-02 49699333+dependabot[bot]@users.noreply.github.com Bump jinja2 from 2.11.3 to 3.1.3 2024-02-02 kjlubick@google.com Update Bazel rules after vulkan_header update 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll ANGLE from 1abfdc37320f to f8c06f103a1f (3 revisions) 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from 82b9fb3ddb57 to 07589c29ccba (6 revisions) 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll SK Tool from e83737e1d145 to b8aac8b590af 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll Skia Infra from 6e97526ab534 to e83737e1d145 (14 revisions) 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll SwiftShader from 78d1799ee43e to eb75201a4e03 (2 revisions) 2024-02-02 skia-autoroll@skia-public.iam.gserviceaccount.com Roll Dawn from 2c3a08ff64ba to dd852a700405 (8 revisions) 2024-02-02 bungeman@google.com Revert "[pdf] Properly handle drawing not on a page" 2024-02-02 johnstiles@google.com Add BUILD.gn cleanups to zlib. 2024-02-01 kjlubick@google.com Make SkScalarContext for Windows DirectWrite explicitly decode PNGs ... --- DEPS | 2 +- ci/licenses_golden/licenses_skia | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/DEPS b/DEPS index cb3f2c5e4bb2c..aac6f3aed6cc4 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { 'flutter_git': 'https://flutter.googlesource.com', 'skia_git': 'https://skia.googlesource.com', 'llvm_git': 'https://llvm.googlesource.com', - 'skia_revision': 'cdf214adfb4d88561c8aea0800f6cdc556502103', + 'skia_revision': 'c29a20702356a118d09d173755d66315b16127cf', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. diff --git a/ci/licenses_golden/licenses_skia b/ci/licenses_golden/licenses_skia index 3d504fcbe41a5..1d91a434cb0a6 100644 --- a/ci/licenses_golden/licenses_skia +++ b/ci/licenses_golden/licenses_skia @@ -1,4 +1,4 @@ -Signature: 369883e596b730b1331c02542375f231 +Signature: 1b1ebe15c644fb78b31443b98595da7e ==================================================================================================== LIBRARY: etc1 @@ -396,7 +396,10 @@ FILE: ../../../flutter/third_party/skia/modules/pathkit/perf/pathops.bench.js FILE: ../../../flutter/third_party/skia/modules/pathkit/perf/perfReporter.js FILE: ../../../flutter/third_party/skia/modules/skparagraph/test.html FILE: ../../../flutter/third_party/skia/package-lock.json +FILE: ../../../flutter/third_party/skia/relnotes/TickToProcessEvents.md FILE: ../../../flutter/third_party/skia/relnotes/VkDeviceLostCallback.md +FILE: ../../../flutter/third_party/skia/relnotes/anim-codec-player.md +FILE: ../../../flutter/third_party/skia/relnotes/codec-image.md FILE: ../../../flutter/third_party/skia/src/gpu/gpu_workaround_list.txt FILE: ../../../flutter/third_party/skia/src/ports/fontations/Cargo.toml FILE: ../../../flutter/third_party/skia/src/sksl/generated/sksl_compute.minified.sksl @@ -5015,7 +5018,6 @@ ORIGIN: ../../../flutter/third_party/skia/include/private/base/SkSafe32.h + ../. ORIGIN: ../../../flutter/third_party/skia/include/private/base/SkSpan_impl.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/include/private/base/SkTo.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/include/private/gpu/vk/SkiaVulkan.h + ../../../flutter/third_party/skia/LICENSE -ORIGIN: ../../../flutter/third_party/skia/include/utils/SkAnimCodecPlayer.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/include/utils/SkTextUtils.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/skcms/skcms.cc + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/skcms/src/Transform_inl.h + ../../../flutter/third_party/skia/LICENSE @@ -5033,6 +5035,8 @@ ORIGIN: ../../../flutter/third_party/skia/modules/skottie/src/layers/TextLayer.c ORIGIN: ../../../flutter/third_party/skia/modules/skottie/src/layers/shapelayer/ShapeLayer.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/skottie/utils/SkottieUtils.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/skottie/utils/SkottieUtils.h + ../../../flutter/third_party/skia/LICENSE +ORIGIN: ../../../flutter/third_party/skia/modules/skresources/src/SkAnimCodecPlayer.cpp + ../../../flutter/third_party/skia/LICENSE +ORIGIN: ../../../flutter/third_party/skia/modules/skresources/src/SkAnimCodecPlayer.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/sksg/include/SkSGClipEffect.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/sksg/include/SkSGColorFilter.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/modules/sksg/include/SkSGGradient.h + ../../../flutter/third_party/skia/LICENSE @@ -5163,7 +5167,6 @@ ORIGIN: ../../../flutter/third_party/skia/src/sksl/codegen/SkSLPipelineStageCode ORIGIN: ../../../flutter/third_party/skia/src/sksl/ir/SkSLVariableReference.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/text/gpu/SDFMaskFilter.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/text/gpu/SkChromeRemoteGlyphCache.cpp + ../../../flutter/third_party/skia/LICENSE -ORIGIN: ../../../flutter/third_party/skia/src/utils/SkAnimCodecPlayer.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/utils/SkCallableTraits.h + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/utils/SkJSON.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/utils/SkJSON.h + ../../../flutter/third_party/skia/LICENSE @@ -5230,7 +5233,6 @@ FILE: ../../../flutter/third_party/skia/include/private/base/SkSafe32.h FILE: ../../../flutter/third_party/skia/include/private/base/SkSpan_impl.h FILE: ../../../flutter/third_party/skia/include/private/base/SkTo.h FILE: ../../../flutter/third_party/skia/include/private/gpu/vk/SkiaVulkan.h -FILE: ../../../flutter/third_party/skia/include/utils/SkAnimCodecPlayer.h FILE: ../../../flutter/third_party/skia/include/utils/SkTextUtils.h FILE: ../../../flutter/third_party/skia/modules/skcms/skcms.cc FILE: ../../../flutter/third_party/skia/modules/skcms/src/Transform_inl.h @@ -5248,6 +5250,8 @@ FILE: ../../../flutter/third_party/skia/modules/skottie/src/layers/TextLayer.cpp FILE: ../../../flutter/third_party/skia/modules/skottie/src/layers/shapelayer/ShapeLayer.cpp FILE: ../../../flutter/third_party/skia/modules/skottie/utils/SkottieUtils.cpp FILE: ../../../flutter/third_party/skia/modules/skottie/utils/SkottieUtils.h +FILE: ../../../flutter/third_party/skia/modules/skresources/src/SkAnimCodecPlayer.cpp +FILE: ../../../flutter/third_party/skia/modules/skresources/src/SkAnimCodecPlayer.h FILE: ../../../flutter/third_party/skia/modules/sksg/include/SkSGClipEffect.h FILE: ../../../flutter/third_party/skia/modules/sksg/include/SkSGColorFilter.h FILE: ../../../flutter/third_party/skia/modules/sksg/include/SkSGGradient.h @@ -5378,7 +5382,6 @@ FILE: ../../../flutter/third_party/skia/src/sksl/codegen/SkSLPipelineStageCodeGe FILE: ../../../flutter/third_party/skia/src/sksl/ir/SkSLVariableReference.cpp FILE: ../../../flutter/third_party/skia/src/text/gpu/SDFMaskFilter.cpp FILE: ../../../flutter/third_party/skia/src/text/gpu/SkChromeRemoteGlyphCache.cpp -FILE: ../../../flutter/third_party/skia/src/utils/SkAnimCodecPlayer.cpp FILE: ../../../flutter/third_party/skia/src/utils/SkCallableTraits.h FILE: ../../../flutter/third_party/skia/src/utils/SkJSON.cpp FILE: ../../../flutter/third_party/skia/src/utils/SkJSON.h @@ -9502,11 +9505,13 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ==================================================================================================== LIBRARY: skia +ORIGIN: ../../../flutter/third_party/skia/gm/emptyshader.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/gm/imagedither.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/sksl/analysis/SkSLCheckSymbolTableCorrectness.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/sksl/ir/SkSLSymbol.cpp + ../../../flutter/third_party/skia/LICENSE ORIGIN: ../../../flutter/third_party/skia/src/sksl/transform/SkSLFindAndDeclareBuiltinStructs.cpp + ../../../flutter/third_party/skia/LICENSE TYPE: LicenseType.bsd +FILE: ../../../flutter/third_party/skia/gm/emptyshader.cpp FILE: ../../../flutter/third_party/skia/gm/imagedither.cpp FILE: ../../../flutter/third_party/skia/src/sksl/analysis/SkSLCheckSymbolTableCorrectness.cpp FILE: ../../../flutter/third_party/skia/src/sksl/ir/SkSLSymbol.cpp From 7c2f6c811270237bfab5d2d4bc508ea5a015bca9 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 5 Feb 2024 18:43:27 -0800 Subject: [PATCH 04/18] [Impeller] Do not skip the GLES render pass if the command list is empty (#50381) The render pass may need to apply other changes such as painting the clear color. See https://github.com/flutter/flutter/issues/142639 --- impeller/renderer/backend/gles/render_pass_gles.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 7a1cfea53322a..0bb937320c384 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -151,10 +151,6 @@ struct RenderPassData { const std::shared_ptr& tracer) { TRACE_EVENT0("impeller", "RenderPassGLES::EncodeCommandsInReactor"); - if (commands.empty()) { - return true; - } - const auto& gl = reactor.GetProcTable(); #ifdef IMPELLER_DEBUG tracer->MarkFrameStart(gl); @@ -517,9 +513,6 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { if (!IsValid()) { return false; } - if (commands_.empty()) { - return true; - } const auto& render_target = GetRenderTarget(); if (!render_target.HasColorAttachment(0u)) { return false; From 319ff6c8cbc96753f1763c4d489126fa18f9e050 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 5 Feb 2024 20:35:05 -0800 Subject: [PATCH 05/18] [Impeller] Cache RenderPass/Framebuffer objects on the resolve texture sources. (#50142) Cache vk::RenderPass and vk::Framebuffer objects on the resolve texture of any render target attachments. Use these on the next frame unconditionally. Fixes https://github.com/flutter/flutter/issues/141750 --- ci/licenses_golden/excluded_files | 1 + impeller/renderer/backend/vulkan/BUILD.gn | 2 + .../vulkan/render_pass_cache_unittests.cc | 51 +++++++++++++++++++ .../renderer/backend/vulkan/render_pass_vk.cc | 38 +++++++++++--- .../renderer/backend/vulkan/render_pass_vk.h | 2 + .../backend/vulkan/swapchain_image_vk.h | 1 + .../backend/vulkan/texture_source_vk.h | 2 + .../renderer/backend/vulkan/texture_vk.cc | 18 +++++++ impeller/renderer/backend/vulkan/texture_vk.h | 28 ++++++++++ 9 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index e5abdae233f47..e4900a92bd7cc 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -176,6 +176,7 @@ ../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/fence_waiter_vk_unittests.cc +../../../flutter/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/test ../../../flutter/impeller/renderer/blit_pass_unittests.cc diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index dfd33336b921c..5d9e26df17fe3 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -14,6 +14,7 @@ impeller_component("vulkan_unittests") { "context_vk_unittests.cc", "descriptor_pool_vk_unittests.cc", "fence_waiter_vk_unittests.cc", + "render_pass_cache_unittests.cc", "resource_manager_vk_unittests.cc", "test/gpu_tracer_unittests.cc", "test/mock_vulkan.cc", @@ -23,6 +24,7 @@ impeller_component("vulkan_unittests") { ] deps = [ ":vulkan", + "../../../playground:playground_test", "//flutter/testing:testing_lib", ] } diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc new file mode 100644 index 0000000000000..e2fc63ea5796a --- /dev/null +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -0,0 +1,51 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" +#include "gtest/gtest.h" +#include "impeller/playground/playground_test.h" +#include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "impeller/renderer/render_target.h" + +namespace impeller { +namespace testing { + +using RendererTest = PlaygroundTest; + +TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { + if (GetBackend() != PlaygroundBackend::kVulkan) { + GTEST_SKIP() << "Test only applies to Vulkan"; + } + + auto allocator = std::make_shared( + GetContext()->GetResourceAllocator()); + + auto render_target = RenderTarget::CreateOffscreenMSAA( + *GetContext(), *allocator, {100, 100}, 1); + auto resolve_texture = + render_target.GetColorAttachments().find(0u)->second.resolve_texture; + auto& texture_vk = TextureVK::Cast(*resolve_texture); + + EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + + auto buffer = GetContext()->CreateCommandBuffer(); + auto render_pass = buffer->CreateRenderPass(render_target); + + EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + + render_pass->EncodeCommands(); + GetContext()->GetCommandQueue()->Submit({buffer}); + + // Can be reused without error. + auto buffer_2 = GetContext()->CreateCommandBuffer(); + auto render_pass_2 = buffer_2->CreateRenderPass(render_target); + + EXPECT_TRUE(render_pass_2->EncodeCommands()); + EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index a8ce872f6db47..f942d99ea64e2 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -25,6 +25,7 @@ #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -79,6 +80,7 @@ static std::vector GetVKClearValues( SharedHandleVK RenderPassVK::CreateVKRenderPass( const ContextVK& context, + const SharedHandleVK& recycled_renderpass, const std::shared_ptr& command_buffer) const { BarrierVK barrier; barrier.new_layout = vk::ImageLayout::eGeneral; @@ -127,6 +129,15 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( TextureVK::Cast(*stencil->texture).SetLayout(barrier); } + // There may exist a previous recycled render pass that we can continue using. + // This is probably compatible with the render pass we are about to construct, + // but I have not conclusively proven this. If there are scenarios that + // produce validation warnings, we could use them to determine if we need + // additional checks at this point to determine reusability. + if (recycled_renderpass != nullptr) { + return recycled_renderpass; + } + auto pass = builder.Build(context.GetDevice()); if (!pass) { @@ -143,6 +154,11 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, const RenderTarget& target, std::shared_ptr command_buffer) : RenderPass(context, target), command_buffer_(std::move(command_buffer)) { + color_image_vk_ = + render_target_.GetColorAttachments().find(0u)->second.texture; + resolve_image_vk_ = + render_target_.GetColorAttachments().find(0u)->second.resolve_texture; + const auto& vk_context = ContextVK::Cast(*context); const std::shared_ptr& encoder = command_buffer_->GetEncoder(); @@ -154,16 +170,26 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, return true; }); + SharedHandleVK recycled_render_pass; + SharedHandleVK recycled_framebuffer; + if (resolve_image_vk_) { + recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass(); + recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer(); + } + const auto& target_size = render_target_.GetRenderTargetSize(); - render_pass_ = CreateVKRenderPass(vk_context, command_buffer_); + render_pass_ = + CreateVKRenderPass(vk_context, recycled_render_pass, command_buffer_); if (!render_pass_) { VALIDATION_LOG << "Could not create renderpass."; is_valid_ = false; return; } - auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass_); + auto framebuffer = (recycled_framebuffer == nullptr) + ? CreateVKFramebuffer(vk_context, *render_pass_) + : recycled_framebuffer; if (!framebuffer) { VALIDATION_LOG << "Could not create framebuffer."; is_valid_ = false; @@ -174,6 +200,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, is_valid_ = false; return; } + if (resolve_image_vk_) { + TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer); + TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_); + } auto clear_values = GetVKClearValues(render_target_); @@ -205,10 +235,6 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, .setExtent(vk::Extent2D(sc.GetWidth(), sc.GetHeight())); command_buffer_vk_.setScissor(0, 1, &scissor); - color_image_vk_ = - render_target_.GetColorAttachments().find(0u)->second.texture; - resolve_image_vk_ = - render_target_.GetColorAttachments().find(0u)->second.resolve_texture; is_valid_ = true; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 4836cf4c86c81..bf05e523d7ced 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -11,6 +11,7 @@ #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" +#include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -123,6 +124,7 @@ class RenderPassVK final : public RenderPass { SharedHandleVK CreateVKRenderPass( const ContextVK& context, + const SharedHandleVK& recycled_renderpass, const std::shared_ptr& command_buffer) const; SharedHandleVK CreateVKFramebuffer( diff --git a/impeller/renderer/backend/vulkan/swapchain_image_vk.h b/impeller/renderer/backend/vulkan/swapchain_image_vk.h index 1c0b9c2e911e8..48673ebe4845d 100644 --- a/impeller/renderer/backend/vulkan/swapchain_image_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_image_vk.h @@ -9,6 +9,7 @@ #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/texture_source_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_handles.hpp" namespace impeller { diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.h b/impeller/renderer/backend/vulkan/texture_source_vk.h index 0983bf74535d8..4f76a8067cf4f 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.h +++ b/impeller/renderer/backend/vulkan/texture_source_vk.h @@ -10,7 +10,9 @@ #include "impeller/core/texture_descriptor.h" #include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" +#include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" +#include "vulkan/vulkan_handles.hpp" namespace impeller { diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index f96b4940a4d98..b105d31b96e85 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -173,4 +173,22 @@ vk::ImageView TextureVK::GetRenderTargetView() const { return source_->GetRenderTargetView(); } +void TextureVK::SetFramebuffer( + const SharedHandleVK& framebuffer) { + framebuffer_ = framebuffer; +} + +void TextureVK::SetRenderPass( + const SharedHandleVK& render_pass) { + render_pass_ = render_pass; +} + +SharedHandleVK TextureVK::GetFramebuffer() const { + return framebuffer_; +} + +SharedHandleVK TextureVK::GetRenderPass() const { + return render_pass_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/texture_vk.h b/impeller/renderer/backend/vulkan/texture_vk.h index fd36213988bca..5826e3df78174 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.h +++ b/impeller/renderer/backend/vulkan/texture_vk.h @@ -44,9 +44,37 @@ class TextureVK final : public Texture, public BackendCast { bool IsSwapchainImage() const { return source_->IsSwapchainImage(); } + // These methods should only be used by render_pass_vk.h + + /// Store the last framebuffer object used with this texture. + /// + /// This field is only set if this texture is used as the resolve texture + /// of a render pass. By construction, this framebuffer should be compatible + /// with any future render passes. + void SetFramebuffer(const SharedHandleVK& framebuffer); + + /// Store the last render pass object used with this texture. + /// + /// This field is only set if this texture is used as the resolve texture + /// of a render pass. By construction, this framebuffer should be compatible + /// with any future render passes. + void SetRenderPass(const SharedHandleVK& render_pass); + + /// Retrieve the last framebuffer object used with this texture. + /// + /// May be nullptr if no previous framebuffer existed. + SharedHandleVK GetFramebuffer() const; + + /// Retrieve the last render pass object used with this texture. + /// + /// May be nullptr if no previous render pass existed. + SharedHandleVK GetRenderPass() const; + private: std::weak_ptr context_; std::shared_ptr source_; + SharedHandleVK framebuffer_ = nullptr; + SharedHandleVK render_pass_ = nullptr; // |Texture| void SetLabel(std::string_view label) override; From 4b877fba244caefc7ac8848e2d8c65b4f6d3b0c5 Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Tue, 6 Feb 2024 04:45:17 -0500 Subject: [PATCH 06/18] Roll Skia from c29a20702356 to 9e68ed6caf6d (1 revision) (#50392) https://skia.googlesource.com/skia.git/+log/c29a20702356..9e68ed6caf6d 2024-02-06 skia-autoroll@skia-public.iam.gserviceaccount.com Roll ANGLE from 5a0615588a1a to e62bd70a6c74 (4 revisions) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/skia-flutter-autoroll Please CC brianosman@google.com,bungeman@google.com,chinmaygarde@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index aac6f3aed6cc4..17be3865d5f4f 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { 'flutter_git': 'https://flutter.googlesource.com', 'skia_git': 'https://skia.googlesource.com', 'llvm_git': 'https://llvm.googlesource.com', - 'skia_revision': 'c29a20702356a118d09d173755d66315b16127cf', + 'skia_revision': '9e68ed6caf6dd06f01a8b15f0d2f97a480070bfd', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. From c4265e7e5e3668ff539d57df641b3c9f6bc57c87 Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Tue, 6 Feb 2024 06:09:21 -0500 Subject: [PATCH 07/18] Roll Skia from 9e68ed6caf6d to 44106ee8edea (1 revision) (#50393) https://skia.googlesource.com/skia.git/+log/9e68ed6caf6d..44106ee8edea 2024-02-06 skia-autoroll@skia-public.iam.gserviceaccount.com Roll vulkan-deps from 83223ec30684 to 32051b1fe7ae (9 revisions) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/skia-flutter-autoroll Please CC brianosman@google.com,bungeman@google.com,chinmaygarde@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 17be3865d5f4f..b99b933edc2a9 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { 'flutter_git': 'https://flutter.googlesource.com', 'skia_git': 'https://skia.googlesource.com', 'llvm_git': 'https://llvm.googlesource.com', - 'skia_revision': '9e68ed6caf6dd06f01a8b15f0d2f97a480070bfd', + 'skia_revision': '44106ee8edea8886e7408c1099164eda04836846', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. From da62280332cb6e73d082b7f5e0288972991c712e Mon Sep 17 00:00:00 2001 From: Jonny Wang Date: Tue, 6 Feb 2024 15:34:37 +0000 Subject: [PATCH 08/18] [fuchsia] Bump Fuchsia's API level to 16 (#50358) Update Fuchsia's API level to 16. b/322503140 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. --- .../fuchsia/dart_runner/dart_component_controller.cc | 5 +++++ shell/platform/fuchsia/flutter/component_v2.cc | 5 +++++ shell/platform/fuchsia/runtime/dart/utils/vmservice_object.h | 5 +++++ tools/fuchsia/target_api_level | 2 +- 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc index 1fc887cca4c08..a2dbb35ea4a23 100644 --- a/shell/platform/fuchsia/dart_runner/dart_component_controller.cc +++ b/shell/platform/fuchsia/dart_runner/dart_component_controller.cc @@ -216,9 +216,14 @@ bool DartComponentController::CreateAndBindNamespace() { fdio_service_connect_at(dart_outgoing_dir_ptr_.channel().get(), "svc", dart_public_dir.NewRequest().TakeChannel().release()); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + auto composed_service_dir = std::make_unique(); composed_service_dir->set_fallback(std::move(dart_public_dir)); +#pragma clang diagnostic pop + // Clone and check if client is servicing the directory. dart_outgoing_dir_ptr_->Clone( fuchsia::io::OpenFlags::DESCRIBE | diff --git a/shell/platform/fuchsia/flutter/component_v2.cc b/shell/platform/fuchsia/flutter/component_v2.cc index a10b003167566..ef616bbf4bf4e 100644 --- a/shell/platform/fuchsia/flutter/component_v2.cc +++ b/shell/platform/fuchsia/flutter/component_v2.cc @@ -259,9 +259,14 @@ ComponentV2::ComponentV2( fdio_service_connect_at(directory_ptr_.channel().get(), "svc", request.release()); +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + auto composed_service_dir = std::make_unique(); composed_service_dir->set_fallback(std::move(flutter_public_dir)); +#pragma clang diagnostic pop + // Clone and check if client is servicing the directory. directory_ptr_->Clone(fuchsia::io::OpenFlags::DESCRIBE | fuchsia::io::OpenFlags::CLONE_SAME_RIGHTS, diff --git a/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.h b/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.h index 697922491a277..c02d609e51506 100644 --- a/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.h +++ b/shell/platform/fuchsia/runtime/dart/utils/vmservice_object.h @@ -9,6 +9,9 @@ namespace dart_utils { +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" + class VMServiceObject : public vfs::LazyDir { public: static constexpr const char* kDirName = "DartVM"; @@ -21,6 +24,8 @@ class VMServiceObject : public vfs::LazyDir { std::string name) const override; }; +#pragma clang diagnostic pop + } // namespace dart_utils #endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_RUNTIME_DART_UTILS_VMSERVICE_OBJECT_H_ diff --git a/tools/fuchsia/target_api_level b/tools/fuchsia/target_api_level index 60d3b2f4a4cd5..b6a7d89c68e0c 100644 --- a/tools/fuchsia/target_api_level +++ b/tools/fuchsia/target_api_level @@ -1 +1 @@ -15 +16 From 2f40a27c01246333e7a0e78f912198f80c0d7f29 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 6 Feb 2024 08:42:14 -0800 Subject: [PATCH 09/18] Capture `FAILURES!!!` when running Android `scenario_app` tests. (#50255) I expect `Linux linux_android_emulator_tests` and `Linux linux_android_emulator_tests_api_33` to fail. When they fail, I'll fix the assertion (https://github.com/flutter/flutter/issues/142746) before merging this PR. --- .ci.yaml | 2 + .../bin/android_integration_tests.dart | 66 +++++++++++++++---- testing/scenario_app/bin/utils/logs.dart | 4 +- .../bin/utils/process_manager_extension.dart | 7 ++ testing/scenario_app/lib/main.dart | 15 +++-- testing/scenario_app/run_android_tests.sh | 5 +- .../tool/run_android_tests_smoke.sh | 18 +++++ 7 files changed, 99 insertions(+), 18 deletions(-) create mode 100755 testing/scenario_app/tool/run_android_tests_smoke.sh diff --git a/.ci.yaml b/.ci.yaml index edc4faa761885..5b25a136b70d0 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -60,6 +60,7 @@ targets: - DEPS - lib/ui/** - shell/platform/android/** + - testing/scenario_app/** # Task to run Linux linux_android_emulator_tests on AVDs running Android 33 # instead of 34 for investigating https://github.com/flutter/flutter/issues/137947. @@ -76,6 +77,7 @@ targets: - DEPS - lib/ui/** - shell/platform/android/** + - testing/scenario_app/** - name: Linux builder_cache enabled_branches: diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index 02fbdfa265290..ccbd0729df800 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -18,14 +18,49 @@ import 'utils/screenshot_transformer.dart'; const int tcpPort = 3001; void main(List args) async { - const ProcessManager pm = LocalProcessManager(); final ArgParser parser = ArgParser() - ..addOption('adb', help: 'absolute path to the adb tool', mandatory: true) - ..addOption('out-dir', help: 'out directory', mandatory: true); + ..addOption( + 'adb', + help: 'absolute path to the adb tool', + mandatory: true, + ) + ..addOption( + 'out-dir', + help: 'out directory', + mandatory: true, + ) + ..addFlag( + 'smoke-test', + help: 'runs a single test to verify the setup', + negatable: false, + defaultsTo: true, + ); - final ArgResults results = parser.parse(args); - final Directory outDir = Directory(results['out-dir'] as String); - final File adb = File(results['adb'] as String); + runZonedGuarded( + () async { + final ArgResults results = parser.parse(args); + final Directory outDir = Directory(results['out-dir'] as String); + final File adb = File(results['adb'] as String); + final bool smokeTest = results['smoke-test'] as bool; + await _run(outDir: outDir, adb: adb, smokeTest: smokeTest); + exit(0); + }, + (Object error, StackTrace stackTrace) { + if (error is! Panic) { + stderr.writeln(error); + stderr.writeln(stackTrace); + } + exit(1); + }, + ); +} + +Future _run({ + required Directory outDir, + required File adb, + required bool smokeTest, +}) async { + const ProcessManager pm = LocalProcessManager(); if (!outDir.existsSync()) { panic(['out-dir does not exist: $outDir', 'make sure to build the selected engine variant']); @@ -170,15 +205,25 @@ void main(List args) async { }); await step('Running instrumented tests...', () async { - final int exitCode = await pm.runAndForward([ + final (int exitCode, StringBuffer out) = await pm.runAndCapture([ adb.path, 'shell', 'am', 'instrument', - '-w', 'dev.flutter.scenarios.test/dev.flutter.TestRunner', + '-w', + if (smokeTest) + '-e class dev.flutter.scenarios.EngineLaunchE2ETest', + 'dev.flutter.scenarios.test/dev.flutter.TestRunner', ]); if (exitCode != 0) { - panic(['could not install test apk']); + panic(['instrumented tests failed to run']); + } + // Unfortunately adb shell am instrument does not return a non-zero exit + // code when tests fail, but it does seem to print "FAILURES!!!" to + // stdout, so we can use that as a signal that something went wrong. + if (out.toString().contains('FAILURES!!!')) { + stdout.write(out); + panic(['1 or more tests failed']); } }); } finally { @@ -221,8 +266,7 @@ void main(List args) async { await step('Flush logcat...', () async { await logcat.flush(); + await logcat.close(); }); - - exit(0); } } diff --git a/testing/scenario_app/bin/utils/logs.dart b/testing/scenario_app/bin/utils/logs.dart index 0c7c8a6873bf3..3132164300dde 100644 --- a/testing/scenario_app/bin/utils/logs.dart +++ b/testing/scenario_app/bin/utils/logs.dart @@ -23,9 +23,11 @@ void log(String msg) { stdout.writeln('$_gray$msg$_reset'); } +final class Panic extends Error {} + void panic(List messages) { for (final String message in messages) { stderr.writeln('$_red$message$_reset'); } - throw 'panic'; + throw Panic(); } diff --git a/testing/scenario_app/bin/utils/process_manager_extension.dart b/testing/scenario_app/bin/utils/process_manager_extension.dart index fb705f8f1144e..02499ae3f5fdd 100644 --- a/testing/scenario_app/bin/utils/process_manager_extension.dart +++ b/testing/scenario_app/bin/utils/process_manager_extension.dart @@ -45,4 +45,11 @@ extension RunAndForward on ProcessManager { Future runAndForward(List cmd) async { return pipeProcessStreams(await start(cmd), out: stdout); } + + /// Runs [cmd], and captures the stdout and stderr pipes. + Future<(int, StringBuffer)> runAndCapture(List cmd) async { + final StringBuffer buffer = StringBuffer(); + final int exitCode = await pipeProcessStreams(await start(cmd), out: buffer); + return (exitCode, buffer); + } } diff --git a/testing/scenario_app/lib/main.dart b/testing/scenario_app/lib/main.dart index 5e76f1d7306bf..6116bb1a80d82 100644 --- a/testing/scenario_app/lib/main.dart +++ b/testing/scenario_app/lib/main.dart @@ -25,14 +25,19 @@ void main() { channelBuffers.setListener('driver', _handleDriverMessage); channelBuffers.setListener('write_timeline', _handleWriteTimelineMessage); - final FlutterView view = PlatformDispatcher.instance.implicitView!; + // TODO(matanlurey): https://github.com/flutter/flutter/issues/142746. + // This Dart program is used for every test, but there is at least one test + // (EngineLaunchE2ETest.java) that does not create a FlutterView, so the + // implicit view's size is not initialized (and the assert would be tripped). + // + // final FlutterView view = PlatformDispatcher.instance.implicitView!; // Asserting that this is greater than zero since this app runs on different // platforms with different sizes. If it is greater than zero, it has been // initialized to some meaningful value at least. - assert( - view.display.size > Offset.zero, - 'Expected ${view.display} to be initialized.', - ); + // assert( + // view.display.size > Offset.zero, + // 'Expected ${view.display} to be initialized.', + // ); final ByteData data = ByteData(1); data.setUint8(0, 1); diff --git a/testing/scenario_app/run_android_tests.sh b/testing/scenario_app/run_android_tests.sh index eae473cf4cbdd..ec4abf49cb1c8 100755 --- a/testing/scenario_app/run_android_tests.sh +++ b/testing/scenario_app/run_android_tests.sh @@ -33,7 +33,10 @@ function follow_links() ( ) SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")") -SRC_DIR="$(cd "$SCRIPT_DIR/../../.."; pwd -P)" +SRC_DIR="$( + cd "$SCRIPT_DIR/../../.." + pwd -P +)" OUT_DIR="$SRC_DIR/out/$BUILD_VARIANT" # Dump the logcat and symbolize stack traces before exiting. diff --git a/testing/scenario_app/tool/run_android_tests_smoke.sh b/testing/scenario_app/tool/run_android_tests_smoke.sh new file mode 100755 index 0000000000000..f04da36fc6594 --- /dev/null +++ b/testing/scenario_app/tool/run_android_tests_smoke.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# This is a debugging script that runs a single Android E2E test on a connected +# device or emulator, and reports the exit code. It was largely created to debug +# why `./testing/scenario_app/run_android_tests.sh` did or did not report +# failures correctly. + +# Run this command and print out the exit code. +../third_party/dart/tools/sdks/dart-sdk/bin/dart ./testing/scenario_app/bin/android_integration_tests.dart \ + --adb="../third_party/android_tools/sdk/platform-tools/adb" \ + --out-dir="../out/android_debug_unopt_arm64" \ + --smoke-test + +echo "Exit code: $?" +echo "Done" From 10000e0c916cb691d6d8e8a257d5c4b06ba18cbb Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Tue, 6 Feb 2024 11:48:31 -0500 Subject: [PATCH 10/18] Roll Dart SDK from b62066b42af0 to 29265c94a6e8 (10 revisions) (#50398) https://dart.googlesource.com/sdk.git/+log/b62066b42af0..29265c94a6e8 2024-02-06 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-114.0.dev 2024-02-06 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-113.0.dev 2024-02-06 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-112.0.dev 2024-02-05 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-111.0.dev 2024-02-05 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-110.0.dev 2024-02-05 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-109.0.dev 2024-02-05 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-108.0.dev 2024-02-03 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-107.0.dev 2024-02-03 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-106.0.dev 2024-02-02 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-105.0.dev If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/dart-sdk-flutter-engine Please CC chinmaygarde@google.com,dart-vm-team@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter Engine: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- ci/licenses_golden/licenses_dart | 4 ++-- sky/packages/sky_engine/LICENSE | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DEPS b/DEPS index b99b933edc2a9..29461086f96a6 100644 --- a/DEPS +++ b/DEPS @@ -62,7 +62,7 @@ vars = { # Dart is: https://github.com/dart-lang/sdk/blob/main/DEPS # You can use //tools/dart/create_updated_flutter_deps.py to produce # updated revision list of existing dependencies. - 'dart_revision': 'b62066b42af0627ecb31af80b147a75834172745', + 'dart_revision': '29265c94a6e83e1a66015e76754a1b2a2540ea0c', # WARNING: DO NOT EDIT MANUALLY # The lines between blank lines above and below are generated by a script. See create_updated_flutter_deps.py diff --git a/ci/licenses_golden/licenses_dart b/ci/licenses_golden/licenses_dart index 327ad362a8f1e..e3e9e8332f233 100644 --- a/ci/licenses_golden/licenses_dart +++ b/ci/licenses_golden/licenses_dart @@ -1,4 +1,4 @@ -Signature: f2bb9d804c685455ce1abef16b3d0943 +Signature: 1c56d4fc6cbb1e41505c2d67d9b13747 ==================================================================================================== LIBRARY: dart @@ -4750,7 +4750,7 @@ Exhibit B - "Incompatible With Secondary Licenses" Notice This Source Code Form is "Incompatible With Secondary Licenses", as defined by the Mozilla Public License, v. 2.0. -You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/b62066b42af0627ecb31af80b147a75834172745 +You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/29265c94a6e83e1a66015e76754a1b2a2540ea0c /third_party/fallback_root_certificates/ ==================================================================================================== diff --git a/sky/packages/sky_engine/LICENSE b/sky/packages/sky_engine/LICENSE index 1d82290088c22..7f7f20609e8a5 100644 --- a/sky/packages/sky_engine/LICENSE +++ b/sky/packages/sky_engine/LICENSE @@ -31342,7 +31342,7 @@ Exhibit B - "Incompatible With Secondary Licenses" Notice This Source Code Form is "Incompatible With Secondary Licenses", as defined by the Mozilla Public License, v. 2.0. -You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/b62066b42af0627ecb31af80b147a75834172745 +You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/29265c94a6e83e1a66015e76754a1b2a2540ea0c /third_party/fallback_root_certificates/ -------------------------------------------------------------------------------- From e116f89ff506bd9d59d91c85a584a7dc135ca922 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 6 Feb 2024 09:15:08 -0800 Subject: [PATCH 11/18] Run all Android `scenario_app` tests, not just the smoke test. (#50400) I was making sure that I could get at least one test running and passing on CI (as expected), now turn all of them on. --- testing/scenario_app/bin/android_integration_tests.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/scenario_app/bin/android_integration_tests.dart b/testing/scenario_app/bin/android_integration_tests.dart index ccbd0729df800..9517ee350ddf8 100644 --- a/testing/scenario_app/bin/android_integration_tests.dart +++ b/testing/scenario_app/bin/android_integration_tests.dart @@ -33,7 +33,6 @@ void main(List args) async { 'smoke-test', help: 'runs a single test to verify the setup', negatable: false, - defaultsTo: true, ); runZonedGuarded( From 911bd8d76368811c502bd477e7f8e0efd4079917 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 6 Feb 2024 09:33:45 -0800 Subject: [PATCH 12/18] [Impeller] Specify if Angle or SwiftShader is being used in the title. (#50376) Minor QOL improvement. Tried taking a frame capture unsuccessfully before realizing that I was using SwiftShader. --- impeller/playground/playground_test.cc | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/impeller/playground/playground_test.cc b/impeller/playground/playground_test.cc index d9439dacc2a7c..676c3b0412992 100644 --- a/impeller/playground/playground_test.cc +++ b/impeller/playground/playground_test.cc @@ -56,15 +56,27 @@ RuntimeStage::Map PlaygroundTest::OpenAssetAsRuntimeStage( return RuntimeStage::DecodeRuntimeStages(fixture); } -static std::string FormatWindowTitle(const std::string& test_name) { - std::stringstream stream; - stream << "Impeller Playground for '" << test_name << "' (Press ESC to quit)"; - return stream.str(); -} - // |Playground| std::string PlaygroundTest::GetWindowTitle() const { - return FormatWindowTitle(flutter::testing::GetCurrentTestName()); + std::stringstream stream; + stream << "Impeller Playground for '" + << flutter::testing::GetCurrentTestName() << "' "; + switch (GetBackend()) { + case PlaygroundBackend::kMetal: + break; + case PlaygroundBackend::kOpenGLES: + if (switches_.use_angle) { + stream << " (Angle) "; + } + break; + case PlaygroundBackend::kVulkan: + if (switches_.use_swiftshader) { + stream << " (SwiftShader) "; + } + break; + } + stream << " (Press ESC to quit)"; + return stream.str(); } // |Playground| From a09980eec32705ee402c75aef99f4e99d8bda64f Mon Sep 17 00:00:00 2001 From: "zijiehe@" <68449066+zijiehe-google-com@users.noreply.github.com> Date: Tue, 6 Feb 2024 09:34:53 -0800 Subject: [PATCH 13/18] Revert "Revert "[Fuchsia] Execute most of the testing/fuchsia/test_suites.yaml on debug and release builds"" (#50295) Reverts flutter/engine#50291, https://github.com/flutter/flutter/issues/142811 Following is the original change description. This change implements a BundledTestRunner to run most of the tests in testing/fuchsia/test_suites.yaml as ExecutableTestRunner. - Tests with packages out of out/fuchsia_*_x64/ are ignored for now. - Tests with extra test command line parameters are ignored for now. The BundledTestRunner can share most of the logic in ExecutableTestRunner and avoid reinventing the wheel. This change also fixes the build break of fuchsia_tests in fuchsia_release_x64 which allows tests to run on the build as well. - Tests not built with AOT are filtered out with variant field in test_suites.yaml. Bug: https://github.com/flutter/flutter/issues/140179 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .ci.yaml | 7 +- ci/builders/linux_fuchsia.json | 24 +++- .../fuchsia/dart_runner/embedder/BUILD.gn | 3 +- .../fuchsia/dart_runner/kernel/BUILD.gn | 3 +- .../fuchsia/dart_runner/vmservice/BUILD.gn | 2 +- .../platform/fuchsia/flutter/kernel/BUILD.gn | 3 +- testing/fuchsia/run_tests.py | 115 ++++++++++++++---- testing/fuchsia/test_suites.yaml | 4 + 8 files changed, 128 insertions(+), 33 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index 5b25a136b70d0..fa5bd99a109b4 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -195,7 +195,12 @@ targets: - name: Linux linux_fuchsia recipe: engine_v2/engine_v2 - timeout: 60 + # Temporarily increase the timeout of this builder to 75 minutes to avoid + # being penetrated by cold goma / rbclient cache. + # TODO(zijiehe-google-com): Drop the timeout to 60 minutes once the release + # builder can finish within a reasonable time. + # https://github.com/flutter/flutter/issues/142932 + timeout: 75 properties: release_build: "true" config_name: linux_fuchsia diff --git a/ci/builders/linux_fuchsia.json b/ci/builders/linux_fuchsia.json index 011a13618aa11..facbae0827b6c 100644 --- a/ci/builders/linux_fuchsia.json +++ b/ci/builders/linux_fuchsia.json @@ -115,10 +115,12 @@ { "drone_dimensions": [ "device_type=none", + "kvm=1", "os=Linux" ], "gclient_variables": { - "download_android_deps": false + "download_android_deps": false, + "run_fuchsia_emu": true }, "gn": [ "--fuchsia", @@ -131,9 +133,22 @@ "ninja": { "config": "fuchsia_release_x64", "targets": [ - "flutter/shell/platform/fuchsia:fuchsia" + "flutter/shell/platform/fuchsia:fuchsia", + "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", + "fuchsia_tests" ] - } + }, + "tests": [ + { + "name": "x64 emulator based release tests", + "language": "python3", + "script": "flutter/tools/fuchsia/with_envs.py", + "parameters": [ + "testing/fuchsia/run_tests.py", + "fuchsia_release_x64" + ] + } + ] }, { "drone_dimensions": [ @@ -158,6 +173,7 @@ "config": "fuchsia_debug_x64", "targets": [ "flutter/shell/platform/fuchsia:fuchsia", + "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", "fuchsia_tests" ] }, @@ -176,7 +192,7 @@ ] }, { - "name": "x64 emulator based tests", + "name": "x64 emulator based debug tests", "language": "python3", "script": "flutter/tools/fuchsia/with_envs.py", "parameters": [ diff --git a/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn b/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn index afd12c4b51be8..562191cbcf6b0 100644 --- a/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn @@ -54,8 +54,7 @@ template("create_aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (!(flutter_runtime_mode == "profile") || is_debug)) { + if (!invoker.product && (flutter_runtime_mode == "debug" || is_debug)) { args += [ "--enable_asserts" ] } args += [ rebase_path(shim_kernel) ] diff --git a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn index af896f65e28b8..c990742404703 100644 --- a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn @@ -69,8 +69,7 @@ template("create_kernel_core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (is_debug || !(flutter_runtime_mode == "profile"))) { + if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn b/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn index 8aa7da550baab..c299edb5f3f78 100644 --- a/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn @@ -65,7 +65,7 @@ template("aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!product && (!(flutter_runtime_mode == "profile") || is_debug)) { + if (!product && (flutter_runtime_mode == "debug" || is_debug)) { args += [ "--enable_asserts" ] } diff --git a/shell/platform/fuchsia/flutter/kernel/BUILD.gn b/shell/platform/fuchsia/flutter/kernel/BUILD.gn index bc9853d5cd7f9..dd77441bfd3cd 100644 --- a/shell/platform/fuchsia/flutter/kernel/BUILD.gn +++ b/shell/platform/fuchsia/flutter/kernel/BUILD.gn @@ -73,8 +73,7 @@ template("core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && - (is_debug || !(flutter_runtime_mode == "profile"))) { + if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/testing/fuchsia/run_tests.py b/testing/fuchsia/run_tests.py index 2a301c828299c..40c66be2b96a1 100755 --- a/testing/fuchsia/run_tests.py +++ b/testing/fuchsia/run_tests.py @@ -1,12 +1,28 @@ -#!/usr/bin/env python3 +#!/usr/bin/env vpython3 + +# [VPYTHON:BEGIN] +# python_version: "3.8" +# wheel < +# name: "infra/python/wheels/pyyaml/${platform}_${py_python}_${py_abi}" +# version: "version:5.4.1.chromium.1" +# > +# [VPYTHON:END] + # Copyright (c) 2013, the Flutter project 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 argparse +import logging import os import sys +from subprocess import CompletedProcess +from typing import List + +# The import is coming from vpython wheel and pylint cannot find it. +import yaml # pylint: disable=import-error + # The imports are coming from fuchsia/test_scripts and pylint cannot find them # without setting a global init-hook which is less favorable. # But this file will be executed as part of the CI, its correctness of importing @@ -25,32 +41,89 @@ from run_executable_test import ExecutableTestRunner from test_runner import TestRunner -# TODO(https://github.com/flutter/flutter/issues/140179): Respect build -# configurations. -OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out/fuchsia_debug_x64') +if len(sys.argv) == 2: + VARIANT = sys.argv[1] + sys.argv.pop() +elif len(sys.argv) == 1: + VARIANT = 'fuchsia_debug_x64' +else: + assert False, 'Expect only one parameter as the compile output directory.' +OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out', VARIANT) -# TODO(https://github.com/flutter/flutter/issues/140179): Execute all the tests -# in -# https://github.com/flutter/engine/blob/main/testing/fuchsia/test_suites.yaml -# and avoid hardcoded paths. -def _get_test_runner(runner_args: argparse.Namespace, *_) -> TestRunner: - return ExecutableTestRunner( - OUT_DIR, [], - 'fuchsia-pkg://fuchsia.com/dart_runner_tests#meta/dart_runner_tests.cm', - runner_args.target_id, None, '/tmp/log', - [os.path.join(OUT_DIR, 'dart_runner_tests.far')], None +class BundledTestRunner(TestRunner): + + # private, use bundled_test_runner_of function instead. + def __init__( + self, target_id: str, package_deps: List[str], tests: List[str], + logs_dir: str + ): + super().__init__(OUT_DIR, [], None, target_id, package_deps) + self.tests = tests + self.logs_dir = logs_dir + + def run_test(self) -> CompletedProcess: + returncode = 0 + for test in self.tests: + # pylint: disable=protected-access + test_runner = ExecutableTestRunner( + OUT_DIR, [], test, self._target_id, None, self.logs_dir, [], None + ) + test_runner._package_deps = self._package_deps + result = test_runner.run_test().returncode + logging.info('Result of test %s is %s', test, result) + if result != 0: + returncode = result + return CompletedProcess(args='', returncode=returncode) + + +def bundled_test_runner_of(target_id: str) -> BundledTestRunner: + log_dir = os.environ.get('FLUTTER_LOGS_DIR', '/tmp/log') + with open(os.path.join(os.path.dirname(__file__), 'test_suites.yaml'), + 'r') as file: + tests = yaml.safe_load(file) + # TODO(zijiehe-google-com): Run tests with multiple packages or with extra + # test arguments, https://github.com/flutter/flutter/issues/140179. + tests = list( + filter( + lambda test: test['test_command'].startswith('test run ') and test[ + 'test_command'].endswith('.cm'), tests + ) + ) + tests = list( + filter( + lambda test: 'package' in test and test['package'].endswith('-0.far'), + tests + ) + ) + tests = list( + filter( + lambda test: not 'variant' in test or VARIANT == test['variant'], + tests + ) + ) + for test in tests: + original_package = test['package'] + test['package'] = os.path.join( + OUT_DIR, test['package'].replace('-0.far', '.far') + ) + try: + os.remove(test['package']) + except FileNotFoundError: + pass + os.symlink(original_package, test['package']) + return BundledTestRunner( + target_id, [test['package'] for test in tests], + [test['test_command'][len('test run '):] for test in tests], log_dir ) +def _get_test_runner(runner_args: argparse.Namespace, *_) -> TestRunner: + return bundled_test_runner_of(runner_args.target_id) + + if __name__ == '__main__': - try: - os.remove(os.path.join(OUT_DIR, 'dart_runner_tests.far')) - except FileNotFoundError: - pass - os.symlink( - 'dart_runner_tests-0.far', os.path.join(OUT_DIR, 'dart_runner_tests.far') - ) + logging.info('Running tests in %s', OUT_DIR) sys.argv.append('--out-dir=' + OUT_DIR) # The 'flutter-test-type' is a place holder and has no specific meaning; the # _get_test_runner is overrided. diff --git a/testing/fuchsia/test_suites.yaml b/testing/fuchsia/test_suites.yaml index c311fe2fc28dc..876a51ad8914f 100644 --- a/testing/fuchsia/test_suites.yaml +++ b/testing/fuchsia/test_suites.yaml @@ -19,16 +19,20 @@ package: flow_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/runtime_tests#meta/runtime_tests.cm package: runtime_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/shell_tests#meta/shell_tests.cm package: shell_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/testing_tests#meta/testing_tests.cm package: testing_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/txt_tests#meta/txt_tests.cm -- --gtest_filter=-ParagraphTest.* package: txt_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/ui_tests#meta/ui_tests.cm package: ui_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/embedder_tests#meta/embedder_tests.cm package: embedder_tests-0.far + variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/dart_utils_tests#meta/dart_utils_tests.cm package: dart_utils_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/dart-jit-runner-integration-test#meta/dart-jit-runner-integration-test.cm From 9938df6a8bd33cb3a44e87d4297c96488c27739b Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Tue, 6 Feb 2024 17:35:47 +0000 Subject: [PATCH 14/18] Reverts "[web] Fix Scene clip bounds. Trigger resize on DPR Change." (#50404) Reverts flutter/engine#50161 Initiated by: jonahwilliams Reason for reverting: This is causing what looks like bogus goldens on the framework -> engine roll: https://github.com/flutter/flutter/pull/142966 Original PR Author: ditman Reviewed By: {yjbanov, mdebbar} This change reverts the following previous change: Original Description: The Scene of the HTML renderer is passing incorrect size information to the engine, and when DPR<1, it can result in elements being culled off of the viewport. In addition to that, when an app is embedded, not all changes in DPR cause a Resize event (only those some of the dimensions fails by a rounding error!), so this PR ensures that all DPR events in embedded trigger a resize event. ### Issues Fixes https://github.com/flutter/flutter/issues/129182 ### Testing Looking good at: https://dit-astral-test.web.app [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- ci/licenses_golden/licenses_flutter | 2 - lib/web_ui/lib/src/engine.dart | 1 - lib/web_ui/lib/src/engine/html/scene.dart | 12 +-- .../custom_element_dimensions_provider.dart | 40 +++----- .../dimensions_provider.dart | 25 ++--- .../full_page_dimensions_provider.dart | 5 +- .../view_embedder/display_dpr_stream.dart | 93 ------------------- ...stom_element_dimensions_provider_test.dart | 34 ++----- .../dimensions_provider_test.dart | 14 +++ .../full_page_dimensions_provider_test.dart | 3 + .../display_dpr_stream_test.dart | 45 --------- 11 files changed, 48 insertions(+), 226 deletions(-) delete mode 100644 lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart delete mode 100644 lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 6419812c072a5..26dfb762b3930 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -6134,7 +6134,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart + ../../../f ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart + ../../../flutter/LICENSE @@ -8998,7 +8997,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart -FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 14bda2390fdcc..57c7b35fafb8a 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -190,7 +190,6 @@ export 'engine/vector_math.dart'; export 'engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart'; -export 'engine/view_embedder/display_dpr_stream.dart'; export 'engine/view_embedder/dom_manager.dart'; export 'engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart'; export 'engine/view_embedder/embedding_strategy/embedding_strategy.dart'; diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index b9f69996b1097..c9703038b6103 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -45,15 +45,9 @@ class PersistedScene extends PersistedContainerSurface { @override void recomputeTransformAndClip() { - // The scene clip is the size of the entire window **in Logical pixels**. - // - // Even though the majority of the engine uses `physicalSize`, there are some - // bits (like the HTML renderer, or dynamic view sizing) that are implemented - // using CSS, and CSS operates in logical pixels. - // - // See also: [EngineFlutterView.resize]. - final ui.Size bounds = window.physicalSize / window.devicePixelRatio; - localClipBounds = ui.Rect.fromLTRB(0, 0, bounds.width, bounds.height); + // The scene clip is the size of the entire window. + final ui.Size screen = window.physicalSize; + localClipBounds = ui.Rect.fromLTRB(0, 0, screen.width, screen.height); projectedClip = null; } diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart index 25f77afdd5038..7b39ca908e576 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart @@ -4,7 +4,6 @@ import 'dart:async'; -import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -13,36 +12,21 @@ import 'dimensions_provider.dart'; /// This class provides observable, real-time dimensions of a host element. /// -/// This class needs a `Stream` of `devicePixelRatio` changes, like the one -/// provided by [DisplayDprStream], because html resize observers do not report -/// DPR changes. -/// /// All the measurements returned from this class are potentially *expensive*, /// and should be cached as needed. Every call to every method on this class /// WILL perform actual DOM measurements. -/// -/// This broadcasts `null` size events, to match the implementation of the -/// FullPageDimensionsProvider, but it could broadcast the size coming from the -/// DomResizeObserverEntry. Further changes in the engine are required for this -/// to be effective. class CustomElementDimensionsProvider extends DimensionsProvider { /// Creates a [CustomElementDimensionsProvider] from a [_hostElement]. - CustomElementDimensionsProvider(this._hostElement, { - Stream? onDprChange, - }) { - // Send a resize event when the page DPR changes. - _dprChangeStreamSubscription = onDprChange?.listen((_) { - _broadcastSize(null); - }); - + CustomElementDimensionsProvider(this._hostElement) { // Hook up a resize observer on the hostElement (if supported!). _hostElementResizeObserver = createDomResizeObserver(( List entries, DomResizeObserver _, ) { - for (final DomResizeObserverEntry _ in entries) { - _broadcastSize(null); - } + entries + .map((DomResizeObserverEntry entry) => + ui.Size(entry.contentRect.width, entry.contentRect.height)) + .forEach(_broadcastSize); }); assert(() { @@ -61,12 +45,11 @@ class CustomElementDimensionsProvider extends DimensionsProvider { // Handle resize events late DomResizeObserver? _hostElementResizeObserver; - late StreamSubscription? _dprChangeStreamSubscription; - final StreamController _onResizeStreamController = - StreamController.broadcast(); + final StreamController _onResizeStreamController = + StreamController.broadcast(); // Broadcasts the last seen `Size`. - void _broadcastSize(ui.Size? size) { + void _broadcastSize(ui.Size size) { _onResizeStreamController.add(size); } @@ -75,17 +58,16 @@ class CustomElementDimensionsProvider extends DimensionsProvider { super.close(); _hostElementResizeObserver?.disconnect(); // ignore:unawaited_futures - _dprChangeStreamSubscription?.cancel(); - // ignore:unawaited_futures _onResizeStreamController.close(); } @override - Stream get onResize => _onResizeStreamController.stream; + Stream get onResize => _onResizeStreamController.stream; @override ui.Size computePhysicalSize() { - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); + return ui.Size( _hostElement.clientWidth * devicePixelRatio, _hostElement.clientHeight * devicePixelRatio, diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart index 469f3a197d25b..2fcf44834395e 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart @@ -5,11 +5,11 @@ import 'dart:async'; import 'package:meta/meta.dart'; -import 'package:ui/src/engine/dom.dart'; -import 'package:ui/src/engine/view_embedder/display_dpr_stream.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; +import '../../display.dart'; +import '../../dom.dart'; import 'custom_element_dimensions_provider.dart'; import 'full_page_dimensions_provider.dart'; @@ -32,15 +32,18 @@ abstract class DimensionsProvider { /// Creates the appropriate DimensionsProvider depending on the incoming [hostElement]. factory DimensionsProvider.create({DomElement? hostElement}) { if (hostElement != null) { - return CustomElementDimensionsProvider( - hostElement, - onDprChange: DisplayDprStream.instance.dprChanged, - ); + return CustomElementDimensionsProvider(hostElement); } else { return FullPageDimensionsProvider(); } } + /// Returns the DPI reported by the browser. + double getDevicePixelRatio() { + // This is overridable in tests. + return EngineFlutterDisplay.instance.devicePixelRatio; + } + /// Returns the [ui.Size] of the "viewport". /// /// This function is expensive. It triggers browser layout if there are @@ -54,16 +57,6 @@ abstract class DimensionsProvider { ); /// Returns a Stream with the changes to [ui.Size] (when cheap to get). - /// - /// Currently this Stream always returns `null` measurements because the - /// resize event that we use for [FullPageDimensionsProvider] does not contain - /// the new size, so users of this Stream everywhere immediately retrieve the - /// new `physicalSize` from the window. - /// - /// The [CustomElementDimensionsProvider] *could* broadcast the new size, but - /// to keep both implementations consistent (and their consumers), for now all - /// events from this Stream are going to be `null` (until we find a performant - /// way to retrieve the dimensions in full-page mode). Stream get onResize; /// Whether the [DimensionsProvider] instance has been closed or not. diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart index bcacba4c869f3..8221d95457aef 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'package:ui/src/engine/browser_detection.dart'; -import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -68,7 +67,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { late double windowInnerWidth; late double windowInnerHeight; final DomVisualViewport? viewport = domWindow.visualViewport; - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); if (viewport != null) { if (operatingSystem == OperatingSystem.iOs) { @@ -103,7 +102,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { double physicalHeight, bool isEditingOnMobile, ) { - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); final DomVisualViewport? viewport = domWindow.visualViewport; late double windowInnerHeight; diff --git a/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart b/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart deleted file mode 100644 index d73eeb5aec244..0000000000000 --- a/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2013 The Flutter 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 'dart:async'; -import 'dart:js_interop'; - -import 'package:meta/meta.dart'; -import 'package:ui/src/engine/display.dart'; -import 'package:ui/src/engine/dom.dart'; -import 'package:ui/ui.dart' as ui show Display; - -/// Provides a stream of `devicePixelRatio` changes for the given display. -/// -/// Note that until the Window Management API is generally available, this class -/// only monitors the global `devicePixelRatio` property, provided by the default -/// [EngineFlutterDisplay.instance]. -/// -/// See: https://developer.mozilla.org/en-US/docs/Web/API/Window_Management_API -class DisplayDprStream { - DisplayDprStream( - this._display, { - @visibleForTesting DebugDisplayDprStreamOverrides? overrides, - }) : _currentDpr = _display.devicePixelRatio, - _debugOverrides = overrides { - // Start listening to DPR changes. - _subscribeToMediaQuery(); - } - - /// A singleton instance of DisplayDprStream. - static DisplayDprStream instance = - DisplayDprStream(EngineFlutterDisplay.instance); - - // The display object that will provide the DPR information. - final ui.Display _display; - - // Last reported value of DPR. - double _currentDpr; - - // Controls the [dprChanged] broadcast Stream. - final StreamController _dprStreamController = - StreamController.broadcast(); - - // Object that fires a `change` event for the `_currentDpr`. - late DomEventTarget _dprMediaQuery; - - // Creates the media query for the latest known DPR value, and adds a change listener to it. - void _subscribeToMediaQuery() { - if (_debugOverrides?.getMediaQuery != null) { - _dprMediaQuery = _debugOverrides!.getMediaQuery!(_currentDpr); - } else { - _dprMediaQuery = domWindow.matchMedia('(resolution: ${_currentDpr}dppx)'); - } - _dprMediaQuery.addEventListenerWithOptions( - 'change', - createDomEventListener(_onDprMediaQueryChange), - { - // We only listen `once` because this event only triggers once when the - // DPR changes from `_currentDpr`. Once that happens, we need a new - // `_dprMediaQuery` that is watching the new `_currentDpr`. - // - // By using `once`, we don't need to worry about detaching the event - // listener from the old mediaQuery after we're done with it. - 'once': true, - 'passive': true, - }, - ); - } - - // Handler of the _dprMediaQuery 'change' event. - // - // This calls subscribe again because events are listened to with `once: true`. - JSVoid _onDprMediaQueryChange(DomEvent _) { - _currentDpr = _display.devicePixelRatio; - _dprStreamController.add(_currentDpr); - // Re-subscribe... - _subscribeToMediaQuery(); - } - - /// A stream that emits the latest value of [EngineFlutterDisplay.instance.devicePixelRatio]. - Stream get dprChanged => _dprStreamController.stream; - - // The overrides object that is used for testing. - final DebugDisplayDprStreamOverrides? _debugOverrides; -} - -@visibleForTesting -class DebugDisplayDprStreamOverrides { - DebugDisplayDprStreamOverrides({ - this.getMediaQuery, - }); - final DomEventTarget Function(double currentValue)? getMediaQuery; -} diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart index 4416aa59bc56c..9ee7a95083c05 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'dart:async'; import 'package:test/bootstrap/browser.dart'; @@ -106,48 +109,23 @@ void doTests() { }); test('funnels resize events on sizeSource', () async { - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.7); - sizeSource ..style.width = '100px' ..style.height = '100px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(270, 270)); + expect(await provider.onResize.first, const ui.Size(100, 100)); sizeSource ..style.width = '200px' ..style.height = '200px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(540, 540)); + expect(await provider.onResize.first, const ui.Size(200, 200)); sizeSource ..style.width = '300px' ..style.height = '300px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(810, 810)); - }); - - test('funnels DPR change events too', () async { - // Override the source of DPR events... - final StreamController dprController = - StreamController.broadcast(); - - // Inject the dprController stream into the CustomElementDimensionsProvider. - final CustomElementDimensionsProvider provider = - CustomElementDimensionsProvider( - sizeSource, - onDprChange: dprController.stream, - ); - - // Set and broadcast the mock DPR value - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(3.2); - dprController.add(3.2); - - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(32, 32)); + expect(await provider.onResize.first, const ui.Size(300, 300)); }); test('closed by onHotRestart', () async { diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart index 7c355f94fdd17..ba81aa32f01e3 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; @@ -28,4 +31,15 @@ void doTests() { expect(provider, isA()); }); }); + + group('getDevicePixelRatio', () { + test('Returns the correct pixelRatio', () async { + // Override the DPI to something known, but weird... + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(33930); + + final DimensionsProvider provider = DimensionsProvider.create(); + + expect(provider.getDevicePixelRatio(), 33930); + }); + }); } diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart index cd633da01526d..88a037c053e45 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'dart:async'; import 'package:test/bootstrap/browser.dart'; diff --git a/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart b/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart deleted file mode 100644 index a32ff78620fcf..0000000000000 --- a/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2013 The Flutter 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 'dart:async'; - -import 'package:test/bootstrap/browser.dart'; -import 'package:test/test.dart'; -import 'package:ui/src/engine.dart'; - -void main() { - internalBootstrapBrowserTest(() => doTests); -} - -void doTests() { - final DomEventTarget eventTarget = createDomElement('div'); - - group('dprChanged Stream', () { - late DisplayDprStream dprStream; - - setUp(() async { - dprStream = DisplayDprStream(EngineFlutterDisplay.instance, - overrides: DebugDisplayDprStreamOverrides( - getMediaQuery: (_) => eventTarget, - )); - }); - - test('funnels display DPR on every mediaQuery "change" event.', () async { - final Future> dprs = dprStream.dprChanged - .take(3) - .timeout(const Duration(seconds: 1)) - .toList(); - - // Simulate the events - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(6.9); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(4.2); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(0.71); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - - expect(await dprs, [6.9, 4.2, 0.71]); - }); - }); -} From 441a3c8ca8db657d9e4c0669c5398770141f838c Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 6 Feb 2024 10:23:06 -0800 Subject: [PATCH 15/18] Skip flaking test on Windows nobody is fixing. (#50401) Filed https://github.com/flutter/flutter/issues/142991 to re-enable. --- lib/web_ui/test/engine/pointer_binding_test.dart | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/test/engine/pointer_binding_test.dart b/lib/web_ui/test/engine/pointer_binding_test.dart index d877f9b74e378..e5a3c0f0cdc0a 100644 --- a/lib/web_ui/test/engine/pointer_binding_test.dart +++ b/lib/web_ui/test/engine/pointer_binding_test.dart @@ -2620,8 +2620,9 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) { void testWithSemantics( String description, - Future Function() body, - ) { + Future Function() body, { + Object? skip, + }) { test( description, () async { @@ -2631,6 +2632,7 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) { await body(); EngineSemantics.instance.semanticsEnabled = false; }, + skip: skip, ); } @@ -2913,8 +2915,8 @@ void _testClickDebouncer({required PointerBinding Function() getBinding}) { semanticsActions, isEmpty, ); - }); - + // TODO(yjbanov): https://github.com/flutter/flutter/issues/142991. + }, skip: operatingSystem == OperatingSystem.windows); testWithSemantics('Forwards click if enough time passed after the last flushed pointerup', () async { expect(EnginePlatformDispatcher.instance.semanticsEnabled, true); From 269ed9089fd919d243ee675eeee27732e7d2d623 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Tue, 6 Feb 2024 18:49:25 +0000 Subject: [PATCH 16/18] Reverts "Revert "Revert "[Fuchsia] Execute most of the testing/fuchsia/test_suites.yaml on debug and release builds""" (#50407) Reverts "Reland "[Fuchsia] Execute most of the testing/fuchsia/test_suites.yaml on debug and release builds""" Reverts flutter/engine#50295 Initiated by: zanderso Reason for reverting: Timing out on CI Original PR Author: zijiehe-google-com Reviewed By: {keyonghan} This change reverts the following previous change: Original Description: Reverts flutter/engine#50291, https://github.com/flutter/flutter/issues/142811 Following is the original change description. This change implements a BundledTestRunner to run most of the tests in testing/fuchsia/test_suites.yaml as ExecutableTestRunner. - Tests with packages out of out/fuchsia_*_x64/ are ignored for now. - Tests with extra test command line parameters are ignored for now. The BundledTestRunner can share most of the logic in ExecutableTestRunner and avoid reinventing the wheel. This change also fixes the build break of fuchsia_tests in fuchsia_release_x64 which allows tests to run on the build as well. - Tests not built with AOT are filtered out with variant field in test_suites.yaml. Bug: https://github.com/flutter/flutter/issues/140179 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .ci.yaml | 7 +- ci/builders/linux_fuchsia.json | 24 +--- .../fuchsia/dart_runner/embedder/BUILD.gn | 3 +- .../fuchsia/dart_runner/kernel/BUILD.gn | 3 +- .../fuchsia/dart_runner/vmservice/BUILD.gn | 2 +- .../platform/fuchsia/flutter/kernel/BUILD.gn | 3 +- testing/fuchsia/run_tests.py | 115 ++++-------------- testing/fuchsia/test_suites.yaml | 4 - 8 files changed, 33 insertions(+), 128 deletions(-) diff --git a/.ci.yaml b/.ci.yaml index fa5bd99a109b4..5b25a136b70d0 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -195,12 +195,7 @@ targets: - name: Linux linux_fuchsia recipe: engine_v2/engine_v2 - # Temporarily increase the timeout of this builder to 75 minutes to avoid - # being penetrated by cold goma / rbclient cache. - # TODO(zijiehe-google-com): Drop the timeout to 60 minutes once the release - # builder can finish within a reasonable time. - # https://github.com/flutter/flutter/issues/142932 - timeout: 75 + timeout: 60 properties: release_build: "true" config_name: linux_fuchsia diff --git a/ci/builders/linux_fuchsia.json b/ci/builders/linux_fuchsia.json index facbae0827b6c..011a13618aa11 100644 --- a/ci/builders/linux_fuchsia.json +++ b/ci/builders/linux_fuchsia.json @@ -115,12 +115,10 @@ { "drone_dimensions": [ "device_type=none", - "kvm=1", "os=Linux" ], "gclient_variables": { - "download_android_deps": false, - "run_fuchsia_emu": true + "download_android_deps": false }, "gn": [ "--fuchsia", @@ -133,22 +131,9 @@ "ninja": { "config": "fuchsia_release_x64", "targets": [ - "flutter/shell/platform/fuchsia:fuchsia", - "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", - "fuchsia_tests" + "flutter/shell/platform/fuchsia:fuchsia" ] - }, - "tests": [ - { - "name": "x64 emulator based release tests", - "language": "python3", - "script": "flutter/tools/fuchsia/with_envs.py", - "parameters": [ - "testing/fuchsia/run_tests.py", - "fuchsia_release_x64" - ] - } - ] + } }, { "drone_dimensions": [ @@ -173,7 +158,6 @@ "config": "fuchsia_debug_x64", "targets": [ "flutter/shell/platform/fuchsia:fuchsia", - "flutter/shell/platform/fuchsia/dart_runner:dart_runner_tests", "fuchsia_tests" ] }, @@ -192,7 +176,7 @@ ] }, { - "name": "x64 emulator based debug tests", + "name": "x64 emulator based tests", "language": "python3", "script": "flutter/tools/fuchsia/with_envs.py", "parameters": [ diff --git a/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn b/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn index 562191cbcf6b0..afd12c4b51be8 100644 --- a/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/embedder/BUILD.gn @@ -54,7 +54,8 @@ template("create_aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && (flutter_runtime_mode == "debug" || is_debug)) { + if (!invoker.product && + (!(flutter_runtime_mode == "profile") || is_debug)) { args += [ "--enable_asserts" ] } args += [ rebase_path(shim_kernel) ] diff --git a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn index c990742404703..af896f65e28b8 100644 --- a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn @@ -69,7 +69,8 @@ template("create_kernel_core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { + if (!invoker.product && + (is_debug || !(flutter_runtime_mode == "profile"))) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn b/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn index c299edb5f3f78..8aa7da550baab 100644 --- a/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn +++ b/shell/platform/fuchsia/dart_runner/vmservice/BUILD.gn @@ -65,7 +65,7 @@ template("aot_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!product && (flutter_runtime_mode == "debug" || is_debug)) { + if (!product && (!(flutter_runtime_mode == "profile") || is_debug)) { args += [ "--enable_asserts" ] } diff --git a/shell/platform/fuchsia/flutter/kernel/BUILD.gn b/shell/platform/fuchsia/flutter/kernel/BUILD.gn index dd77441bfd3cd..bc9853d5cd7f9 100644 --- a/shell/platform/fuchsia/flutter/kernel/BUILD.gn +++ b/shell/platform/fuchsia/flutter/kernel/BUILD.gn @@ -73,7 +73,8 @@ template("core_snapshot") { # No asserts in debug or release product. # No asserts in release with flutter_profile=true (non-product) # Yes asserts in non-product debug. - if (!invoker.product && (is_debug || flutter_runtime_mode == "debug")) { + if (!invoker.product && + (is_debug || !(flutter_runtime_mode == "profile"))) { args += [ "--enable_asserts" ] } args += [ rebase_path(platform_dill) ] diff --git a/testing/fuchsia/run_tests.py b/testing/fuchsia/run_tests.py index 40c66be2b96a1..2a301c828299c 100755 --- a/testing/fuchsia/run_tests.py +++ b/testing/fuchsia/run_tests.py @@ -1,28 +1,12 @@ -#!/usr/bin/env vpython3 - -# [VPYTHON:BEGIN] -# python_version: "3.8" -# wheel < -# name: "infra/python/wheels/pyyaml/${platform}_${py_python}_${py_abi}" -# version: "version:5.4.1.chromium.1" -# > -# [VPYTHON:END] - +#!/usr/bin/env python3 # Copyright (c) 2013, the Flutter project 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 argparse -import logging import os import sys -from subprocess import CompletedProcess -from typing import List - -# The import is coming from vpython wheel and pylint cannot find it. -import yaml # pylint: disable=import-error - # The imports are coming from fuchsia/test_scripts and pylint cannot find them # without setting a global init-hook which is less favorable. # But this file will be executed as part of the CI, its correctness of importing @@ -41,89 +25,32 @@ from run_executable_test import ExecutableTestRunner from test_runner import TestRunner -if len(sys.argv) == 2: - VARIANT = sys.argv[1] - sys.argv.pop() -elif len(sys.argv) == 1: - VARIANT = 'fuchsia_debug_x64' -else: - assert False, 'Expect only one parameter as the compile output directory.' -OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out', VARIANT) - - -class BundledTestRunner(TestRunner): - - # private, use bundled_test_runner_of function instead. - def __init__( - self, target_id: str, package_deps: List[str], tests: List[str], - logs_dir: str - ): - super().__init__(OUT_DIR, [], None, target_id, package_deps) - self.tests = tests - self.logs_dir = logs_dir - - def run_test(self) -> CompletedProcess: - returncode = 0 - for test in self.tests: - # pylint: disable=protected-access - test_runner = ExecutableTestRunner( - OUT_DIR, [], test, self._target_id, None, self.logs_dir, [], None - ) - test_runner._package_deps = self._package_deps - result = test_runner.run_test().returncode - logging.info('Result of test %s is %s', test, result) - if result != 0: - returncode = result - return CompletedProcess(args='', returncode=returncode) - - -def bundled_test_runner_of(target_id: str) -> BundledTestRunner: - log_dir = os.environ.get('FLUTTER_LOGS_DIR', '/tmp/log') - with open(os.path.join(os.path.dirname(__file__), 'test_suites.yaml'), - 'r') as file: - tests = yaml.safe_load(file) - # TODO(zijiehe-google-com): Run tests with multiple packages or with extra - # test arguments, https://github.com/flutter/flutter/issues/140179. - tests = list( - filter( - lambda test: test['test_command'].startswith('test run ') and test[ - 'test_command'].endswith('.cm'), tests - ) - ) - tests = list( - filter( - lambda test: 'package' in test and test['package'].endswith('-0.far'), - tests - ) - ) - tests = list( - filter( - lambda test: not 'variant' in test or VARIANT == test['variant'], - tests - ) - ) - for test in tests: - original_package = test['package'] - test['package'] = os.path.join( - OUT_DIR, test['package'].replace('-0.far', '.far') - ) - try: - os.remove(test['package']) - except FileNotFoundError: - pass - os.symlink(original_package, test['package']) - return BundledTestRunner( - target_id, [test['package'] for test in tests], - [test['test_command'][len('test run '):] for test in tests], log_dir - ) +# TODO(https://github.com/flutter/flutter/issues/140179): Respect build +# configurations. +OUT_DIR = os.path.join(DIR_SRC_ROOT, 'out/fuchsia_debug_x64') +# TODO(https://github.com/flutter/flutter/issues/140179): Execute all the tests +# in +# https://github.com/flutter/engine/blob/main/testing/fuchsia/test_suites.yaml +# and avoid hardcoded paths. def _get_test_runner(runner_args: argparse.Namespace, *_) -> TestRunner: - return bundled_test_runner_of(runner_args.target_id) + return ExecutableTestRunner( + OUT_DIR, [], + 'fuchsia-pkg://fuchsia.com/dart_runner_tests#meta/dart_runner_tests.cm', + runner_args.target_id, None, '/tmp/log', + [os.path.join(OUT_DIR, 'dart_runner_tests.far')], None + ) if __name__ == '__main__': - logging.info('Running tests in %s', OUT_DIR) + try: + os.remove(os.path.join(OUT_DIR, 'dart_runner_tests.far')) + except FileNotFoundError: + pass + os.symlink( + 'dart_runner_tests-0.far', os.path.join(OUT_DIR, 'dart_runner_tests.far') + ) sys.argv.append('--out-dir=' + OUT_DIR) # The 'flutter-test-type' is a place holder and has no specific meaning; the # _get_test_runner is overrided. diff --git a/testing/fuchsia/test_suites.yaml b/testing/fuchsia/test_suites.yaml index 876a51ad8914f..c311fe2fc28dc 100644 --- a/testing/fuchsia/test_suites.yaml +++ b/testing/fuchsia/test_suites.yaml @@ -19,20 +19,16 @@ package: flow_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/runtime_tests#meta/runtime_tests.cm package: runtime_tests-0.far - variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/shell_tests#meta/shell_tests.cm package: shell_tests-0.far - variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/testing_tests#meta/testing_tests.cm package: testing_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/txt_tests#meta/txt_tests.cm -- --gtest_filter=-ParagraphTest.* package: txt_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/ui_tests#meta/ui_tests.cm package: ui_tests-0.far - variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/embedder_tests#meta/embedder_tests.cm package: embedder_tests-0.far - variant: fuchsia_debug_x64 - test_command: test run fuchsia-pkg://fuchsia.com/dart_utils_tests#meta/dart_utils_tests.cm package: dart_utils_tests-0.far - test_command: test run fuchsia-pkg://fuchsia.com/dart-jit-runner-integration-test#meta/dart-jit-runner-integration-test.cm From b2ba57e547c47b414f37b5ca91a937e6e28a1cc9 Mon Sep 17 00:00:00 2001 From: keyonghan <54558023+keyonghan@users.noreply.github.com> Date: Tue, 6 Feb 2024 11:18:59 -0800 Subject: [PATCH 17/18] Add use_rbe to gclient variables for Framework Smoke Tests (#50403) This is to enable rbe support. Part of https://github.com/flutter/flutter/issues/142986 --- .ci.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.ci.yaml b/.ci.yaml index 5b25a136b70d0..45cbaacfe22de 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -218,6 +218,9 @@ targets: enabled_branches: - main timeout: 60 + properties: + gclient_variables: >- + {"use_rbe": true} - name: Linux linux_clang_tidy recipe: engine_v2/engine_v2 From 07cdaab7f5314b9bc19a5c42796b832ac25d2614 Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Tue, 6 Feb 2024 14:19:01 -0500 Subject: [PATCH 18/18] Roll Dart SDK from 29265c94a6e8 to 35269fa71956 (1 revision) (#50402) https://dart.googlesource.com/sdk.git/+log/29265c94a6e8..35269fa71956 2024-02-06 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.4.0-115.0.dev If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/dart-sdk-flutter-engine Please CC chinmaygarde@google.com,dart-vm-team@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter Engine: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPS b/DEPS index 29461086f96a6..7ff51b746a43b 100644 --- a/DEPS +++ b/DEPS @@ -62,7 +62,7 @@ vars = { # Dart is: https://github.com/dart-lang/sdk/blob/main/DEPS # You can use //tools/dart/create_updated_flutter_deps.py to produce # updated revision list of existing dependencies. - 'dart_revision': '29265c94a6e83e1a66015e76754a1b2a2540ea0c', + 'dart_revision': '35269fa71956e0c8eb919dda7176e0fb0ede7f1f', # WARNING: DO NOT EDIT MANUALLY # The lines between blank lines above and below are generated by a script. See create_updated_flutter_deps.py @@ -77,7 +77,7 @@ vars = { 'dart_perfetto_rev': '13ce0c9e13b0940d2476cd0cff2301708a9a2e2b', 'dart_protobuf_gn_rev': 'ca669f79945418f6229e4fef89b666b2a88cbb10', 'dart_protobuf_rev': 'a293fb9c866b1def3d3e7fffc5f6763a2ec59cc9', - 'dart_pub_rev': 'a3689f03168c896dd1cb0db8a60c568b38ee16bf', + 'dart_pub_rev': '4ab2e3663f0a98be40427e004e789caebf3ea72e', 'dart_tools_rev': 'f6e67f2223fd5f9c6bdb3ace908d22c73ea02bc5', 'dart_watcher_rev': '66cd694ffb7ee6e09ff0cde6c9f788aa47ee1a23', 'dart_webdev_rev': '2539d54bca7143a2dd1a1ec5d55fa38ca481639c',