Uh oh!
There was an error while loading. Please reload this page.
Add support for Uniform Buffer objects - #64
Conversation
… simplifying camera code.
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive support for uniform buffer objects (UBOs) to the rendering system, enabling shaders to receive structured constant data through bind groups. The implementation spans platform bindings, high-level API wrappers, documentation, and working examples.
- Core bind group and bind group layout builders with uniform buffer support and dynamic offset capabilities
- Scene math utilities for common 3D transformations (model, view, projection matrices)
- Validation helpers for alignment and offset checking
- Complete tutorial and specification documentation with working example
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/tutorials/uniform-buffers.md | Tutorial demonstrating uniform buffer usage with a spinning triangle example |
| docs/tutorials/README.md | Index page linking to the uniform buffers tutorial |
| docs/specs/uniform-buffers-and-bind-groups.md | Specification detailing UBO architecture, API design, and constraints |
| docs/specs/_spec-template.md | Template for future specification documents |
| crates/lambda-rs/src/render/validation.rs | Alignment and dynamic offset validation helpers |
| crates/lambda-rs/src/render/scene_math.rs | Camera and transform matrix computation utilities |
| crates/lambda-rs/src/render/pipeline.rs | Pipeline builder extended with bind group layouts and culling mode |
| crates/lambda-rs/src/render/mod.rs | Module exports and render context updates for bind groups |
| crates/lambda-rs/src/render/command.rs | SetBindGroup command for binding resources during render passes |
| crates/lambda-rs/src/render/buffer.rs | Buffer write utilities and UniformBuffer wrapper type |
| crates/lambda-rs/src/render/bind.rs | High-level bind group and layout builders |
| crates/lambda-rs/examples/uniform_buffer_triangle.rs | Working example demonstrating uniform buffer usage |
| crates/lambda-rs/examples/push_constants.rs | Updated to use scene_math helpers |
| crates/lambda-rs-platform/src/wgpu/mod.rs | Platform module exports for bind functionality |
| crates/lambda-rs-platform/src/wgpu/bind.rs | Platform-layer bind group wrappers and builders |
| README.md | Updated with tutorials section and links |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| &camera, | ||
| width.max(1), | ||
| height.max(1), | ||
| [0.0, -1.0 / 3.0, 0.0], // pivot |
There was a problem hiding this comment.
The inline comment for the pivot parameter does not explain why this specific value (-1/3 on Y axis) is chosen. Consider adding context about what geometric feature this pivot represents (e.g., 'pivot at triangle centroid' or 'pivot at geometric center').
| [0.0, -1.0 / 3.0, 0.0], // pivot | |
| [0.0, -1.0 / 3.0, 0.0], // pivot at triangle centroid (geometric center) |
| } | ||
| let align = alignment.max(1); | ||
| for (i, off) in offsets.iter().enumerate() { | ||
| if (*off as u32) % align != 0 { |
There was a problem hiding this comment.
The cast (*off as u32) is redundant since off is already dereferenced from &[u32] and is therefore type u32. Simplify to if *off % align != 0.
| if(*offasu32) % align != 0{ | |
| if*off % align != 0{ |
| pub fn with_layouts(mut self, layouts: &[&bind::BindGroupLayout]) -> Self { | ||
| self.bind_group_layouts = layouts | ||
| .iter() | ||
| .map(|l| std::rc::Rc::new(l.raw().clone())) |
There was a problem hiding this comment.
Cloning the underlying wgpu::BindGroupLayout for each layout entry may be expensive. Consider whether the high-level BindGroupLayout can be stored directly with Rc to avoid cloning the GPU resource, or document why the clone is necessary here.
| pubfn with_layouts(mutself,layouts:&[&bind::BindGroupLayout]) -> Self{ | |
| self.bind_group_layouts = layouts | |
| .iter() | |
| .map(|l| std::rc::Rc::new(l.raw().clone())) | |
| pubfn with_layouts(mutself,layouts:&[std::rc::Rc<bind::BindGroupLayout>]) -> Self{ | |
| self.bind_group_layouts = layouts | |
| .iter() | |
| .cloned() |
| let bytes = unsafe { | ||
| std::slice::from_raw_parts( | ||
| (data as *const T) as *const u8, | ||
| std::mem::size_of::<T>(), | ||
| ) | ||
| }; |
There was a problem hiding this comment.
The unsafe block lacks a SAFETY comment explaining why the raw pointer dereference is safe. Add documentation stating that data is a valid reference to T, T is Copy (ensuring no invalid bit patterns), and the lifetime of the slice does not exceed the borrow of data.
| &camera, | ||
| self.width.max(1), | ||
| self.height.max(1), | ||
| [0.0, -1.0 / 3.0, 0.0], |
There was a problem hiding this comment.
The magic number -1.0 / 3.0 appears multiple times in the file (lines 195, 300) without explanation. Consider defining this as a named constant like TRIANGLE_PIVOT_Y to improve code clarity and maintainability.
… wrap the actual gpu resources.
Uh oh!
There was an error while loading. Please reload this page.
No description provided.