Revert "Apply trivial frontend defaults by copy, and recursively" This reverts commit ba44daa08b94402e1d0ef4add3ff5c43fdd9c248. Reason for revert: Revert so I can revert https://dawn-review.googlesource.com/c/dawn/+/166821 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> TBR=cwallez@chromium.org,kainino@chromium.org,noreply+kokoro@google.com,dawn-scoped@luci-project-accounts.iam.gserviceaccount.com,lokokung@google.com Change-Id: I60dbcad27ad2297b7000a4587c0d35354aa907f8 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: dawn:2224 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/168720 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Kai Ninomiya <kainino@google.com> Kokoro: Kai Ninomiya <kainino@chromium.org> Commit-Queue: Kai Ninomiya <kainino@chromium.org> Auto-Submit: Kai Ninomiya <kainino@chromium.org>
diff --git a/docs/dawn/codegen.md b/docs/dawn/codegen.md index 4293887..ad90e5d 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 `WithTrivialFrontendDefaults()` 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 `ApplyTrivialFrontendDefaults()` 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 87d76ba..078bb73 100644 --- a/generator/dawn_json_generator.py +++ b/generator/dawn_json_generator.py
@@ -203,6 +203,11 @@ 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 @@ -211,19 +216,6 @@ 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 4b4c529..5e11d83 100644 --- a/generator/templates/dawn/native/api_structs.cpp +++ b/generator/templates/dawn/native/api_structs.cpp
@@ -62,12 +62,6 @@ 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}}), @@ -75,33 +69,13 @@ {% endfor %} {% if type.any_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 %} + void {{CppType}}::ApplyTrivialFrontendDefaults() { + {% for member in type.members if member.requires_struct_defaulting %} {% set memberName = member.name.camelCase() %} - {% 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 %} + if ({{memberName}} == {{namespace}}::{{as_cppType(member.type.name)}}::Undefined) { + {{memberName}} = {{namespace}}::{{as_cppType(member.type.name)}}::{{as_cppEnum(Name(member.default_value))}}; + } {% 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 ae28708..481f9b9 100644 --- a/generator/templates/dawn/native/api_structs.h +++ b/generator/templates/dawn/native/api_structs.h
@@ -97,12 +97,10 @@ {% endfor %} {% if type.any_member_requires_struct_defaulting %} - // 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; + // 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(); {% 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 b609fc4..273a930 100644 --- a/src/dawn/native/BindGroupLayoutInternal.cpp +++ b/src/dawn/native/BindGroupLayoutInternal.cpp
@@ -400,14 +400,16 @@ bindingInfo.sampler = binding->sampler; } else if (binding->texture.sampleType != wgpu::TextureSampleType::Undefined) { bindingInfo.bindingType = BindingInfoType::Texture; - bindingInfo.texture = binding->texture.WithTrivialFrontendDefaults(); + bindingInfo.texture = binding->texture; + bindingInfo.texture.ApplyTrivialFrontendDefaults(); 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.WithTrivialFrontendDefaults(); + bindingInfo.storageTexture = binding->storageTexture; + bindingInfo.storageTexture.ApplyTrivialFrontendDefaults(); 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 94e6ffd..f7ae4e1 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/src/dawn/native/CommandEncoder.cpp
@@ -1577,7 +1577,8 @@ void CommandEncoder::APICopyBufferToTexture(const ImageCopyBuffer* source, const ImageCopyTexture* destinationOrig, const Extent3D* copySize) { - ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture destination = *destinationOrig; + destination.ApplyTrivialFrontendDefaults(); mEncodingContext.TryEncode( this, @@ -1666,7 +1667,8 @@ void CommandEncoder::APICopyTextureToBuffer(const ImageCopyTexture* sourceOrig, const ImageCopyBuffer* destination, const Extent3D* copySize) { - ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture source = *sourceOrig; + source.ApplyTrivialFrontendDefaults(); mEncodingContext.TryEncode( this, @@ -1765,8 +1767,10 @@ void CommandEncoder::APICopyTextureToTexture(const ImageCopyTexture* sourceOrig, const ImageCopyTexture* destinationOrig, const Extent3D* copySize) { - ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults(); - ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture source = *sourceOrig; + source.ApplyTrivialFrontendDefaults(); + ImageCopyTexture destination = *destinationOrig; + destination.ApplyTrivialFrontendDefaults(); mEncodingContext.TryEncode( this,
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 5cf0b1c..e51370b 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp
@@ -1840,8 +1840,9 @@ SamplerDescriptor descriptor = {}; if (descriptorOrig) { - descriptor = descriptorOrig->WithTrivialFrontendDefaults(); + descriptor = *descriptorOrig; } + descriptor.ApplyTrivialFrontendDefaults(); if (IsValidationEnabled()) { DAWN_TRY_CONTEXT(ValidateSamplerDescriptor(this, &descriptor), "validating %s", @@ -1903,7 +1904,8 @@ ResultOrError<Ref<TextureBase>> DeviceBase::CreateTexture(const TextureDescriptor* descriptorOrig) { DAWN_TRY(ValidateIsAlive()); - TextureDescriptor rawDescriptor = descriptorOrig->WithTrivialFrontendDefaults(); + TextureDescriptor rawDescriptor = *descriptorOrig; + rawDescriptor.ApplyTrivialFrontendDefaults(); UnpackedPtr<TextureDescriptor> descriptor; if (IsValidationEnabled()) {
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp index f83b4c6..0d519c9 100644 --- a/src/dawn/native/Queue.cpp +++ b/src/dawn/native/Queue.cpp
@@ -449,7 +449,8 @@ size_t dataSize, const TextureDataLayout& dataLayout, const Extent3D* writeSize) { - ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture destination = *destinationOrig; + destination.ApplyTrivialFrontendDefaults(); DAWN_TRY(ValidateWriteTexture(&destination, dataSize, dataLayout, writeSize)); @@ -525,8 +526,10 @@ const ImageCopyTexture* destinationOrig, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { - ImageCopyTexture source = sourceOrig->WithTrivialFrontendDefaults(); - ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture source = *sourceOrig; + source.ApplyTrivialFrontendDefaults(); + ImageCopyTexture destination = *destinationOrig; + destination.ApplyTrivialFrontendDefaults(); if (GetDevice()->IsValidationEnabled()) { DAWN_TRY_CONTEXT( @@ -542,7 +545,8 @@ const ImageCopyTexture* destinationOrig, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { - ImageCopyTexture destination = destinationOrig->WithTrivialFrontendDefaults(); + ImageCopyTexture destination = *destinationOrig; + destination.ApplyTrivialFrontendDefaults(); 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 5c2dd51..b264226 100644 --- a/src/dawn/native/RenderPipeline.cpp +++ b/src/dawn/native/RenderPipeline.cpp
@@ -855,7 +855,8 @@ // Make a local copy with defaulting applied, before copying the // now-defaulted values into mVertexBufferInfos. - VertexBufferLayout buffer = bufferOrig.WithTrivialFrontendDefaults(); + VertexBufferLayout buffer = bufferOrig; + buffer.ApplyTrivialFrontendDefaults(); mVertexBuffersUsed.set(slot); mVertexBufferInfos[slot].arrayStride = buffer.arrayStride; @@ -900,7 +901,8 @@ } } - mPrimitive = descriptor->primitive.WithTrivialFrontendDefaults(); + mPrimitive = descriptor->primitive; + mPrimitive.ApplyTrivialFrontendDefaults(); UnpackedPtr<PrimitiveState> unpackedPrimitive = Unpack(&mPrimitive); if (auto* depthClipControl = unpackedPrimitive.Get<PrimitiveDepthClipControl>()) { mUnclippedDepth = depthClipControl->unclippedDepth; @@ -909,7 +911,9 @@ mMultisample = descriptor->multisample; if (mAttachmentState->HasDepthStencilAttachment()) { - mDepthStencil = descriptor->depthStencil->WithTrivialFrontendDefaults(); + mDepthStencil = *descriptor->depthStencil; + mDepthStencil.stencilFront.ApplyTrivialFrontendDefaults(); + mDepthStencil.stencilBack.ApplyTrivialFrontendDefaults(); // Reify depth option for stencil-only formats const Format& format = device->GetValidInternalFormat(mDepthStencil.format); @@ -955,7 +959,9 @@ mTargets[i] = *target; if (target->blend != nullptr) { - mTargetBlend[i] = target->blend->WithTrivialFrontendDefaults(); + mTargetBlend[i] = *target->blend; + mTargetBlend[i].alpha.ApplyTrivialFrontendDefaults(); + mTargetBlend[i].color.ApplyTrivialFrontendDefaults(); mTargets[i].blend = &mTargetBlend[i]; } }
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp index 06479e7..197342b 100644 --- a/src/dawn/native/Texture.cpp +++ b/src/dawn/native/Texture.cpp
@@ -645,8 +645,9 @@ TextureViewDescriptor desc = {}; if (descriptor) { - desc = descriptor->WithTrivialFrontendDefaults(); + desc = *descriptor; } + 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.