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

Conversation

@jiahaog
Copy link
Member

@jiahaog jiahaog commented Jul 15, 2020

Description

In release mode, it is possible for the app to start too quickly, which causes window.physicalSize to return Size.zero and trip the following assertion in the engine:

transform_layer.cc(20)] Check failed: transform_.isFinite().

Returning Size.zero is a documented behavior here

This occurs when createViewConfiguration is overiden (#2805 related), otherwise it falls back to the default window size of 800x600. As such, this PR makes ensureInitialized wait for a non-zero window.physicalSize before proceeding.

I'm not sure if this is the best solution, because:

  1. Breaking change to ensureInitialized. I'm also not sure if it's a convention for ensureInitialized to be async. The key thing we need to do is to set the view configuration correctly before handing control back to the user code, but this does not seem possible with the current sync APIs.
  2. Accessing dart:ui.window which is recommended to avoid, though this does fall into the exception where:

The only place that WidgetsBinding.instance.window is inappropriate is if a Window is required before invoking runApp(). In that case, it is acceptable (though unfortunate) to use this object statically.

Some other alternatives I can think of:

  • Introduce another method to wait for the physical size, but this seems more clunky to users.
  • Recommend users call setSurfaceSize instead and explicitly pass in the desired size
  • Don't use window.physicalSize and instead always use the defaults of 800x600

Related Issues

#2805
Maybe #36533
b/161208057

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

jiahaog added a commit to jiahaog/flutter-plugins that referenced this pull request Jul 15, 2020
…lutter#2875

In release mode, it is possible for the app to start too quickly, which causes `window.physicalSize` to return `Size.zero` and trip the following assertion in the engine:

```
transform_layer.cc(20)] Check failed: transform_.isFinite().
```

Returning `Size.zero` is a documented behavior [here](https://api.flutter.dev/flutter/dart-ui/Window/physicalSize.html)

This occurs when `createViewConfiguration` is overiden (flutter#2805 related), otherwise it falls back to the default window size of 800x600. As such, this PR makes `ensureInitialized` wait for a non-zero `window.physicalSize` before proceeding.

I'm not sure if this is the best solution, because:

1. Breaking change to `ensureInitialized`. I'm also not sure if it's a convention for `ensureInitialized` to be async. The key thing we need to do is to set the view configuration correctly before handing control back to the user code, but this does not seem possible with the current sync APIs.
2. Accessing dart:ui.window which is recommended to [avoid](https://api.flutter.dev/flutter/dart-ui/window.html), though this does fall into the exception where:

> The only place that `WidgetsBinding.instance.window` is inappropriate is if a `Window` is required before invoking `runApp()`. In that case, it is acceptable (though unfortunate) to use this object statically.

Some other alternatives I can think of:
- Introduce another method to wait for the physical size, but this seems more clunky to users.
- Recommend users call `setSurfaceSize` instead and explicitly pass in the desired size
- Don't use `window.physicalSize` and instead always use the defaults of 800x600
@jiahaog jiahaog force-pushed the e2e-release-mode branch from a0ce651 to 1723d19 Compare July 15, 2020 01:37
jiahaog added a commit to jiahaog/flutter-plugins that referenced this pull request Jul 15, 2020
…lutter#2875

In release mode, it is possible for the app to start too quickly, which causes `window.physicalSize` to return `Size.zero` and trip the following assertion in the engine:

```
transform_layer.cc(20)] Check failed: transform_.isFinite().
```

Returning `Size.zero` is a documented behavior [here](https://api.flutter.dev/flutter/dart-ui/Window/physicalSize.html)

This occurs when `createViewConfiguration` is overiden (flutter#2805 related), otherwise it falls back to the default window size of 800x600. As such, this PR makes `ensureInitialized` wait for a non-zero `window.physicalSize` before proceeding.

I'm not sure if this is the best solution, because:

1. Breaking change to `ensureInitialized`. I'm also not sure if it's a convention for `ensureInitialized` to be async. The key thing we need to do is to set the view configuration correctly before handing control back to the user code, but this does not seem possible with the current sync APIs.
2. Accessing dart:ui.window which is recommended to [avoid](https://api.flutter.dev/flutter/dart-ui/window.html), though this does fall into the exception where:

> The only place that `WidgetsBinding.instance.window` is inappropriate is if a `Window` is required before invoking `runApp()`. In that case, it is acceptable (though unfortunate) to use this object statically.

Some other alternatives I can think of:
- Introduce another method to wait for the physical size, but this seems more clunky to users.
- Recommend users call `setSurfaceSize` instead and explicitly pass in the desired size
- Don't use `window.physicalSize` and instead always use the defaults of 800x600
@jiahaog jiahaog force-pushed the e2e-release-mode branch from 1723d19 to e4df438 Compare July 15, 2020 01:37
@jiahaog jiahaog requested a review from dnfield July 15, 2020 01:38
@jiahaog
Copy link
Member Author

jiahaog commented Jul 15, 2020

@dnfield not sure if there are better ways to do this, let me know what you think!

…lutter#2875

In release mode, it is possible for the app to start too quickly, which causes `window.physicalSize` to return `Size.zero` and trip the following assertion in the engine:

```
transform_layer.cc(20)] Check failed: transform_.isFinite().
```

Returning `Size.zero` is a documented behavior [here](https://api.flutter.dev/flutter/dart-ui/Window/physicalSize.html)

This occurs when `createViewConfiguration` is overiden (flutter#2805 related), otherwise it falls back to the default window size of 800x600. As such, this PR makes `ensureInitialized` wait for a non-zero `window.physicalSize` before proceeding.

I'm not sure if this is the best solution, because:

1. Breaking change to `ensureInitialized`. I'm also not sure if it's a convention for `ensureInitialized` to be async. The key thing we need to do is to set the view configuration correctly before handing control back to the user code, but this does not seem possible with the current sync APIs.
2. Accessing dart:ui.window which is recommended to [avoid](https://api.flutter.dev/flutter/dart-ui/window.html), though this does fall into the exception where:

> The only place that `WidgetsBinding.instance.window` is inappropriate is if a `Window` is required before invoking `runApp()`. In that case, it is acceptable (though unfortunate) to use this object statically.

Some other alternatives I can think of:
- Introduce another method to wait for the physical size, but this seems more clunky to users.
- Recommend users call `setSurfaceSize` instead and explicitly pass in the desired size
- Don't use `window.physicalSize` and instead always use the defaults of 800x600
@jiahaog jiahaog force-pushed the e2e-release-mode branch from e4df438 to e6ad2dc Compare July 15, 2020 05:41
@jiahaog
Copy link
Member Author

jiahaog commented Jul 15, 2020

Upon further investigation this only happens when I try to build the engine from source, closing for now

@jiahaog jiahaog closed this Jul 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants