Skip to content

Commit

Permalink
Add API to query stage of varying parameter (#302)
Browse files Browse the repository at this point in the history 
Fixes #301

The problem here is that if you have input GLSL code like:

```glsl
// example.vs
in vec3 pos;
```

and:

```glsl
// example.fs
in vec3 worldPos;
```

Then both `pos` and `worldPos` are reflected as global variables (parameters of the *program*), which both get bound to "varying input" resources, but there is no way to tell through the API that `pos` is a vertex parameter while `worldPos` is a fragment one.

The original request in issue #301 was to expose parameters like this not as a global variables, but rather as parameters of the entry point in their specific file. That is, treat it as if the user had written, e.g.:

```glsl
// example.vs
void vsMain(in vec3 pos) { ... }
```

Doing that would unify the GLSL and HLSL/Slang cases a bit, but would require the Slang reflection API to lie about the structure of code the user wrote. At a more basic level, that would have been hard to implement because the current reflection API just exposes the underlying AST, and the AST *needs* to leave `pos` at the global scope so that when we go and spit GLSL back out we retain the original structure.

This PR implements a more simplistic solution, where the user is allowed to query the stage that a varying parameter "belongs" to. For right now I'm only enabling this to work for varying parameters (but it doesn't care if they are entry-point or global-scope varyings). Despite what I said on #301, this should work for both the top-level parameter's variable layout, *and* any variable layouts for fields within its type reflection.

In terms of implementation, I took the simple but wasteful route: every `VarLayout` now has a `stage` field that is by default initialized to `SLANG_STAGE_NONE`. When collecting varying parameters, I take advantage of the fact that everything bottlenecks through `processEntryPointParameter()` which takes an `EntryPointParameterState` so that I can set the `VarLayout::stage` field for any varying parameter in one place.

While I was making this change, I also did a bit of cleanup so that the "official" names for the varying parameter categories are `VARYING_INPUT` and `VARYING_OUTPUT`, with `VERTEX_INPUT` and `FRAGMENT_OUTPUT` being "deprecated" in principle. I didn't do the bulk rename inside the codebase yet.
  • Loading branch information
Tim Foley authored Nov 29, 2017
1 parent 7139380 commit b487516
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 20 deletions.
33 changes: 29 additions & 4 deletions slang.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,8 @@ extern "C"
SLANG_PARAMETER_CATEGORY_CONSTANT_BUFFER,
SLANG_PARAMETER_CATEGORY_SHADER_RESOURCE,
SLANG_PARAMETER_CATEGORY_UNORDERED_ACCESS,
SLANG_PARAMETER_CATEGORY_VERTEX_INPUT,
SLANG_PARAMETER_CATEGORY_FRAGMENT_OUTPUT,
SLANG_PARAMETER_CATEGORY_VARYING_INPUT,
SLANG_PARAMETER_CATEGORY_VARYING_OUTPUT,
SLANG_PARAMETER_CATEGORY_SAMPLER_STATE,
SLANG_PARAMETER_CATEGORY_UNIFORM,
SLANG_PARAMETER_CATEGORY_DESCRIPTOR_TABLE_SLOT,
Expand All @@ -605,6 +605,11 @@ extern "C"

//
SLANG_PARAMETER_CATEGORY_COUNT,


// DEPRECATED:
SLANG_PARAMETER_CATEGORY_VERTEX_INPUT = SLANG_PARAMETER_CATEGORY_VARYING_INPUT,
SLANG_PARAMETER_CATEGORY_FRAGMENT_OUTPUT = SLANG_PARAMETER_CATEGORY_VARYING_OUTPUT,
};

typedef SlangUInt32 SlangStage;
Expand All @@ -618,6 +623,7 @@ extern "C"
SLANG_STAGE_FRAGMENT,
SLANG_STAGE_COMPUTE,

// alias:
SLANG_STAGE_PIXEL = SLANG_STAGE_FRAGMENT,
};

Expand Down Expand Up @@ -672,6 +678,16 @@ extern "C"
SLANG_API char const* spReflectionVariableLayout_GetSemanticName(SlangReflectionVariableLayout* var);
SLANG_API size_t spReflectionVariableLayout_GetSemanticIndex(SlangReflectionVariableLayout* var);

/** Get the stage that a variable belongs to (if any).
A variable "belongs" to a specific stage when it is a varying input/output
parameter either defined as part of the parameter list for an entry
point *or* at the global scope of a stage-specific GLSL code file (e.g.,
an `in` parameter in a GLSL `.vs` file belongs to the vertex stage).
*/
SLANG_API SlangStage spReflectionVariableLayout_getStage(
SlangReflectionVariableLayout* var);

// Shader Parameter Reflection

typedef SlangReflectionVariableLayout SlangReflectionParameter;
Expand Down Expand Up @@ -857,15 +873,19 @@ namespace slang
ConstantBuffer = SLANG_PARAMETER_CATEGORY_CONSTANT_BUFFER,
ShaderResource = SLANG_PARAMETER_CATEGORY_SHADER_RESOURCE,
UnorderedAccess = SLANG_PARAMETER_CATEGORY_UNORDERED_ACCESS,
VertexInput = SLANG_PARAMETER_CATEGORY_VERTEX_INPUT,
FragmentOutput = SLANG_PARAMETER_CATEGORY_FRAGMENT_OUTPUT,
VaryingInput = SLANG_PARAMETER_CATEGORY_VARYING_INPUT,
VaryingOutput = SLANG_PARAMETER_CATEGORY_VARYING_OUTPUT,
SamplerState = SLANG_PARAMETER_CATEGORY_SAMPLER_STATE,
Uniform = SLANG_PARAMETER_CATEGORY_UNIFORM,
DescriptorTableSlot = SLANG_PARAMETER_CATEGORY_DESCRIPTOR_TABLE_SLOT,
SpecializationConstant = SLANG_PARAMETER_CATEGORY_SPECIALIZATION_CONSTANT,
PushConstantBuffer = SLANG_PARAMETER_CATEGORY_PUSH_CONSTANT_BUFFER,
RegisterSpace = SLANG_PARAMETER_CATEGORY_REGISTER_SPACE,
GenericResource = SLANG_PARAMETER_CATEGORY_GENERIC,

// DEPRECATED:
VertexInput = SLANG_PARAMETER_CATEGORY_VERTEX_INPUT,
FragmentOutput = SLANG_PARAMETER_CATEGORY_FRAGMENT_OUTPUT,
};

struct TypeLayoutReflection
Expand Down Expand Up @@ -1057,6 +1077,11 @@ namespace slang
{
return spReflectionVariableLayout_GetSemanticIndex((SlangReflectionVariableLayout*) this);
}

SlangStage getStage()
{
return spReflectionVariableLayout_getStage((SlangReflectionVariableLayout*) this);
}
};

struct EntryPointReflection
Expand Down
1 change: 1 addition & 0 deletions source/slang/ast-legalize.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3309,6 +3309,7 @@ struct LoweringVisitor
RefPtr<VarLayout> newFieldLayout = new VarLayout();
newFieldLayout->typeLayout = fieldLayout->typeLayout;
newFieldLayout->flags = fieldLayout->flags;
newFieldLayout->stage = fieldLayout->stage;
newFieldLayout->varDecl = fieldLayout->varDecl;
newFieldLayout->systemValueSemantic = fieldLayout->systemValueSemantic;
newFieldLayout->systemValueSemanticIndex = fieldLayout->systemValueSemanticIndex;
Expand Down
2 changes: 2 additions & 0 deletions source/slang/ir-legalize-types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,8 @@ static LegalVal declareSimpleVar(
// well as all the offset information that has accumulated
// along the chain of parent variables.

// TODO: this logic needs to propagate through semantics...

varLayout = new VarLayout();
varLayout->typeLayout = typeLayout;

Expand Down
10 changes: 9 additions & 1 deletion source/slang/parameter-binding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ struct EntryPointParameterState
int* ioSemanticIndex = nullptr;
EntryPointParameterDirectionMask directionMask;
int semanticSlotCount;
Stage stage = Stage::Unknown;
};


Expand All @@ -639,14 +640,15 @@ static RefPtr<TypeLayout> processEntryPointParameter(
static void collectGlobalScopeGLSLVaryingParameter(
ParameterBindingContext* context,
RefPtr<VarDeclBase> varDecl,
RefPtr<Type> effectiveType,
RefPtr<Type> effectiveType,
EntryPointParameterDirection direction)
{
int defaultSemanticIndex = 0;

EntryPointParameterState state;
state.directionMask = direction;
state.ioSemanticIndex = &defaultSemanticIndex;
state.stage = context->stage;

RefPtr<VarLayout> varLayout = new VarLayout();
varLayout->varDecl = makeDeclRef(varDecl.Ptr());
Expand Down Expand Up @@ -1333,6 +1335,10 @@ static RefPtr<TypeLayout> processEntryPointParameter(
varLayout->flags |= VarLayoutFlag::HasSemantic;
}

if (varLayout)
{
varLayout->stage = state.stage;
}

// Scalar and vector types are treated as outputs directly
if(auto basicType = type->As<BasicExpressionType>())
Expand Down Expand Up @@ -1485,6 +1491,7 @@ static void collectEntryPointParameters(
state.ioSemanticIndex = &defaultSemanticIndex;
state.optSemanticName = nullptr;
state.semanticSlotCount = 0;
state.stage = entryPoint->profile.GetStage();

for( auto m : entryPointFuncDecl->Members )
{
Expand Down Expand Up @@ -2020,6 +2027,7 @@ RefPtr<ProgramLayout> specializeProgramLayout(
RefPtr<VarLayout> newVarLayout = new VarLayout();
RefPtr<ParameterInfo> paramInfo = new ParameterInfo();
newVarLayout->varDecl = varLayout->varDecl;
newVarLayout->stage = varLayout->stage;
newVarLayout->typeLayout = newTypeLayout;
paramInfo->varLayouts.Add(newVarLayout);
completeBindingsForParameter(&context, paramInfo);
Expand Down
30 changes: 30 additions & 0 deletions source/slang/reflection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,36 @@ SLANG_API size_t spReflectionVariableLayout_GetSemanticIndex(SlangReflectionVari
return varLayout->semanticIndex;
}

SLANG_API SlangStage spReflectionVariableLayout_getStage(
SlangReflectionVariableLayout* inVarLayout)
{
auto varLayout = convert(inVarLayout);
if(!varLayout) return SLANG_STAGE_NONE;

// A parameter that is not a varying input or output is
// not considered to belong to a single stage.
//
// TODO: We might need to reconsider this for, e.g., entry
// point parameters, where they might be stage-specific even
// if they are uniform.
if (!varLayout->FindResourceInfo(Slang::LayoutResourceKind::VaryingInput)
&& !varLayout->FindResourceInfo(Slang::LayoutResourceKind::VaryingOutput))
{
return SLANG_STAGE_NONE;
}

// TODO: We should find the stage for a variable layout by
// walking up the tree of layout information, until we find
// something that has a definitive stage attached to it (e.g.,
// either an entry point or a GLSL translation unit).
//
// We don't currently have parent links in the reflection layout
// information, so doing that walk would be tricky right now, so
// it is easier to just bloat the representation and store yet another
// field on every variable layout.
return (SlangStage) varLayout->stage;
}


// Shader Parameter Reflection

Expand Down
6 changes: 6 additions & 0 deletions source/slang/type-layout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,12 @@ class VarLayout : public Layout
String semanticName;
int semanticIndex;

// The stage this variable belongs to, in case it is
// stage-specific.
// TODO: This is wasteful to be storing on every single
// variable layout.
Stage stage = Stage::Unknown;

// The start register(s) for any resources
struct ResourceInfo
{
Expand Down
6 changes: 4 additions & 2 deletions tests/reflection/sample-rate-input.glsl.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ standard output = {
},
{
"name": "uv",
"binding": {"kind": "vertexInput", "index": 0},
"stage": "fragment",
"binding": {"kind": "varyingInput", "index": 0},
"type": {
"kind": "vector",
"elementCount": 2,
Expand All @@ -33,7 +34,8 @@ standard output = {
},
{
"name": "c",
"binding": {"kind": "fragmentOutput", "index": 0},
"stage": "fragment",
"binding": {"kind": "varyingOutput", "index": 0},
"type": {
"kind": "vector",
"elementCount": 4,
Expand Down
33 changes: 22 additions & 11 deletions tests/reflection/vertex-input-semantics.hlsl.expected
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ standard output = {
"parameters": [
{
"name": "a",
"binding": {"kind": "vertexInput", "index": 0},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 0},
"semanticName": "A",
"type": {
"kind": "vector",
Expand All @@ -26,7 +27,8 @@ standard output = {
},
{
"name": "b",
"binding": {"kind": "vertexInput", "index": 1, "count": 3},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 1, "count": 3},
"semanticName": "B",
"type": {
"kind": "struct",
Expand All @@ -42,7 +44,8 @@ standard output = {
"scalarType": "int32"
}
},
"binding": {"kind": "vertexInput", "index": 0},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 0},
"semanticName": "B"
},
{
Expand All @@ -61,7 +64,8 @@ standard output = {
"scalarType": "float32"
}
},
"binding": {"kind": "vertexInput", "index": 0},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 0},
"semanticName": "B",
"semanticIndex": 1
},
Expand All @@ -75,13 +79,15 @@ standard output = {
"scalarType": "float32"
}
},
"binding": {"kind": "vertexInput", "index": 1},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 1},
"semanticName": "B",
"semanticIndex": 2
}
]
},
"binding": {"kind": "vertexInput", "index": 1, "count": 2},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 1, "count": 2},
"semanticName": "B",
"semanticIndex": 1
}
Expand All @@ -90,7 +96,8 @@ standard output = {
},
{
"name": "c",
"binding": {"kind": "vertexInput", "index": 4, "count": 3},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 4, "count": 3},
"type": {
"kind": "struct",
"name": "C",
Expand All @@ -111,7 +118,8 @@ standard output = {
"scalarType": "float32"
}
},
"binding": {"kind": "vertexInput", "index": 0},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 0},
"semanticName": "CX"
},
{
Expand All @@ -124,13 +132,15 @@ standard output = {
"scalarType": "float32"
}
},
"binding": {"kind": "vertexInput", "index": 1},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 1},
"semanticName": "CX",
"semanticIndex": 1
}
]
},
"binding": {"kind": "vertexInput", "index": 0, "count": 2},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 0, "count": 2},
"semanticName": "CX"
},
{
Expand All @@ -143,7 +153,8 @@ standard output = {
"scalarType": "int32"
}
},
"binding": {"kind": "vertexInput", "index": 2},
"stage": "vertex",
"binding": {"kind": "varyingInput", "index": 2},
"semanticName": "CY"
}
]
Expand Down
26 changes: 24 additions & 2 deletions tools/slang-reflection-test/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ static void emitReflectionVarBindingInfoJSON(
CASE(CONSTANT_BUFFER, constantBuffer);
CASE(SHADER_RESOURCE, shaderResource);
CASE(UNORDERED_ACCESS, unorderedAccess);
CASE(VERTEX_INPUT, vertexInput);
CASE(FRAGMENT_OUTPUT, fragmentOutput);
CASE(VARYING_INPUT, varyingInput);
CASE(VARYING_OUTPUT, varyingOutput);
CASE(SAMPLER_STATE, samplerState);
CASE(UNIFORM, uniform);
CASE(DESCRIPTOR_TABLE_SLOT, descriptorTableSlot);
Expand Down Expand Up @@ -146,6 +146,28 @@ static void emitReflectionVarBindingInfoJSON(
PrettyWriter& writer,
slang::VariableLayoutReflection* var)
{
auto stage = var->getStage();
if (stage != SLANG_STAGE_NONE)
{
char const* stageName = "UNKNOWN";
switch (stage)
{
case SLANG_STAGE_VERTEX: stageName = "vertex"; break;
case SLANG_STAGE_HULL: stageName = "hull"; break;
case SLANG_STAGE_DOMAIN: stageName = "domain"; break;
case SLANG_STAGE_GEOMETRY: stageName = "geometry"; break;
case SLANG_STAGE_FRAGMENT: stageName = "fragment"; break;
case SLANG_STAGE_COMPUTE: stageName = "compute"; break;

default:
break;
}

write(writer, "\"stage\": \"");
write(writer, stageName);
write(writer, "\",\n");
}

auto typeLayout = var->getTypeLayout();
auto categoryCount = var->getCategoryCount();

Expand Down

0 comments on commit b487516

Please sign in to comment.