From e5cc4660c634a0dd35a9813e03192d380f253332 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Thu, 29 Nov 2018 07:48:38 -0800 Subject: Fix uses of dynamic_cast on types in reflection API (#731) The `Type` infrastructure uses a class hierarchy, but blindly `dynamic_cast`ing to a desired case doesn't always give the expected result, because a `Type` could represent a `typedef` (a `NamedExpressionType`) that itself resolves to, e.g, a vector type (a `VectorExpressionType`). In that case a `dynamic_cast(someType)` would fail, even though the type logically represents a vector. The `Type::As()` method is designed to handle this case, by "looking through" simple `typedef`s to get at the real definition of a type. The fix in this case is to use `Type::As()` at various points in the reflection code (`reflection.cpp`) instead of `dynamic_cast`. This problem surfaced with a `StructuredBuffer` not reflecting correctly, because the element type (`float2`) is actually a `typedef` (for `vector`), so I've included a test case that stresses that case. Getting the right output in the test required tweaking the `slang-reflection-test` tool to produce additional output for resource types (currently narrowed down to only affect structured buffers to avoid large diffs in expected test outputs). --- source/slang/reflection.cpp | 34 +++++----- tests/reflection/structured-buffer.slang | 19 ++++++ tests/reflection/structured-buffer.slang.expected | 75 +++++++++++++++++++++++ tests/reflection/thread-group-size.hlsl.expected | 6 +- tests/reflection/unbounded-arrays.hlsl.1.expected | 10 ++- tools/slang-reflection-test/main.cpp | 21 +++++++ 6 files changed, 146 insertions(+), 19 deletions(-) create mode 100644 tests/reflection/structured-buffer.slang create mode 100644 tests/reflection/structured-buffer.slang.expected diff --git a/source/slang/reflection.cpp b/source/slang/reflection.cpp index 73b616377..44920fb9f 100644 --- a/source/slang/reflection.cpp +++ b/source/slang/reflection.cpp @@ -189,7 +189,7 @@ SLANG_API unsigned int spReflectionType_GetFieldCount(SlangReflectionType* inTyp // TODO: maybe filter based on kind - if(auto declRefType = dynamic_cast(type)) + if(auto declRefType = type->As()) { auto declRef = declRefType->declRef; if( auto structDeclRef = declRef.As()) @@ -208,7 +208,7 @@ SLANG_API SlangReflectionVariable* spReflectionType_GetFieldByIndex(SlangReflect // TODO: maybe filter based on kind - if(auto declRefType = dynamic_cast(type)) + if(auto declRefType = type->As()) { auto declRef = declRefType->declRef; if( auto structDeclRef = declRef.As()) @@ -226,11 +226,11 @@ SLANG_API size_t spReflectionType_GetElementCount(SlangReflectionType* inType) auto type = convert(inType); if(!type) return 0; - if(auto arrayType = dynamic_cast(type)) + if(auto arrayType = type->As()) { return arrayType->ArrayLength ? (size_t) GetIntVal(arrayType->ArrayLength) : 0; } - else if( auto vectorType = dynamic_cast(type)) + else if( auto vectorType = type->As()) { return (size_t) GetIntVal(vectorType->elementCount); } @@ -243,19 +243,19 @@ SLANG_API SlangReflectionType* spReflectionType_GetElementType(SlangReflectionTy auto type = convert(inType); if(!type) return nullptr; - if(auto arrayType = dynamic_cast(type)) + if(auto arrayType = type->As()) { return (SlangReflectionType*) arrayType->baseType.Ptr(); } - else if( auto constantBufferType = dynamic_cast(type)) + else if( auto constantBufferType = type->As()) { return convert(constantBufferType->elementType.Ptr()); } - else if( auto vectorType = dynamic_cast(type)) + else if( auto vectorType = type->As()) { return convert(vectorType->elementType.Ptr()); } - else if( auto matrixType = dynamic_cast(type)) + else if( auto matrixType = type->As()) { return convert(matrixType->getElementType()); } @@ -268,15 +268,15 @@ SLANG_API unsigned int spReflectionType_GetRowCount(SlangReflectionType* inType) auto type = convert(inType); if(!type) return 0; - if(auto matrixType = dynamic_cast(type)) + if(auto matrixType = type->As()) { return (unsigned int) GetIntVal(matrixType->getRowCount()); } - else if(auto vectorType = dynamic_cast(type)) + else if(auto vectorType = type->As()) { return 1; } - else if( auto basicType = dynamic_cast(type) ) + else if( auto basicType = type->As() ) { return 1; } @@ -289,15 +289,15 @@ SLANG_API unsigned int spReflectionType_GetColumnCount(SlangReflectionType* inTy auto type = convert(inType); if(!type) return 0; - if(auto matrixType = dynamic_cast(type)) + if(auto matrixType = type->As()) { return (unsigned int) GetIntVal(matrixType->getColumnCount()); } - else if(auto vectorType = dynamic_cast(type)) + else if(auto vectorType = type->As()) { return (unsigned int) GetIntVal(vectorType->elementCount); } - else if( auto basicType = dynamic_cast(type) ) + else if( auto basicType = type->As() ) { return 1; } @@ -310,16 +310,16 @@ SLANG_API SlangScalarType spReflectionType_GetScalarType(SlangReflectionType* in auto type = convert(inType); if(!type) return 0; - if(auto matrixType = dynamic_cast(type)) + if(auto matrixType = type->As()) { type = matrixType->getElementType(); } - else if(auto vectorType = dynamic_cast(type)) + else if(auto vectorType = type->As()) { type = vectorType->elementType.Ptr(); } - if(auto basicType = dynamic_cast(type)) + if(auto basicType = type->As()) { switch (basicType->baseType) { diff --git a/tests/reflection/structured-buffer.slang b/tests/reflection/structured-buffer.slang new file mode 100644 index 000000000..491d61486 --- /dev/null +++ b/tests/reflection/structured-buffer.slang @@ -0,0 +1,19 @@ +//TEST:REFLECTION:-profile ps_4_0 -target hlsl + +// Confirm that we reflect the contents of structure-buffer types correctly. + +struct S +{ + float2 a; + float b; + uint c; +}; + +StructuredBuffer x; +StructuredBuffer y; +StructuredBuffer z; + +float4 main() : SV_Target +{ + return x[0] + y[0].xyxy + z[0].a.xyxy; +} \ No newline at end of file diff --git a/tests/reflection/structured-buffer.slang.expected b/tests/reflection/structured-buffer.slang.expected new file mode 100644 index 000000000..70ceb64f2 --- /dev/null +++ b/tests/reflection/structured-buffer.slang.expected @@ -0,0 +1,75 @@ +result code = 0 +standard error = { +} +standard output = { +{ + "parameters": [ + { + "name": "x", + "binding": {"kind": "shaderResource", "index": 0}, + "type": { + "kind": "resource", + "baseShape": "structuredBuffer", + "resultType": { + "kind": "scalar", + "scalarType": "uint32" + } + } + }, + { + "name": "y", + "binding": {"kind": "shaderResource", "index": 1}, + "type": { + "kind": "resource", + "baseShape": "structuredBuffer", + "resultType": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + } + } + }, + { + "name": "z", + "binding": {"kind": "shaderResource", "index": 2}, + "type": { + "kind": "resource", + "baseShape": "structuredBuffer", + "resultType": { + "kind": "struct", + "fields": [ + "name": "a", + "type": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "name": "b", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "name": "c", + "type": { + "kind": "scalar", + "scalarType": "uint32" + } + ] + } + } + } + ], + "entryPoints": [ + { + "name": "main", + "stage:": "fragment" + } + ] +} +} diff --git a/tests/reflection/thread-group-size.hlsl.expected b/tests/reflection/thread-group-size.hlsl.expected index cd5d09e35..46b0eb87e 100644 --- a/tests/reflection/thread-group-size.hlsl.expected +++ b/tests/reflection/thread-group-size.hlsl.expected @@ -10,7 +10,11 @@ standard output = { "type": { "kind": "resource", "baseShape": "structuredBuffer", - "access": "readWrite" + "access": "readWrite", + "resultType": { + "kind": "scalar", + "scalarType": "float32" + } } } ], diff --git a/tests/reflection/unbounded-arrays.hlsl.1.expected b/tests/reflection/unbounded-arrays.hlsl.1.expected index 382fc25bc..60d894ba7 100644 --- a/tests/reflection/unbounded-arrays.hlsl.1.expected +++ b/tests/reflection/unbounded-arrays.hlsl.1.expected @@ -98,7 +98,15 @@ standard output = { "type": { "kind": "resource", "baseShape": "structuredBuffer", - "access": "readWrite" + "access": "readWrite", + "resultType": { + "kind": "vector", + "elementCount": 4, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + } } } ], diff --git a/tools/slang-reflection-test/main.cpp b/tools/slang-reflection-test/main.cpp index 7e5f268f3..872d2ff3a 100644 --- a/tools/slang-reflection-test/main.cpp +++ b/tools/slang-reflection-test/main.cpp @@ -372,6 +372,27 @@ static void emitReflectionTypeInfoJSON( } write(writer, "\""); } + + // TODO: We should really print the result type for all resource + // types, but current test output depends on the old behavior, so + // we only add result type output for structured buffers at first. + // + switch (shape & SLANG_RESOURCE_BASE_SHAPE_MASK) + { + default: + break; + + case SLANG_STRUCTURED_BUFFER: + if( auto resultType = type->getResourceResultType() ) + { + write(writer, ",\n"); + write(writer, "\"resultType\": "); + emitReflectionTypeJSON( + writer, + resultType); + } + break; + } } break; -- cgit v1.2.3