feat(lambda-rs): buffers to default to DEVICE_LOCAL as opposed to CPU_VISIBLE #180
Conversation
vmarcella
added
lambda-rs
There was a problem hiding this comment.
Pull request overview
This PR updates lambda-rs render buffer defaults to favor GPU-optimal allocations by making Properties::default() (and thus BufferBuilder::new()) default to DEVICE_LOCAL rather than CPU_VISIBLE, and adjusts docs/tests accordingly.
Changes:
- Change
Properties::default()toProperties::DEVICE_LOCALand updateBufferBuilder::new()to inherit that default. - Update
BufferBuilder::build_from_meshto create vertex buffers asDEVICE_LOCAL. - Expand documentation around
CPU_VISIBLEvsDEVICE_LOCALand add unit tests for the new defaults.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// use lambda::render::buffer::{BufferBuilder, Usage, Properties, BufferType}; | ||
| /// let vertices: Vec<Vertex> = build_vertices(); | ||
| /// let vb = BufferBuilder::new() | ||
| /// .with_usage(Usage::VERTEX) | ||
| /// .with_properties(Properties::DEVICE_LOCAL) | ||
| /// // Defaults to `Properties::DEVICE_LOCAL` (recommended for static geometry). | ||
| /// .with_buffer_type(BufferType::Vertex) |
There was a problem hiding this comment.
This doc example still imports Properties but no longer uses it after removing the explicit .with_properties(...) call. To keep the example self-consistent, either drop Properties from the use list or keep an explicit .with_properties(Properties::DEVICE_LOCAL) call (with a comment that it matches the default).
| /// Use `CPU_VISIBLE` for buffers you plan to update from the CPU using | ||
| /// `Buffer::write_*` (this enables `wgpu::Queue::write_buffer` by adding the | ||
| /// required `COPY_DST` usage). | ||
| /// | ||
| /// Prefer `DEVICE_LOCAL` for static geometry uploaded once and never modified. | ||
| /// This is typically the best default for vertex and index buffers on discrete | ||
| /// GPUs, where CPU-visible memory may live in system RAM rather than VRAM. |
There was a problem hiding this comment.
The docs here imply CPU_VISIBLE impacts actual memory residency (system RAM vs VRAM). In this codebase, cpu_visible ultimately only adds wgpu::BufferUsages::COPY_DST (see lambda-rs-platform buffer builder) and does not request mapping (e.g., MAP_WRITE), so it likely doesn’t control where the buffer is placed. Suggest rewording to focus on “uploadable via queue writes (COPY_DST)” vs “not uploadable”, and avoid claims about RAM/VRAM placement unless the platform layer truly enforces that.
| /// Use `CPU_VISIBLE` for buffers you plan to update from the CPU using | ||
| /// `Buffer::write_*` (this enables `wgpu::Queue::write_buffer` by adding the | ||
| /// required `COPY_DST` usage). |
There was a problem hiding this comment.
With Properties now defaulting to DEVICE_LOCAL (i.e., no COPY_DST), it becomes easy for callers to create a buffer and then call Buffer::write_*, which will hit a wgpu validation error/panic because Queue::write_buffer requires COPY_DST. Consider making write_bytes/write_value/write_slice return a Result (or adding an explicit runtime check that produces a clear error) when the underlying buffer wasn’t created with CPU_VISIBLE/COPY_DST.
✅ Coverage Report📊 View Full HTML Report (download artifact) Overall Coverage
Changed Files in This PR
PR Files Coverage: 87.87% (261/297 lines) Generated by cargo-llvm-cov · Latest main coverage Last updated: 2026-02-09 21:54:54 UTC · Commit: |
1 participant
Summary
This PR changes buffer allocation defaults to favor GPU-optimal residency.
Previously,
Properties::default()(andBufferBuilder::new()) defaulted toCPU_VISIBLE, which biases buffers toward CPU-visible memory and implicitly addsCOPY_DST. That default is suboptimal for static vertex/index buffers on discrete GPUs because the memory is being allocated on the hosts RAM as opposed to the GPUs VRAM.Now, buffers default to
DEVICE_LOCAL, and callers must opt intoCPU_VISIBLEonly when they intend to update the buffer from the CPU viaBuffer::write_*.Related Issues
Changes
Properties::default()to returnProperties::DEVICE_LOCAL.BufferBuilder::new()to inherit thePropertiesdefault.BufferBuilder::build_from_meshto create vertex buffers asDEVICE_LOCAL.Type of Change
Affected Crates
lambda-rslambda-rs-platformlambda-rs-argslambda-rs-loggingChecklist
cargo +nightly fmt --all)cargo clippy --workspace --all-targets -- -D warnings)cargo test --workspace)Testing
Commands run:
cargo test -p lambda-rsManual verification steps (if applicable):
BufferBuilder::build_from_mesh(static geometry) and confirm it still renders correctly.CPU_VISIBLE.Screenshots/Recordings
N/A (no UI change).
Platform Testing
Additional Notes
Buffer::write_value/write_slice/write_bytes), add.with_properties(Properties::CPU_VISIBLE)when constructing that buffer.