encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add more validations for input state 

This change sets limit for stride in VertexInputDescriptor and
offset in VertexAttributeDescriptor, and adds validation code
for them.

It also uses existing descriptors to replace redundant definitions.

BUG=dawn:107

Change-Id: Ifbb07f48ec9a5baae8ae8d21865dc384670b759a
Reviewed-on: https://dawn-review.googlesource.com/c/4901
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Yunchao He 2019-02-22 15:08:03 +00:00 committed by Commit Bot service account
parent fda0617505
commit 87bf834109
6 changed files with 92 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/common/Constants.h
View File

@ -22,7 +22,14 @@ static constexpr uint32_t kMaxBindGroups = 4u;
// TODO(cwallez@chromium.org): investigate bindgroup limits
static constexpr uint32_t kMaxBindingsPerGroup = 16u;
static constexpr uint32_t kMaxVertexAttributes = 16u;
// Vulkan has a standalone limit named maxVertexInputAttributeOffset (2047u at least) for vertex
// attribute offset. The limit might be meaningless because Vulkan has another limit named
// maxVertexInputBindingStride (2048u at least). We use maxVertexAttributeEnd (2048u) here to verify
// vertex attribute offset, which equals to maxOffset + smallest size of vertex format (char). We
// may use maxVertexInputStride instead in future.
static constexpr uint32_t kMaxVertexAttributeEnd = 2048u;
static constexpr uint32_t kMaxVertexInputs = 16u;
static constexpr uint32_t kMaxVertexInputStride = 2048u;
static constexpr uint32_t kNumStages = 3;
static constexpr uint32_t kMaxColorAttachments = 4u;
static constexpr uint32_t kTextureRowPitchAlignment = 256u;

28
src/dawn_native/InputState.cpp
View File

@ -95,7 +95,7 @@ namespace dawn_native {
return mAttributesSetMask;
}
const InputStateBase::AttributeInfo& InputStateBase::GetAttribute(uint32_t location) const {
const VertexAttributeDescriptor& InputStateBase::GetAttribute(uint32_t location) const {
ASSERT(mAttributesSetMask[location]);
return mAttributeInfos[location];
}
@ -104,7 +104,7 @@ namespace dawn_native {
return mInputsSetMask;
}
const InputStateBase::InputInfo& InputStateBase::GetInput(uint32_t slot) const {
const VertexInputDescriptor& InputStateBase::GetInput(uint32_t slot) const {
ASSERT(mInputsSetMask[slot]);
return mInputInfos[slot];
}
@ -135,16 +135,24 @@ namespace dawn_native {
HandleError("Binding slot out of bounds");
return;
}
// If attribute->offset is close to 0xFFFFFFFF, the validation below to add
// attribute->offset and VertexFormatSize(attribute->format) might overflow on a
// 32bit machine, then it can pass the validation incorrectly. We need to catch it.
if (attribute->offset >= kMaxVertexAttributeEnd) {
HandleError("Setting attribute offset out of bounds");
return;
}
if (attribute->offset + VertexFormatSize(attribute->format) > kMaxVertexAttributeEnd) {
HandleError("Setting attribute offset out of bounds");
return;
}
if (mAttributesSetMask[attribute->shaderLocation]) {
HandleError("Setting already set attribute");
return;
}
mAttributesSetMask.set(attribute->shaderLocation);
auto& info = mAttributeInfos[attribute->shaderLocation];
info.inputSlot = attribute->inputSlot;
info.offset = attribute->offset;
info.format = attribute->format;
mAttributeInfos[attribute->shaderLocation] = *attribute;
}
void InputStateBuilder::SetInput(const VertexInputDescriptor* input) {
@ -152,15 +160,17 @@ namespace dawn_native {
HandleError("Setting input out of bounds");
return;
}
if (input->stride > kMaxVertexInputStride) {
HandleError("Setting input stride out of bounds");
return;
}
if (mInputsSetMask[input->inputSlot]) {
HandleError("Setting already set input");
return;
}
mInputsSetMask.set(input->inputSlot);
auto& info = mInputInfos[input->inputSlot];
info.stride = input->stride;
info.stepMode = input->stepMode;
mInputInfos[input->inputSlot] = *input;
}
} // namespace dawn_native

23
src/dawn_native/InputState.h
View File

@ -36,27 +36,16 @@ namespace dawn_native {
public:
InputStateBase(InputStateBuilder* builder);
struct AttributeInfo {
uint32_t inputSlot;
dawn::VertexFormat format;
uint32_t offset;
};
struct InputInfo {
uint32_t stride;
dawn::InputStepMode stepMode;
};
const std::bitset<kMaxVertexAttributes>& GetAttributesSetMask() const;
const AttributeInfo& GetAttribute(uint32_t location) const;
const VertexAttributeDescriptor& GetAttribute(uint32_t location) const;
const std::bitset<kMaxVertexInputs>& GetInputsSetMask() const;
const InputInfo& GetInput(uint32_t slot) const;
const VertexInputDescriptor& GetInput(uint32_t slot) const;
private:
std::bitset<kMaxVertexAttributes> mAttributesSetMask;
std::array<AttributeInfo, kMaxVertexAttributes> mAttributeInfos;
std::array<VertexAttributeDescriptor, kMaxVertexAttributes> mAttributeInfos;
std::bitset<kMaxVertexInputs> mInputsSetMask;
std::array<InputInfo, kMaxVertexInputs> mInputInfos;
std::array<VertexInputDescriptor, kMaxVertexInputs> mInputInfos;
};
class InputStateBuilder : public Builder<InputStateBase> {
@ -73,9 +62,9 @@ namespace dawn_native {
InputStateBase* GetResultImpl() override;
std::bitset<kMaxVertexAttributes> mAttributesSetMask;
std::array<InputStateBase::AttributeInfo, kMaxVertexAttributes> mAttributeInfos;
std::array<VertexAttributeDescriptor, kMaxVertexAttributes> mAttributeInfos;
std::bitset<kMaxVertexInputs> mInputsSetMask;
std::array<InputStateBase::InputInfo, kMaxVertexInputs> mInputInfos;
std::array<VertexInputDescriptor, kMaxVertexInputs> mInputInfos;
};
} // namespace dawn_native

4
src/dawn_native/d3d12/InputStateD3D12.cpp
View File

@ -71,7 +71,7 @@ namespace dawn_native { namespace d3d12 {
D3D12_INPUT_ELEMENT_DESC& inputElementDescriptor = mInputElementDescriptors[count++];
const AttributeInfo& attribute = GetAttribute(i);
const VertexAttributeDescriptor& attribute = GetAttribute(i);
// If the HLSL semantic is TEXCOORDN the SemanticName should be "TEXCOORD" and the
// SemanticIndex N
@ -80,7 +80,7 @@ namespace dawn_native { namespace d3d12 {
inputElementDescriptor.Format = VertexFormatType(attribute.format);
inputElementDescriptor.InputSlot = attribute.inputSlot;
const InputInfo& input = GetInput(attribute.inputSlot);
const VertexInputDescriptor& input = GetInput(attribute.inputSlot);
inputElementDescriptor.AlignedByteOffset = attribute.offset;
inputElementDescriptor.InputSlotClass = InputStepModeFunction(input.stepMode);

4
src/dawn_native/metal/InputStateMTL.mm
View File

@ -66,7 +66,7 @@ namespace dawn_native { namespace metal {
if (!attributesSetMask[i]) {
continue;
}
const AttributeInfo& info = GetAttribute(i);
const VertexAttributeDescriptor& info = GetAttribute(i);
auto attribDesc = [MTLVertexAttributeDescriptor new];
attribDesc.format = VertexFormatType(info.format);
@ -77,7 +77,7 @@ namespace dawn_native { namespace metal {
}
for (uint32_t i : IterateBitSet(GetInputsSetMask())) {
const InputInfo& info = GetInput(i);
const VertexInputDescriptor& info = GetInput(i);
auto layoutDesc = [MTLVertexBufferLayoutDescriptor new];
if (info.stride == 0) {

64
src/tests/unittests/validation/InputStateValidationTests.cpp
View File

@ -146,9 +146,9 @@ TEST_F(InputStateTest, AlreadySetInput) {
.GetResult();
}
// Check out of bounds condition on SetInput
TEST_F(InputStateTest, SetInputOutOfBounds) {
// Control case, setting last input
// Check out of bounds condition on input slot
TEST_F(InputStateTest, SetInputSlotOutOfBounds) {
// Control case, setting last input slot
dawn::VertexInputDescriptor input;
input.inputSlot = kMaxVertexInputs - 1;
input.stride = 0;
@ -156,11 +156,25 @@ TEST_F(InputStateTest, SetInputOutOfBounds) {
AssertWillBeSuccess(device.CreateInputStateBuilder()).SetInput(&input).GetResult();
// Test OOB
// Test input slot OOB
input.inputSlot = kMaxVertexInputs;
AssertWillBeError(device.CreateInputStateBuilder()).SetInput(&input).GetResult();
}
// Check out of bounds condition on input stride
TEST_F(InputStateTest, SetInputStrideOutOfBounds) {
// Control case, setting max input stride
dawn::VertexInputDescriptor input;
input.inputSlot = 0;
input.stride = kMaxVertexInputStride;
input.stepMode = dawn::InputStepMode::Vertex;
AssertWillBeSuccess(device.CreateInputStateBuilder()).SetInput(&input).GetResult();
// Test input stride OOB
input.stride = kMaxVertexInputStride + 1;
AssertWillBeError(device.CreateInputStateBuilder()).SetInput(&input).GetResult();
}
// Test that we cannot set an already set attribute
TEST_F(InputStateTest, AlreadySetAttribute) {
// Control case, setting last attribute
@ -183,9 +197,9 @@ TEST_F(InputStateTest, AlreadySetAttribute) {
.GetResult();
}
// Check out of bounds condition on SetAttribute
TEST_F(InputStateTest, SetAttributeOutOfBounds) {
// Control case, setting last attribute
// Check out of bounds condition on attribute shader location
TEST_F(InputStateTest, SetAttributeLocationOutOfBounds) {
// Control case, setting last attribute shader location
dawn::VertexAttributeDescriptor attribute;
attribute.shaderLocation = kMaxVertexAttributes - 1;
attribute.inputSlot = 0;
@ -197,7 +211,7 @@ TEST_F(InputStateTest, SetAttributeOutOfBounds) {
.SetAttribute(&attribute)
.GetResult();
// Test OOB
// Test attribute location OOB
attribute.shaderLocation = kMaxVertexAttributes;
AssertWillBeError(device.CreateInputStateBuilder())
.SetInput(&kBaseInput)
@ -205,6 +219,40 @@ TEST_F(InputStateTest, SetAttributeOutOfBounds) {
.GetResult();
}
// Check attribute offset out of bounds
TEST_F(InputStateTest, SetAttributeOffsetOutOfBounds) {
// Control case, setting max attribute offset for FloatR32 vertex format
dawn::VertexAttributeDescriptor attribute;
attribute.shaderLocation = 0;
attribute.inputSlot = 0;
attribute.offset = kMaxVertexAttributeEnd - sizeof(dawn::VertexFormat::FloatR32);
attribute.format = dawn::VertexFormat::FloatR32;
AssertWillBeSuccess(device.CreateInputStateBuilder())
.SetInput(&kBaseInput)
.SetAttribute(&attribute)
.GetResult();
// Test attribute offset out of bounds
attribute.offset = kMaxVertexAttributeEnd - 1;
AssertWillBeError(device.CreateInputStateBuilder())
.SetInput(&kBaseInput)
.SetAttribute(&attribute)
.GetResult();
}
// Check attribute offset overflow
TEST_F(InputStateTest, SetAttributeOffsetOverflow) {
dawn::VertexAttributeDescriptor attribute;
attribute.shaderLocation = 0;
attribute.inputSlot = 0;
attribute.offset = std::numeric_limits<uint32_t>::max();
attribute.format = dawn::VertexFormat::FloatR32;
AssertWillBeError(device.CreateInputStateBuilder())
.SetInput(&kBaseInput)
.SetAttribute(&attribute)
.GetResult();
}
// Check that all attributes must be backed by an input
TEST_F(InputStateTest, RequireInputForAttribute) {
// Control case