Refactor rendering API #66
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors render pipeline creation by introducing builder patterns in the platform layer. The changes extract shader module creation, pipeline layout construction, and render pipeline building from the application layer into reusable platform-layer builders.
- Adds new builder types (
ShaderModule,PipelineLayoutBuilder,RenderPipelineBuilder) to the platform layer - Migrates render pipeline construction from verbose wgpu descriptor creation to a cleaner builder-based API
- Simplifies the application-layer pipeline code by delegating low-level wgpu details to the platform layer
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
crates/lambda-rs-platform/src/wgpu/pipeline.rs |
New platform-layer module providing builders for shader modules, pipeline layouts, and render pipelines |
crates/lambda-rs-platform/src/wgpu/mod.rs |
Adds the new pipeline module to the platform layer's public API |
crates/lambda-rs/src/render/pipeline.rs |
Refactored to use the new platform-layer builders, removing direct wgpu descriptor construction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[derive(Debug)] | ||
| /// Wrapper around `wgpu::ShaderModule` that preserves a label. | ||
| pub struct ShaderModule { |
There was a problem hiding this comment.
Doc comment should appear before the derive attribute, not between it and the struct declaration. Move the doc comment above line 5.
| #[derive(Debug)] | ||
| /// Wrapper around `wgpu::PipelineLayout`. |
There was a problem hiding this comment.
Doc comment should appear before the derive attribute, not between it and the struct declaration. Move the doc comment above line 35.
| #[derive(Debug)] | |
| /// Wrapper around `wgpu::PipelineLayout`. | |
| /// Wrapper around `wgpu::PipelineLayout`. | |
| #[derive(Debug)] |
| impl<'a> PipelineLayoutBuilder<'a> { | ||
| /// New builder with no layouts or push constants. | ||
| pub fn new() -> Self { |
There was a problem hiding this comment.
The builder implements new() but doesn't implement the Default trait. Either implement Default for PipelineLayoutBuilder or add #[allow(clippy::new_without_default)] to suppress the clippy warning.
…tly used inside the high level rendering engine.
… few other minor things.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| /// Public, engine-facing surface configuration that avoids exposing `wgpu`. |
There was a problem hiding this comment.
[nitpick] Inconsistent hyphenation: 'engine-facing' should be 'engine‑facing' to match the endash style used elsewhere in the PR.
| /// Public, engine-facing surface configuration that avoids exposing `wgpu`. | |
| /// Public, engine‑facing surface configuration that avoids exposing `wgpu`. |
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug)] | ||
| /// Public, engine-facing subset of device limits. |
There was a problem hiding this comment.
[nitpick] Inconsistent hyphenation: 'engine-facing' should be 'engine‑facing' to match the endash style used elsewhere in the PR.
| /// Public, engine-facing subset of device limits. | |
| /// Public, engine‑facing subset of device limits. |
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. | ||
| pub(crate) fn adapter(&self) -> &wgpu::Adapter { | ||
| &self.adapter | ||
| } | ||
|
|
||
| /// Borrow the logical device for resource creation. | ||
| /// | ||
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. | ||
| pub(crate) fn device(&self) -> &wgpu::Device { | ||
| &self.device | ||
| } | ||
|
|
||
| /// Borrow the submission queue for command submission. | ||
| /// | ||
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. |
There was a problem hiding this comment.
[nitpick] Inconsistent hyphenation: 'Crate-visible' should be 'Crate‑visible' to match the endash style used elsewhere in the PR.
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. | |
| pub(crate) fn adapter(&self) -> &wgpu::Adapter { | |
| &self.adapter | |
| } | |
| /// Borrow the logical device for resource creation. | |
| /// | |
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. | |
| pub(crate) fn device(&self) -> &wgpu::Device { | |
| &self.device | |
| } | |
| /// Borrow the submission queue for command submission. | |
| /// | |
| /// Crate-visible to avoid exposing raw `wgpu` to higher layers. | |
| /// Crate‑visible to avoid exposing raw `wgpu` to higher layers. | |
| pub(crate) fn adapter(&self) -> &wgpu::Adapter { | |
| &self.adapter | |
| } | |
| /// Borrow the logical device for resource creation. | |
| /// | |
| /// Crate‑visible to avoid exposing raw `wgpu` to higher layers. | |
| pub(crate) fn device(&self) -> &wgpu::Device { | |
| &self.device | |
| } | |
| /// Borrow the submission queue for command submission. | |
| /// | |
| /// Crate‑visible to avoid exposing raw `wgpu` to higher layers. |
No description provided.