Skip to content

Commit 415167f

Browse files
committed
[update] render context to better handle depth attachment for stencil only passes, fix depth clearing on stencil only passses, and add tests.
1 parent 4c07ca3 commit 415167f

File tree

2 files changed

+95
-35
lines changed

2 files changed

+95
-35
lines changed

crates/lambda-rs-platform/src/wgpu/render_pass.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,12 @@ impl RenderPassBuilder {
365365
// Apply operations to all provided attachments.
366366
attachments.set_operations_for_all(operations);
367367

368-
// Optional depth/stencil attachment. Include stencil ops only when provided
369-
// to avoid referring to a stencil aspect on depth-only formats.
368+
// Optional depth/stencil attachment. Include depth or stencil ops only
369+
// when provided to avoid touching aspects that were not requested.
370370
let depth_stencil_attachment = depth_view.map(|v| {
371-
// Map depth ops (defaulting when not provided)
372-
let dop = depth_ops.unwrap_or_default();
373-
let mapped_depth_ops = Some(match dop.load {
371+
// Map depth ops only when explicitly provided; when `None`, preserve the
372+
// depth aspect, which is important for stencil-only passes.
373+
let mapped_depth_ops = depth_ops.map(|dop| match dop.load {
374374
DepthLoadOp::Load => wgpu::Operations {
375375
load: wgpu::LoadOp::Load,
376376
store: match dop.store {
@@ -387,7 +387,7 @@ impl RenderPassBuilder {
387387
},
388388
});
389389

390-
// Map stencil ops only if explicitly provided
390+
// Map stencil ops only if explicitly provided.
391391
let mapped_stencil_ops = stencil_ops.map(|sop| match sop.load {
392392
StencilLoadOp::Load => wgpu::Operations {
393393
load: wgpu::LoadOp::Load,

crates/lambda-rs/src/render/mod.rs

Lines changed: 89 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -482,8 +482,10 @@ impl RenderContext {
482482
}
483483

484484
// Depth/stencil attachment when either depth or stencil requested.
485-
let want_depth_attachment = pass.depth_operations().is_some()
486-
|| pass.stencil_operations().is_some();
485+
let want_depth_attachment = Self::has_depth_attachment(
486+
pass.depth_operations(),
487+
pass.stencil_operations(),
488+
);
487489

488490
let (depth_view, depth_ops) = if want_depth_attachment {
489491
// Ensure depth texture exists, with proper sample count and format.
@@ -533,29 +535,10 @@ impl RenderContext {
533535
.expect("depth texture should be present")
534536
.view_ref();
535537

536-
// Map depth ops; default when not explicitly provided.
537-
let mapped = match pass.depth_operations() {
538-
Some(dops) => platform::render_pass::DepthOperations {
539-
load: match dops.load {
540-
render_pass::DepthLoadOp::Load => {
541-
platform::render_pass::DepthLoadOp::Load
542-
}
543-
render_pass::DepthLoadOp::Clear(v) => {
544-
platform::render_pass::DepthLoadOp::Clear(v as f32)
545-
}
546-
},
547-
store: match dops.store {
548-
render_pass::StoreOp::Store => {
549-
platform::render_pass::StoreOp::Store
550-
}
551-
render_pass::StoreOp::Discard => {
552-
platform::render_pass::StoreOp::Discard
553-
}
554-
},
555-
},
556-
None => platform::render_pass::DepthOperations::default(),
557-
};
558-
(Some(view_ref), Some(mapped))
538+
// Map depth operations when explicitly provided; leave depth
539+
// untouched for stencil-only passes.
540+
let depth_ops = Self::map_depth_ops(pass.depth_operations());
541+
(Some(view_ref), depth_ops)
559542
} else {
560543
(None, None)
561544
};
@@ -593,7 +576,7 @@ impl RenderContext {
593576
self.encode_pass(
594577
&mut pass_encoder,
595578
pass.uses_color(),
596-
pass.depth_operations().is_some(),
579+
want_depth_attachment,
597580
pass.stencil_operations().is_some(),
598581
viewport,
599582
&mut command_iter,
@@ -618,7 +601,7 @@ impl RenderContext {
618601
&self,
619602
pass: &mut platform::render_pass::RenderPass<'_>,
620603
uses_color: bool,
621-
pass_has_depth: bool,
604+
pass_has_depth_attachment: bool,
622605
pass_has_stencil: bool,
623606
initial_viewport: viewport::Viewport,
624607
commands: &mut I,
@@ -675,7 +658,9 @@ impl RenderContext {
675658
label
676659
)));
677660
}
678-
if !pass_has_depth && pipeline_ref.expects_depth_stencil() {
661+
if !pass_has_depth_attachment
662+
&& pipeline_ref.expects_depth_stencil()
663+
{
679664
let label = pipeline_ref.pipeline().label().unwrap_or("unnamed");
680665
return Err(RenderError::Configuration(format!(
681666
"Render pipeline '{}' expects a depth/stencil attachment but the current pass has none",
@@ -711,7 +696,7 @@ impl RenderContext {
711696
);
712697
util::warn_once(&key, &msg);
713698
}
714-
if pass_has_depth
699+
if pass_has_depth_attachment
715700
&& !pipeline_ref.expects_depth_stencil()
716701
&& warned_no_depth_for_pipeline.insert(pipeline)
717702
{
@@ -865,6 +850,38 @@ impl RenderContext {
865850
self.config = config;
866851
return Ok(());
867852
}
853+
854+
/// Determine whether a pass requires a depth attachment based on depth or
855+
/// stencil operations.
856+
fn has_depth_attachment(
857+
depth_ops: Option<render_pass::DepthOperations>,
858+
stencil_ops: Option<render_pass::StencilOperations>,
859+
) -> bool {
860+
return depth_ops.is_some() || stencil_ops.is_some();
861+
}
862+
863+
/// Map high-level depth operations to platform depth operations, returning
864+
/// `None` when no depth operations were requested.
865+
fn map_depth_ops(
866+
depth_ops: Option<render_pass::DepthOperations>,
867+
) -> Option<platform::render_pass::DepthOperations> {
868+
return depth_ops.map(|dops| platform::render_pass::DepthOperations {
869+
load: match dops.load {
870+
render_pass::DepthLoadOp::Load => {
871+
platform::render_pass::DepthLoadOp::Load
872+
}
873+
render_pass::DepthLoadOp::Clear(value) => {
874+
platform::render_pass::DepthLoadOp::Clear(value as f32)
875+
}
876+
},
877+
store: match dops.store {
878+
render_pass::StoreOp::Store => platform::render_pass::StoreOp::Store,
879+
render_pass::StoreOp::Discard => {
880+
platform::render_pass::StoreOp::Discard
881+
}
882+
},
883+
});
884+
}
868885
}
869886

870887
/// Errors reported while preparing or presenting a frame.
@@ -909,3 +926,46 @@ impl core::fmt::Display for RenderContextError {
909926
}
910927

911928
impl std::error::Error for RenderContextError {}
929+
930+
#[cfg(test)]
931+
mod tests {
932+
use super::*;
933+
use crate::render::render_pass;
934+
935+
#[test]
936+
fn has_depth_attachment_false_when_no_depth_or_stencil() {
937+
let has_attachment = RenderContext::has_depth_attachment(None, None);
938+
assert!(!has_attachment);
939+
}
940+
941+
#[test]
942+
fn has_depth_attachment_true_for_depth_only() {
943+
let depth_ops = Some(render_pass::DepthOperations::default());
944+
let has_attachment = RenderContext::has_depth_attachment(depth_ops, None);
945+
assert!(has_attachment);
946+
}
947+
948+
#[test]
949+
fn has_depth_attachment_true_for_stencil_only() {
950+
let stencil_ops = Some(render_pass::StencilOperations::default());
951+
let has_attachment = RenderContext::has_depth_attachment(None, stencil_ops);
952+
assert!(has_attachment);
953+
}
954+
955+
#[test]
956+
fn map_depth_ops_none_when_no_depth_operations() {
957+
let mapped = RenderContext::map_depth_ops(None);
958+
assert!(mapped.is_none());
959+
}
960+
961+
#[test]
962+
fn map_depth_ops_maps_clear_and_store() {
963+
let depth_ops = render_pass::DepthOperations {
964+
load: render_pass::DepthLoadOp::Clear(0.5),
965+
store: render_pass::StoreOp::Store,
966+
};
967+
let mapped = RenderContext::map_depth_ops(Some(depth_ops)).expect("mapped");
968+
assert_eq!(mapped.load, platform::render_pass::DepthLoadOp::Clear(0.5));
969+
assert_eq!(mapped.store, platform::render_pass::StoreOp::Store);
970+
}
971+
}

0 commit comments

Comments
 (0)