Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Mar 4, 2025

Attempted fix for #163218

There are very few macOS devices with multiple GPUs and they were only brielfy sold. We seem to have a problem o the specific devices when choosing the dedicated GPU. INstead, lets try forcing the integrated GPU when its available.

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. a: desktop Running on desktop platform-macos labels Mar 4, 2025
@jonahwilliams jonahwilliams requested review from gaaclarke and knopp March 4, 2025 19:22
@loic-sharma
Copy link
Member

Not sure if you're aware, but on Windows we recently added an API so that users can set a GPU preference. If you add an API, ideally it should mirror the Windows API: #162490

@knopp
Copy link
Member

knopp commented Mar 4, 2025

Not sure if you're aware, but on Windows we recently added an API so that users can set a GPU preference. If you add an API, ideally it should mirror the Windows API: #162490

Yeah, the issue is that right now it crashes with the dedicated one :)

@jonahwilliams
Copy link
Contributor Author

@loic-sharma I think if they were still making macbooks with multiple GPUs we'd probably support via enum like with windows, but support for multiple GPUs was removed with the arm mac so it seems like a dead end.

@loic-sharma
Copy link
Member

Sounds good!

Copy link
Member

@knopp knopp left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonahwilliams jonahwilliams marked this pull request as ready for review March 4, 2025 20:12
Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

The code looks fine to me. I'm still fuzzy on the premise. We want this because we've written the code to require unified memory and all integrated graphics cards we care about have unified memory? I'm not sure if that's going to be the case.

id<MTLDevice> SelectMetalDevice() {
NSArray<id<MTLDevice>>* devices = MTLCopyAllDevices();
for (id<MTLDevice> device in devices) {
if (!device.isRemovable && device.isLowPower) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use hasUnifiedMemory perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams
Copy link
Contributor Author

We don't actually need unified memory AFAIK. I can run on a much older macbook (2015) which I think doesn't have unified memory and it works fine 🤷

@jonahwilliams
Copy link
Contributor Author

Let me double check on my 2015 Mac to see if it has unified memory.

@jonahwilliams
Copy link
Contributor Author

Okay so looks like the 2015 macbook also has unified memory. So the crash seems to be specific to using GPUs without unified memory, which means a bug when using Shared memory types. I think as long as it works, its probably what we should prefer.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Mar 5, 2025
Merged via the queue into flutter:master with commit ccf9922 Mar 5, 2025
175 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
@gaaclarke
Copy link
Member

So the crash seems to be specific to using GPUs without unified memory

Yay, glad to have gotten to the bottom of that. That was the most plausible explanation. It makes sense too since we have no test coverage from discrete gpus.

We should get this fixed since there are probably some android devices that don't have unified memory, like TVs, streaming boxes and cars.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 6, 2025
flutter/flutter@2578d97...321fbc0

2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)
2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)
2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)
2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)
2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)
2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)
2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)
2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)
2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)
2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)
2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694)
2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229)
2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)
2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635)
2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)
2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)
2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)
2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630)
2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)
2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)
2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)
2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)
2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)
2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569)
2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)
2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)
2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)
2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)
2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)

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 [email protected],[email protected] 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
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
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
flutter/flutter@2578d97...321fbc0

2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)
2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)
2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)
2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)
2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)
2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)
2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)
2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)
2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)
2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)
2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694)
2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229)
2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)
2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635)
2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)
2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)
2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)
2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630)
2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)
2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)
2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)
2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)
2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)
2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569)
2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)
2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)
2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)
2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)
2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)

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 [email protected],[email protected] 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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
flutter/flutter@2578d97...321fbc0

2025-03-06 [email protected] Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)
2025-03-06 [email protected] [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)
2025-03-06 [email protected] [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)
2025-03-06 [email protected] Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)
2025-03-06 [email protected] Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)
2025-03-06 [email protected] Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)
2025-03-06 [email protected] [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)
2025-03-05 [email protected] Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)
2025-03-05 [email protected] Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)
2025-03-05 [email protected] Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)
2025-03-05 [email protected] Adds animateToItem to the CarouselController (flutter/flutter#162694)
2025-03-05 [email protected] Cleanup content context (flutter/flutter#164229)
2025-03-05 [email protected] Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)
2025-03-05 [email protected] [Impeller] fix macOS managed memory. (flutter/flutter#164635)
2025-03-05 [email protected] [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)
2025-03-05 [email protected] Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)
2025-03-05 [email protected] Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)
2025-03-05 [email protected] Roll Packages from 9e4684e to abba683 (8 revisions) (flutter/flutter#164630)
2025-03-05 [email protected] Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)
2025-03-05 [email protected] Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)
2025-03-05 [email protected] Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)
2025-03-05 [email protected] Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)
2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)
2025-03-05 [email protected] [macos] prefer integrated GPU. (flutter/flutter#164569)
2025-03-05 [email protected] Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)
2025-03-05 [email protected] Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)
2025-03-05 [email protected] Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)
2025-03-05 [email protected] when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)
2025-03-05 [email protected] Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)

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 [email protected],[email protected] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop engine flutter/engine related. See also e: labels. platform-macos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants