webgpu.h: Re-number enums, with defaulting: SamplerDescriptor
Trivial defaulting (matching what's defined in the upstream WebIDL) is
done via a new method .ApplyTrivialFrontendDefaults() called in various
places in the frontend as needed.
Part 1 implements the following, which occur directly in CreateSampler:
- SamplerDescriptor.addressMode[UVW]: AddressMode = ClampToEdge
- SamplerDescriptor.{min,mag}Filter: FilterMode = Nearest
- SamplerDescriptor.mipmapFilter: MipmapFilterMode = Nearest
Spot tests are added for all of these. They won't necessarily catch if
the defaulting is _wrong_, but should crash if it is _missing_.
Bug: dawn:2224
Change-Id: Ib325274c2e29cf822a6e3664412495b0c49ad7f0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/165340
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Kai Ninomiya <kainino@chromium.org>
diff --git a/docs/dawn/codegen.md b/docs/dawn/codegen.md
index a677c34..ad90e5d 100644
--- a/docs/dawn/codegen.md
+++ b/docs/dawn/codegen.md
@@ -44,6 +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.
- `"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 f9c900c..0fedc0a 100644
--- a/generator/dawn_json_generator.py
+++ b/generator/dawn_json_generator.py
@@ -117,17 +117,22 @@
Type.__init__(self, name, json_data)
self.values = []
+ self.hasUndefined = False
self.contiguousFromZero = True
lastValue = -1
for m in self.json_data['values']:
if not is_enabled(m):
continue
value = m['value']
+ name = m['name']
+ if name == "undefined":
+ assert value == 0
+ self.hasUndefined = True
if value != lastValue + 1:
self.contiguousFromZero = False
lastValue = value
self.values.append(
- EnumValue(Name(m['name']), value, m.get('valid', True), m))
+ EnumValue(Name(name), value, m.get('valid', True), m))
# Assert that all values are unique in enums
all_values = set()
@@ -198,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
@@ -297,6 +307,11 @@
return True
return False
+ @property
+ def any_member_requires_struct_defaulting(self):
+ return any(member.requires_struct_defaulting
+ for member in self.members)
+
class ConstantDefinition():
def __init__(self, is_enabled, name, json_data):
diff --git a/generator/templates/api_cpp.h b/generator/templates/api_cpp.h
index 1f9ca0a..1335065 100644
--- a/generator/templates/api_cpp.h
+++ b/generator/templates/api_cpp.h
@@ -188,7 +188,7 @@
CType mHandle = nullptr;
};
-{% macro render_cpp_default_value(member, is_struct=True, force_default=False) -%}
+{% macro render_cpp_default_value(member, is_struct, force_default=False) -%}
{%- if member.json_data.get("no_default", false) -%}
{%- elif member.annotation in ["*", "const*"] and member.optional or member.default_value == "nullptr" -%}
{{" "}}= nullptr
@@ -280,7 +280,7 @@
ChainedStruct{{Out}} {{const}} * nextInChain = nullptr;
{% endif %}
{% for member in type.members %}
- {% set member_declaration = as_annotated_cppType(member, type.has_free_members_function) + render_cpp_default_value(member, False, type.has_free_members_function) %}
+ {% set member_declaration = as_annotated_cppType(member, type.has_free_members_function) + render_cpp_default_value(member, True, type.has_free_members_function) %}
{% if type.chained and loop.first %}
//* Align the first member after ChainedStruct to match the C struct layout.
//* It has to be aligned both to its natural and ChainedStruct's alignment.
diff --git a/generator/templates/dawn/native/api_structs.cpp b/generator/templates/dawn/native/api_structs.cpp
index bd0f3a3..5e11d83 100644
--- a/generator/templates/dawn/native/api_structs.cpp
+++ b/generator/templates/dawn/native/api_structs.cpp
@@ -68,6 +68,16 @@
"offsetof mismatch for {{CppType}}::{{memberName}}");
{% endfor %}
+ {% if type.any_member_requires_struct_defaulting %}
+ void {{CppType}}::ApplyTrivialFrontendDefaults() {
+ {% for member in type.members if member.requires_struct_defaulting %}
+ {% 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))}};
+ }
+ {% endfor %}
+ }
+ {% endif %}
bool {{CppType}}::operator==(const {{as_cppType(type.name)}}& rhs) const {
return {% if type.extensible or type.chained -%}
(nextInChain == rhs.nextInChain) &&
diff --git a/generator/templates/dawn/native/api_structs.h b/generator/templates/dawn/native/api_structs.h
index f484bcd..481f9b9 100644
--- a/generator/templates/dawn/native/api_structs.h
+++ b/generator/templates/dawn/native/api_structs.h
@@ -96,6 +96,12 @@
{% endif %}
{% 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();
+ {% endif %}
// Equality operators, mostly for testing. Note that this tests
// strict pointer-pointer equality if the struct contains member pointers.
bool operator==(const {{as_cppType(type.name)}}& rhs) const;
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index befb134..f452a40 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -226,9 +226,10 @@
"address mode": {
"category": "enum",
"values": [
- {"value": 0, "name": "repeat"},
- {"value": 1, "name": "mirror repeat"},
- {"value": 2, "name": "clamp to edge"}
+ {"value": 0, "name": "undefined", "jsrepr": "undefined"},
+ {"value": 1, "name": "clamp to edge"},
+ {"value": 2, "name": "repeat"},
+ {"value": 3, "name": "mirror repeat"}
]
},
"backend type": {
@@ -1976,8 +1977,9 @@
"filter mode": {
"category": "enum",
"values": [
- {"value": 0, "name": "nearest"},
- {"value": 1, "name": "linear"}
+ {"value": 0, "name": "undefined", "jsrepr": "undefined"},
+ {"value": 1, "name": "nearest"},
+ {"value": 2, "name": "linear"}
]
},
"float": {
@@ -2199,8 +2201,9 @@
"mipmap filter mode": {
"category": "enum",
"values": [
- {"value": 0, "name": "nearest"},
- {"value": 1, "name": "linear"}
+ {"value": 0, "name": "undefined", "jsrepr": "undefined"},
+ {"value": 1, "name": "nearest"},
+ {"value": 2, "name": "linear"}
]
},
"store op": {
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index a23503b..4892f58 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -1822,14 +1822,21 @@
return CreateUninitializedRenderPipelineImpl(Unpack(&appliedDescriptor));
}
-ResultOrError<Ref<SamplerBase>> DeviceBase::CreateSampler(const SamplerDescriptor* descriptor) {
- const SamplerDescriptor defaultDescriptor = {};
+ResultOrError<Ref<SamplerBase>> DeviceBase::CreateSampler(const SamplerDescriptor* descriptorOrig) {
DAWN_TRY(ValidateIsAlive());
- descriptor = descriptor != nullptr ? descriptor : &defaultDescriptor;
- if (IsValidationEnabled()) {
- DAWN_TRY_CONTEXT(ValidateSamplerDescriptor(this, descriptor), "validating %s", descriptor);
+
+ SamplerDescriptor descriptor = {};
+ if (descriptorOrig) {
+ descriptor = *descriptorOrig;
}
- return GetOrCreateSampler(descriptor);
+ descriptor.ApplyTrivialFrontendDefaults();
+
+ if (IsValidationEnabled()) {
+ DAWN_TRY_CONTEXT(ValidateSamplerDescriptor(this, &descriptor), "validating %s",
+ &descriptor);
+ }
+
+ return GetOrCreateSampler(&descriptor);
}
ResultOrError<Ref<ShaderModuleBase>> DeviceBase::CreateShaderModule(
diff --git a/src/dawn/native/d3d11/SamplerD3D11.cpp b/src/dawn/native/d3d11/SamplerD3D11.cpp
index d32186f..118c802 100644
--- a/src/dawn/native/d3d11/SamplerD3D11.cpp
+++ b/src/dawn/native/d3d11/SamplerD3D11.cpp
@@ -45,7 +45,10 @@
return D3D11_TEXTURE_ADDRESS_MIRROR;
case wgpu::AddressMode::ClampToEdge:
return D3D11_TEXTURE_ADDRESS_CLAMP;
+ case wgpu::AddressMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
D3D11_FILTER_TYPE D3D11FilterType(wgpu::FilterMode mode) {
@@ -54,7 +57,10 @@
return D3D11_FILTER_TYPE_POINT;
case wgpu::FilterMode::Linear:
return D3D11_FILTER_TYPE_LINEAR;
+ case wgpu::FilterMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
D3D11_FILTER_TYPE D3D11MipmapFilterType(wgpu::MipmapFilterMode mode) {
@@ -63,7 +69,10 @@
return D3D11_FILTER_TYPE_POINT;
case wgpu::MipmapFilterMode::Linear:
return D3D11_FILTER_TYPE_LINEAR;
+ case wgpu::MipmapFilterMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
} // namespace
diff --git a/src/dawn/native/d3d12/SamplerD3D12.cpp b/src/dawn/native/d3d12/SamplerD3D12.cpp
index 4188a2b..a55b077 100644
--- a/src/dawn/native/d3d12/SamplerD3D12.cpp
+++ b/src/dawn/native/d3d12/SamplerD3D12.cpp
@@ -43,7 +43,10 @@
return D3D12_TEXTURE_ADDRESS_MODE_MIRROR;
case wgpu::AddressMode::ClampToEdge:
return D3D12_TEXTURE_ADDRESS_MODE_CLAMP;
+ case wgpu::AddressMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
} // namespace
@@ -62,6 +65,8 @@
case wgpu::FilterMode::Linear:
minFilter = D3D12_FILTER_TYPE_LINEAR;
break;
+ case wgpu::FilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
D3D12_FILTER_TYPE magFilter;
@@ -72,6 +77,8 @@
case wgpu::FilterMode::Linear:
magFilter = D3D12_FILTER_TYPE_LINEAR;
break;
+ case wgpu::FilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
D3D12_FILTER_TYPE mipmapFilter;
@@ -82,6 +89,8 @@
case wgpu::MipmapFilterMode::Linear:
mipmapFilter = D3D12_FILTER_TYPE_LINEAR;
break;
+ case wgpu::MipmapFilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
D3D12_FILTER_REDUCTION_TYPE reduction = descriptor->compare == wgpu::CompareFunction::Undefined
diff --git a/src/dawn/native/metal/SamplerMTL.mm b/src/dawn/native/metal/SamplerMTL.mm
index 48d212b..b79aba1f 100644
--- a/src/dawn/native/metal/SamplerMTL.mm
+++ b/src/dawn/native/metal/SamplerMTL.mm
@@ -39,7 +39,10 @@
return MTLSamplerMinMagFilterNearest;
case wgpu::FilterMode::Linear:
return MTLSamplerMinMagFilterLinear;
+ case wgpu::FilterMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
MTLSamplerMipFilter FilterModeToMipFilter(wgpu::MipmapFilterMode mode) {
@@ -48,7 +51,10 @@
return MTLSamplerMipFilterNearest;
case wgpu::MipmapFilterMode::Linear:
return MTLSamplerMipFilterLinear;
+ case wgpu::MipmapFilterMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
MTLSamplerAddressMode AddressMode(wgpu::AddressMode mode) {
@@ -59,7 +65,10 @@
return MTLSamplerAddressModeMirrorRepeat;
case wgpu::AddressMode::ClampToEdge:
return MTLSamplerAddressModeClampToEdge;
+ case wgpu::AddressMode::Undefined:
+ break;
}
+ DAWN_UNREACHABLE();
}
} // namespace
diff --git a/src/dawn/native/opengl/SamplerGL.cpp b/src/dawn/native/opengl/SamplerGL.cpp
index 20123b5..7269ada 100644
--- a/src/dawn/native/opengl/SamplerGL.cpp
+++ b/src/dawn/native/opengl/SamplerGL.cpp
@@ -40,6 +40,8 @@
return GL_NEAREST;
case wgpu::FilterMode::Linear:
return GL_LINEAR;
+ case wgpu::FilterMode::Undefined:
+ break;
}
DAWN_UNREACHABLE();
}
@@ -52,6 +54,8 @@
return GL_NEAREST_MIPMAP_NEAREST;
case wgpu::MipmapFilterMode::Linear:
return GL_NEAREST_MIPMAP_LINEAR;
+ case wgpu::MipmapFilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
case wgpu::FilterMode::Linear:
switch (mipMapFilter) {
@@ -59,7 +63,11 @@
return GL_LINEAR_MIPMAP_NEAREST;
case wgpu::MipmapFilterMode::Linear:
return GL_LINEAR_MIPMAP_LINEAR;
+ case wgpu::MipmapFilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
+ case wgpu::FilterMode::Undefined:
+ DAWN_UNREACHABLE();
}
DAWN_UNREACHABLE();
}
@@ -72,6 +80,8 @@
return GL_MIRRORED_REPEAT;
case wgpu::AddressMode::ClampToEdge:
return GL_CLAMP_TO_EDGE;
+ case wgpu::AddressMode::Undefined:
+ break;
}
DAWN_UNREACHABLE();
}
diff --git a/src/dawn/native/vulkan/SamplerVk.cpp b/src/dawn/native/vulkan/SamplerVk.cpp
index 7c1a83c..eff581c 100644
--- a/src/dawn/native/vulkan/SamplerVk.cpp
+++ b/src/dawn/native/vulkan/SamplerVk.cpp
@@ -45,6 +45,8 @@
return VK_SAMPLER_ADDRESS_MODE_MIRRORED_REPEAT;
case wgpu::AddressMode::ClampToEdge:
return VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE;
+ case wgpu::AddressMode::Undefined:
+ break;
}
DAWN_UNREACHABLE();
}
@@ -55,6 +57,8 @@
return VK_FILTER_LINEAR;
case wgpu::FilterMode::Nearest:
return VK_FILTER_NEAREST;
+ case wgpu::FilterMode::Undefined:
+ break;
}
DAWN_UNREACHABLE();
}
@@ -65,6 +69,8 @@
return VK_SAMPLER_MIPMAP_MODE_LINEAR;
case wgpu::MipmapFilterMode::Nearest:
return VK_SAMPLER_MIPMAP_MODE_NEAREST;
+ case wgpu::MipmapFilterMode::Undefined:
+ break;
}
DAWN_UNREACHABLE();
}
diff --git a/src/dawn/tests/end2end/TextureViewTests.cpp b/src/dawn/tests/end2end/TextureViewTests.cpp
index d111a27..f99776e 100644
--- a/src/dawn/tests/end2end/TextureViewTests.cpp
+++ b/src/dawn/tests/end2end/TextureViewTests.cpp
@@ -133,17 +133,14 @@
mRenderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
- wgpu::FilterMode kFilterMode = wgpu::FilterMode::Nearest;
- wgpu::MipmapFilterMode kMipmapFilterMode = wgpu::MipmapFilterMode::Nearest;
- wgpu::AddressMode kAddressMode = wgpu::AddressMode::ClampToEdge;
-
wgpu::SamplerDescriptor samplerDescriptor = {};
- samplerDescriptor.minFilter = kFilterMode;
- samplerDescriptor.magFilter = kFilterMode;
- samplerDescriptor.mipmapFilter = kMipmapFilterMode;
- samplerDescriptor.addressModeU = kAddressMode;
- samplerDescriptor.addressModeV = kAddressMode;
- samplerDescriptor.addressModeW = kAddressMode;
+ // (Off-topic) spot-test for defaulting of these six fields.
+ samplerDescriptor.minFilter = wgpu::FilterMode::Undefined;
+ samplerDescriptor.magFilter = wgpu::FilterMode::Undefined;
+ samplerDescriptor.mipmapFilter = wgpu::MipmapFilterMode::Undefined;
+ samplerDescriptor.addressModeU = wgpu::AddressMode::Undefined;
+ samplerDescriptor.addressModeV = wgpu::AddressMode::Undefined;
+ samplerDescriptor.addressModeW = wgpu::AddressMode::Undefined;
mSampler = device.CreateSampler(&samplerDescriptor);
mVSModule = CreateDefaultVertexShaderModule(device);
