Apply trivial frontend defaults by copy, and recursively
struct.WithTrivialFrontendDefaults() makes a member-by-member copy of
the struct, applying defaults where needed and recursing where possible.
Also adds a few missing static_asserts for `type.chained` structs.
Bug: dawn:2224
Change-Id: I5bb00d3c04f9417778114ffe79382266884208f7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/168160
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Auto-Submit: Kai Ninomiya <kainino@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/docs/dawn/codegen.md b/docs/dawn/codegen.md
index ad90e5d..4293887 100644
--- a/docs/dawn/codegen.md
+++ b/docs/dawn/codegen.md
@@ -44,7 +44,7 @@
- `"length"` (default to 1 if not set), a string. Defines length of the array pointed to for pointer arguments. If not set the length is implicitly 1 (so not an array), but otherwise it can be set to the name of another member in the same record that will contain the length of the array (this is heavily used in the `fooCount` `foos` pattern in the API). As a special case `"strlen"` can be used for `const char*` record members to denote that the length should be determined with `strlen`.
- `"optional"` (default to false) a boolean that says whether this member is optional. Member records can be optional if they are pointers (otherwise dawn_wire will always try to dereference them), objects (otherwise dawn_wire will always try to encode their ID and crash), or if they have a `"default"` key. Optional pointers and objects will always default to `nullptr` (unless `"no_default"` is set to `true`).
- `"default"` (optional) a number or string. If set the record member will use that value as default value. Depending on the member's category it can be a number, a string containing a number, or the name of an enum/bitmask value.
- - Dawn implements "trivial defaulting" for enums, similarly to the upstream WebGPU spec's WebIDL: if a zero-valued enum (usually called `Undefined`) is passed in, Dawn applies the default value specified here. See `ApplyTrivialFrontendDefaults()` in `api_structs.h` for how this works.
+ - Dawn implements "trivial defaulting" for enums, similarly to the upstream WebGPU spec's WebIDL: if a zero-valued enum (usually called `Undefined`) is passed in, Dawn applies the default value specified here. See `WithTrivialFrontendDefaults()` in `api_structs.h` for how this works.
- `"wire_is_data_only"` (default to false) a boolean that says whether it is safe to directly return a pointer of this member that is pointing to a piece of memory in the transfer buffer into dawn_wire. To prevent TOCTOU attacks, by default in dawn_wire we must ensure every single value returned to dawn_native a copy of what's in the wire, so `"wire_is_data_only"` is set to true only when the member is data-only and don't impact control flow.
**`"native"`** native types that can be referenced by name in other things.
diff --git a/generator/dawn_json_generator.py b/generator/dawn_json_generator.py
index 078bb73..87d76ba 100644
--- a/generator/dawn_json_generator.py
+++ b/generator/dawn_json_generator.py
@@ -203,11 +203,6 @@
self.default_value = default_value
self.skip_serialize = skip_serialize
- self.requires_struct_defaulting = False
- if self.default_value not in [None, "undefined"] and self.annotation == "value" and \
- self.type.category == "enum" and self.type.hasUndefined:
- self.requires_struct_defaulting = True
-
def set_handle_type(self, handle_type):
assert self.type.dict_name == "ObjectHandle"
self.handle_type = handle_type
@@ -216,6 +211,19 @@
assert self.type.dict_name == "ObjectId"
self.id_type = id_type
+ @property
+ def requires_struct_defaulting(self):
+ if self.annotation != "value":
+ return False
+
+ if self.type.category == "structure":
+ return self.type.any_member_requires_struct_defaulting
+ elif self.type.category == "enum":
+ return (self.type.hasUndefined
+ and self.default_value not in [None, "undefined"])
+ else:
+ return False
+
Method = namedtuple(
'Method', ['name', 'return_type', 'arguments', 'autolock', 'json_data'])
diff --git a/generator/templates/dawn/native/api_structs.cpp b/generator/templates/dawn/native/api_structs.cpp
index 5e11d83..4b4c529 100644
--- a/generator/templates/dawn/native/api_structs.cpp
+++ b/generator/templates/dawn/native/api_structs.cpp
@@ -62,6 +62,12 @@
static_assert(offsetof({{CppType}}, nextInChain) == offsetof({{CType}}, nextInChain),
"offsetof mismatch for {{CppType}}::nextInChain");
{% endif %}
+ {% if type.chained %}
+ static_assert(offsetof({{CppType}}, nextInChain) == offsetof({{CType}}, chain) + offsetof(WGPUChainedStruct, next),
+ "offsetof mismatch for {{CppType}}::nextInChain");
+ static_assert(offsetof({{CppType}}, sType) == offsetof({{CType}}, chain) + offsetof(WGPUChainedStruct, sType),
+ "offsetof mismatch for {{CppType}}::sType");
+ {% endif %}
{% for member in type.members %}
{% set memberName = member.name.camelCase() %}
static_assert(offsetof({{CppType}}, {{memberName}}) == offsetof({{CType}}, {{memberName}}),
@@ -69,13 +75,33 @@
{% endfor %}
{% if type.any_member_requires_struct_defaulting %}
- void {{CppType}}::ApplyTrivialFrontendDefaults() {
- {% for member in type.members if member.requires_struct_defaulting %}
+ {{CppType}} {{CppType}}::WithTrivialFrontendDefaults() const {
+ {{CppType}} copy;
+ {% if type.extensible %}
+ copy.nextInChain = nextInChain;
+ {% endif %}
+ {% if type.chained %}
+ copy.nextInChain = nextInChain;
+ copy.sType = sType;
+ {% endif %}
+ {% for member in type.members %}
{% set memberName = member.name.camelCase() %}
- if ({{memberName}} == {{namespace}}::{{as_cppType(member.type.name)}}::Undefined) {
- {{memberName}} = {{namespace}}::{{as_cppType(member.type.name)}}::{{as_cppEnum(Name(member.default_value))}};
- }
+ {% if member.requires_struct_defaulting %}
+ {% if member.type.category == "structure" %}
+ copy.{{memberName}} = {{memberName}}.WithTrivialFrontendDefaults();
+ {% elif member.type.category == "enum" %}
+ {% set Enum = namespace + "::" + as_cppType(member.type.name) %}
+ copy.{{memberName}} = ({{memberName}} == {{Enum}}::Undefined)
+ ? {{Enum}}::{{as_cppEnum(Name(member.default_value))}}
+ : {{memberName}};
+ {% else %}
+ {{assert(False, "other types do not currently support defaulting")}}
+ {% endif %}
+ {% else %}
+ copy.{{memberName}} = {{memberName}};
+ {% endif %}
{% endfor %}
+ return copy;
}
{% endif %}
bool {{CppType}}::operator==(const {{as_cppType(type.name)}}& rhs) const {
diff --git a/generator/templates/dawn/native/api_structs.h b/generator/templates/dawn/native/api_structs.h
index 481f9b9..ae28708 100644
--- a/generator/templates/dawn/native/api_structs.h
+++ b/generator/templates/dawn/native/api_structs.h
@@ -97,10 +97,12 @@
{% endfor %}
{% if type.any_member_requires_struct_defaulting %}
- // For any enum members with trivial defaulting (where something like
- // "Undefined" is replaced with a default), this method applies all of the
- // defaults for the struct. It must be called in an appropriate place in Dawn.
- void ApplyTrivialFrontendDefaults();
+ // This method makes a copy of the struct, then, for any enum members with trivial
+ // defaulting (where something like "Undefined" is replaced with a default), applies
+ // all of the defaults for the struct, and recursively its by-value substructs (but
+ // NOT by-pointer substructs since they are const*). It must be called in an
+ // appropriate place in Dawn.
+ [[nodiscard]] {{as_cppType(type.name)}} WithTrivialFrontendDefaults() const;
{% endif %}
// Equality operators, mostly for testing. Note that this tests
// strict pointer-pointer equality if the struct contains member pointers.
diff --git a/src/dawn/native/BindGroupLayoutInternal.cpp b/src/dawn/native/BindGroupLayoutInternal.cpp
index 273a930..b609fc4 100644
--- a/src/dawn/native/BindGroupLayoutInternal.cpp
+++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -400,16 +400,14 @@
bindingInfo.sampler = binding->sampler;
} else if (binding->texture.sampleType != wgpu::TextureSampleType::Undefined) {
bindingInfo.bindingType = BindingInfoType::Texture;
- bindingInfo.texture = binding->texture;
- bindingInfo.texture.ApplyTrivialFrontendDefaults();
+ bindingInfo.texture = binding->texture.WithTrivialFrontendDefaults();
if (binding->texture.viewDimension == wgpu::TextureViewDimension::Undefined) {
bindingInfo.texture.viewDimension = wgpu::TextureViewDimension::e2D;
}
} else if (binding->storageTexture.access != wgpu::StorageTextureAccess::Undefined) {
bindingInfo.bindingType = BindingInfoType::StorageTexture;
- bindingInfo.storageTexture = binding->storageTexture;
- bindingInfo.storageTexture.ApplyTrivialFrontendDefaults();
+ bindingInfo.storageTexture = binding->storageTexture.WithTrivialFrontendDefaults();
if (binding->storageTexture.viewDimension == wgpu::TextureViewDimension::Undefined) {
bindingInfo.storageTexture.viewDimension = wgpu::TextureViewDimension::e2D;
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index f7ae4e1..94e6ffd 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -1577,8 +1577,7 @@
void CommandEncoder::APICopyBufferToTexture(const ImageCopyBuffer* source,
const ImageCopyTexture* destinationOrig,
const Extent3D* copySize) {
- ImageCopyTexture destination = *destinationOrig;
- destination.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults();
mEncodingContext.TryEncode(
this,
@@ -1667,8 +1666,7 @@
void CommandEncoder::APICopyTextureToBuffer(const ImageCopyTexture* sourceOrig,
const ImageCopyBuffer* destination,
const Extent3D* copySize) {
- ImageCopyTexture source = *sourceOrig;
- source.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults();
mEncodingContext.TryEncode(
this,
@@ -1767,10 +1765,8 @@
void CommandEncoder::APICopyTextureToTexture(const ImageCopyTexture* sourceOrig,
const ImageCopyTexture* destinationOrig,
const Extent3D* copySize) {
- ImageCopyTexture source = *sourceOrig;
- source.ApplyTrivialFrontendDefaults();
- ImageCopyTexture destination = *destinationOrig;
- destination.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults();
+ ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults();
mEncodingContext.TryEncode(
this,
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index e51370b..5cf0b1c 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -1840,9 +1840,8 @@
SamplerDescriptor descriptor = {};
if (descriptorOrig) {
- descriptor = *descriptorOrig;
+ descriptor = descriptorOrig->WithTrivialFrontendDefaults();
}
- descriptor.ApplyTrivialFrontendDefaults();
if (IsValidationEnabled()) {
DAWN_TRY_CONTEXT(ValidateSamplerDescriptor(this, &descriptor), "validating %s",
@@ -1904,8 +1903,7 @@
ResultOrError<Ref<TextureBase>> DeviceBase::CreateTexture(const TextureDescriptor* descriptorOrig) {
DAWN_TRY(ValidateIsAlive());
- TextureDescriptor rawDescriptor = *descriptorOrig;
- rawDescriptor.ApplyTrivialFrontendDefaults();
+ TextureDescriptor rawDescriptor = descriptorOrig->WithTrivialFrontendDefaults();
UnpackedPtr<TextureDescriptor> descriptor;
if (IsValidationEnabled()) {
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp
index 0d519c9..f83b4c6 100644
--- a/src/dawn/native/Queue.cpp
+++ b/src/dawn/native/Queue.cpp
@@ -449,8 +449,7 @@
size_t dataSize,
const TextureDataLayout& dataLayout,
const Extent3D* writeSize) {
- ImageCopyTexture destination = *destinationOrig;
- destination.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults();
DAWN_TRY(ValidateWriteTexture(&destination, dataSize, dataLayout, writeSize));
@@ -526,10 +525,8 @@
const ImageCopyTexture* destinationOrig,
const Extent3D* copySize,
const CopyTextureForBrowserOptions* options) {
- ImageCopyTexture source = *sourceOrig;
- source.ApplyTrivialFrontendDefaults();
- ImageCopyTexture destination = *destinationOrig;
- destination.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults();
+ ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults();
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY_CONTEXT(
@@ -545,8 +542,7 @@
const ImageCopyTexture* destinationOrig,
const Extent3D* copySize,
const CopyTextureForBrowserOptions* options) {
- ImageCopyTexture destination = *destinationOrig;
- destination.ApplyTrivialFrontendDefaults();
+ ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults();
if (GetDevice()->IsValidationEnabled()) {
DAWN_TRY_CONTEXT(ValidateCopyExternalTextureForBrowser(GetDevice(), source, &destination,
diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp
index b264226..5c2dd51 100644
--- a/src/dawn/native/RenderPipeline.cpp
+++ b/src/dawn/native/RenderPipeline.cpp
@@ -855,8 +855,7 @@
// Make a local copy with defaulting applied, before copying the
// now-defaulted values into mVertexBufferInfos.
- VertexBufferLayout buffer = bufferOrig;
- buffer.ApplyTrivialFrontendDefaults();
+ VertexBufferLayout buffer = bufferOrig.WithTrivialFrontendDefaults();
mVertexBuffersUsed.set(slot);
mVertexBufferInfos[slot].arrayStride = buffer.arrayStride;
@@ -901,8 +900,7 @@
}
}
- mPrimitive = descriptor->primitive;
- mPrimitive.ApplyTrivialFrontendDefaults();
+ mPrimitive = descriptor->primitive.WithTrivialFrontendDefaults();
UnpackedPtr<PrimitiveState> unpackedPrimitive = Unpack(&mPrimitive);
if (auto* depthClipControl = unpackedPrimitive.Get<PrimitiveDepthClipControl>()) {
mUnclippedDepth = depthClipControl->unclippedDepth;
@@ -911,9 +909,7 @@
mMultisample = descriptor->multisample;
if (mAttachmentState->HasDepthStencilAttachment()) {
- mDepthStencil = *descriptor->depthStencil;
- mDepthStencil.stencilFront.ApplyTrivialFrontendDefaults();
- mDepthStencil.stencilBack.ApplyTrivialFrontendDefaults();
+ mDepthStencil = descriptor->depthStencil->WithTrivialFrontendDefaults();
// Reify depth option for stencil-only formats
const Format& format = device->GetValidInternalFormat(mDepthStencil.format);
@@ -959,9 +955,7 @@
mTargets[i] = *target;
if (target->blend != nullptr) {
- mTargetBlend[i] = *target->blend;
- mTargetBlend[i].alpha.ApplyTrivialFrontendDefaults();
- mTargetBlend[i].color.ApplyTrivialFrontendDefaults();
+ mTargetBlend[i] = target->blend->WithTrivialFrontendDefaults();
mTargets[i].blend = &mTargetBlend[i];
}
}
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp
index 197342b..06479e7 100644
--- a/src/dawn/native/Texture.cpp
+++ b/src/dawn/native/Texture.cpp
@@ -645,9 +645,8 @@
TextureViewDescriptor desc = {};
if (descriptor) {
- desc = *descriptor;
+ desc = descriptor->WithTrivialFrontendDefaults();
}
- desc.ApplyTrivialFrontendDefaults();
// The default value for the view dimension depends on the texture's dimension with a
// special case for 2DArray being chosen if texture is 2D but has more than one array layer.
