Skip to content

Added METAL platform, Metal Shader Converter output and Windows host support - #25

Merged
dzhdanNV merged 6 commits into
NVIDIA-RTX:mainfrom
selimsandal:main
Oct 1, 2026
Merged

Added METAL platform, Metal Shader Converter output and Windows host support#25
dzhdanNV merged 6 commits into
NVIDIA-RTX:mainfrom
selimsandal:main

Conversation

@selimsandal

@selimsandal selimsandal commented Sep 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  1. Fixed a macOS build error: libc++ can use a 128-bit fs::file_time_type rep.
  2. -p METAL compiles MSL into .metallib. New options: --metalSdk, --metalStd, --metalMinOS.
  3. -p METAL --metalFromDXIL compiles HLSL with DXC, converts it with Metal Shader Converter and writes a .metalbundle (metallib + reflection JSON, read with ShaderMake::ParseMetalConverterBundle). Converter options go through --metalShaderConverterOptions.
  4. Windows hosts use Metal Developer Tools for Windows and Metal Shader Converter for Windows.
  5. The SHADERMAKE_TOOL ExternalProject is always built (a no-op without changes), so pin bumps no longer leave a stale binary.

@selimsandal selimsandal changed the title Fix filesystem timestamp signatures with 128-bit clocks Add metal support & Fix filesystem timestamp signatures with 128-bit clocks Sep 27, 2026
@selimsandal
selimsandal force-pushed the main branch 2 times, most recently from 42181e9 to b96586d Compare September 28, 2026 22:56
@dzhdanNV

dzhdanNV commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Hey Selim. Is this PR ready for a review or are you still working on it?

@selimsandal

selimsandal commented Sep 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@dzhdanNV Hey! I think this one's ready. I want to merge this as part of the Metal 4 backend for NRI I'm working on.

One question for this PR, --metalFromDXIL hardcodes some NRI specifics (container layout, framebuffer fetch space 998). Is that ok in ShaderMake or should I split that commit out?

Also for the rest of the Metal 4 work here are my forks, I haven't opened PRs yet but it would be great if you can take a look (or if an issue in NRI repo is better that would work for me too for discussion)

@dzhdanNV

dzhdanNV commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

I want to merge this as part of the Metal 4 backend for NRI I'm working on.

\O/

One question for this PR, --metalFromDXIL hardcodes some NRI specifics (container layout, framebuffer fetch space 998). Is that ok in ShaderMake or should I split that commit out?

It's a bit concerning, but not a blocker. Try to invent a way to avoid hardcoded stuff. Please, provide some details.

I think this one's ready.

Let's get rid of hard coded stuff if we can first

Also for the rest of the Metal 4 work here are my forks, I haven't opened PRs yet but it would be great if you can take a look (or if an issue in NRI repo is better that would work for me too for discussion)

I have been working on many things simultaneously. I can't jump in right now. But feel free to start with opening an NRI PR first. I have a swarm of AI buddies, who will be happy to review, suggest fixes or fix if necessary.

@selimsandal

selimsandal commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

I'll make the ShaderMake path generic and let you know. Thanks!

Will open the PR on NRI too.

@selimsandal selimsandal changed the title Add metal support & Fix filesystem timestamp signatures with 128-bit clocks Added METAL platform, Metal Shader Converter output and Windows host support Sep 29, 2026
@selimsandal

selimsandal commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

@dzhdanNV pr ready to review now, removed the hardcoding, updated pr message

@selimsandal
Fix filesystem timestamp signatures with 128-bit clocks
3d9a7ea
@selimsandal
Added METAL platform
477abc9 
'-p METAL' compiles Metal Shading Language sources into '.metallib' libraries
using 'xcrun -sdk <sdk> metal' (macOS only). New options: '--metalSdk',
'--metalStd' and '--metalMinOS'; they and the resolved Metal compiler are part
of the build signature.
@selimsandal
Added METAL output from DXIL via Metal Shader Converter
6cce0bf 
'-p METAL --metalFromDXIL' compiles HLSL with DXC (same options as DXIL),
converts it with 'metal-shaderconverter' and writes Metal converter bundles
('.metalbundle': metallib and reflection JSON, see 'ShaderBlob.h'), usable
without the converter at runtime (e.g. on iOS). New options:
'--metalShaderConverter' and '--metalShaderConverterOptions' (also per config
line), passed to the converter as is. The deployment OS and version come from
'--metalSdk' and '--metalMinOS'. The converter, its library, the options and
contents of files referenced by them are part of the build signature.
@selimsandal
Added Windows host support for METAL platform
8df9541
@selimsandal
Fixed stale ShaderMake binary with SHADERMAKE_TOOL
4e748aa 
- the ExternalProject is always built, so source changes (e.g. after bumping a pinned revision) rebuild the tool
@selimsandal

selimsandal commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

synced to upstream

@dzhdanNV ChatGPT Codex Connector

dzhdanNV commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Read-only review of head 4e748aa: NoGo for now. I found three issues to address before merging:

  1. Windows Metal compiler discovery: FindMetalTool only probes %PROGRAMFILES%\Metal Developer Tools\bin\metal.exe (then PATH). Installations using %PROGRAMFILES%\Metal Developer Tools\macos\bin\metal.exe—the layout used by Dawn and The Forge—will fail the advertised default lookup unless the caller supplies --compiler or modifies PATH. Please probe the installed toolchain layout, including the target-specific location where applicable.

  2. Incremental dependency tracking with --metalFromDXIL: GetHierarchicalUpdateTime now skips every unresolved #include <...> whenever the platform is METAL. This also covers the DXC conversion path. For example, an HLSL header found through -I in --compilerOptions is invisible to ShaderMake's includeDirs lookup; editing it can leave an existing .metalbundle unchanged. Please limit the exemption to actual Metal toolchain headers, or otherwise track compiler-resolved includes.

  3. Bundle validation: ParseMetalConverterBundle checks bounds and the JSON terminator, but accepts offsets that overlap the header or each other. A header with metallibOffset = 0 can pass and return the bundle header as metallib data. Please require the metallib to start after the header and the reflection to start after the metallib, with non-overlapping ranges.

The current Build workflow run is action_required with no jobs, and the workflow has no macOS Metal job. A small Metal smoke test on the intended host/toolchain paths would help verify the fixes.

@dzhdanNV

dzhdanNV commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Feel free to fix what you can. The rest I will handle myself.

@selimsandal
Addressed review feedback
247dd7d 
- Windows: "metal.exe" is also looked up in the per-target "Metal Developer Tools\macos\bin" or "ios\bin" (selected by "--metalSdk"), after "bin" and before PATH
- METAL: only "<metal_*>" and "<simd/...>" includes are exempt from dependency tracking and only for native MSL, HLSL with "--metalFromDXIL" is tracked like DXIL
- ParseMetalConverterBundle: the metallib must follow the header and the reflection must follow the metallib, without overlaps, the metallib must be non-empty
- CI: added a macOS build with a METAL smoke test (macosx, iphoneos, iphonesimulator)
@selimsandal

selimsandal commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

added macos ci, addressed other feedback too

@dzhdanNV

dzhdanNV commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Thanks. That's a lot. Let's merge!

@dzhdanNV
dzhdanNV merged commit 9c978c3 into NVIDIA-RTX:main Oct 1, 2026
1 check failed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 1, 2026
@dzhdanNV

dzhdanNV commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Wow, thanks for auto-build on MacOS!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@selimsandal @dzhdanNV