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

Use MSL directly for SDL_GPU #74

Merged
merged 4 commits into from
Nov 3, 2024
Merged

Conversation

TheSpydog
Copy link
Contributor

@TheSpydog TheSpydog commented Nov 2, 2024

This is a quickly-slapped-together proof of concept for using MojoShader's MSL emitter for the SDLGPU driver rather than going through SPIRV -> ShaderCross. The code organization and OS detection logic (particularly that it only checks for Mac and not iOS/tvOS) could be massively improved, so marking this as a draft for now.

I also removed ShaderCross references entirely from mojoshader_sdlgpu but that's definitely overkill. We'll need it for at least D3D12, but we could dynamically load it instead via SDL_LoadObject.

The only change on the emitter side was changing the hardcoded uniform buffer index 16 to 0 to match SDL's binding order requirements.

@flibitijibibo
Copy link
Collaborator

Honestly I wouldn't say this is overkill necessarily; we were planning on skipping D3D12 until the memory manager is in, and since we're strapped for resources we might actually get Shader Model 7 first anyway... precompiled blobs won't be affected either, so unless someone does all that extra D3D work we won't be missing out on anything.

@TheSpydog TheSpydog marked this pull request as ready for review November 3, 2024 02:01
@TheSpydog
Copy link
Contributor Author

Fair points, and in the event that we do set up D3D12's memory manager in the near-ish future, we'll still need to rework shadercross integration anyway since this was still using the old single-header version. (Which is now deleted by this PR!)

I think this is ready for review now. Wasn't totally sure how to select the shader format so I went with preprocessor checks, since it's only Apple platforms that need special handling.

Copy link
Collaborator

@flibitijibibo flibitijibibo left a comment

Choose a reason for hiding this comment

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

Overall idea makes sense to me - let's test this with Flotilla, as long as that one's happy we're good to go.

mojoshader_sdlgpu.c Outdated Show resolved Hide resolved
mojoshader_sdlgpu.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@flibitijibibo flibitijibibo 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, if someone can make a macOS x86_64 dylib for FNA3D with this patched in I can test locally.

@flibitijibibo flibitijibibo merged commit 32ae391 into icculus:main Nov 3, 2024
3 checks passed
@TheSpydog TheSpydog deleted the sdlgpu_msl branch November 4, 2024 00:17
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.

2 participants