From 6c18fc697b2b2eeed69a9af6e963e72078d40608 Mon Sep 17 00:00:00 2001 From: vmarcella Date: Sat, 1 Nov 2025 16:32:15 -0700 Subject: [PATCH 1/3] [add] buffer abstraction and replace direct wgpu usage in lambda-rs. --- crates/lambda-rs-platform/src/wgpu/buffer.rs | 161 +++++++++++++++++++ crates/lambda-rs-platform/src/wgpu/mod.rs | 1 + crates/lambda-rs/src/render/buffer.rs | 54 +++---- 3 files changed, 185 insertions(+), 31 deletions(-) create mode 100644 crates/lambda-rs-platform/src/wgpu/buffer.rs diff --git a/crates/lambda-rs-platform/src/wgpu/buffer.rs b/crates/lambda-rs-platform/src/wgpu/buffer.rs new file mode 100644 index 00000000..4901f03c --- /dev/null +++ b/crates/lambda-rs-platform/src/wgpu/buffer.rs @@ -0,0 +1,161 @@ +//! Buffer wrappers and builders for the platform layer. +//! +//! This module provides a thin wrapper over `wgpu::Buffer` plus a small +//! builder that handles common initialization patterns and keeps label and +//! usage metadata for debugging/inspection. + +use crate::wgpu::{ + types as wgpu, + types::util::DeviceExt, +}; + +#[derive(Clone, Copy, Debug)] +/// Platform buffer usage flags. +pub struct Usage(pub(crate) wgpu::BufferUsages); + +impl Usage { + /// Vertex buffer usage. + pub const VERTEX: Usage = Usage(wgpu::BufferUsages::VERTEX); + /// Index buffer usage. + pub const INDEX: Usage = Usage(wgpu::BufferUsages::INDEX); + /// Uniform buffer usage. + pub const UNIFORM: Usage = Usage(wgpu::BufferUsages::UNIFORM); + /// Storage buffer usage. + pub const STORAGE: Usage = Usage(wgpu::BufferUsages::STORAGE); + /// Copy destination (for CPU-visible uploads). + pub const COPY_DST: Usage = Usage(wgpu::BufferUsages::COPY_DST); + + pub(crate) fn to_wgpu(self) -> wgpu::BufferUsages { + return self.0; + } +} + +impl std::ops::BitOr for Usage { + type Output = Usage; + fn bitor(self, rhs: Usage) -> Usage { + return Usage(self.0 | rhs.0); + } +} + +impl Default for Usage { + fn default() -> Self { + return Usage(wgpu::BufferUsages::VERTEX); + } +} + +#[derive(Debug)] +/// Wrapper around `wgpu::Buffer` with metadata. +pub struct Buffer { + pub(crate) raw: wgpu::Buffer, + pub(crate) label: Option, + pub(crate) size: wgpu::BufferAddress, + pub(crate) usage: wgpu::BufferUsages, +} + +impl Buffer { + /// Borrow the underlying `wgpu::Buffer`. + pub fn raw(&self) -> &wgpu::Buffer { + return &self.raw; + } + + /// Optional debug label. + pub fn label(&self) -> Option<&str> { + return self.label.as_deref(); + } + + /// Size in bytes at creation time. + pub fn size(&self) -> wgpu::BufferAddress { + return self.size; + } + + /// Usage flags used to create the buffer. + pub fn usage(&self) -> wgpu::BufferUsages { + return self.usage; + } +} + +#[derive(Default)] +/// Builder for creating a `Buffer` with optional initial contents. +pub struct BufferBuilder { + label: Option, + size: usize, + usage: Usage, + cpu_visible: bool, +} + +impl BufferBuilder { + /// Create a new builder with zero size and VERTEX usage. + pub fn new() -> Self { + return Self { + label: None, + size: 0, + usage: Usage::VERTEX, + cpu_visible: false, + }; + } + + /// Attach a label for debugging/profiling. + pub fn with_label(mut self, label: &str) -> Self { + self.label = Some(label.to_string()); + return self; + } + + /// Set the total size in bytes. If zero, size is inferred from data length. + pub fn with_size(mut self, size: usize) -> Self { + self.size = size; + return self; + } + + /// Set usage flags. + pub fn with_usage(mut self, usage: Usage) -> Self { + self.usage = usage; + return self; + } + + /// Hint that buffer will be updated from CPU via queue writes. + pub fn with_cpu_visible(mut self, cpu_visible: bool) -> Self { + self.cpu_visible = cpu_visible; + return self; + } + + /// Create a buffer initialized with `contents`. + pub fn build_init(self, device: &wgpu::Device, contents: &[u8]) -> Buffer { + let size = if self.size == 0 { + contents.len() + } else { + self.size + }; + + let mut usage = self.usage.to_wgpu(); + if self.cpu_visible { + usage |= wgpu::BufferUsages::COPY_DST; + } + + let raw = device.create_buffer_init(&wgpu::util::BufferInitDescriptor { + label: self.label.as_deref(), + contents, + usage, + }); + + return Buffer { + raw, + label: self.label, + size: size as wgpu::BufferAddress, + usage, + }; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn usage_bitor_combines_flags() { + let u = Usage::VERTEX | Usage::INDEX | Usage::UNIFORM; + let flags = u.to_wgpu(); + assert!(flags.contains(wgpu::BufferUsages::VERTEX)); + assert!(flags.contains(wgpu::BufferUsages::INDEX)); + assert!(flags.contains(wgpu::BufferUsages::UNIFORM)); + } +} diff --git a/crates/lambda-rs-platform/src/wgpu/mod.rs b/crates/lambda-rs-platform/src/wgpu/mod.rs index b15807dc..5caeeb31 100644 --- a/crates/lambda-rs-platform/src/wgpu/mod.rs +++ b/crates/lambda-rs-platform/src/wgpu/mod.rs @@ -16,6 +16,7 @@ use wgpu::rwh::{ use crate::winit::WindowHandle; pub mod bind; +pub mod buffer; #[derive(Debug, Clone)] /// Builder for creating a `wgpu::Instance` with consistent defaults. diff --git a/crates/lambda-rs/src/render/buffer.rs b/crates/lambda-rs/src/render/buffer.rs index 71e96efa..de6d8423 100644 --- a/crates/lambda-rs/src/render/buffer.rs +++ b/crates/lambda-rs/src/render/buffer.rs @@ -2,9 +2,9 @@ use std::rc::Rc; -use lambda_platform::wgpu::types::{ - self as wgpu, - util::DeviceExt, +use lambda_platform::wgpu::{ + buffer as platform_buffer, + types as wgpu, }; use super::{ @@ -26,22 +26,20 @@ pub enum BufferType { } #[derive(Clone, Copy, Debug)] -/// A thin newtype for `wgpu::BufferUsages` that supports bitwise ops while -/// keeping explicit construction points in the API surface. -pub struct Usage(wgpu::BufferUsages); +/// Buffer usage flags (engine-facing), mapped to platform usage internally. +pub struct Usage(platform_buffer::Usage); impl Usage { /// Mark buffer usable as a vertex buffer. - pub const VERTEX: Usage = Usage(wgpu::BufferUsages::VERTEX); + pub const VERTEX: Usage = Usage(platform_buffer::Usage::VERTEX); /// Mark buffer usable as an index buffer. - pub const INDEX: Usage = Usage(wgpu::BufferUsages::INDEX); + pub const INDEX: Usage = Usage(platform_buffer::Usage::INDEX); /// Mark buffer usable as a uniform buffer. - pub const UNIFORM: Usage = Usage(wgpu::BufferUsages::UNIFORM); + pub const UNIFORM: Usage = Usage(platform_buffer::Usage::UNIFORM); /// Mark buffer usable as a storage buffer. - pub const STORAGE: Usage = Usage(wgpu::BufferUsages::STORAGE); + pub const STORAGE: Usage = Usage(platform_buffer::Usage::STORAGE); - /// Extract the inner `wgpu` flags. - pub fn to_wgpu(self) -> wgpu::BufferUsages { + fn to_platform(self) -> platform_buffer::Usage { self.0 } } @@ -50,7 +48,7 @@ impl std::ops::BitOr for Usage { type Output = Usage; fn bitor(self, rhs: Usage) -> Usage { - Usage(self.0 | rhs.0) + return Usage(self.0 | rhs.0); } } @@ -90,8 +88,8 @@ impl Default for Properties { /// when binding to pipeline inputs. #[derive(Debug)] pub struct Buffer { - buffer: Rc, - stride: wgpu::BufferAddress, + buffer: Rc, + stride: u64, buffer_type: BufferType, } @@ -101,14 +99,10 @@ impl Buffer { pub fn destroy(self, _render_context: &RenderContext) {} pub(super) fn raw(&self) -> &wgpu::Buffer { - return self.buffer.as_ref(); + return self.buffer.raw(); } - pub(super) fn raw_rc(&self) -> Rc { - return self.buffer.clone(); - } - - pub(super) fn stride(&self) -> wgpu::BufferAddress { + pub(super) fn stride(&self) -> u64 { return self.stride; } @@ -244,7 +238,6 @@ impl BufferBuilder { render_context: &mut RenderContext, data: Vec, ) -> Result { - let device = render_context.device(); let element_size = std::mem::size_of::(); let buffer_length = if self.buffer_length == 0 { element_size * data.len() @@ -266,20 +259,19 @@ impl BufferBuilder { ) }; - let mut usage = self.usage.to_wgpu(); - if self.properties.cpu_visible() { - usage |= wgpu::BufferUsages::COPY_DST; + let mut builder = platform_buffer::BufferBuilder::new() + .with_size(buffer_length) + .with_usage(self.usage.to_platform()) + .with_cpu_visible(self.properties.cpu_visible()); + if let Some(label) = &self.label { + builder = builder.with_label(label); } - let buffer = device.create_buffer_init(&wgpu::util::BufferInitDescriptor { - label: self.label.as_deref(), - contents: bytes, - usage, - }); + let buffer = builder.build_init(render_context.device(), bytes); return Ok(Buffer { buffer: Rc::new(buffer), - stride: element_size as wgpu::BufferAddress, + stride: element_size as u64, buffer_type: self.buffer_type, }); } From 0bfe374c59e32630490c15e08c66d3314d456e95 Mon Sep 17 00:00:00 2001 From: vmarcella Date: Sat, 1 Nov 2025 16:40:50 -0700 Subject: [PATCH 2/3] [add] more unit tests for buffer and render pass abstraction. --- crates/lambda-rs-platform/src/wgpu/mod.rs | 111 ++++++++++++++++++++++ crates/lambda-rs/src/render/buffer.rs | 58 +++++++++-- crates/lambda-rs/src/render/mod.rs | 34 ++----- 3 files changed, 170 insertions(+), 33 deletions(-) diff --git a/crates/lambda-rs-platform/src/wgpu/mod.rs b/crates/lambda-rs-platform/src/wgpu/mod.rs index 5caeeb31..b30569ff 100644 --- a/crates/lambda-rs-platform/src/wgpu/mod.rs +++ b/crates/lambda-rs-platform/src/wgpu/mod.rs @@ -343,6 +343,117 @@ impl Frame { } } +// ---------------------- Command Encoding Abstractions ----------------------- + +#[derive(Debug)] +/// Thin wrapper around `wgpu::CommandEncoder` with convenience helpers. +pub struct CommandEncoder { + raw: wgpu::CommandEncoder, +} + +impl CommandEncoder { + /// Create a new command encoder with an optional label. + pub fn new(device: &wgpu::Device, label: Option<&str>) -> Self { + let raw = + device.create_command_encoder(&wgpu::CommandEncoderDescriptor { label }); + return Self { raw }; + } + + /// Begin a render pass targeting a single color attachment with the provided + /// load/store operations. Depth/stencil is not attached by this helper. + pub fn begin_render_pass<'view>( + &'view mut self, + label: Option<&str>, + view: &'view wgpu::TextureView, + ops: wgpu::Operations, + ) -> RenderPass<'view> { + let color_attachment = wgpu::RenderPassColorAttachment { + view, + resolve_target: None, + depth_slice: None, + ops, + }; + let color_attachments = [Some(color_attachment)]; + let pass = self.raw.begin_render_pass(&wgpu::RenderPassDescriptor { + label, + color_attachments: &color_attachments, + depth_stencil_attachment: None, + timestamp_writes: None, + occlusion_query_set: None, + }); + return RenderPass { raw: pass }; + } + + /// Finish recording and return the command buffer. + pub fn finish(self) -> wgpu::CommandBuffer { + return self.raw.finish(); + } +} + +#[derive(Debug)] +/// Wrapper around `wgpu::RenderPass<'_>` exposing the operations needed by the +/// Lambda renderer without leaking raw `wgpu` types at the call sites. +pub struct RenderPass<'a> { + raw: wgpu::RenderPass<'a>, +} + +impl<'a> RenderPass<'a> { + /// Set the active render pipeline. + pub fn set_pipeline(&mut self, pipeline: &wgpu::RenderPipeline) { + self.raw.set_pipeline(pipeline); + } + + /// Apply viewport state. + pub fn set_viewport( + &mut self, + x: f32, + y: f32, + width: f32, + height: f32, + min_depth: f32, + max_depth: f32, + ) { + self + .raw + .set_viewport(x, y, width, height, min_depth, max_depth); + } + + /// Apply scissor rectangle. + pub fn set_scissor_rect(&mut self, x: u32, y: u32, width: u32, height: u32) { + self.raw.set_scissor_rect(x, y, width, height); + } + + /// Bind a group with optional dynamic offsets. + pub fn set_bind_group( + &mut self, + set: u32, + group: &wgpu::BindGroup, + dynamic_offsets: &[u32], + ) { + self.raw.set_bind_group(set, group, dynamic_offsets); + } + + /// Bind a vertex buffer slot. + pub fn set_vertex_buffer(&mut self, slot: u32, buffer: &wgpu::Buffer) { + self.raw.set_vertex_buffer(slot, buffer.slice(..)); + } + + /// Upload push constants. + pub fn set_push_constants( + &mut self, + stages: wgpu::ShaderStages, + offset: u32, + data: &[u8], + ) { + self.raw.set_push_constants(stages, offset, data); + } + + /// Issue a non-indexed draw over a vertex range. + pub fn draw(&mut self, vertices: std::ops::Range) { + self.raw.draw(vertices, 0..1); + } +} + #[derive(Debug, Clone)] /// Builder for a `Gpu` (adapter, device, queue) with feature validation. pub struct GpuBuilder { diff --git a/crates/lambda-rs/src/render/buffer.rs b/crates/lambda-rs/src/render/buffer.rs index de6d8423..0b9c4b55 100644 --- a/crates/lambda-rs/src/render/buffer.rs +++ b/crates/lambda-rs/src/render/buffer.rs @@ -239,15 +239,7 @@ impl BufferBuilder { data: Vec, ) -> Result { let element_size = std::mem::size_of::(); - let buffer_length = if self.buffer_length == 0 { - element_size * data.len() - } else { - self.buffer_length - }; - - if buffer_length == 0 { - return Err("Attempted to create a buffer with zero length."); - } + let buffer_length = self.resolve_length(element_size, data.len())?; // SAFETY: Converting data to bytes is safe because it's underlying // type, Data, is constrianed to Copy and the lifetime of the slice does @@ -290,3 +282,51 @@ impl BufferBuilder { .build(render_context, mesh.vertices().to_vec()); } } + +impl BufferBuilder { + /// Resolve the effective buffer length from explicit size or data length. + /// Returns an error if the resulting length would be zero. + pub(crate) fn resolve_length( + &self, + element_size: usize, + data_len: usize, + ) -> Result { + let buffer_length = if self.buffer_length == 0 { + element_size * data_len + } else { + self.buffer_length + }; + if buffer_length == 0 { + return Err("Attempted to create a buffer with zero length."); + } + return Ok(buffer_length); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_length_rejects_zero() { + let builder = BufferBuilder::new(); + let result = builder.resolve_length(std::mem::size_of::(), 0); + assert!(result.is_err()); + } + + #[test] + fn label_is_recorded_on_builder() { + let mut builder = BufferBuilder::new(); + builder.with_label("buffer-test"); + // Indirect check via building a small buffer would require a device; ensure + // the label setter stores the value locally instead. + // Access through an internal helper to avoid exposing label publicly. + #[allow(clippy::redundant_closure_call)] + { + // Create a small closure to read the private label field. + // The test module shares the parent scope, so it can access fields. + let read = |b: &BufferBuilder| b.label.as_deref(); + assert_eq!(read(&builder), Some("buffer-test")); + } + } +} diff --git a/crates/lambda-rs/src/render/mod.rs b/crates/lambda-rs/src/render/mod.rs index 7c3072af..29bf25df 100644 --- a/crates/lambda-rs/src/render/mod.rs +++ b/crates/lambda-rs/src/render/mod.rs @@ -19,6 +19,7 @@ use std::iter; use lambda_platform::wgpu::{ types as wgpu, + CommandEncoder as PlatformCommandEncoder, Gpu, GpuBuilder, Instance, @@ -251,12 +252,10 @@ impl RenderContext { }; let view = frame.texture_view(); - let mut encoder = - self - .device() - .create_command_encoder(&wgpu::CommandEncoderDescriptor { - label: Some("lambda-render-command-encoder"), - }); + let mut encoder = PlatformCommandEncoder::new( + self.device(), + Some("lambda-render-command-encoder"), + ); let mut command_iter = commands.into_iter(); while let Some(command) = command_iter.next() { @@ -271,21 +270,8 @@ impl RenderContext { )) })?; - let color_attachment = wgpu::RenderPassColorAttachment { - view, - depth_slice: None, - resolve_target: None, - ops: pass.color_ops(), - }; - let color_attachments = [Some(color_attachment)]; let mut pass_encoder = - encoder.begin_render_pass(&wgpu::RenderPassDescriptor { - label: pass.label(), - color_attachments: &color_attachments, - depth_stencil_attachment: None, - timestamp_writes: None, - occlusion_query_set: None, - }); + encoder.begin_render_pass(pass.label(), view, pass.color_ops()); self.encode_pass(&mut pass_encoder, viewport, &mut command_iter)?; } @@ -306,7 +292,7 @@ impl RenderContext { /// Encode a single render pass and consume commands until `EndRenderPass`. fn encode_pass( &mut self, - pass: &mut wgpu::RenderPass<'_>, + pass: &mut lambda_platform::wgpu::RenderPass<'_>, initial_viewport: viewport::Viewport, commands: &mut I, ) -> Result<(), RenderError> @@ -372,7 +358,7 @@ impl RenderContext { )); })?; - pass.set_vertex_buffer(buffer as u32, buffer_ref.raw().slice(..)); + pass.set_vertex_buffer(buffer as u32, buffer_ref.raw()); } RenderCommand::PushConstants { pipeline, @@ -394,7 +380,7 @@ impl RenderContext { pass.set_push_constants(stage.to_wgpu(), offset, slice); } RenderCommand::Draw { vertices } => { - pass.draw(vertices, 0..1); + pass.draw(vertices); } RenderCommand::BeginRenderPass { .. } => { return Err(RenderError::Configuration( @@ -411,7 +397,7 @@ impl RenderContext { /// Apply both viewport and scissor state to the active pass. fn apply_viewport( - pass: &mut wgpu::RenderPass<'_>, + pass: &mut lambda_platform::wgpu::RenderPass<'_>, viewport: &viewport::Viewport, ) { let (x, y, width, height, min_depth, max_depth) = viewport.viewport_f32(); From 9e378aaca960f0da388782a955e088afc29c24f0 Mon Sep 17 00:00:00 2001 From: vmarcella Date: Sat, 1 Nov 2025 16:51:36 -0700 Subject: [PATCH 3/3] [fix] test for labels. --- crates/lambda-rs/src/render/buffer.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/crates/lambda-rs/src/render/buffer.rs b/crates/lambda-rs/src/render/buffer.rs index 0b9c4b55..f45d162e 100644 --- a/crates/lambda-rs/src/render/buffer.rs +++ b/crates/lambda-rs/src/render/buffer.rs @@ -318,15 +318,8 @@ mod tests { fn label_is_recorded_on_builder() { let mut builder = BufferBuilder::new(); builder.with_label("buffer-test"); - // Indirect check via building a small buffer would require a device; ensure - // the label setter stores the value locally instead. - // Access through an internal helper to avoid exposing label publicly. - #[allow(clippy::redundant_closure_call)] - { - // Create a small closure to read the private label field. - // The test module shares the parent scope, so it can access fields. - let read = |b: &BufferBuilder| b.label.as_deref(); - assert_eq!(read(&builder), Some("buffer-test")); - } + // Indirect check: validate the internal label is stored on the builder. + // Test module is a child of this module and can access private fields. + assert_eq!(builder.label.as_deref(), Some("buffer-test")); } }