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

Ensure vulkan subgroups are disabled for MoltenVK #67915

Merged

Conversation

RevoluPowered
Copy link
Contributor

We found they don't work on intel macbooks properly at all.

Possible future solutions:

  • update to moltenvk.
  • update to spirv may resolve it.

We found they don't work on intel macbooks properly at all.

Possible future solutions:
- update to moltenvk.
- update to spirv may resolve it.
@RevoluPowered RevoluPowered force-pushed the fix-intel-macbook-crash-subgroups branch from 53d105f to 381d457 Compare October 26, 2022 19:12
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good to me. In my mind this is more of stopgap until we can figure out why exactly subgroups are crashing on MacOS. But we should fix the crash now and then fix subgroups if/when we can

@clayjohn clayjohn requested a review from RandomShaper October 28, 2022 18:24
@RandomShaper
Copy link
Member

Yeah, let's keep an eye on this to be sure we do the right thing. Aside, the changes about unsigned correctness would belong in a separate commit/PR, but I wouldn't block merging on that. Just for next time.

@akien-mga
Copy link
Member

I'll merge for beta 4. For beta 5, I'll be updating all our Vulkan components to the latest Vulkan SDK released a week ago (including new MoltenVK and I've seen mentions of regression fixes), so please test again then and see if this is still needed.

@akien-mga akien-mga merged commit d147adc into godotengine:master Oct 31, 2022
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants