From 9621fb3021ac2ea9248746ea570b8ba6786cb77d Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 28 May 2019 11:07:29 -0400 Subject: [PATCH] Validate vertex/instance/index buffer ranges --- ffi/wgpu-remote.h | 2 +- ffi/wgpu.h | 4 +- wgpu-native/src/command/mod.rs | 7 -- wgpu-native/src/command/render.rs | 116 ++++++++++++++++++++++++++---- wgpu-native/src/device.rs | 5 ++ wgpu-native/src/pipeline.rs | 1 + wgpu-native/src/resource.rs | 1 + 7 files changed, 114 insertions(+), 22 deletions(-) diff --git a/ffi/wgpu-remote.h b/ffi/wgpu-remote.h index 65336655d..e41ecdc9e 100644 --- a/ffi/wgpu-remote.h +++ b/ffi/wgpu-remote.h @@ -1,6 +1,6 @@ -/* Generated with cbindgen:0.8.7 */ +/* Generated with cbindgen:0.8.6 */ #include #include diff --git a/ffi/wgpu.h b/ffi/wgpu.h index 7e800dbec..971d78be6 100644 --- a/ffi/wgpu.h +++ b/ffi/wgpu.h @@ -1,6 +1,6 @@ #define WGPU_LOCAL -/* Generated with cbindgen:0.8.7 */ +/* Generated with cbindgen:0.8.6 */ #include #include @@ -11,6 +11,8 @@ #define WGPUMAX_COLOR_TARGETS 4 +#define WGPUMAX_VERTEX_BUFFERS 8 + typedef enum { WGPUAddressMode_ClampToEdge = 0, WGPUAddressMode_Repeat = 1, diff --git a/wgpu-native/src/command/mod.rs b/wgpu-native/src/command/mod.rs index dccd7fbc2..19fbd6003 100644 --- a/wgpu-native/src/command/mod.rs +++ b/wgpu-native/src/command/mod.rs @@ -19,7 +19,6 @@ use crate::{ RenderPassKey, }, hub::{Storage, HUB}, - pipeline::IndexFormat, resource::TexturePlacement, swap_chain::{SwapChainLink, SwapImageEpoch}, track::{DummyUsage, Stitch, TrackerSet}, @@ -393,11 +392,6 @@ pub fn command_encoder_begin_render_pass( depth_stencil: depth_stencil_attachment.map(|at| view_guard[at.attachment].format), }; - let index_state = IndexState { - bound_buffer_view: None, - format: IndexFormat::Uint16, - }; - RenderPass::new( current_comb, Stored { @@ -405,7 +399,6 @@ pub fn command_encoder_begin_render_pass( ref_count: cmb.life_guard.ref_count.clone(), }, context, - index_state, ) } diff --git a/wgpu-native/src/command/render.rs b/wgpu-native/src/command/render.rs index c0256f2c0..45f6e43ab 100644 --- a/wgpu-native/src/command/render.rs +++ b/wgpu-native/src/command/render.rs @@ -1,9 +1,9 @@ use crate::{ command::bind::{Binder, LayoutChange}, conv, - device::RenderPassContext, + device::{MAX_VERTEX_BUFFERS, RenderPassContext}, hub::HUB, - pipeline::{IndexFormat, PipelineFlags}, + pipeline::{IndexFormat, InputStepMode, PipelineFlags}, resource::BufferUsage, track::{Stitch, TrackerSet}, BindGroupId, @@ -20,7 +20,7 @@ use crate::{ use hal::command::RawCommandBuffer; -use std::{iter, slice}; +use std::{iter, ops::Range, slice}; #[derive(Debug, PartialEq)] enum OptionalState { @@ -50,8 +50,63 @@ enum DrawError { #[derive(Debug)] pub struct IndexState { - pub(crate) bound_buffer_view: Option<(BufferId, BufferAddress)>, - pub(crate) format: IndexFormat, + bound_buffer_view: Option<(BufferId, Range)>, + format: IndexFormat, + limit: u32, +} + +impl IndexState { + fn update_limit(&mut self) { + self.limit = match self.bound_buffer_view { + Some((_, ref range)) => { + let shift = match self.format { + IndexFormat::Uint16 => 1, + IndexFormat::Uint32 => 2, + }; + ((range.end - range.start) >> shift) as u32 + } + None => 0, + } + } +} + +#[derive(Clone, Copy, Debug)] +pub struct VertexBufferState { + total_size: BufferAddress, + stride: BufferAddress, + rate: InputStepMode, +} + +impl VertexBufferState { + const EMPTY: Self = VertexBufferState { + total_size: 0, + stride: 0, + rate: InputStepMode::Vertex, + }; +} + +#[derive(Debug)] +pub struct VertexState { + inputs: [VertexBufferState; MAX_VERTEX_BUFFERS], + vertex_limit: u32, + instance_limit: u32, +} + +impl VertexState { + fn update_limits(&mut self) { + self.vertex_limit = !0; + self.instance_limit = !0; + for vbs in &self.inputs { + if vbs.stride == 0 { + continue + } + let limit = (vbs.total_size / vbs.stride) as u32; + match vbs.rate { + InputStepMode::Vertex => self.vertex_limit = self.vertex_limit.min(limit), + InputStepMode::Instance => self.instance_limit = self.instance_limit.min(limit), + } + } + } } pub struct RenderPass { @@ -63,6 +118,7 @@ pub struct RenderPass { blend_color_status: OptionalState, stencil_reference_status: OptionalState, index_state: IndexState, + vertex_state: VertexState, } impl RenderPass { @@ -70,7 +126,6 @@ impl RenderPass { raw: B::CommandBuffer, cmb_id: Stored, context: RenderPassContext, - index_state: IndexState, ) -> Self { RenderPass { raw, @@ -80,7 +135,16 @@ impl RenderPass { trackers: TrackerSet::new(), blend_color_status: OptionalState::Unused, stencil_reference_status: OptionalState::Unused, - index_state, + index_state: IndexState { + bound_buffer_view: None, + format: IndexFormat::Uint16, + limit: 0, + }, + vertex_state: VertexState { + inputs: [VertexBufferState::EMPTY; MAX_VERTEX_BUFFERS], + vertex_limit: 0, + instance_limit: 0, + }, } } @@ -208,6 +272,10 @@ pub extern "C" fn wgpu_render_pass_set_index_buffer( .get_with_extended_usage(&*buffer_guard, buffer_id, BufferUsage::INDEX) .unwrap(); + let range = offset .. buffer.size; + pass.index_state.bound_buffer_view = Some((buffer_id, range)); + pass.index_state.update_limit(); + let view = hal::buffer::IndexBufferView { buffer: &buffer.raw, offset, @@ -217,8 +285,6 @@ pub extern "C" fn wgpu_render_pass_set_index_buffer( unsafe { pass.raw.bind_index_buffer(view); } - - pass.index_state.bound_buffer_view = Some((buffer_id, offset)); } #[no_mangle] @@ -234,12 +300,18 @@ pub extern "C" fn wgpu_render_pass_set_vertex_buffers( let offsets = unsafe { slice::from_raw_parts(offset_ptr, count) }; let pass = &mut pass_guard[pass_id]; - for &id in buffers { - pass.trackers + for (vbs, (&id, &offset)) in pass.vertex_state.inputs.iter_mut().zip(buffers.iter().zip(offsets)) { + let buffer = pass.trackers .buffers .get_with_extended_usage(&*buffer_guard, id, BufferUsage::VERTEX) .unwrap(); + vbs.total_size = buffer.size - offset; } + for vbs in pass.vertex_state.inputs[count..].iter_mut() { + vbs.total_size = 0; + } + + pass.vertex_state.update_limits(); let buffers = buffers .iter() @@ -263,6 +335,9 @@ pub extern "C" fn wgpu_render_pass_draw( let pass = &mut pass_guard[pass_id]; pass.is_ready().unwrap(); + assert!(first_vertex + vertex_count <= pass.vertex_state.vertex_limit, "Vertex out of range!"); + assert!(first_instance + instance_count <= pass.vertex_state.instance_limit, "Instance out of range!"); + unsafe { pass.raw.draw( first_vertex .. first_vertex + vertex_count, @@ -284,6 +359,10 @@ pub extern "C" fn wgpu_render_pass_draw_indexed( let pass = &mut pass_guard[pass_id]; pass.is_ready().unwrap(); + //TODO: validate that base_vertex + max_index() is within the provided range + assert!(first_index + index_count <= pass.index_state.limit, "Index out of range!"); + assert!(first_instance + instance_count <= pass.vertex_state.instance_limit, "Instance out of range!"); + unsafe { pass.raw.draw_indexed( first_index .. first_index + index_count, @@ -352,8 +431,9 @@ pub extern "C" fn wgpu_render_pass_set_pipeline( // Rebind index buffer if the index format has changed with the pipeline switch if pass.index_state.format != pipeline.index_format { pass.index_state.format = pipeline.index_format; + pass.index_state.update_limit(); - if let Some((buffer_id, offset)) = pass.index_state.bound_buffer_view { + if let Some((buffer_id, ref range)) = pass.index_state.bound_buffer_view { let buffer_guard = HUB.buffers.read(); let buffer = pass .trackers @@ -363,7 +443,7 @@ pub extern "C" fn wgpu_render_pass_set_pipeline( let view = hal::buffer::IndexBufferView { buffer: &buffer.raw, - offset, + offset: range.start, index_type: conv::map_index_format(pass.index_state.format), }; @@ -372,6 +452,16 @@ pub extern "C" fn wgpu_render_pass_set_pipeline( } } } + // Update vertex buffer limits + for (vbs, &(stride, rate)) in pass.vertex_state.inputs.iter_mut().zip(&pipeline.vertex_strides) { + vbs.stride = stride; + vbs.rate = rate; + } + for vbs in pass.vertex_state.inputs[pipeline.vertex_strides.len() .. ].iter_mut() { + vbs.stride = 0; + vbs.rate = InputStepMode::Vertex; + } + pass.vertex_state.update_limits(); } #[no_mangle] diff --git a/wgpu-native/src/device.rs b/wgpu-native/src/device.rs index 17b11ca27..e0dc1db4a 100644 --- a/wgpu-native/src/device.rs +++ b/wgpu-native/src/device.rs @@ -58,6 +58,7 @@ use std::{collections::hash_map::Entry, ffi, iter, ptr, slice, sync::atomic::Ord const CLEANUP_WAIT_MS: u64 = 5000; pub const MAX_COLOR_TARGETS: usize = 4; +pub const MAX_VERTEX_BUFFERS: usize = 8; pub fn all_buffer_stages() -> hal::pso::PipelineStage { use hal::pso::PipelineStage as Ps; @@ -602,6 +603,7 @@ pub fn device_create_buffer( }, memory_properties, memory, + size: desc.size, mapped_write_ranges: Vec::new(), pending_map_operation: None, life_guard: LifeGuard::new(), @@ -1445,9 +1447,11 @@ pub fn device_create_render_pipeline( desc.vertex_input.vertex_buffers_count, ) }; + let mut vertex_strides = Vec::with_capacity(desc_vbs.len()); let mut vertex_buffers = Vec::with_capacity(desc_vbs.len()); let mut attributes = Vec::new(); for (i, vb_state) in desc_vbs.iter().enumerate() { + vertex_strides.alloc().init((vb_state.stride, vb_state.step_mode)); if vb_state.attributes_count == 0 { continue; } @@ -1558,6 +1562,7 @@ pub fn device_create_render_pipeline( pass_context, flags, index_format: desc.vertex_input.index_format, + vertex_strides, } } diff --git a/wgpu-native/src/pipeline.rs b/wgpu-native/src/pipeline.rs index e14fea4e7..0369fec57 100644 --- a/wgpu-native/src/pipeline.rs +++ b/wgpu-native/src/pipeline.rs @@ -293,4 +293,5 @@ pub struct RenderPipeline { pub(crate) pass_context: RenderPassContext, pub(crate) flags: PipelineFlags, pub(crate) index_format: IndexFormat, + pub(crate) vertex_strides: Vec<(BufferAddress, InputStepMode)>, } diff --git a/wgpu-native/src/resource.rs b/wgpu-native/src/resource.rs index 00a2d06c2..234765b32 100644 --- a/wgpu-native/src/resource.rs +++ b/wgpu-native/src/resource.rs @@ -62,6 +62,7 @@ pub struct Buffer { pub(crate) device_id: Stored, pub(crate) memory_properties: hal::memory::Properties, pub(crate) memory: B::Memory, + pub(crate) size: BufferAddress, pub(crate) mapped_write_ranges: Vec>, pub(crate) pending_map_operation: Option, pub(crate) life_guard: LifeGuard,