Make ComboVertexStateDescriptor non-copyable non-movable.
The VertexState tests were failing on MSVC debug x64 because the
ComboVertexStateDescriptor was copied but internal pointer to
itself weren't updated. This commit makes the structure non-movable and
non-copyable so the problem doesn't happen in the future and fixes
compilation errors.
Bug: dawn:602
Change-Id: Idd3a68933920eb47939217362e9e137dd7581014
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38400
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/tests/end2end/VertexBufferRobustnessTests.cpp b/src/tests/end2end/VertexBufferRobustnessTests.cpp
index abd6367..d6b6f3a 100644
--- a/src/tests/end2end/VertexBufferRobustnessTests.cpp
+++ b/src/tests/end2end/VertexBufferRobustnessTests.cpp
@@ -52,7 +52,7 @@
// Runs the test, a true |expectation| meaning success
void DoTest(const std::string& attributes,
const std::string& successExpression,
- utils::ComboVertexStateDescriptor vertexState,
+ const utils::ComboVertexStateDescriptor& vertexState,
wgpu::Buffer vertexBuffer,
uint64_t bufferOffset,
bool expectation) {
@@ -72,7 +72,7 @@
descriptor.vertexStage.module = vsModule;
descriptor.cFragmentStage.module = fsModule;
descriptor.primitiveTopology = wgpu::PrimitiveTopology::PointList;
- descriptor.cVertexState = std::move(vertexState);
+ descriptor.vertexState = &vertexState;
descriptor.cColorStates[0].format = renderPass.colorFormat;
renderPass.renderPassInfo.cColorAttachments[0].clearColor = {0, 0, 0, 1};
@@ -108,8 +108,7 @@
wgpu::Buffer vertexBuffer = utils::CreateBufferFromData(device, kVertices, sizeof(kVertices),
wgpu::BufferUsage::Vertex);
- DoTest("[[location(0)]] var<in> a : f32;", "a == 473.0", std::move(vertexState), vertexBuffer,
- 0, false);
+ DoTest("[[location(0)]] var<in> a : f32;", "a == 473.0", vertexState, vertexBuffer, 0, false);
}
TEST_P(VertexBufferRobustnessTest, FloatClamp) {
@@ -126,8 +125,7 @@
wgpu::Buffer vertexBuffer = utils::CreateBufferFromData(device, kVertices, sizeof(kVertices),
wgpu::BufferUsage::Vertex);
- DoTest("[[location(0)]] var<in> a : f32;", "a == 473.0", std::move(vertexState), vertexBuffer,
- 4, true);
+ DoTest("[[location(0)]] var<in> a : f32;", "a == 473.0", vertexState, vertexBuffer, 4, true);
}
TEST_P(VertexBufferRobustnessTest, IntClamp) {
@@ -144,8 +142,7 @@
wgpu::Buffer vertexBuffer = utils::CreateBufferFromData(device, kVertices, sizeof(kVertices),
wgpu::BufferUsage::Vertex);
- DoTest("[[location(0)]] var<in> a : i32;", "a == 473", std::move(vertexState), vertexBuffer, 4,
- true);
+ DoTest("[[location(0)]] var<in> a : i32;", "a == 473", vertexState, vertexBuffer, 4, true);
}
TEST_P(VertexBufferRobustnessTest, UIntClamp) {
@@ -162,8 +159,7 @@
wgpu::Buffer vertexBuffer = utils::CreateBufferFromData(device, kVertices, sizeof(kVertices),
wgpu::BufferUsage::Vertex);
- DoTest("[[location(0)]] var<in> a : u32;", "a == 473", std::move(vertexState), vertexBuffer, 4,
- true);
+ DoTest("[[location(0)]] var<in> a : u32;", "a == 473", vertexState, vertexBuffer, 4, true);
}
TEST_P(VertexBufferRobustnessTest, Float2Clamp) {
@@ -199,8 +195,7 @@
wgpu::BufferUsage::Vertex);
DoTest("[[location(0)]] var<in> a : vec3<f32>;",
- "a[0] == 473.0 && a[1] == 473.0 && a[2] == 473.0", std::move(vertexState), vertexBuffer,
- 12, true);
+ "a[0] == 473.0 && a[1] == 473.0 && a[2] == 473.0", vertexState, vertexBuffer, 12, true);
}
TEST_P(VertexBufferRobustnessTest, Float4Clamp) {
@@ -218,8 +213,8 @@
wgpu::BufferUsage::Vertex);
DoTest("[[location(0)]] var<in> a : vec4<f32>;",
- "a[0] == 473.0 && a[1] == 473.0 && a[2] == 473.0 && a[3] == 473.0",
- std::move(vertexState), vertexBuffer, 16, true);
+ "a[0] == 473.0 && a[1] == 473.0 && a[2] == 473.0 && a[3] == 473.0", vertexState,
+ vertexBuffer, 16, true);
}
DAWN_INSTANTIATE_TEST(VertexBufferRobustnessTest, MetalBackend({"metal_enable_vertex_pulling"}));
diff --git a/src/tests/end2end/VertexStateTests.cpp b/src/tests/end2end/VertexStateTests.cpp
index 4b2df8e..44bf889 100644
--- a/src/tests/end2end/VertexStateTests.cpp
+++ b/src/tests/end2end/VertexStateTests.cpp
@@ -150,32 +150,30 @@
std::vector<VertexAttributeSpec> attributes;
};
- utils::ComboVertexStateDescriptor MakeVertexState(
- const std::vector<VertexBufferSpec>& buffers) {
- utils::ComboVertexStateDescriptor vertexState;
+ void MakeVertexState(const std::vector<VertexBufferSpec>& buffers,
+ utils::ComboVertexStateDescriptor* vertexState) {
uint32_t vertexBufferCount = 0;
uint32_t totalNumAttributes = 0;
for (const VertexBufferSpec& buffer : buffers) {
- vertexState.cVertexBuffers[vertexBufferCount].arrayStride = buffer.arrayStride;
- vertexState.cVertexBuffers[vertexBufferCount].stepMode = buffer.step;
+ vertexState->cVertexBuffers[vertexBufferCount].arrayStride = buffer.arrayStride;
+ vertexState->cVertexBuffers[vertexBufferCount].stepMode = buffer.step;
- vertexState.cVertexBuffers[vertexBufferCount].attributes =
- &vertexState.cAttributes[totalNumAttributes];
+ vertexState->cVertexBuffers[vertexBufferCount].attributes =
+ &vertexState->cAttributes[totalNumAttributes];
for (const VertexAttributeSpec& attribute : buffer.attributes) {
- vertexState.cAttributes[totalNumAttributes].shaderLocation = attribute.location;
- vertexState.cAttributes[totalNumAttributes].offset = attribute.offset;
- vertexState.cAttributes[totalNumAttributes].format = attribute.format;
+ vertexState->cAttributes[totalNumAttributes].shaderLocation = attribute.location;
+ vertexState->cAttributes[totalNumAttributes].offset = attribute.offset;
+ vertexState->cAttributes[totalNumAttributes].format = attribute.format;
totalNumAttributes++;
}
- vertexState.cVertexBuffers[vertexBufferCount].attributeCount =
+ vertexState->cVertexBuffers[vertexBufferCount].attributeCount =
static_cast<uint32_t>(buffer.attributes.size());
vertexBufferCount++;
}
- vertexState.vertexBufferCount = vertexBufferCount;
- return vertexState;
+ vertexState->vertexBufferCount = vertexBufferCount;
}
template <typename T>
@@ -235,8 +233,9 @@
// Test compilation and usage of the fixture :)
TEST_P(VertexStateTest, Basic) {
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
- {{4 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{4 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
@@ -255,8 +254,8 @@
// This test was failing only on AMD but the OpenGL backend doesn't gather PCI info yet.
DAWN_SKIP_TEST_IF(IsLinux() && IsOpenGL());
- utils::ComboVertexStateDescriptor vertexState =
- MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}}, &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 0, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
@@ -276,8 +275,8 @@
// R32F case
{
- utils::ComboVertexStateDescriptor vertexState =
- MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float}}}}, &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 0, {{0, VertexFormat::Float, InputStepMode::Vertex}});
@@ -286,8 +285,8 @@
}
// RG32F case
{
- utils::ComboVertexStateDescriptor vertexState =
- MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float2}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float2}}}}, &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 0, {{0, VertexFormat::Float2, InputStepMode::Vertex}});
@@ -296,8 +295,8 @@
}
// RGB32F case
{
- utils::ComboVertexStateDescriptor vertexState =
- MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float3}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{0, InputStepMode::Vertex, {{0, 0, VertexFormat::Float3}}}}, &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 0, {{0, VertexFormat::Float3, InputStepMode::Vertex}});
@@ -311,8 +310,9 @@
// This test was failing only on AMD but the OpenGL backend doesn't gather PCI info yet.
DAWN_SKIP_TEST_IF(IsLinux() && IsOpenGL());
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
- {{8 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{8 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
@@ -328,10 +328,12 @@
// Test two attributes at an offset, vertex version
TEST_P(VertexStateTest, TwoAttributesAtAnOffsetVertex) {
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState(
{{8 * sizeof(float),
InputStepMode::Vertex,
- {{0, 0, VertexFormat::Float4}, {1, 4 * sizeof(float), VertexFormat::Float4}}}});
+ {{0, 0, VertexFormat::Float4}, {1, 4 * sizeof(float), VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
@@ -347,10 +349,12 @@
// Test two attributes at an offset, instance version
TEST_P(VertexStateTest, TwoAttributesAtAnOffsetInstance) {
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState(
{{8 * sizeof(float),
InputStepMode::Instance,
- {{0, 0, VertexFormat::Float4}, {1, 4 * sizeof(float), VertexFormat::Float4}}}});
+ {{0, 0, VertexFormat::Float4}, {1, 4 * sizeof(float), VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
@@ -366,8 +370,9 @@
// Test a pure-instance input state
TEST_P(VertexStateTest, PureInstance) {
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
- {{4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
@@ -385,13 +390,15 @@
// Test with mixed everything, vertex vs. instance, different stride and offsets
// different attribute types
TEST_P(VertexStateTest, MixedEverything) {
- utils::ComboVertexStateDescriptor vertexState = MakeVertexState(
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState(
{{12 * sizeof(float),
InputStepMode::Vertex,
{{0, 0, VertexFormat::Float}, {1, 6 * sizeof(float), VertexFormat::Float2}}},
{10 * sizeof(float),
InputStepMode::Instance,
- {{2, 0, VertexFormat::Float3}, {3, 5 * sizeof(float), VertexFormat::Float4}}}});
+ {{2, 0, VertexFormat::Float3}, {3, 5 * sizeof(float), VertexFormat::Float4}}}},
+ &vertexState);
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1,
{{0, VertexFormat::Float, InputStepMode::Vertex},
@@ -419,9 +426,10 @@
// Test input state is unaffected by unused vertex slot
TEST_P(VertexStateTest, UnusedVertexSlot) {
// Instance input state, using slot 1
- utils::ComboVertexStateDescriptor instanceVertexState = MakeVertexState(
- {{0, InputStepMode::Vertex, {}},
- {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor instanceVertexState;
+ MakeVertexState({{0, InputStepMode::Vertex, {}},
+ {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}},
+ &instanceVertexState);
wgpu::RenderPipeline instancePipeline = MakeTestPipeline(
instanceVertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
@@ -458,15 +466,17 @@
// SetVertexBuffer should be reapplied when the input state changes.
TEST_P(VertexStateTest, MultiplePipelinesMixedVertexState) {
// Basic input state, using slot 0
- utils::ComboVertexStateDescriptor vertexVertexState = MakeVertexState(
- {{4 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor vertexVertexState;
+ MakeVertexState({{4 * sizeof(float), InputStepMode::Vertex, {{0, 0, VertexFormat::Float4}}}},
+ &vertexVertexState);
wgpu::RenderPipeline vertexPipeline =
MakeTestPipeline(vertexVertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
// Instance input state, using slot 1
- utils::ComboVertexStateDescriptor instanceVertexState = MakeVertexState(
- {{0, InputStepMode::Instance, {}},
- {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
+ utils::ComboVertexStateDescriptor instanceVertexState;
+ MakeVertexState({{0, InputStepMode::Instance, {}},
+ {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}},
+ &instanceVertexState);
wgpu::RenderPipeline instancePipeline = MakeTestPipeline(
instanceVertexState, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
@@ -529,16 +539,17 @@
utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 3, 3);
- utils::ComboVertexStateDescriptor vertexState =
- MakeVertexState({{16,
- InputStepMode::Vertex,
- {
- // "****" represents the bytes we'll actually read in the shader.
- {0, 0 /* offset */, VertexFormat::Float4}, // |****|----|----|----|
- {1, 4 /* offset */, VertexFormat::UInt2}, // |****|****|
- {2, 8 /* offset */, VertexFormat::Half4}, // |-----****|
- {3, 0 /* offset */, VertexFormat::Float}, // |****|
- }}});
+ utils::ComboVertexStateDescriptor vertexState;
+ MakeVertexState({{16,
+ InputStepMode::Vertex,
+ {
+ // "****" represents the bytes we'll actually read in the shader.
+ {0, 0 /* offset */, VertexFormat::Float4}, // |****|----|----|----|
+ {1, 4 /* offset */, VertexFormat::UInt2}, // |****|****|
+ {2, 8 /* offset */, VertexFormat::Half4}, // |-----****|
+ {3, 0 /* offset */, VertexFormat::Float}, // |****|
+ }}},
+ &vertexState);
struct Data {
float fvalue;
diff --git a/src/utils/ComboRenderPipelineDescriptor.h b/src/utils/ComboRenderPipelineDescriptor.h
index 067b79e..ce8eb30 100644
--- a/src/utils/ComboRenderPipelineDescriptor.h
+++ b/src/utils/ComboRenderPipelineDescriptor.h
@@ -27,6 +27,11 @@
public:
ComboVertexStateDescriptor();
+ ComboVertexStateDescriptor(const ComboVertexStateDescriptor&) = delete;
+ ComboVertexStateDescriptor& operator=(const ComboVertexStateDescriptor&) = delete;
+ ComboVertexStateDescriptor(ComboVertexStateDescriptor&&) = delete;
+ ComboVertexStateDescriptor& operator=(ComboVertexStateDescriptor&&) = delete;
+
std::array<wgpu::VertexBufferLayoutDescriptor, kMaxVertexBuffers> cVertexBuffers;
std::array<wgpu::VertexAttributeDescriptor, kMaxVertexAttributes> cAttributes;
};
