summaryrefslogtreecommitdiffstats
path: root/tools
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-11-21 14:06:19 -0800
committerGitHub <noreply@github.com>2019-11-21 14:06:19 -0800
commit2ea64ff4f2c7c43b72ff24650330fca79a87500f (patch)
tree715f12e80ae04fd3b5073b0b300b578d3a605da8 /tools
parentcc37fb01b55a351c5fd703160dfd5249cad640e7 (diff)
Remove support for explicit register/binding syntax on TEST_INPUT (#1132)
The `TEST_INPUT` facility allows textual Slang test cases to provide two kinds of information to the `render-test` tool: 1. Information on what shader inputs exist 2. Information on what values/objects to bind into those shader inputs Under the first category of information, there exists supporting for attaching a `dxbinding(...)` annotation to a `TEST_INPUT` which seemingly indicates what HLSL `register` the input uses. There is a similar `glbinding(...)` annotation, used for OpenGL and Vulkan. It turns out that these annotations were, in practice, completely ignored and had no bearing on how `render-test` allocates or bindings graphics API objects. There was some amount of code attempting to validate that explicit registers/bindings were being set appropriately, but the actual values were being ignored. The visible consequence of the `dxbinding` and `glbinding` annotations being ignored is issue #1036: the order of `TEST_INPUT` lines was *de facto* determining the registers/bindings that were being used by `render-test`. This change simply removes the placebo features and strips things down to what is implemented in practice: the `TEST_INPUT` lines do not need target-API-specific binding/register numbers, because their order in the file implicitly defines them. I added logic to the parsing of `TEST_INPUT` lines to make sure I got an error message on any leftover annotations, and went ahead and systematicaly deleted all of the placebo annotations from our test cases. If we decide to make `TEST_INPUT` lines *not* depend on order of declaration in the future, we can build it up as a new and better considered feature. The main alternative I considered was to keep the annotations in place, and change `render-test` and the `gfx` abstraction layer to properly respect them, but that path actually creates much more opportunity for breakage (since every single test case would suddenly be specifying its root signature / pipeline layout via a different path using data that has never been tested). The approach in this change has the benefit of giving me high confidence that all the test cases continue to work just as they had before.
Diffstat (limited to 'tools')
-rw-r--r--tools/render-test/render-test-main.cpp2
-rw-r--r--tools/render-test/shader-input-layout.cpp26
-rw-r--r--tools/render-test/shader-input-layout.h2
-rw-r--r--tools/render-test/shader-renderer-util.cpp54
-rw-r--r--tools/render-test/shader-renderer-util.h3
5 files changed, 6 insertions, 81 deletions
diff --git a/tools/render-test/render-test-main.cpp b/tools/render-test/render-test-main.cpp
index 13af422a9..1355402cf 100644
--- a/tools/render-test/render-test-main.cpp
+++ b/tools/render-test/render-test-main.cpp
@@ -89,7 +89,7 @@ class RenderTestApp : public WindowListener
uint64_t m_startTicks;
// variables for state to be used for rendering...
- uintptr_t m_constantBufferSize, m_computeResultBufferSize;
+ uintptr_t m_constantBufferSize;
RefPtr<Renderer> m_renderer;
diff --git a/tools/render-test/shader-input-layout.cpp b/tools/render-test/shader-input-layout.cpp
index 3ba20c2cb..7a9001c9e 100644
--- a/tools/render-test/shader-input-layout.cpp
+++ b/tools/render-test/shader-input-layout.cpp
@@ -463,27 +463,6 @@ namespace renderer_test
entry.isCPUOnly = true;
parser.ReadToken();
}
- else if (parser.LookAhead("dxbinding"))
- {
- parser.ReadToken();
- parser.Read("(");
- entry.hlslBinding = parser.ReadInt();
- parser.Read(")");
- }
- else if (parser.LookAhead("glbinding"))
- {
- parser.ReadToken();
- parser.Read("(");
- while (!parser.IsEnd() && !parser.LookAhead(")"))
- {
- entry.glslBinding.add(parser.ReadInt());
- if (parser.LookAhead(","))
- parser.Read(",");
- else
- break;
- }
- parser.Read(")");
- }
else if (parser.LookAhead("out"))
{
parser.ReadToken();
@@ -541,6 +520,11 @@ namespace renderer_test
entry.name = builder;
}
+ else
+ {
+ fprintf(stderr, "Invalid TEST_INPUT syntax '%s'\n", parser.NextToken().Content.getBuffer());
+ break;
+ }
if (parser.LookAhead(","))
parser.Read(",");
diff --git a/tools/render-test/shader-input-layout.h b/tools/render-test/shader-input-layout.h
index e57e1ed8b..d2f67a61e 100644
--- a/tools/render-test/shader-input-layout.h
+++ b/tools/render-test/shader-input-layout.h
@@ -67,8 +67,6 @@ public:
ArrayDesc arrayDesc;
bool isOutput = false;
bool isCPUOnly = false;
- int hlslBinding = -1;
- Slang::List<int> glslBinding;
Slang::String name; ///< Optional name. Useful for binding through reflection.
};
diff --git a/tools/render-test/shader-renderer-util.cpp b/tools/render-test/shader-renderer-util.cpp
index 55ef4a3dc..f73595f4f 100644
--- a/tools/render-test/shader-renderer-util.cpp
+++ b/tools/render-test/shader-renderer-util.cpp
@@ -156,53 +156,6 @@ static RefPtr<SamplerState> _createSamplerState(
return renderer->createSamplerState(_calcSamplerDesc(srcDesc));
}
-/* static */BindingStateImpl::RegisterRange ShaderRendererUtil::calcRegisterRange(Renderer* renderer, const ShaderInputLayoutEntry& entry)
-{
- typedef BindingStateImpl::RegisterRange RegisterRange;
-
- BindingStyle bindingStyle = RendererUtil::getBindingStyle(renderer->getRendererType());
-
- switch (bindingStyle)
- {
- case BindingStyle::DirectX:
- {
- return RegisterRange::makeSingle(entry.hlslBinding);
- }
- case BindingStyle::Vulkan:
- {
- // USe OpenGls for now
- // fallthru
- }
- case BindingStyle::OpenGl:
- {
- const int count = int(entry.glslBinding.getCount());
-
- if (count <= 0)
- {
- break;
- }
-
- int baseIndex = entry.glslBinding[0];
- // Make sure they are contiguous
- for (Index i = 1; i < int(entry.glslBinding.getCount()); ++i)
- {
- if (baseIndex + i != entry.glslBinding[i])
- {
- assert(!"Bindings must be contiguous");
- break;
- }
- }
- return RegisterRange::makeRange(baseIndex, count);
- }
- /* case BindingStyle::Vulkan:
- {
- } */
- default: break;
- }
- // Return invalid
- return RegisterRange::makeInvalid();
-}
-
/* static */Result ShaderRendererUtil::createBindingState(const ShaderInputLayout& layout, Renderer* renderer, BufferResource* addedConstantBuffer, BindingStateImpl** outBindingState)
{
auto srcEntries = layout.entries.getBuffer();
@@ -225,13 +178,6 @@ static RefPtr<SamplerState> _createSamplerState(
const ShaderInputLayoutEntry& srcEntry = srcEntries[i];
SLANG_ASSERT(srcEntry.isCPUOnly == false);
- const BindingStateImpl::RegisterRange registerSet = calcRegisterRange(renderer, srcEntry);
- if (!registerSet.isValid())
- {
- assert(!"Couldn't find a binding");
- return SLANG_FAIL;
- }
-
DescriptorSetLayout::SlotRangeDesc slotRangeDesc;
switch (srcEntry.type)
diff --git a/tools/render-test/shader-renderer-util.h b/tools/render-test/shader-renderer-util.h
index bbdea2af6..872f21768 100644
--- a/tools/render-test/shader-renderer-util.h
+++ b/tools/render-test/shader-renderer-util.h
@@ -62,9 +62,6 @@ struct ShaderRendererUtil
/// Create BindingState::Desc from the contents of layout
static Slang::Result createBindingState(const ShaderInputLayout& layout, Renderer* renderer, BufferResource* addedConstantBuffer, BindingStateImpl** outBindingState);
- /// Get the binding register associated with this binding (or -1 if none defined)
- static BindingStateImpl::RegisterRange calcRegisterRange(Renderer* renderer, const ShaderInputLayoutEntry& entry);
-
private:
/// Create BindingState::Desc from a list of ShaderInputLayout entries
static Slang::Result _createBindingState(ShaderInputLayoutEntry* srcEntries, int numEntries, Renderer* renderer, BufferResource* addedConstantBuffer, BindingStateImpl** outBindingState);