Sitelet https://github.com/flutter/flutter/pull/162785
Skip to content

replace deprecated [UIScreen mainScreen] in iOS - #162785

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
abaltatech:replace-mainscreen-issue130832-issue130825
Feb 19, 2025
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
abaltatech:replace-mainscreen-issue130832-issue130825

Conversation

@dkyurtov

@dkyurtov dkyurtov commented Feb 6, 2025

Copy link
Copy Markdown
Contributor
  1. Replaced mainScreen in FlutterPlatformViewsController with the screeScale of the screen that the flutterViewController is attached to. Updated API usage of FlutterClippingMaskViewPool and OverlayLayerPool by providing the correct screenScale from the screen that the flutterViewController is attached to.
  2. Replaced mainScreen in OverlayLayerPool by adding an API for the screenScale to be provided by the class user.
  3. Replaced mainScreen in FlutterClippingMaskViewPool by adding an API for the screenScale to be provided by the class user.
  4. Replaced mainScreen in FlutterPlatformViewsTest by adding a mocked FlutterViewController which has a mocked screen with a mocked screenScale. The flutterViewController is then provided the to the flutterPlatformViewsController.

All of the above changes allow platform views and their overlays displayed on an external screen to be properly scaled when rendered.

Fixes #130832. I believe this fixed #130825 since FlutterPlatformViews_Internal.mm does not exist but the definition of the interfaces declared in FlutterPlatformViews_Internal.h are part of the files modified by this pr. This pr also replaces mainScreen in overlay_layer_pool.mm.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla

google-cla Bot commented Feb 6, 2025

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions Bot added platform-ios iOS applications specifically engine flutter/engine related. See also e: labels. labels Feb 6, 2025
	- Replaced mainScreen in FlutterPlatformViewsController with the
	  screeScale of the screen that the flutterViewController is
attached to. Updated API usage of FlutterClippingMaskViewPool and
OverlayLayerPool by providing the correct screenScale from the screen
that the flutterViewController is attached to.
	- Replaced mainScreen in OverlayLayerPool by adding an API for
	  the screenScale to be provided by the class user.
	- Replaced mainScreen in FlutterClippingMaskViewPool by adding an API for
          the screenScale to be provided by the class user.
	- Replaced mainScreen FlutterPlatformViewsTest by adding a
	  mocked FlutterViewController which has a mocked screen with a mocked screenScale. The viewController is then provided the to the flutterPlatformViewsController.
@dkyurtov
dkyurtov force-pushed the replace-mainscreen-issue130832-issue130825 branch from 932fd68 to 5496c4a Compare February 11, 2025 13:43
@github-actions github-actions Bot added the team-ios Owned by iOS platform team label Feb 11, 2025

@jmagman jmagman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

@jmagman jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 14, 2025
@dkyurtov

Copy link
Copy Markdown
Contributor Author

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

@jmagman Glad to contribute! We did not choose to tackle this issue, it chose us. We were experiencing issues with platform view scaling on a secondary display. Only after resolving it did we find that there is an open issue for that.

@hellohuanlin

Copy link
Copy Markdown
Contributor

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

@jmagman Glad to contribute! We did not choose to tackle this issue, it chose us. We were experiencing issues with platform view scaling on a secondary display. Only after resolving it did we find that there is an open issue for that.

Oh I didn't know this actually fixes a bug. That's nice.

@jmagman

jmagman commented Feb 18, 2025 •

Copy link
Copy Markdown
Member

@dkyurtov Sorry you're hitting a tricky infrastructure issue which is why "Merge Queue Guard" is stuck. Could you rebase/merge master and push again? Or give me permission to https://github.com/abaltatech/flutter/tree/replace-mainscreen-issue130832-issue130825 and I can do it?

@dkyurtov

Copy link
Copy Markdown
Contributor Author

@dkyurtov Sorry you're hitting a tricky infrastructure issue which is why "Merge Queue Guard" is stuck. Could you rebase/merge master and push again? Or give me permission to https://github.com/abaltatech/flutter/tree/replace-mainscreen-issue130832-issue130825 and I can do it?

@jmagman No worries, I merged the master. Let me know if anything else is needed.

@flutter-dashboard

Copy link
Copy Markdown

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #162785 at sha 501ddb2

@jmagman

jmagman commented Feb 19, 2025

Copy link
Copy Markdown
Member

@dkyurtov if you're further interested in this, I just spotted #134062.

Merged via the queue into flutter:master with commit 3d2ab87 Feb 19, 2025
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 19, 2025
@dkyurtov

Copy link
Copy Markdown
Contributor Author

@dkyurtov if you're further interested in this, I just spotted #134062.

@jmagman Okay, thanks for looking out, I will take a look.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025
hellohuanlin added a commit to hellohuanlin/flutter that referenced this pull request Mar 27, 2025
hellohuanlin added a commit to hellohuanlin/flutter that referenced this pull request Mar 28, 2025
github-merge-queue Bot pushed a commit that referenced this pull request Mar 28, 2025
This PR fully reverts #162785

Due to crash from an internal customer

Fixes #165648


## 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], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Jenn Magder <magder@google.com>
@jmagman

jmagman commented Mar 28, 2025

Copy link
Copy Markdown
Member

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648.
If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

hellohuanlin added a commit to hellohuanlin/flutter that referenced this pull request Mar 28, 2025
…r#166080)

This PR fully reverts flutter#162785

Due to crash from an internal customer

Fixes flutter#165648


## 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], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Jenn Magder <magder@google.com>
@dkyurtov

Copy link
Copy Markdown
Contributor Author

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.

If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

@hellohuanlin

Copy link
Copy Markdown
Contributor

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.

If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

The crash happens internally for some customers, and I wasn't able to reproduce it. What I know is there are at least 2 divide-by-zero, both inside platform view logic.

Strawman idea: maybe cache the screen scale since it usually don't change? (unless in rare case when you switch to external monitor).

@dkyurtov

dkyurtov commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.
If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

The crash happens internally for some customers, and I wasn't able to reproduce it. What I know is there are at least 2 divide-by-zero, both inside platform view logic.

Strawman idea: maybe cache the screen scale since it usually don't change? (unless in rare case when you switch to external monitor).

Okay, thank you for the additional information. I would assume that the crash happened when flutterScreenIfViewLoaded returned nil. I will investigate when does that happen and how to handle this case. I will think about your idea but my initial thoughts are that it will problematic for the reasons that you mentioned in the brackets.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
zhangyuang pushed a commit to zhangyuang/flutter-fork that referenced this pull request Jun 9, 2025
…r#166080)

This PR fully reverts flutter#162785

Due to crash from an internal customer

Fixes flutter#165648


## 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], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Jenn Magder <magder@google.com>
romanejaquez pushed a commit to romanejaquez/flutter that referenced this pull request Aug 14, 2025
…r#166080)

This PR fully reverts flutter#162785

Due to crash from an internal customer

Fixes flutter#165648


## 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], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Jenn Magder <magder@google.com>
pblazej added a commit to webrtc-sdk/webrtc that referenced this pull request May 11, 2026
Drop `[UIScreen mainScreen] / [NSScreen mainScreen]` from
`currentScaleFactor`.

Still requires some testing, especially in multiscreen scenarios.

### References

-
[livekit/client-sdk-swift#998](livekit/client-sdk-swift#998)
— App Store rejection on iOS 26 citing deprecated UIKit APIs in
`LiveKitWebRTC.framework`.
- [Apple Developer Forums — `UITraitCollection.displayScale` is the
preferred replacement](https://developer.apple.com/forums/thread/733516)
(Apple Frameworks Engineer).
- [Apple —
`UITraitCollection.displayScale`](https://developer.apple.com/documentation/uikit/uitraitcollection/1623519-displayscale)
- [Apple Metal Best Practices: Native Screen
Scale](https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/NativeScreenScale.html)
— future TODO to switch to `nativeScale` for drawable sizing.

Prior art in other SDKs:

-
[airbnb/MagazineLayout#112](airbnb/MagazineLayout#112)
— replaced `UIScreen.main` with `traitCollection.displayScale` directly.
-
[jellyfin/Swiftfin#1809](jellyfin/Swiftfin#1809)
— same migration.
-
[flutter/flutter#162785](flutter/flutter#162785)
— chose `view.window.windowScene.screen.scale`, [reverted in
#166080](flutter/flutter#166080) after a
downstream `scale == 0` crash (informed the `MAX(scale, 1.0)` safety net
here).
-
[react-native-community#493](react-native-community/discussions-and-proposals#493)
— community discussion of the same migration.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cloudwebrtc pushed a commit to webrtc-sdk/webrtc that referenced this pull request Jul 16, 2026
Drop `[UIScreen mainScreen] / [NSScreen mainScreen]` from
`currentScaleFactor`.

Still requires some testing, especially in multiscreen scenarios.

### References

-
[livekit/client-sdk-swift#998](livekit/client-sdk-swift#998)
— App Store rejection on iOS 26 citing deprecated UIKit APIs in
`LiveKitWebRTC.framework`.
- [Apple Developer Forums — `UITraitCollection.displayScale` is the
preferred replacement](https://developer.apple.com/forums/thread/733516)
(Apple Frameworks Engineer).
- [Apple —
`UITraitCollection.displayScale`](https://developer.apple.com/documentation/uikit/uitraitcollection/1623519-displayscale)
- [Apple Metal Best Practices: Native Screen
Scale](https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/NativeScreenScale.html)
— future TODO to switch to `nativeScale` for drawable sizing.

Prior art in other SDKs:

-
[airbnb/MagazineLayout#112](airbnb/MagazineLayout#112)
— replaced `UIScreen.main` with `traitCollection.displayScale` directly.
-
[jellyfin/Swiftfin#1809](jellyfin/Swiftfin#1809)
— same migration.
-
[flutter/flutter#162785](flutter/flutter#162785)
— chose `view.window.windowScene.screen.scale`, [reverted in
#166080](flutter/flutter#166080) after a
downstream `scale == 0` crash (informed the `MAX(scale, 1.0)` safety net
here).
-
[react-native-community#493](react-native-community/discussions-and-proposals#493)
— community discussion of the same migration.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-ios iOS applications specifically team-ios Owned by iOS platform team will affect goldens Changes to golden files

Projects

None yet

3 participants