From 862ebf2696052244ec68e457098279a9e66c4741 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 1 Nov 2022 16:46:43 +0100 Subject: [PATCH 1/6] Added new BUFFER_USAGE_COMBINE_VERTEX_INDEX downlevel flag. WebGL doesn't support buffers that are are used both as vertex and index buffer. --- wgpu-core/src/device/mod.rs | 7 +++++++ wgpu-core/src/resource.rs | 4 +++- wgpu-hal/src/dx11/adapter.rs | 5 +++-- wgpu-hal/src/gles/adapter.rs | 6 ++++++ wgpu-types/src/lib.rs | 5 +++++ 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 57c6b943fc..fc558b1fac 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -590,6 +590,13 @@ impl Device { }); } + if desc + .usage + .contains(wgt::BufferUsages::VERTEX | wgt::BufferUsages::INDEX) + { + self.require_downlevel_flags(wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX)?; + } + let mut usage = conv::map_buffer_usage(desc.usage); if desc.usage.is_empty() { diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index e106188543..a008a1d422 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -1,5 +1,5 @@ use crate::{ - device::{DeviceError, HostMap, MissingFeatures}, + device::{DeviceError, HostMap, MissingDownlevelFlags, MissingFeatures}, hub::{Global, GlobalIdentityHandlerFactory, HalApi, Resource, Token}, id::{AdapterId, DeviceId, SurfaceId, TextureId, Valid}, init_tracker::{BufferInitTracker, TextureInitTracker}, @@ -248,6 +248,8 @@ pub enum CreateBufferError { UsageMismatch(wgt::BufferUsages), #[error("Buffer size {requested} is greater than the maximum buffer size ({maximum})")] MaxBufferSize { requested: u64, maximum: u64 }, + #[error(transparent)] + MissingDownlevelFlags(#[from] MissingDownlevelFlags), } impl Resource for Buffer { diff --git a/wgpu-hal/src/dx11/adapter.rs b/wgpu-hal/src/dx11/adapter.rs index 9ea3243ab8..d8655e6778 100644 --- a/wgpu-hal/src/dx11/adapter.rs +++ b/wgpu-hal/src/dx11/adapter.rs @@ -91,8 +91,9 @@ impl super::Adapter { | wgt::Features::CLEAR_TEXTURE | wgt::Features::TEXTURE_FORMAT_16BIT_NORM | wgt::Features::ADDRESS_MODE_CLAMP_TO_ZERO; - let mut downlevel = - wgt::DownlevelFlags::BASE_VERTEX | wgt::DownlevelFlags::READ_ONLY_DEPTH_STENCIL; + let mut downlevel = wgt::DownlevelFlags::BASE_VERTEX + | wgt::DownlevelFlags::READ_ONLY_DEPTH_STENCIL + | wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX; // Features from queries downlevel.set( diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index 53b11029d0..bda01fe3cd 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -312,6 +312,12 @@ impl super::Adapter { wgt::DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED, !(cfg!(target_arch = "wasm32") || is_angle), ); + // https://registry.khronos.org/webgl/specs/latest/1.0/#5.14.5 + // "A given WebGLBuffer object may only be bound to one of the ARRAY_BUFFER or ELEMENT_ARRAY_BUFFER target in its lifetime." + downlevel_flags.set( + wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX, + !cfg!(target_arch = "wasm32"), + ); let mut features = wgt::Features::empty() | wgt::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index ec5b0dd8a3..91c983255f 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -1087,6 +1087,11 @@ bitflags::bitflags! { /// /// WebGL doesn't support this. const BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED = 1 << 15; + + /// Supports buffers that have both [`BufferUsages::INDEX`] and [`BufferUsages::VERTEX`] set. + /// + /// WebGL doesn't support this. + const BUFFER_USAGE_COMBINE_VERTEX_INDEX = 1 << 16; } } From 8bdce19b9c0291f6bd178057801ec7afb8de3484 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 1 Nov 2022 16:54:06 +0100 Subject: [PATCH 2/6] changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c75a41c40..30a4bf1601 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Bottom level categories: - Convert all `Default` Implementations on Enums to `derive(Default)` - Implement `Default` for `CompositeAlphaMode` - Improve compute shader validation error message. By @haraldreingruber in [#3139](https://github.com/gfx-rs/wgpu/pull/3139) +- New downlevel feature `BUFFER_USAGE_COMBINE_VERTEX_INDEX` to indicate support of buffers that have both usages `VERTEX` and `INDEX` (unsupported on WebGL). By @Wumpf in [#3157](https://github.com/gfx-rs/wgpu/pull/3157) #### WebGPU - Implement `queue_validate_write_buffer` by @jinleili in [#3098](https://github.com/gfx-rs/wgpu/pull/3098) From a6a4e1604d95e81c37f29dfe6a34e36a1d50c65d Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Tue, 1 Nov 2022 21:07:18 +0100 Subject: [PATCH 3/6] rename BUFFER_USAGE_COMBINE_VERTEX_INDEX to UNRESTRICTED_INDEX_BUFFER and use it on buffer copies as well --- CHANGELOG.md | 2 +- wgpu-core/src/command/transfer.rs | 23 +++++++++++++++++++++++ wgpu-core/src/device/mod.rs | 12 ++++++++---- wgpu-hal/src/dx11/adapter.rs | 2 +- wgpu-hal/src/gles/adapter.rs | 5 ++--- wgpu-types/src/lib.rs | 6 ++++-- 6 files changed, 39 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30a4bf1601..f955046adf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ Bottom level categories: - Convert all `Default` Implementations on Enums to `derive(Default)` - Implement `Default` for `CompositeAlphaMode` - Improve compute shader validation error message. By @haraldreingruber in [#3139](https://github.com/gfx-rs/wgpu/pull/3139) -- New downlevel feature `BUFFER_USAGE_COMBINE_VERTEX_INDEX` to indicate support of buffers that have both usages `VERTEX` and `INDEX` (unsupported on WebGL). By @Wumpf in [#3157](https://github.com/gfx-rs/wgpu/pull/3157) +- New downlevel feature `UNRESTRICTED_INDEX_BUFFER` to indicate support for using `INDEX` together with other non-copy/map usages (unsupported on WebGL). By @Wumpf in [#3157](https://github.com/gfx-rs/wgpu/pull/3157) #### WebGPU - Implement `queue_validate_write_buffer` by @jinleili in [#3098](https://github.com/gfx-rs/wgpu/pull/3098) diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index dd46e30a26..ee80dd1bcf 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -541,10 +541,13 @@ impl Global { let hub = A::hub(self); let mut token = Token::root(); + let (device_guard, mut token) = hub.devices.read(&mut token); let (mut cmd_buf_guard, mut token) = hub.command_buffers.write(&mut token); let cmd_buf = CommandBuffer::get_encoder_mut(&mut *cmd_buf_guard, command_encoder_id)?; let (buffer_guard, _) = hub.buffers.read(&mut token); + let device = &device_guard[cmd_buf.device_id.value]; + #[cfg(feature = "trace")] if let Some(ref mut list) = cmd_buf.commands { list.push(TraceCommand::CopyBufferToBuffer { @@ -594,6 +597,26 @@ impl Global { if destination_offset % wgt::COPY_BUFFER_ALIGNMENT != 0 { return Err(TransferError::UnalignedBufferOffset(destination_offset).into()); } + if !device + .downlevel + .flags + .contains(wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER) + && (src_buffer.usage.contains(BufferUsages::INDEX) + || dst_buffer.usage.contains(BufferUsages::INDEX)) + { + let non_index_non_copy_buffer_usages = wgt::BufferUsages::VERTEX + | wgt::BufferUsages::UNIFORM + | wgt::BufferUsages::INDIRECT + | wgt::BufferUsages::STORAGE; + if src_buffer.usage.contains(non_index_non_copy_buffer_usages) + || dst_buffer.usage.contains(non_index_non_copy_buffer_usages) + { + return Err(TransferError::MissingDownlevelFlags(MissingDownlevelFlags( + wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER, + )) + .into()); + } + } let source_end_offset = source_offset + size; let destination_end_offset = destination_offset + size; diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index fc558b1fac..1dfcd1d8d4 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -590,11 +590,15 @@ impl Device { }); } - if desc - .usage - .contains(wgt::BufferUsages::VERTEX | wgt::BufferUsages::INDEX) + if desc.usage.contains(wgt::BufferUsages::INDEX) + && desc.usage.contains( + wgt::BufferUsages::VERTEX + | wgt::BufferUsages::UNIFORM + | wgt::BufferUsages::INDIRECT + | wgt::BufferUsages::STORAGE, + ) { - self.require_downlevel_flags(wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX)?; + self.require_downlevel_flags(wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER)?; } let mut usage = conv::map_buffer_usage(desc.usage); diff --git a/wgpu-hal/src/dx11/adapter.rs b/wgpu-hal/src/dx11/adapter.rs index d8655e6778..d30ba8fa90 100644 --- a/wgpu-hal/src/dx11/adapter.rs +++ b/wgpu-hal/src/dx11/adapter.rs @@ -93,7 +93,7 @@ impl super::Adapter { | wgt::Features::ADDRESS_MODE_CLAMP_TO_ZERO; let mut downlevel = wgt::DownlevelFlags::BASE_VERTEX | wgt::DownlevelFlags::READ_ONLY_DEPTH_STENCIL - | wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX; + | wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER; // Features from queries downlevel.set( diff --git a/wgpu-hal/src/gles/adapter.rs b/wgpu-hal/src/gles/adapter.rs index bda01fe3cd..9b5d357895 100644 --- a/wgpu-hal/src/gles/adapter.rs +++ b/wgpu-hal/src/gles/adapter.rs @@ -312,10 +312,9 @@ impl super::Adapter { wgt::DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED, !(cfg!(target_arch = "wasm32") || is_angle), ); - // https://registry.khronos.org/webgl/specs/latest/1.0/#5.14.5 - // "A given WebGLBuffer object may only be bound to one of the ARRAY_BUFFER or ELEMENT_ARRAY_BUFFER target in its lifetime." + // see https://registry.khronos.org/webgl/specs/latest/2.0/#BUFFER_OBJECT_BINDING downlevel_flags.set( - wgt::DownlevelFlags::BUFFER_USAGE_COMBINE_VERTEX_INDEX, + wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER, !cfg!(target_arch = "wasm32"), ); diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 91c983255f..ccac16210b 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -1088,10 +1088,12 @@ bitflags::bitflags! { /// WebGL doesn't support this. const BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED = 1 << 15; - /// Supports buffers that have both [`BufferUsages::INDEX`] and [`BufferUsages::VERTEX`] set. + /// Supports buffers to combine [`BufferUsages::INDEX`] with usages other than [`BufferUsages::COPY_DST`] and [`BufferUsages::COPY_SRC`]. + /// Furthermore, in absence of this feature it is not allowed to copy index buffers from/to buffers with usages other than + /// [`BufferUsages::INDEX`]/[`BufferUsages::COPY_DST`]/[`BufferUsages::COPY_SRC`]. /// /// WebGL doesn't support this. - const BUFFER_USAGE_COMBINE_VERTEX_INDEX = 1 << 16; + const UNRESTRICTED_INDEX_BUFFER = 1 << 16; } } From caac31159627e0e9e5ba13f8a2bdd0e150df5b95 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Thu, 3 Nov 2022 17:39:04 +0100 Subject: [PATCH 4/6] fix index buffer transfer restrictions --- wgpu-core/src/command/transfer.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index ee80dd1bcf..1c01c89ba8 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -601,15 +601,15 @@ impl Global { .downlevel .flags .contains(wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER) - && (src_buffer.usage.contains(BufferUsages::INDEX) - || dst_buffer.usage.contains(BufferUsages::INDEX)) + && (src_buffer.usage.contains(wgt::BufferUsages::INDEX) + || dst_buffer.usage.contains(wgt::BufferUsages::INDEX)) { - let non_index_non_copy_buffer_usages = wgt::BufferUsages::VERTEX + let forbidden_usages = wgt::BufferUsages::VERTEX | wgt::BufferUsages::UNIFORM | wgt::BufferUsages::INDIRECT | wgt::BufferUsages::STORAGE; - if src_buffer.usage.contains(non_index_non_copy_buffer_usages) - || dst_buffer.usage.contains(non_index_non_copy_buffer_usages) + if src_buffer.usage.intersects(forbidden_usages) + || dst_buffer.usage.intersects(forbidden_usages) { return Err(TransferError::MissingDownlevelFlags(MissingDownlevelFlags( wgt::DownlevelFlags::UNRESTRICTED_INDEX_BUFFER, From 484725ffcd22ebf68ddf3c8ed64fa2d0686c6ddd Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Sat, 5 Nov 2022 23:58:46 +0100 Subject: [PATCH 5/6] fix feature flag comment --- wgpu-types/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index ccac16210b..d48d37456d 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -1063,7 +1063,7 @@ bitflags::bitflags! { /// Supports samplers with anisotropic filtering. Note this isn't actually required by /// WebGPU, the implementation is allowed to completely ignore aniso clamp. This flag is - /// here for native backends so they can comunicate to the user of aniso is enabled. + /// here for native backends so they can communi cate to the user of aniso is enabled. /// /// All backends and all devices support anisotropic filtering. const ANISOTROPIC_FILTERING = 1 << 10; @@ -1089,8 +1089,8 @@ bitflags::bitflags! { const BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED = 1 << 15; /// Supports buffers to combine [`BufferUsages::INDEX`] with usages other than [`BufferUsages::COPY_DST`] and [`BufferUsages::COPY_SRC`]. - /// Furthermore, in absence of this feature it is not allowed to copy index buffers from/to buffers with usages other than - /// [`BufferUsages::INDEX`]/[`BufferUsages::COPY_DST`]/[`BufferUsages::COPY_SRC`]. + /// Furthermore, in absence of this feature it is not allowed to copy index buffers from/to buffers with a set of usage flags containing + /// [`BufferUsages::VERTEX`]/[`BufferUsages::UNIFORM`]/[`BufferUsages::STORAGE`] or [`BufferUsages::INDIRECT`]. /// /// WebGL doesn't support this. const UNRESTRICTED_INDEX_BUFFER = 1 << 16; From 77f7d6f10b69465679c634b86f8a71a766b21641 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 7 Nov 2022 21:35:22 +0100 Subject: [PATCH 6/6] Fix comment Co-authored-by: Connor Fitzgerald --- wgpu-types/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index d48d37456d..f88747f4c2 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -1063,7 +1063,7 @@ bitflags::bitflags! { /// Supports samplers with anisotropic filtering. Note this isn't actually required by /// WebGPU, the implementation is allowed to completely ignore aniso clamp. This flag is - /// here for native backends so they can communi cate to the user of aniso is enabled. + /// here for native backends so they can communicate to the user of aniso is enabled. /// /// All backends and all devices support anisotropic filtering. const ANISOTROPIC_FILTERING = 1 << 10;