-
Notifications
You must be signed in to change notification settings - Fork 378
Implement SampleCmpBias and SampleCmpGrad #8931
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
Conversation
c89bffb to
2e5adbe
Compare
2e5adbe to
55e4727
Compare
-these are new for HLSL shader model 6.8 -also change glsl.meta.slang to use existing sampler typealias in hlsl.meta.slang fixes #8929
55e4727 to
ef9b27e
Compare
jkwak-work
left a comment
There was a problem hiding this 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.
But ... can you also address TODO-s in glsl.meta.slang that seems to be related?
There are more than one location in glsl.meta.slang with the following comment:
// TODO: Need to apply bias
Here is one of the examples,
slang/source/slang/glsl.meta.slang
Lines 2005 to 2006 in d9b92cc
| // TODO: Need to apply bias | |
| return sampler.SampleCmp(location, compareValue); |
When I was writing the GLSL functions, there was no way to apply bias in HLSL.
The functions in glsl.meta.slang should simply call the counter part functions in hlsl.meta.slang that can take bias value.
|
Thanks @jkwak-work, I filed #9038 for the GLSL work since GLSL is lower priority compared to my other tasks. |
| float SampleCmpGrad(vector<float, Shape.dimensions+isArray> location, float compareValue, vector<float, Shape.dimensions> gradX, vector<float, Shape.dimensions> gradY, constexpr vector<int, Shape.planeDimensions> offset) | ||
| { | ||
| static_assert(!(Shape is __ShapeCube), "SampleCmpGrad with offset is not supported for TextureCube or TextureCubeArray"); | ||
| static_assert(!(Shape is __ShapeCube && isArray != 0), "SampleCmpGrad is not supported for TextureCubeArray"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the second assert redundant? The first one will always fire first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll move the stricter assert first, I think both should be kept
gtong-nv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
these are new for HLSL shader model 6.8
fixes #8929