From 13a2ca6135284a63a178918bd0c7c7becc56eaf2 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 6 Nov 2019 14:01:53 -0800 Subject: Fixes to how "pending" data is reported in type layouts (#1108) These fixes were needed to get a Falcor example that uses Slang `interface`s for shader specialization to work. The basic fixes are: * In `applyOffsetToTypeLayout`, we need to account for the case where the type being offset had "pending" data, and make sure to offset it as well. This also means that when computing if offsetting is needed, we also need to check the pending data, if any. * In `_createParameterGroupTypeLayout` make sure to call `applyOffsetToTypeLayout` *after* the layout for "pending" data, if any, has been computed. * In `_createTypeLayout`, when computing layout for an `ExistentialSpecializedType` (a type that has had existential/interface-type slots specialized to known types), don't forget to set the type layout of the variable layout we create to store pending data. The first two items relate a "legacy" reflection API that the client code really shouldn't be using (but it was easier to fix the "do what I mean" behavior in Slang than to refactor the client), and the last is a detailed corner case in the layout of interface-based types that we are not currently equiped to test directly. --- source/slang/slang-type-layout.cpp | 116 +++++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 30 deletions(-) (limited to 'source') diff --git a/source/slang/slang-type-layout.cpp b/source/slang/slang-type-layout.cpp index 29ebe7de6..88e62323b 100644 --- a/source/slang/slang-type-layout.cpp +++ b/source/slang/slang-type-layout.cpp @@ -1287,6 +1287,20 @@ RefPtr applyOffsetToTypeLayout( break; } } + if( auto oldPendingTypeLayout = oldTypeLayout->pendingDataTypeLayout ) + { + if( auto pendingOffsetVarLayout = offsetVarLayout->pendingVarLayout ) + { + for (auto oldResInfo : oldPendingTypeLayout->resourceInfos) + { + if (auto offsetResInfo = pendingOffsetVarLayout->FindResourceInfo(oldResInfo.kind)) + { + anyHit = true; + break; + } + } + } + } if (!anyHit) return oldTypeLayout; @@ -1324,6 +1338,35 @@ RefPtr applyOffsetToTypeLayout( } } + if( auto oldPendingField = oldField->pendingVarLayout ) + { + RefPtr newPendingField = new VarLayout(); + newPendingField->varDecl = oldPendingField->varDecl; + newPendingField->typeLayout = oldPendingField->typeLayout; + newPendingField->flags = oldPendingField->flags; + newPendingField->semanticIndex = oldPendingField->semanticIndex; + newPendingField->semanticName = oldPendingField->semanticName; + newPendingField->stage = oldPendingField->stage; + newPendingField->systemValueSemantic = oldPendingField->systemValueSemantic; + newPendingField->systemValueSemanticIndex = oldPendingField->systemValueSemanticIndex; + + newField->pendingVarLayout = newPendingField; + + for (auto oldResInfo : oldPendingField->resourceInfos) + { + auto newResInfo = newPendingField->findOrAddResourceInfo(oldResInfo.kind); + newResInfo->index = oldResInfo.index; + newResInfo->space = oldResInfo.space; + if( auto pendingOffsetVarLayout = offsetVarLayout->pendingVarLayout ) + { + if (auto offsetResInfo = pendingOffsetVarLayout->FindResourceInfo(oldResInfo.kind)) + { + newResInfo->index += offsetResInfo->index; + } + } + } + } + newStructTypeLayout->fields.add(newField); mapOldFieldToNew.Add(oldField.Ptr(), newField.Ptr()); @@ -1354,6 +1397,14 @@ RefPtr applyOffsetToTypeLayout( newResInfo->count = oldResInfo.count; } + if( auto oldPendingTypeLayout = oldTypeLayout->pendingDataTypeLayout ) + { + if( auto pendingOffsetVarLayout = offsetVarLayout->pendingVarLayout ) + { + newTypeLayout->pendingDataTypeLayout = applyOffsetToTypeLayout(oldPendingTypeLayout, pendingOffsetVarLayout); + } + } + return newTypeLayout; } @@ -1707,36 +1758,6 @@ static RefPtr _createParameterGroupTypeLayout( } } - // The existing Slang reflection API was created before we really - // understood the wrinkle that the "container" and elements parts - // of a parameter group could collide on some resource kinds, - // so the API doesn't currently expose the nice `VarLayout`s we've - // just computed. - // - // Instead, the API allows the user to query the element type layout - // for the group, and the user just assumes that the offsetting - // is magically applied there. To go back to the earlier example: - // - // struct MyMaterial { Texture2D t; SamplerState s; }; - // ConstantBuffer gMaterial; - // - // A user of the existing reflection API expects to be able to - // query the `binding` of `gMaterial` and get back zero, then - // query the `binding` of the `t` field of the element type - // and get *one*. It is clear that in the abstract, the - // `MyMaterial::t` field should have an offset of zero (as - // the first field in a `struct`), so to meet the user's - // expectations, some cleverness is needed. - // - // We will use a subroutine `applyOffsetToTypeLayout` - // that tries to recursively walk an existing `TypeLayout` - // and apply an offset to its fields. This is currently - // quite ad hoc, but that doesn't matter much as it - // handles `struct` types which are the 99% case for - // parameter blocks. - // - typeLayout->offsetElementTypeLayout = applyOffsetToTypeLayout(rawElementTypeLayout, elementVarLayout); - // Next, resource usage from the container and element // types may need to "bleed through" to the overall // parameter group type. @@ -1934,8 +1955,42 @@ static RefPtr _createParameterGroupTypeLayout( // Now we need to update the type layout to what we've done. // typeLayout->pendingDataTypeLayout = unmaskedPendingDataTypeLayout; + + // TODO: we should probably adjust the size reported by the element type + // to include any "pending" data that was allocated into the group, so + // that it can be easier for client code to allocate their instances. } + // The existing Slang reflection API was created before we really + // understood the wrinkle that the "container" and elements parts + // of a parameter group could collide on some resource kinds, + // so the API doesn't currently expose the nice `VarLayout`s we've + // just computed. + // + // Instead, the API allows the user to query the element type layout + // for the group, and the user just assumes that the offsetting + // is magically applied there. To go back to the earlier example: + // + // struct MyMaterial { Texture2D t; SamplerState s; }; + // ConstantBuffer gMaterial; + // + // A user of the existing reflection API expects to be able to + // query the `binding` of `gMaterial` and get back zero, then + // query the `binding` of the `t` field of the element type + // and get *one*. It is clear that in the abstract, the + // `MyMaterial::t` field should have an offset of zero (as + // the first field in a `struct`), so to meet the user's + // expectations, some cleverness is needed. + // + // We will use a subroutine `applyOffsetToTypeLayout` + // that tries to recursively walk an existing `TypeLayout` + // and apply an offset to its fields. This is currently + // quite ad hoc, but that doesn't matter much as it + // handles `struct` types which are the 99% case for + // parameter blocks. + // + typeLayout->offsetElementTypeLayout = applyOffsetToTypeLayout(rawElementTypeLayout, elementVarLayout); + return typeLayout; } @@ -3351,6 +3406,7 @@ static TypeLayoutResult _createTypeLayout( RefPtr pendingDataVarLayout = new VarLayout(); if(auto pendingDataTypeLayout = baseTypeLayoutResult.layout->pendingDataTypeLayout) { + pendingDataVarLayout->typeLayout = pendingDataTypeLayout; for( auto pendingResInfo : pendingDataTypeLayout->resourceInfos ) { auto kind = pendingResInfo.kind; -- cgit v1.2.3