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

Fix missing family check flag #5744

Merged
merged 2 commits into from
May 26, 2024

Conversation

VladasZ
Copy link
Contributor

@VladasZ VladasZ commented May 25, 2024

Description
iOS 12 doesn't have supports_family method and it crashes when this method is called. There was already a check for that using family_check flag. But it was missing in one place.

Xcode crash logs:

2024-05-25 20:52:31.043060+0300 TestGame[355:12589] [DYMTLInitPlatform] platform initialization successful
2024-05-25 20:52:31.154605+0300 TestGame[355:12422] Metal GPU Frame Capture Enabled
2024-05-25 20:52:31.157163+0300 TestGame[355:12422] Metal API Validation Enabled
2024-05-25 20:52:31.194143+0300 TestGame[355:12422] -[MTLDebugDevice supportsFamily:]: unrecognized selector sent to instance 0x102209200
2024-05-25 20:52:31.201718+0300 TestGame[355:12422] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[MTLDebugDevice supportsFamily:]: unrecognized selector sent to instance 0x102209200'

Testing
I ran it on my iPhone 5s and it didn't crash.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@VladasZ VladasZ changed the base branch from trunk to v0.20 May 25, 2024 17:57
@VladasZ VladasZ marked this pull request as ready for review May 25, 2024 18:11
@VladasZ VladasZ requested a review from a team as a code owner May 25, 2024 18:11
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Good catch!
Wondering though why this line here isn't an issue? https://github.com/gfx-rs/wgpu/blob/trunk/wgpu-hal/src/metal/adapter.rs#L515 It seems that in order to determine family_check we first do a family check..?

@Wumpf Wumpf merged commit 687180a into gfx-rs:v0.20 May 26, 2024
25 checks passed
@VladasZ
Copy link
Contributor Author

VladasZ commented May 26, 2024

That is weird. Documentation says that iOS 13.0+ is minimal supported version:
https://developer.apple.com/documentation/metal/mtldevice/3143473-supportsfamily .
Maybe actually the method does exist but MTLGPUFamily enum ends somewhere between Apple2 and `Apple4
on iOS 12?

version.major < 8 is false so supports_family is never called in this expression.

@VladasZ VladasZ deleted the fix-missing-family-check-flag branch May 26, 2024 10:05
@Friz64
Copy link

Friz64 commented May 29, 2024

This only landed in the v0.20 branch, was this intentional?

@Wumpf
Copy link
Member

Wumpf commented May 29, 2024

oh shoot, I didn't pay enough attention there. Thank you so much @Friz64 for pointing this out!

@VladasZ we usually first push to trunk and then cherry-pick back for main. As is, this fix would get lost with the next major release. Can you please create a PR against trunk?

(luckily this patch would be perfectly fine for backporting, so it's alright to release it with 0.20.1)

@VladasZ
Copy link
Contributor Author

VladasZ commented May 29, 2024

@Wumpf sorry. Didn't know that. I tried to use trunk at first but it had some breaking changes and I didn't want to use it. So I fixed it in v0.20.
Here is the new PR: #5754

morr0ne pushed a commit to verdiwm/wgpu that referenced this pull request Jul 28, 2024
* fix: missed family check flag

* fix: fmt and changelog
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.

3 participants