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

[MSL] textureGrad -> sample_compare translation doesn't work on macOS #796

Closed
tgjones opened this issue Dec 31, 2018 · 5 comments
Closed
Labels
bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on

Comments

@tgjones
Copy link

tgjones commented Dec 31, 2018

Shader Playground link

This GLSL:

float sample_depth_from_function(texture2DArray uT, samplerShadow uS)
{
    return textureGrad(
        sampler2DArrayShadow(uT, uS), 
        vec4(0.1, 0.2, 0, 0.5),
    	vec2(0, 0),
    	vec2(0, 0));
}

is currently converted (via glslang) to this MSL:

float sample_depth_from_function(thread const depth2d_array<float> uT, thread const sampler uS)
{
    return uT.sample_compare(uS, float4(0.1, 0.2, 0, 0.5).xy, uint(round(float4(0.1, 0.2, 0, 0.5).z)), float4(0.1, 0.2, 0, 0.5).w, gradient2d(float2(0.0), float2(0.0)));
}

This is correct for iOS. However, as noted in the Metal Shading Language Specification:

On macOS, for sample_compare functions, level is the only supported lod_options value (bias and gradient* are not supported), and lod must be a zero constant.

On macOS, changing the above MSL to the following makes it compile:

float sample_depth_from_function(thread const depth2d_array<float> uT, thread const sampler uS)
{
    return uT.sample_compare(uS, float4(0.1, 0.2, 0, 0.5).xy, uint(round(float4(0.1, 0.2, 0, 0.5).z)), float4(0.1, 0.2, 0, 0.5).w, level(0));
}

I don't know if there is precedent for compiling MSL differently for iOS and macOS?

(Of course, all this would probably be unnecessary if GLSL had a textureLod overload that takes a sampler2DArrayShadow...)

Edit: I was rather laser-focused on my particular usage for cascaded shadow mapping. The above “fix” for macOS of course only works when the gradient parameters are 0.

@billhollings
Copy link
Contributor

Sigh. Okay...looks like Apple introduced that restriction (at least documented it) in MSL 2.1.

Yes...SPRIV-Cross can emit different code for macOS and iOS...but in this case...do we want to?

Do we want to simply output a level(0) on macOS and have it silently render somewhat different than is requested by the shader...or do we let the compile error happen...and force the app to provide different shader code for the platform?

Thoughts, everyone?

@tgjones
Copy link
Author

tgjones commented Dec 31, 2018

It’s not quite the same thing, but a similar translation happens the other way, from SampleCmpLevelZero in HLSL to textureGrad in GLSL:

#207

@Themaister Themaister added needs triage Needs to be reproduced before it can become a different issue type. postponed-to-january labels Jan 1, 2019
@HansKristian-Work
Copy link
Contributor

So, there is a similar case for GLSL already. In HLSL, you can SampleCmpLevelZero for the Cascaded case. Unfortunately, there is no textureLod() for sampler2DArrayShadow, so I go the other way, detect a constant 0 LOD and turn that into a constant 0 gradient. We can do the opposite case for MSL. If the gradient is not possible to prove to be constant 0, we can bail. I don't think we should special case just iOS here unless proven to be vitally important for some obscure use case.

I'll get to it.

@HansKristian-Work HansKristian-Work added bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on and removed needs triage Needs to be reproduced before it can become a different issue type. postponed-to-january labels Jan 7, 2019
@HansKristian-Work
Copy link
Contributor

Was simple enough to just check for macOS vs iOS so implemented that as well. Should be a best-effort implementation for our purpose.

@tgjones
Copy link
Author

tgjones commented Jan 17, 2019

Awesome, thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Feature which should work in SPIRV-Cross does not for some reason. in progress Issue is being actively worked on
Projects
None yet
Development

No branches or pull requests

4 participants