Reland "Apply trivial frontend defaults by copy, and recursively"
This is a (clean) reland of commit ba44daa08b94402e1d0ef4add3ff5c43fdd9c248
crbug.com/1516317 couldn't be fixed by clean revert so I will fix
it directly instead. This revert was only needed to attempt to clean revert another CL.
Original change's description:
> 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>
Bug: chromium:1516317
Bug: dawn:2224
Change-Id: I8e49dcec8f075deff351dccc624f40eaba990c4c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/168701
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Auto-Submit: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Austin Eng <enga@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
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 acf47f4..f7dd037 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -1576,8 +1576,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,
@@ -1666,8 +1665,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,
@@ -1766,10 +1764,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 6f9ebfe..6dbf1ea 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.
