Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverts "[web] Fix Scene clip bounds. Trigger resize on DPR Change." #50404

Merged
merged 1 commit into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion lib/web_ui/lib/src/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
12 changes: 3 additions & 9 deletions lib/web_ui/lib/src/engine/html/scene.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<double>? 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<DomResizeObserverEntry> entries,
DomResizeObserver _,
) {
for (final DomResizeObserverEntry _ in entries) {
_broadcastSize(null);
}
entries
.map((DomResizeObserverEntry entry) =>
ui.Size(entry.contentRect.width, entry.contentRect.height))
.forEach(_broadcastSize);
});

assert(() {
Expand All @@ -61,12 +45,11 @@ class CustomElementDimensionsProvider extends DimensionsProvider {

// Handle resize events
late DomResizeObserver? _hostElementResizeObserver;
late StreamSubscription<double>? _dprChangeStreamSubscription;
final StreamController<ui.Size?> _onResizeStreamController =
StreamController<ui.Size?>.broadcast();
final StreamController<ui.Size> _onResizeStreamController =
StreamController<ui.Size>.broadcast();

// Broadcasts the last seen `Size`.
void _broadcastSize(ui.Size? size) {
void _broadcastSize(ui.Size size) {
_onResizeStreamController.add(size);
}

Expand All @@ -75,17 +58,16 @@ class CustomElementDimensionsProvider extends DimensionsProvider {
super.close();
_hostElementResizeObserver?.disconnect();
// ignore:unawaited_futures
_dprChangeStreamSubscription?.cancel();
// ignore:unawaited_futures
_onResizeStreamController.close();
}

@override
Stream<ui.Size?> get onResize => _onResizeStreamController.stream;
Stream<ui.Size> 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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
Expand All @@ -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<ui.Size?> get onResize;

/// Whether the [DimensionsProvider] instance has been closed or not.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
93 changes: 0 additions & 93 deletions lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<double> dprController =
StreamController<double>.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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -28,4 +31,15 @@ void doTests() {
expect(provider, isA<CustomElementDimensionsProvider>());
});
});

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);
});
});
}
Loading