diff --git a/wgpu-core/src/command/bundle.rs b/wgpu-core/src/command/bundle.rs index aec42a4935..5d474f6a3a 100644 --- a/wgpu-core/src/command/bundle.rs +++ b/wgpu-core/src/command/bundle.rs @@ -96,7 +96,7 @@ use crate::{ hub::Hub, id::{self, RenderBundleId}, init_tracker::{BufferInitTrackerAction, MemoryInitKind, TextureInitTrackerAction}, - pipeline::{self, PipelineFlags, RenderPipeline}, + pipeline::{PipelineFlags, RenderPipeline, VertexStep}, resource::{Resource, ResourceInfo, ResourceType}, resource_log, track::RenderBundleScope, @@ -110,6 +110,91 @@ use thiserror::Error; use hal::CommandEncoder as _; +/// https://gpuweb.github.io/gpuweb/#dom-gpurendercommandsmixin-draw +fn validate_draw( + vertex: &[Option], + step: &[VertexStep], + first_vertex: u32, + vertex_count: u32, + first_instance: u32, + instance_count: u32, +) -> Result<(), DrawError> { + let vertices_end = first_vertex as u64 + vertex_count as u64; + let instances_end = first_instance as u64 + instance_count as u64; + + for (idx, (vbs, step)) in vertex.iter().zip(step).enumerate() { + let Some(vbs) = vbs else { + continue; + }; + + let stride_count = match step.mode { + wgt::VertexStepMode::Vertex => vertices_end, + wgt::VertexStepMode::Instance => instances_end, + }; + + if stride_count == 0 { + continue; + } + + let offset = (stride_count - 1) * step.stride + step.last_stride; + let limit = vbs.range.end - vbs.range.start; + if offset > limit { + return Err(DrawError::VertexOutOfBounds { + step_mode: step.mode, + offset, + limit, + slot: idx as u32, + }); + } + } + + Ok(()) +} + +// See https://gpuweb.github.io/gpuweb/#dom-gpurendercommandsmixin-drawindexed +fn validate_indexed_draw( + vertex: &[Option], + step: &[VertexStep], + index_state: &IndexState, + first_index: u32, + index_count: u32, + first_instance: u32, + instance_count: u32, +) -> Result<(), DrawError> { + let last_index = first_index as u64 + index_count as u64; + let index_limit = index_state.limit(); + if last_index <= index_limit { + return Err(DrawError::IndexBeyondLimit { + last_index, + index_limit, + }); + } + + let stride_count = first_instance as u64 + instance_count as u64; + for (idx, (vbs, step)) in vertex.iter().zip(step).enumerate() { + let Some(vbs) = vbs else { + continue; + }; + + if stride_count == 0 || step.mode != wgt::VertexStepMode::Instance { + continue; + } + + let offset = (stride_count - 1) * step.stride + step.last_stride; + let limit = vbs.range.end - vbs.range.start; + if offset > limit { + return Err(DrawError::VertexOutOfBounds { + step_mode: step.mode, + offset, + limit, + slot: idx as u32, + }); + } + } + + Ok(()) +} + /// Describes a [`RenderBundleEncoder`]. #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] #[cfg_attr(feature = "trace", derive(serde::Serialize))] @@ -495,25 +580,15 @@ impl RenderBundleEncoder { }; let pipeline = state.pipeline(scope)?; let used_bind_groups = pipeline.used_bind_groups; - let vertex_limits = state.vertex_limits(pipeline); - let last_vertex = first_vertex as u64 + vertex_count as u64; - if last_vertex > vertex_limits.vertex_limit { - return Err(DrawError::VertexBeyondLimit { - last_vertex, - vertex_limit: vertex_limits.vertex_limit, - slot: vertex_limits.vertex_limit_slot, - }) - .map_pass_err(scope); - } - let last_instance = first_instance as u64 + instance_count as u64; - if last_instance > vertex_limits.instance_limit { - return Err(DrawError::InstanceBeyondLimit { - last_instance, - instance_limit: vertex_limits.instance_limit, - slot: vertex_limits.instance_limit_slot, - }) - .map_pass_err(scope); - } + + validate_draw( + &state.vertex[..], + &pipeline.steps, + first_vertex, + vertex_count, + first_instance, + instance_count, + ).map_pass_err(scope)?; if instance_count > 0 && vertex_count > 0 { commands.extend(state.flush_vertices()); @@ -539,26 +614,16 @@ impl RenderBundleEncoder { Some(ref index) => index, None => return Err(DrawError::MissingIndexBuffer).map_pass_err(scope), }; - //TODO: validate that base_vertex + max_index() is within the provided range - let vertex_limits = state.vertex_limits(pipeline); - let index_limit = index.limit(); - let last_index = first_index as u64 + index_count as u64; - if last_index > index_limit { - return Err(DrawError::IndexBeyondLimit { - last_index, - index_limit, - }) - .map_pass_err(scope); - } - let last_instance = first_instance as u64 + instance_count as u64; - if last_instance > vertex_limits.instance_limit { - return Err(DrawError::InstanceBeyondLimit { - last_instance, - instance_limit: vertex_limits.instance_limit, - slot: vertex_limits.instance_limit_slot, - }) - .map_pass_err(scope); - } + + validate_indexed_draw( + &state.vertex[..], + &pipeline.steps, + index, + first_index, + index_count, + first_instance, + instance_count, + ).map_pass_err(scope)?; if instance_count > 0 && index_count > 0 { commands.extend(state.flush_index()); @@ -1119,18 +1184,6 @@ struct BindState { is_dirty: bool, } -#[derive(Debug)] -struct VertexLimitState { - /// Length of the shortest vertex rate vertex buffer - vertex_limit: u64, - /// Buffer slot which the shortest vertex rate vertex buffer is bound to - vertex_limit_slot: u32, - /// Length of the shortest instance rate vertex buffer - instance_limit: u64, - /// Buffer slot which the shortest instance rate vertex buffer is bound to - instance_limit_slot: u32, -} - /// The bundle's current pipeline, and some cached information needed for validation. struct PipelineState { /// The pipeline @@ -1138,7 +1191,7 @@ struct PipelineState { /// How this pipeline's vertex shader traverses each vertex buffer, indexed /// by vertex buffer slot number. - steps: Vec, + steps: Vec, /// Ranges of push constants this pipeline uses, copied from the pipeline /// layout. @@ -1223,35 +1276,6 @@ struct State { } impl State { - fn vertex_limits(&self, pipeline: &PipelineState) -> VertexLimitState { - let mut vert_state = VertexLimitState { - vertex_limit: u32::MAX as u64, - vertex_limit_slot: 0, - instance_limit: u32::MAX as u64, - instance_limit_slot: 0, - }; - for (idx, (vbs, step)) in self.vertex.iter().zip(&pipeline.steps).enumerate() { - if let Some(ref vbs) = *vbs { - let limit = (vbs.range.end - vbs.range.start) / step.stride; - match step.mode { - wgt::VertexStepMode::Vertex => { - if limit < vert_state.vertex_limit { - vert_state.vertex_limit = limit; - vert_state.vertex_limit_slot = idx as _; - } - } - wgt::VertexStepMode::Instance => { - if limit < vert_state.instance_limit { - vert_state.instance_limit = limit; - vert_state.instance_limit_slot = idx as _; - } - } - } - } - } - vert_state - } - /// Return the id of the current pipeline, if any. fn pipeline_id(&self) -> Option { self.pipeline.as_ref().map(|p| p.pipeline.as_info().id()) diff --git a/wgpu-core/src/command/draw.rs b/wgpu-core/src/command/draw.rs index 6dc8b4fbb9..058ba81f95 100644 --- a/wgpu-core/src/command/draw.rs +++ b/wgpu-core/src/command/draw.rs @@ -8,7 +8,7 @@ use crate::{ track::UsageConflict, validation::{MissingBufferUsageError, MissingTextureUsageError}, }; -use wgt::{BufferAddress, BufferSize, Color}; +use wgt::{BufferAddress, BufferSize, Color, VertexStepMode}; use std::num::NonZeroU32; use thiserror::Error; @@ -33,6 +33,13 @@ pub enum DrawError { vertex_limit: u64, slot: u32, }, + #[error("{step_mode:?} buffer out of bounds at slot {slot}. Offset {offset} beyond limit {limit}. Did you bind the correct `Vertex` step-rate vertex buffer?")] + VertexOutOfBounds { + step_mode: VertexStepMode, + offset: u64, + limit: u64, + slot: u32, + }, #[error("Instance {last_instance} extends beyond limit {instance_limit} imposed by the buffer in slot {slot}. Did you bind the correct `Instance` step-rate vertex buffer?")] InstanceBeyondLimit { last_instance: u64, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index f4933d898b..9f31d3c6d0 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -361,6 +361,7 @@ impl VertexBufferState { total_size: 0, step: pipeline::VertexStep { stride: 0, + last_stride: 0, mode: VertexStepMode::Vertex, }, bound: false, @@ -384,11 +385,13 @@ struct VertexState { impl VertexState { fn update_limits(&mut self) { - // Ensure that the limits are always smaller than u32::MAX so that - // interger overlows can be prevented via saturating additions. - let max = u32::MAX as u64; - self.vertex_limit = max; - self.instance_limit = max; + // TODO: This isn't entirely spec-compliant. + // We currently require that the buffer range can fit `stride` * count bytes. + // The spec, however, lets a buffer be a bit smaller as long as the size of the + // last element fits in it (the last element can be smaller than the stride between + // elements). + self.vertex_limit = u32::MAX as u64; + self.instance_limit = u32::MAX as u64; for (idx, vbs) in self.inputs.iter().enumerate() { if vbs.step.stride == 0 || !vbs.bound { continue; diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 06aedc00f6..b46bfde691 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2717,8 +2717,13 @@ impl Device { let mut shader_expects_dual_source_blending = false; let mut pipeline_expects_dual_source_blending = false; for (i, vb_state) in desc.vertex.buffers.iter().enumerate() { + let mut last_stride = 0; + for attribute in vb_state.attributes.iter() { + last_stride = last_stride.max(attribute.offset + attribute.format.size()); + } vertex_steps.push(pipeline::VertexStep { stride: vb_state.array_stride, + last_stride, mode: vb_state.step_mode, }); if vb_state.attributes.is_empty() { diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index 1dc2d1eff1..7fb45321f0 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -493,6 +493,9 @@ pub struct VertexStep { /// The byte stride in the buffer between one attribute value and the next. pub stride: wgt::BufferAddress, + /// The byte size required to fit the last vertex in the stream. + pub last_stride: wgt::BufferAddress, + /// Whether the buffer is indexed by vertex number or instance number. pub mode: wgt::VertexStepMode, } @@ -501,6 +504,7 @@ impl Default for VertexStep { fn default() -> Self { Self { stride: 0, + last_stride: 0, mode: wgt::VertexStepMode::Vertex, } }