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 CanvasOcclusionShaderRD format error with double precision build. #85822

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Dec 6, 2023

Fixes 2D shadows in the double precision build.

MoltenVK can't use 64-bit formats since Metal doesn't support it.

Fixes #70879
Fixes #78008

@bruvzg bruvzg added this to the 4.3 milestone Dec 6, 2023
@bruvzg bruvzg requested a review from a team as a code owner December 6, 2023 06:55
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 6, 2023
@sakrel
Copy link
Contributor

sakrel commented Dec 6, 2023

I'm surprised this is only causing issues on macOS, native Vulkan support for this format doesn't look great either:
image
https://vulkan.gpuinfo.org/listbufferformats.php

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.

As Sakrel pointed out, 64 bit vertex formats are not well supported on Vulkan devices. Additionally, they can lead to nasty crashes even on "supported" devices.

I would just remove the ability to use 64 bits here and make everything use the new codepath that you have added for Mac

@bruvzg
Copy link
Member Author

bruvzg commented Dec 6, 2023

I would just remove the ability to use 64 bits here and make everything use the new codepath that you have added for Mac

Done.

@bruvzg bruvzg changed the title [macOS] Fix CanvasOcclusionShaderRD format error with double precision build. Fix CanvasOcclusionShaderRD format error with double precision build. Dec 6, 2023
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 great! Thanks for the quick update

@YuriSizov YuriSizov merged commit 74b6fad into godotengine:master Dec 8, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@YuriSizov YuriSizov added the crash label Dec 8, 2023
@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 25, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

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