diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2018-11-19 09:08:26 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2018-11-19 09:08:26 -0800 |
| commit | 1f4b49596f710e8009d1cc996574049c4afc3627 (patch) | |
| tree | 873cded8658a74227349082eef9cc24656125e10 | |
| parent | 7e0c5ad677e04be4ecd2ce087d856ba26f1667a4 (diff) | |
Fix declaration of RWTexture*.Load() operations (#722)
The `Texture2D.Load()` operation takes a `uint3` of coordinates, with the `.xy` part holding the texel coordinates and the `.z` part holding a mip level.
In contrast, the `RWTexture2D.Load()` operation only takes a `uint2`.
This isn't clearly specified on MSDN, so Slang failed to get the declaration right.
This change fixes it so that we only add the extra coordinate for the mip level on non-multisample, read-only texture types (the previous code only checked for multisample-ness).
I also changed the logic that outputs swizzles to extract the coordinates and mip level so that it only applies when there is a mip level being passed through (this code should never actually be applied, though, because we shouldn't be generating `texelFetch` for RW texures anyway...).
One final change that sneask in here is making the `offset` parameter for one of the load-with-offset cases correctly use the base coordinate count for the texture type (e.g., 2D even for `Texture2DArray`). That is an unrelated fix, but I thought I'd sneak it in here rather than do a separate PR.
| -rw-r--r-- | source/slang/core.meta.slang | 43 | ||||
| -rw-r--r-- | source/slang/core.meta.slang.h | 43 | ||||
| -rw-r--r-- | tests/hlsl/simple/rw-texture.hlsl | 34 |
3 files changed, 112 insertions, 8 deletions
diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang index e7c424c33..14254e99e 100644 --- a/source/slang/core.meta.slang +++ b/source/slang/core.meta.slang @@ -671,7 +671,17 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) if( kBaseTextureTypes[tt].coordCount + isArray < 4 ) { - int loadCoordCount = kBaseTextureTypes[tt].coordCount + isArray + (isMultisample?0:1); + // The `Load()` operation on an ordinary `Texture2D` takes + // an `int3` for the location, where `.xy` holds the texel + // coordinates, and `.z` holds the mip level to use. + // + // The third coordinate for mip level is absent in + // `Texure2DMS.Load()` and `RWTexture2D.Load`. This pattern + // is repreated for all the other texture shapes. + // + bool needsMipLevel = !isMultisample && (access == SLANG_RESOURCE_ACCESS_READ); + + int loadCoordCount = kBaseTextureTypes[tt].coordCount + isArray + (needsMipLevel?1:0); // When translating to GLSL, we need to break apart the `location` argument. // @@ -679,6 +689,13 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) static const char* kGLSLLoadCoordsSwizzle[] = { "", "", "x", "xy", "xyz", "xyzw" }; static const char* kGLSLLoadLODSwizzle[] = { "", "", "y", "z", "w", "error" }; + // TODO: The GLSL translations here only handle the read-only texture + // cases (stuff that lowers to `texture*` in GLSL) and not the stuff + // that lowers to `image*`. + // + // At some point it may make sense to separate the read-only and + // `RW`/`RasterizerOrdered` cases here rather than try to share code. + if (isMultisample) { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; @@ -687,7 +704,16 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) else { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; - sb << "__target_intrinsic(glsl, \"texelFetch($0, ($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount] << ")\")\n"; + sb << "__target_intrinsic(glsl, \"texelFetch($0, "; + if( needsMipLevel ) + { + sb << "($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount]; + } + else + { + sb << "$1, 0"; + } + sb << ")\")\n"; } sb << "T Load("; sb << "int" << loadCoordCount << " location"; @@ -705,7 +731,16 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) else { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; - sb << "__target_intrinsic(glsl, \"texelFetch($0, ($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount] << ", $2)\")\n"; + sb << "__target_intrinsic(glsl, \"texelFetch($0, "; + if( needsMipLevel ) + { + sb << "($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount]; + } + else + { + sb << "$1, 0"; + } + sb << ", $2)\")\n"; } sb << "T Load("; sb << "int" << loadCoordCount << " location"; @@ -713,7 +748,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) { sb << ", int sampleIndex"; } - sb << ", constexpr int" << loadCoordCount << " offset"; + sb << ", constexpr int" << kBaseTextureTypes[tt].coordCount << " offset"; sb << ");\n"; diff --git a/source/slang/core.meta.slang.h b/source/slang/core.meta.slang.h index addc3856f..9a2033384 100644 --- a/source/slang/core.meta.slang.h +++ b/source/slang/core.meta.slang.h @@ -686,7 +686,17 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) if( kBaseTextureTypes[tt].coordCount + isArray < 4 ) { - int loadCoordCount = kBaseTextureTypes[tt].coordCount + isArray + (isMultisample?0:1); + // The `Load()` operation on an ordinary `Texture2D` takes + // an `int3` for the location, where `.xy` holds the texel + // coordinates, and `.z` holds the mip level to use. + // + // The third coordinate for mip level is absent in + // `Texure2DMS.Load()` and `RWTexture2D.Load`. This pattern + // is repreated for all the other texture shapes. + // + bool needsMipLevel = !isMultisample && (access == SLANG_RESOURCE_ACCESS_READ); + + int loadCoordCount = kBaseTextureTypes[tt].coordCount + isArray + (needsMipLevel?1:0); // When translating to GLSL, we need to break apart the `location` argument. // @@ -694,6 +704,13 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) static const char* kGLSLLoadCoordsSwizzle[] = { "", "", "x", "xy", "xyz", "xyzw" }; static const char* kGLSLLoadLODSwizzle[] = { "", "", "y", "z", "w", "error" }; + // TODO: The GLSL translations here only handle the read-only texture + // cases (stuff that lowers to `texture*` in GLSL) and not the stuff + // that lowers to `image*`. + // + // At some point it may make sense to separate the read-only and + // `RW`/`RasterizerOrdered` cases here rather than try to share code. + if (isMultisample) { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; @@ -702,7 +719,16 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) else { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; - sb << "__target_intrinsic(glsl, \"texelFetch($0, ($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount] << ")\")\n"; + sb << "__target_intrinsic(glsl, \"texelFetch($0, "; + if( needsMipLevel ) + { + sb << "($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount]; + } + else + { + sb << "$1, 0"; + } + sb << ")\")\n"; } sb << "T Load("; sb << "int" << loadCoordCount << " location"; @@ -720,7 +746,16 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) else { sb << "__glsl_extension(GL_EXT_samplerless_texture_functions)"; - sb << "__target_intrinsic(glsl, \"texelFetch($0, ($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount] << ", $2)\")\n"; + sb << "__target_intrinsic(glsl, \"texelFetch($0, "; + if( needsMipLevel ) + { + sb << "($1)." << kGLSLLoadCoordsSwizzle[loadCoordCount] << ", ($1)." << kGLSLLoadLODSwizzle[loadCoordCount]; + } + else + { + sb << "$1, 0"; + } + sb << ", $2)\")\n"; } sb << "T Load("; sb << "int" << loadCoordCount << " location"; @@ -728,7 +763,7 @@ for (int tt = 0; tt < kBaseTextureTypeCount; ++tt) { sb << ", int sampleIndex"; } - sb << ", constexpr int" << loadCoordCount << " offset"; + sb << ", constexpr int" << kBaseTextureTypes[tt].coordCount << " offset"; sb << ");\n"; diff --git a/tests/hlsl/simple/rw-texture.hlsl b/tests/hlsl/simple/rw-texture.hlsl new file mode 100644 index 000000000..26916b474 --- /dev/null +++ b/tests/hlsl/simple/rw-texture.hlsl @@ -0,0 +1,34 @@ +// rw-texture.hlsl + +//TEST:COMPARE_HLSL:-no-mangle -profile ps_5_0 -entry main + +// Ensure that we implement the `Load` operations on +// `RWTexture*` types with the correct signature. + +#ifndef __SLANG__ +#define C C_0 +#define SV_Target SV_TARGET +#define u2 u2_0 +#define u3 u3_0 +#define t2 t2_0 +#define t2a t2a_0 +#define t3 t3_0 +#endif + + +cbuffer C : register(b0) +{ + uint2 u2; + uint3 u3; +}; + +RWTexture2D<float4> t2 : register(u1); +RWTexture2DArray<float4> t2a : register(u2); +RWTexture3D<float4> t3 : register(u3); + +float4 main() : SV_Target +{ + return t2.Load(u2) + + t2a.Load(u3) + + t3.Load(u3); +} |
