From 13c472e19648a7f7efbbb06e26304d7628208c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Beaufort?= Date: Thu, 29 Aug 2019 15:56:31 +0000 Subject: [PATCH] Make vertex input descriptor optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following WebGPU spec change at https://github.com/gpuweb/gpuweb/issues/378, vertexInput descriptor from GPURenderPipelineDescriptor should not be required anymore. BUG=dawn:22 Change-Id: I5d2500a758f44b7a7db2d2c23b359f1012221227 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10640 Commit-Queue: François Beaufort Reviewed-by: Austin Eng --- dawn.json | 4 +- src/dawn_native/RenderPipeline.cpp | 16 +++--- src/tests/end2end/VertexInputTests.cpp | 49 +++++++++++++++++++ .../RenderPipelineValidationTests.cpp | 28 +++++++++-- 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/dawn.json b/dawn.json index 8d4851f202..c4a1075b4f 100644 --- a/dawn.json +++ b/dawn.json @@ -648,7 +648,7 @@ "extensible": true, "members": [ {"name": "index format", "type": "index format", "default": "uint32"}, - {"name": "buffer count", "type": "uint32_t"}, + {"name": "buffer count", "type": "uint32_t", "default": 0}, {"name": "buffers", "type": "vertex buffer descriptor", "annotation": "const*", "length": "buffer count"} ] }, @@ -1031,7 +1031,7 @@ {"name": "layout", "type": "pipeline layout"}, {"name": "vertex stage", "type": "pipeline stage descriptor", "annotation": "const*"}, {"name": "fragment stage", "type": "pipeline stage descriptor", "annotation": "const*", "optional": true}, - {"name": "vertex input", "type": "vertex input descriptor", "annotation": "const*"}, + {"name": "vertex input", "type": "vertex input descriptor", "annotation": "const*", "optional": true}, {"name": "primitive topology", "type": "primitive topology"}, {"name": "rasterization state", "type": "rasterization state descriptor", "annotation": "const*", "optional": true}, {"name": "sample count", "type": "uint32_t", "default": "1"}, diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index 5dc6784995..6d9c532228 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -272,17 +272,16 @@ namespace dawn_native { DAWN_TRY(device->ValidateObject(descriptor->layout)); - if (descriptor->vertexInput == nullptr) { - return DAWN_VALIDATION_ERROR("Input state must not be null"); - } - // TODO(crbug.com/dawn/136): Support vertex-only pipelines. if (descriptor->fragmentStage == nullptr) { return DAWN_VALIDATION_ERROR("Null fragment stage is not supported (yet)"); } std::bitset attributesSetMask; - DAWN_TRY(ValidateVertexInputDescriptor(descriptor->vertexInput, &attributesSetMask)); + if (descriptor->vertexInput) { + DAWN_TRY(ValidateVertexInputDescriptor(descriptor->vertexInput, &attributesSetMask)); + } + DAWN_TRY(ValidatePrimitiveTopology(descriptor->primitiveTopology)); DAWN_TRY(ValidatePipelineStageDescriptor(device, descriptor->vertexStage, descriptor->layout, SingleShaderStage::Vertex)); @@ -358,7 +357,6 @@ namespace dawn_native { : PipelineBase(device, descriptor->layout, dawn::ShaderStage::Vertex | dawn::ShaderStage::Fragment), - mVertexInput(*descriptor->vertexInput), mAttachmentState(device->GetOrCreateAttachmentState(descriptor)), mPrimitiveTopology(descriptor->primitiveTopology), mSampleMask(descriptor->sampleMask), @@ -368,6 +366,12 @@ namespace dawn_native { mFragmentModule(descriptor->fragmentStage->module), mFragmentEntryPoint(descriptor->fragmentStage->entryPoint), mIsBlueprint(blueprint) { + if (descriptor->vertexInput != nullptr) { + mVertexInput = *descriptor->vertexInput; + } else { + mVertexInput = VertexInputDescriptor(); + } + for (uint32_t slot = 0; slot < mVertexInput.bufferCount; ++slot) { if (mVertexInput.buffers[slot].attributeCount == 0) { continue; diff --git a/src/tests/end2end/VertexInputTests.cpp b/src/tests/end2end/VertexInputTests.cpp index 5b3906ccc5..c5812ac858 100644 --- a/src/tests/end2end/VertexInputTests.cpp +++ b/src/tests/end2end/VertexInputTests.cpp @@ -518,3 +518,52 @@ DAWN_INSTANTIATE_TEST(VertexInputTest, D3D12Backend, MetalBackend, OpenGLBackend // - Add checks for alignement of vertex buffers and attributes if needed // - Check for attribute narrowing // - Check that the input state and the pipeline vertex input types match + +class OptionalVertexInputTest : public DawnTest {}; + +// Test that vertex input is not required in render pipeline descriptor. +TEST_P(OptionalVertexInputTest, Basic) { + utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 3, 3); + + dawn::ShaderModule vsModule = + utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"( + #version 450 + void main() { + gl_Position = vec4(0.0f, 0.0f, 0.0f, 1.0f); + })"); + + dawn::ShaderModule fsModule = + utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, R"( + #version 450 + layout(location = 0) out vec4 fragColor; + void main() { + fragColor = vec4(0.0f, 1.0f, 0.0f, 1.0f); + })"); + + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.cVertexStage.module = vsModule; + descriptor.cFragmentStage.module = fsModule; + descriptor.primitiveTopology = dawn::PrimitiveTopology::PointList; + descriptor.vertexInput = nullptr; + + dawn::RenderPipeline pipeline = device.CreateRenderPipeline(&descriptor); + + dawn::CommandEncoder encoder = device.CreateCommandEncoder(); + { + dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo); + pass.SetPipeline(pipeline); + pass.Draw(1, 1, 0, 0); + pass.EndPass(); + } + + dawn::CommandBuffer commands = encoder.Finish(); + queue.Submit(1, &commands); + + EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 255, 0, 255), renderPass.color, 1, 1); +} + +DAWN_INSTANTIATE_TEST(OptionalVertexInputTest, + D3D12Backend, + MetalBackend, + OpenGLBackend, + VulkanBackend); diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp index fb89afa1e2..5c5a54a42c 100644 --- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -43,11 +43,31 @@ class RenderPipelineValidationTest : public ValidationTest { // Test cases where creation should succeed TEST_F(RenderPipelineValidationTest, CreationSuccess) { - utils::ComboRenderPipelineDescriptor descriptor(device); - descriptor.cVertexStage.module = vsModule; - descriptor.cFragmentStage.module = fsModule; + { + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.cVertexStage.module = vsModule; + descriptor.cFragmentStage.module = fsModule; - device.CreateRenderPipeline(&descriptor); + device.CreateRenderPipeline(&descriptor); + } + { + // Vertex input should be optional + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.cVertexStage.module = vsModule; + descriptor.cFragmentStage.module = fsModule; + descriptor.vertexInput = nullptr; + + device.CreateRenderPipeline(&descriptor); + } + { + // Rasterization state should be optional + utils::ComboRenderPipelineDescriptor descriptor(device); + descriptor.cVertexStage.module = vsModule; + descriptor.cFragmentStage.module = fsModule; + descriptor.rasterizationState = nullptr; + + device.CreateRenderPipeline(&descriptor); + } } // Tests that at least one color state is required.