Skip to content

Conversation

@inner-daemons
Copy link
Collaborator

@inner-daemons inner-daemons commented Oct 29, 2025

Connections
Closes #8594

Description

Many (most?) of the feature detection logic in the metal backend is completely incorrect. You usually need to check for a specific metal version, a feature family, or specific versions of different OSes. This is often not checked correctly. For example, I have seen checks that ignore the API version completely. Many checks also assume tvOS versions and iOS versions have the same feature support which isn't true.

This addresses that by adjusting a TON of feature checks. This is not exhaustive.

Testing
None

Squash or Rebase?

Squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@inner-daemons inner-daemons marked this pull request as ready for review October 30, 2025 02:28
@inner-daemons
Copy link
Collaborator Author

Due to my hatred of metal docs and the fact that they don't make any sense, and because it seems that feature set related docs have been removed, I'm not gonna look over most of the feature detection code, and just trust that it works well enough.

I will definitely take another few looks over all of this. Its kind of a nightmare to navigate, but I'll try to improve that a little and make sure that reviewers have to put in slightly less effort to follow what's going on.

} else {
version.at_least((11, 0), (10, 0), os_is_mac)
},
supports_memoryless_storage: metal4 || device.supports_family(MTLGPUFamily::Apple2),
Copy link
Contributor

@opstic opstic Oct 30, 2025

Choose a reason for hiding this comment

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

Are you sure about this logic? If a device under iOS 13 runs this, metal4 would be false since family_check fails, and then device.supports_family() would be called. Since this API only became available after iOS 13, it would crash the app. (Past example: #5744) Ditto for the other usages of metal3 and metal4.

Also can you please put the OS version check back since the A7 also supports memoryless storage and its last iOS version is 12.5.7, so family checks wouldn't cover it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Happy to, this is a rough sketch, I went over tons of stuff, I'm gonna look at it more deeply in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I messaged you about this on matrix since I still have concerns.

@inner-daemons inner-daemons marked this pull request as ready for review November 30, 2025 00:16
@inner-daemons inner-daemons added this to the v28 milestone Nov 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Metal feature detection is very broken

2 participants