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

Revert "Dynamic view sizing" #140165

Merged
merged 1 commit into from Dec 14, 2023
Merged

Conversation

chingjun
Copy link
Contributor

Reverts #138648

This caused the app to be stuck in the splash screen on certain phone models.

Context: b/316244317

@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Dec 14, 2023
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 14, 2023
@auto-submit auto-submit bot merged commit d24c01b into master Dec 14, 2023
70 checks passed
@auto-submit auto-submit bot deleted the revert-138648-dynamically-sized-views branch December 14, 2023 19:42
@jiahaog
Copy link
Member

jiahaog commented Dec 14, 2023

To add more context, the affected devices were able to work around the issue by changing the launch mode of the app (to use split screen or a "windowed" mode).

A wild guess of what is happening could be:

  • These devices may be running in a non-standard "full screen" mode by default
  • Dynamic view sizing #138648 causes the framework to incorrectly infer that the size of the view is zero for these devices
  • The engine doesn't produce a frame
  • On Android, the OS only takes down the splash screen when the preDrawListener returns true
  • The embedding is set up to only return true for the preDrawListener after the Flutter first frame
  • Since there are no frames, the splash screen doesn't get taken down.

@goderbauer
Copy link
Member

causes the framework to incorrectly infer that the size of the view is zero for these devices.

This is the one that baffles me. The PR is not really changing how the size is determined for environments where only one size is possible (and even with this change the Android embedding only allows one size). Anyways, looking forward to diving into this once we have the device.

@jiahaog Did this only reproduce with a specific app on those devices or is any Flutter app affected, i.e. can you repro this with the counter app?

@jiahaog
Copy link
Member

jiahaog commented Dec 15, 2023

Thanks to some folks on the affected team, they were able to reproduce it on a Redmi Note 5 pro with the internal minimal app (go/flutter-minimal), which is pretty much the counter app. After launch, It freezes on a white screen (presumably the splash screen for this app) and never shows the counter.

I don't know if it matters, but the minSdkVersion is 23 and the targetSdkVersion 34 for that app.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 15, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 15, 2023
flutter/flutter@a51e33a...2407f69

2023-12-15 58529443+srujzs@users.noreply.github.com Move package:web dependency to dev dependency (flutter/flutter#139696)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9524a185b055 to 986a6fe198dc (1 revision) (flutter/flutter#140221)
2023-12-15 engine-flutter-autoroll@skia.org Roll Packages from 1151191 to 3f2e16b (9 revisions) (flutter/flutter#140218)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7a50221733c2 to 9524a185b055 (1 revision) (flutter/flutter#140217)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 767223f7a4f8 to 7a50221733c2 (1 revision) (flutter/flutter#140216)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 91f65eea0c11 to 767223f7a4f8 (1 revision) (flutter/flutter#140210)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a47da28c9a62 to 91f65eea0c11 (1 revision) (flutter/flutter#140207)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from cde1a596432d to a47da28c9a62 (1 revision) (flutter/flutter#140204)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 46ff5c08a905 to cde1a596432d (1 revision) (flutter/flutter#140200)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a17bb0a63b7e to 46ff5c08a905 (1 revision) (flutter/flutter#140198)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4cb3ba7a85f6 to a17bb0a63b7e (1 revision) (flutter/flutter#140196)
2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 4cb3ba7a85f6 (14 revisions) (flutter/flutter#140195)
2023-12-15 polinach@google.com Increase versions of leak tracker libraries. (flutter/flutter#140018)
2023-12-15 magder@google.com Set compile test iOS app target version to not embed Swift runtime (flutter/flutter#140188)
2023-12-15 58190796+MitchellGoodwin@users.noreply.github.com Cupertino text clear label (flutter/flutter#129727)
2023-12-15 xilaizhang@google.com [github actions] use token from real user flutter mirror bot  (flutter/flutter#140191)
2023-12-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions)" (flutter/flutter#140194)
2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions) (flutter/flutter#140180)
2023-12-14 lsaudon@gmail.com feat: Add onTapAlwaysCalled in TextFormField (flutter/flutter#140089)
2023-12-14 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 3.1.3 to 4.0.0 (flutter/flutter#140177)
2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2140942444ea to 0e7248d43251 (2 revisions) (flutter/flutter#140176)
2023-12-14 ybz975218925@gmail.com fix reorderable_list drop animation (flutter/flutter#139362)
2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 997d3dfa1e74 to 2140942444ea (4 revisions) (flutter/flutter#140171)
2023-12-14 38147403+sharmashashi@users.noreply.github.com Fix BottomNavigationBarItem label overflow (flutter/flutter#120206)
2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from a565cea256c7 to 997d3dfa1e74 (2 revisions) (flutter/flutter#140170)
2023-12-14 chingjun@google.com Revert "Dynamic view sizing" (flutter/flutter#140165)
2023-12-14 aki.nishi.work@gmail.com �: fix cupertionActionSheet design (flutter/flutter#134345)
2023-12-14 104349824+huycozy@users.noreply.github.com Make improvements to existing new issue templates  (flutter/flutter#140142)
2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from caf33276468b to a565cea256c7 (1 revision) (flutter/flutter#140163)
2023-12-14 parlough@gmail.com Expand and update a few release.yml categories (flutter/flutter#140120)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: 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
goderbauer added a commit to goderbauer/flutter that referenced this pull request Dec 20, 2023
goderbauer added a commit to goderbauer/flutter that referenced this pull request Jan 3, 2024
Michal-MK pushed a commit to Michal-MK/flutter that referenced this pull request Jan 8, 2024
Reverts flutter#138648

This caused the app to be stuck in the splash screen on certain phone models.

Context: b/316244317
Markzipan pushed a commit to Markzipan/flutter that referenced this pull request Jan 9, 2024
Reverts flutter#138648

This caused the app to be stuck in the splash screen on certain phone models.

Context: b/316244317
goderbauer added a commit to goderbauer/flutter that referenced this pull request Jan 9, 2024
goderbauer added a commit that referenced this pull request Jan 9, 2024
This reverts commit
d24c01b.

The original change was reverted because it caused some apps to get
stuck on the splash screen on some phones.

An investigation determined that this was due to a rounding error.
Example: The device reports a physical size of 1008.0 x 2198.0 with a
dpr of 1.912500023841858. Flutter would translate that to a logical size
of 527.0588169589221 x 1149.2810314243163 and use that as the input for
its layout algorithm. Since the constraints here are tight, the layout
algorithm would determine that the resulting logical size of the root
render object must be 527.0588169589221 x 1149.2810314243163.
Translating this back to physical pixels by applying the dpr resulted in
a physical size of 1007.9999999999999 x 2198.0 for the frame. Android
now rejected that frame because it didn't match the expected size of
1008.0 x 2198.0 and since no frame had been rendered would never take
down the splash screen.

Prior to dynamically sized views, this wasn't an issue because we would
hard-code the frame size to whatever the requested size was.

Changes in this PR over the original PR:

* The issue has been fixed now by constraining the calculated physical
size to the input physical constraints which makes sure that we always
end up with a size that is acceptable to the operating system.
* The `ViewConfiguration` was refactored to use the slightly more
convenient `BoxConstraints` over the `ViewConstraints` to represent
constraints. Both essentially represent the same thing, but
`BoxConstraints` are more powerful and we avoid a couple of translations
between the two by translating the` ViewConstraints` from the
`FlutterView` to `BoxConstraints` directly when the `ViewConfiguration`
is created.

All changes over the original PR are contained in the second commit of
this PR.

Fixes b/316813075
Part of #134501.
Markzipan pushed a commit to Markzipan/flutter that referenced this pull request Jan 11, 2024
This reverts commit
flutter@d24c01b.

The original change was reverted because it caused some apps to get
stuck on the splash screen on some phones.

An investigation determined that this was due to a rounding error.
Example: The device reports a physical size of 1008.0 x 2198.0 with a
dpr of 1.912500023841858. Flutter would translate that to a logical size
of 527.0588169589221 x 1149.2810314243163 and use that as the input for
its layout algorithm. Since the constraints here are tight, the layout
algorithm would determine that the resulting logical size of the root
render object must be 527.0588169589221 x 1149.2810314243163.
Translating this back to physical pixels by applying the dpr resulted in
a physical size of 1007.9999999999999 x 2198.0 for the frame. Android
now rejected that frame because it didn't match the expected size of
1008.0 x 2198.0 and since no frame had been rendered would never take
down the splash screen.

Prior to dynamically sized views, this wasn't an issue because we would
hard-code the frame size to whatever the requested size was.

Changes in this PR over the original PR:

* The issue has been fixed now by constraining the calculated physical
size to the input physical constraints which makes sure that we always
end up with a size that is acceptable to the operating system.
* The `ViewConfiguration` was refactored to use the slightly more
convenient `BoxConstraints` over the `ViewConstraints` to represent
constraints. Both essentially represent the same thing, but
`BoxConstraints` are more powerful and we avoid a couple of translations
between the two by translating the` ViewConstraints` from the
`FlutterView` to `BoxConstraints` directly when the `ViewConfiguration`
is created.

All changes over the original PR are contained in the second commit of
this PR.

Fixes b/316813075
Part of flutter#134501.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants