archive-gh-me/wgpu
1
0
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2025-12-08 21:26:17 +00:00
Code Issues Projects Releases Wiki Activity

Revert "Restore unintentional support for zero-size buffers"

Browse Source 
This reverts commit c0a580d6f0343a725b3defa8be4fdf0a9691eaad.
This commit is contained in:
Andy Leiserson 2025-07-09 13:05:55 -07:00 committed by Connor Fitzgerald
parent e5b03ffa1d
commit b0527800a1
16 changed files with 101 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
wgpu-core/src/binding_model.rs
View File

@ -106,7 +106,7 @@ pub enum BindingError {
binding_size: u64,
buffer_size: u64,
},
#[error("Buffer {buffer}: Binding offset {offset} is greater than buffer size {buffer_size}")]
#[error("Buffer {buffer}: Binding offset {offset} is greater than or equal to buffer size {buffer_size}")]
BindingOffsetTooLarge {
buffer: ResourceErrorIdent,
offset: wgt::BufferAddress,

30
wgpu-core/src/command/bundle.rs
View File

@ -93,7 +93,6 @@ use core::{
use arrayvec::ArrayVec;
use thiserror::Error;
use wgpu_hal::ShouldBeNonZeroExt;
use wgt::error::{ErrorType, WebGpuError};
use crate::{
@ -505,7 +504,7 @@ impl RenderBundleEncoder {
buffer_id,
index_format,
offset,
size: size.map(NonZeroU64::get),
size,
});
}
}
@ -610,7 +609,7 @@ fn set_index_buffer(
buffer_id: id::Id<id::markers::Buffer>,
index_format: wgt::IndexFormat,
offset: u64,
size: Option<wgt::BufferSizeOrZero>,
size: Option<NonZeroU64>,
) -> Result<(), RenderBundleErrorInner> {
let buffer = buffer_guard.get(buffer_id).get()?;
@ -642,7 +641,7 @@ fn set_vertex_buffer(
slot: u32,
buffer_id: id::Id<id::markers::Buffer>,
offset: u64,
size: Option<wgt::BufferSizeOrZero>,
size: Option<NonZeroU64>,
) -> Result<(), RenderBundleErrorInner> {
let max_vertex_buffers = state.device.limits.max_vertex_buffers;
if slot >= max_vertex_buffers {
@ -1167,8 +1166,11 @@ impl IndexState {
.range
.end
.checked_sub(self.range.start)
.filter(|_| self.range.end <= self.buffer.size)
.expect("index range must be contained in buffer");
.and_then(wgt::BufferSize::new);
assert!(
self.range.end <= self.buffer.size && binding_size.is_some(),
"index buffer range must have non-zero size and be contained in buffer",
);
if self.is_dirty {
self.is_dirty = false;
@ -1176,7 +1178,7 @@ impl IndexState {
buffer: self.buffer.clone(),
index_format: self.format,
offset: self.range.start,
size: Some(binding_size),
size: binding_size,
})
} else {
None
@ -1219,12 +1221,16 @@ impl VertexState {
///
/// `slot` is the index of the vertex buffer slot that `self` tracks.
fn flush(&mut self, slot: u32) -> Option<ArcRenderCommand> {
// This was all checked before, but let's check again just in case.
let binding_size = self
.range
.end
.checked_sub(self.range.start)
.filter(|_| self.range.end <= self.buffer.size)
.expect("vertex range must be contained in buffer");
.and_then(wgt::BufferSize::new);
assert!(
self.range.end <= self.buffer.size && binding_size.is_some(),
"vertex buffer range must have non-zero size and be contained in buffer",
);
if self.is_dirty {
self.is_dirty = false;
@ -1232,7 +1238,7 @@ impl VertexState {
slot,
buffer: self.buffer.clone(),
offset: self.range.start,
size: Some(binding_size),
size: binding_size,
})
} else {
None
@ -1596,7 +1602,7 @@ where
pub mod bundle_ffi {
use super::{RenderBundleEncoder, RenderCommand};
use crate::{id, RawString};
use core::{convert::TryInto, num::NonZeroU64, slice};
use core::{convert::TryInto, slice};
use wgt::{BufferAddress, BufferSize, DynamicOffset, IndexFormat};
/// # Safety
@ -1655,7 +1661,7 @@ pub mod bundle_ffi {
slot,
buffer_id,
offset,
size: size.map(NonZeroU64::get),
size,
});
}

19
wgpu-core/src/command/render.rs
View File

@ -1,17 +1,12 @@
use alloc::{borrow::Cow, sync::Arc, vec::Vec};
use core::{
fmt,
num::{NonZeroU32, NonZeroU64},
str,
};
use hal::ShouldBeNonZeroExt;
use core::{fmt, num::NonZeroU32, str};
use arrayvec::ArrayVec;
use thiserror::Error;
use wgt::{
error::{ErrorType, WebGpuError},
BufferAddress, BufferSize, BufferSizeOrZero, BufferUsages, Color, DynamicOffset, IndexFormat,
ShaderStages, TextureSelector, TextureUsages, TextureViewDimension, VertexStepMode,
BufferAddress, BufferSize, BufferUsages, Color, DynamicOffset, IndexFormat, ShaderStages,
TextureSelector, TextureUsages, TextureViewDimension, VertexStepMode,
};
use crate::command::{
@ -2338,7 +2333,7 @@ fn set_index_buffer(
buffer: Arc<crate::resource::Buffer>,
index_format: IndexFormat,
offset: u64,
size: Option<BufferSizeOrZero>,
size: Option<BufferSize>,
) -> Result<(), RenderPassErrorInner> {
api_log!("RenderPass::set_index_buffer {}", buffer.error_ident());
@ -2378,7 +2373,7 @@ fn set_vertex_buffer(
slot: u32,
buffer: Arc<crate::resource::Buffer>,
offset: u64,
size: Option<BufferSizeOrZero>,
size: Option<BufferSize>,
) -> Result<(), RenderPassErrorInner> {
api_log!(
"RenderPass::set_vertex_buffer {slot} {}",
@ -3089,7 +3084,7 @@ impl Global {
buffer: pass_try!(base, scope, self.resolve_render_pass_buffer_id(buffer_id)),
index_format,
offset,
size: size.map(NonZeroU64::get),
size,
});
Ok(())
@ -3110,7 +3105,7 @@ impl Global {
slot,
buffer: pass_try!(base, scope, self.resolve_render_pass_buffer_id(buffer_id)),
offset,
size: size.map(NonZeroU64::get),
size,
});
Ok(())

21
wgpu-core/src/command/render_command.rs
View File

@ -1,6 +1,6 @@
use alloc::sync::Arc;
use wgt::{BufferAddress, BufferSizeOrZero, Color};
use wgt::{BufferAddress, BufferSize, Color};
use super::{Rect, RenderBundle};
use crate::{
@ -24,13 +24,13 @@ pub enum RenderCommand {
buffer_id: id::BufferId,
index_format: wgt::IndexFormat,
offset: BufferAddress,
size: Option<BufferSizeOrZero>,
size: Option<BufferSize>,
},
SetVertexBuffer {
slot: u32,
buffer_id: id::BufferId,
offset: BufferAddress,
size: Option<BufferSizeOrZero>,
size: Option<BufferSize>,
},
SetBlendConstant(Color),
SetStencilReference(u32),
@ -418,18 +418,21 @@ pub enum ArcRenderCommand {
offset: BufferAddress,
// For a render pass, this reflects the argument passed by the
// application, which may be `None`. For a finished render bundle, this
// reflects the validated size of the binding, and will be populated
// even in the case that the application omitted the size.
size: Option<BufferSizeOrZero>,
// application, which may be `None`. For a render bundle, this reflects
// the validated size of the binding, and will be populated even in the
// case that the application omitted the size.
size: Option<BufferSize>,
},
SetVertexBuffer {
slot: u32,
buffer: Arc<Buffer>,
offset: BufferAddress,
// See comment in `SetIndexBuffer`.
size: Option<BufferSizeOrZero>,
// For a render pass, this reflects the argument passed by the
// application, which may be `None`. For a render bundle, this reflects
// the validated size of the binding, and will be populated even in the
// case that the application omitted the size.
size: Option<BufferSize>,
},
SetBlendConstant(Color),
SetStencilReference(u32),

5
wgpu-core/src/device/resource.rs
View File

@ -8,10 +8,9 @@ use alloc::{
use core::{
fmt,
mem::{self, ManuallyDrop},
num::{NonZeroU32, NonZeroU64},
num::NonZeroU32,
sync::atomic::{AtomicBool, Ordering},
};
use hal::ShouldBeNonZeroExt;
use arrayvec::ArrayVec;
use bitflags::Flags;
@ -2198,7 +2197,7 @@ impl Device {
buffer.check_usage(pub_usage)?;
let bb = buffer.binding(bb.offset, bb.size.map(NonZeroU64::get), snatch_guard)?;
let bb = buffer.binding(bb.offset, bb.size, snatch_guard)?;
let bind_size = bb.size.get();
if bind_size > range_limit as u64 {

51
wgpu-core/src/resource.rs
View File

@ -490,32 +490,35 @@ impl Buffer {
/// If `size` is `None`, then the remainder of the buffer starting from
/// `offset` is used.
///
/// If the binding would overflow the buffer, then an error is returned.
///
/// Zero-size bindings are permitted here for historical reasons. Although
/// zero-size bindings are permitted by WebGPU, they are not permitted by
/// some backends. See [`Buffer::binding`] and
/// [#3170](https://github.com/gfx-rs/wgpu/issues/3170).
/// If the binding would overflow the buffer or is empty (see
/// [`hal::BufferBinding`]), then an error is returned.
pub fn resolve_binding_size(
&self,
offset: wgt::BufferAddress,
binding_size: Option<wgt::BufferSizeOrZero>,
) -> Result<wgt::BufferSizeOrZero, BindingError> {
binding_size: Option<wgt::BufferSize>,
) -> Result<wgt::BufferSize, BindingError> {
let buffer_size = self.size;
match binding_size {
Some(binding_size) => match offset.checked_add(binding_size) {
Some(end) if end <= buffer_size => Ok(binding_size),
_ => Err(BindingError::BindingRangeTooLarge {
buffer: self.error_ident(),
offset,
binding_size,
buffer_size,
}),
},
Some(binding_size) => {
match offset.checked_add(binding_size.get()) {
// `binding_size` is not zero which means `end == buffer_size` is ok.
Some(end) if end <= buffer_size => Ok(binding_size),
_ => Err(BindingError::BindingRangeTooLarge {
buffer: self.error_ident(),
offset,
binding_size: binding_size.get(),
buffer_size,
}),
}
}
None => {
// We require that `buffer_size - offset` converts to
// `BufferSize` (`NonZeroU64`) because bindings must not be
// empty.
buffer_size
.checked_sub(offset)
.and_then(wgt::BufferSize::new)
.ok_or_else(|| BindingError::BindingOffsetTooLarge {
buffer: self.error_ident(),
offset,
@ -531,20 +534,12 @@ impl Buffer {
/// If `size` is `None`, then the remainder of the buffer starting from
/// `offset` is used.
///
/// If the binding would overflow the buffer, then an error is returned.
///
/// Zero-size bindings are permitted here for historical reasons. Although
/// zero-size bindings are permitted by WebGPU, they are not permitted by
/// some backends. Previous documentation for `hal::BufferBinding`
/// disallowed zero-size bindings, but this restriction was not honored
/// elsewhere in the code. Zero-size bindings need to be quashed or remapped
/// to a non-zero size, either universally in wgpu-core, or in specific
/// backends that do not support them. See
/// [#3170](https://github.com/gfx-rs/wgpu/issues/3170).
/// If the binding would overflow the buffer or is empty (see
/// [`hal::BufferBinding`]), then an error is returned.
pub fn binding<'a>(
&'a self,
offset: wgt::BufferAddress,
binding_size: Option<wgt::BufferSizeOrZero>,
binding_size: Option<wgt::BufferSize>,
snatch_guard: &'a SnatchGuard,
) -> Result<hal::BufferBinding<'a, dyn hal::DynBuffer>, BindingError> {
let buf_raw = self.try_raw(snatch_guard)?;

6
wgpu-hal/examples/halmark/main.rs
View File

@ -447,7 +447,11 @@ impl<A: hal::Api> Example<A> {
let global_group = {
let global_buffer_binding = unsafe {
// SAFETY: This is the same size that was specified for buffer creation.
hal::BufferBinding::new_unchecked(&global_buffer, 0, global_buffer_desc.size)
hal::BufferBinding::new_unchecked(
&global_buffer,
0,
global_buffer_desc.size.try_into().unwrap(),
)
};
let texture_binding = hal::TextureBinding {
view: &texture_view,

4
wgpu-hal/src/dx12/command.rs
View File

@ -1136,7 +1136,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
) {
let ibv = Direct3D12::D3D12_INDEX_BUFFER_VIEW {
BufferLocation: binding.resolve_address(),
SizeInBytes: binding.size.try_into().unwrap(),
SizeInBytes: binding.resolve_size() as u32,
Format: auxil::dxgi::conv::map_index_format(format),
};
@ -1149,7 +1149,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
) {
let vb = &mut self.pass.vertex_buffers[index as usize];
vb.BufferLocation = binding.resolve_address();
vb.SizeInBytes = binding.size.try_into().unwrap();
vb.SizeInBytes = binding.resolve_size() as u32;
self.pass.dirty_vertex_buffers |= 1 << index;
}

2
wgpu-hal/src/dx12/device.rs
View File

@ -1442,7 +1442,7 @@ impl crate::Device for super::Device {
let end = start + entry.count as usize;
for data in &desc.buffers[start..end] {
let gpu_address = data.resolve_address();
let mut size = data.size.try_into().unwrap();
let mut size = data.resolve_size() as u32;
if has_dynamic_offset {
match ty {

7
wgpu-hal/src/dx12/mod.rs
View File

@ -865,6 +865,13 @@ unsafe impl Sync for Buffer {}
impl crate::DynBuffer for Buffer {}
impl crate::BufferBinding<'_, Buffer> {
fn resolve_size(&self) -> wgt::BufferAddress {
match self.size {
Some(size) => size.get(),
None => self.buffer.size - self.offset,
}
}
// TODO: Return GPU handle directly?
fn resolve_address(&self) -> wgt::BufferAddress {
(unsafe { self.buffer.resource.GetGPUVirtualAddress() }) + self.offset

3
wgpu-hal/src/gles/device.rs
View File

@ -1261,11 +1261,10 @@ impl crate::Device for super::Device {
let binding = match layout.ty {
wgt::BindingType::Buffer { .. } => {
let bb = &desc.buffers[entry.resource_index as usize];
assert!(bb.size != 0, "zero-size bindings are not supported");
super::RawBinding::Buffer {
raw: bb.buffer.raw.unwrap(),
offset: bb.offset.try_into().unwrap(),
size: bb.size.try_into().unwrap(),
size: bb.size.get().try_into().unwrap(),
}
}
wgt::BindingType::Sampler { .. } => {

53
wgpu-hal/src/lib.rs
View File

@ -297,7 +297,7 @@ use core::{
borrow::Borrow,
error::Error,
fmt,
num::{NonZeroU32, NonZeroU64},
num::NonZeroU32,
ops::{Range, RangeInclusive},
ptr::NonNull,
};
@ -1979,7 +1979,7 @@ pub struct PipelineLayoutDescriptor<'a, B: DynBindGroupLayout + ?Sized> {
///
/// `wgpu_hal` guarantees that shaders compiled with
/// [`ShaderModuleDescriptor::runtime_checks`] set to `true` cannot read or
/// write data via this binding outside the *accessible region* of a buffer:
/// write data via this binding outside the *accessible region* of [`buffer`]:
///
/// - The accessible region starts at [`offset`].
///
@ -2004,14 +2004,14 @@ pub struct PipelineLayoutDescriptor<'a, B: DynBindGroupLayout + ?Sized> {
/// Some back ends cannot tolerate zero-length regions; for example, see
/// [VUID-VkDescriptorBufferInfo-offset-00340][340] and
/// [VUID-VkDescriptorBufferInfo-range-00341][341], or the
/// documentation for GLES's [glBindBufferRange][bbr]. This documentation
/// previously stated that a `BufferBinding` must have `offset` strictly less
/// than the size of the buffer, but this restriction was not honored elsewhere
/// in the code, so has been removed. However, it remains the case that
/// some backends do not support zero-length bindings, so additional
/// logic is needed somewhere to handle this properly. See
/// [#3170](https://github.com/gfx-rs/wgpu/issues/3170).
/// documentation for GLES's [glBindBufferRange][bbr]. For this reason, a valid
/// `BufferBinding` must have `offset` strictly less than the size of the
/// buffer.
///
/// WebGPU allows zero-length bindings, and there is not currently a mechanism
/// in place
///
/// [`buffer`]: BufferBinding::buffer
/// [`offset`]: BufferBinding::offset
/// [`size`]: BufferBinding::size
/// [`Storage`]: wgt::BufferBindingType::Storage
@ -2031,11 +2031,12 @@ pub struct BufferBinding<'a, B: DynBuffer + ?Sized> {
/// The offset at which the bound region starts.
///
/// This must be less or equal to the size of the buffer.
/// Because zero-length bindings are not permitted (see above), this must be
/// strictly less than the size of the buffer.
pub offset: wgt::BufferAddress,
/// The size of the region bound, in bytes.
pub size: wgt::BufferSizeOrZero,
pub size: wgt::BufferSize,
}
// We must implement this manually because `B` is not necessarily `Clone`.
@ -2049,25 +2050,6 @@ impl<B: DynBuffer + ?Sized> Clone for BufferBinding<'_, B> {
}
}
/// Temporary convenience trait to let us call `.get()` on `u64`s in code that
/// really wants to be using `NonZeroU64`.
/// TODO(<https://github.com/gfx-rs/wgpu/issues/3170>): remove this
pub trait ShouldBeNonZeroExt {
fn get(&self) -> u64;
}
impl ShouldBeNonZeroExt for NonZeroU64 {
fn get(&self) -> u64 {
NonZeroU64::get(*self)
}
}
impl ShouldBeNonZeroExt for u64 {
fn get(&self) -> u64 {
*self
}
}
impl<'a, B: DynBuffer + ?Sized> BufferBinding<'a, B> {
/// Construct a `BufferBinding` with the given contents.
///
@ -2080,20 +2062,15 @@ impl<'a, B: DynBuffer + ?Sized> BufferBinding<'a, B> {
///
/// SAFETY: The caller is responsible for ensuring that a binding of `size`
/// bytes starting at `offset` is contained within the buffer.
///
/// The `S` type parameter is a temporary convenience to allow callers to
/// pass either a `u64` or a `NonZeroU64`. When the zero-size binding issue
/// is resolved, the argument should just match the type of the member.
/// TODO(<https://github.com/gfx-rs/wgpu/issues/3170>): remove the parameter
pub unsafe fn new_unchecked<S: Into<wgt::BufferSizeOrZero>>(
pub unsafe fn new_unchecked(
buffer: &'a B,
offset: wgt::BufferAddress,
size: S,
size: wgt::BufferSize,
) -> Self {
Self {
buffer,
offset,
size: size.into(),
size,
}
}
}

10
wgpu-hal/src/metal/command.rs
View File

@ -4,7 +4,7 @@ use alloc::{
borrow::{Cow, ToOwned as _},
vec::Vec,
};
use core::{num::NonZeroU64, ops::Range};
use core::ops::Range;
use metal::{
MTLIndexType, MTLLoadAction, MTLPrimitiveType, MTLScissorRect, MTLSize, MTLStoreAction,
MTLViewport, MTLVisibilityResultMode, NSRange,
@ -977,11 +977,9 @@ impl crate::CommandEncoder for super::CommandEncoder {
let encoder = self.state.render.as_ref().unwrap();
encoder.set_vertex_buffer(buffer_index, Some(&binding.buffer.raw), binding.offset);
// https://github.com/gfx-rs/wgpu/issues/3170
let size =
NonZeroU64::new(binding.size).expect("zero-size vertex buffers are not supported");
self.state.vertex_buffer_size_map.insert(buffer_index, size);
self.state
.vertex_buffer_size_map
.insert(buffer_index, binding.size);
if let Some((index, sizes)) = self
.state

7
wgpu-hal/src/metal/device.rs
View File

@ -1,5 +1,5 @@
use alloc::{borrow::ToOwned as _, sync::Arc, vec::Vec};
use core::{num::NonZeroU64, ptr::NonNull, sync::atomic};
use core::{ptr::NonNull, sync::atomic};
use std::{thread, time};
use parking_lot::Mutex;
@ -928,12 +928,9 @@ impl crate::Device for super::Device {
let end = start + 1;
bg.buffers
.extend(desc.buffers[start..end].iter().map(|source| {
// https://github.com/gfx-rs/wgpu/issues/3170
let source_size = NonZeroU64::new(source.size)
.expect("zero-size bindings are not supported");
let binding_size = match ty {
wgt::BufferBindingType::Storage { .. } => {
Some(source_size)
Some(source.size)
}
_ => None,
};

4
wgpu-hal/src/vulkan/device.rs
View File

@ -1804,12 +1804,10 @@ impl crate::Device for super::Device {
(buffer_infos, local_buffer_infos) =
buffer_infos.extend(desc.buffers[start as usize..end as usize].iter().map(
|binding| {
// https://github.com/gfx-rs/wgpu/issues/3170
assert!(binding.size != 0, "zero-size bindings are not supported");
vk::DescriptorBufferInfo::default()
.buffer(binding.buffer.raw)
.offset(binding.offset)
.range(binding.size)
.range(binding.size.get())
},
));
write.buffer_info(local_buffer_infos)

6
wgpu-types/src/lib.rs
View File

@ -61,12 +61,6 @@ pub type BufferAddress = u64;
/// [`BufferSlice`]: ../wgpu/struct.BufferSlice.html
pub type BufferSize = core::num::NonZeroU64;
/// Integral type used for buffer sizes that may be zero.
///
/// Although the wgpu Rust API disallows zero-size `BufferSlice` and wgpu-hal
/// disallows zero-size bindings, WebGPU permits zero-size buffers and bindings.
pub type BufferSizeOrZero = u64;
/// Integral type used for binding locations in shaders.
///
/// Used in [`VertexAttribute`]s and errors.