Skip to content

Commit b493999

Browse files
committed
fix(splat3d): address Codex review on render-depth certification
Two correctness fixes on the merged #206 code: - certify_batch_simd no longer panics on a depth_var shorter than batch.len: stage16 now reads via get().unwrap_or(0.0), matching certify_batch_scalar's fallback — honors the "does not panic on malformed input" contract. - HEEL-rejected blocks in cascade_block now return RenderDepthCertificate::ZERO (passed=false) instead of certify_depth_scalar(0,0,0,..), which yielded passed=true under default params and could be misread as certified. Promote the shared zeroed/failed certificate to a public associated const RenderDepthCertificate::ZERO. Add regression tests: short-depth_var SIMD parity (no panic), and HEEL-reject carries a failed certificate. https://claude.ai/code/session_017GFLBnDy23AWBqvkbHHC41
1 parent 9ca4004 commit b493999

2 files changed

Lines changed: 60 additions & 18 deletions

File tree

src/hpc/splat3d/depth_cascade.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ pub fn cascade_block(
146146
priority: 0.0,
147147
estimated_error_px: 0.0,
148148
projected_radius_px: 0.0,
149-
certificate: super::depth_cert::certify_depth_scalar(0.0, 0.0, 0.0, &budget.cert_params),
149+
certificate: RenderDepthCertificate::ZERO,
150150
};
151151
}
152152

@@ -304,6 +304,10 @@ mod tests {
304304
let d = cascade_block(&c, &b, &DepthCascadeBudget::default(), 0);
305305
assert_eq!(d.action, HhtlAction::Reject);
306306
assert_eq!(d.tier_reached, HhtlTier::Heel);
307+
// A HEEL-rejected block must carry a failed (zeroed) certificate, never
308+
// a passing one (Codex regression).
309+
assert!(!d.certificate.passed, "rejected block must not be certified");
310+
assert_eq!(d.certificate.covariance_depth_error, 0.0);
307311
}
308312

309313
#[test]

src/hpc/splat3d/depth_cert.rs

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ pub fn certify_batch_scalar(
241241
out.reserve(batch.len);
242242
for i in 0..batch.len {
243243
if batch.valid[i] == 0 {
244-
out.push(ZERO_CERT);
244+
out.push(RenderDepthCertificate::ZERO);
245245
continue;
246246
}
247247
let cov_zz = depth_var.get(i).copied().unwrap_or(0.0);
@@ -264,7 +264,10 @@ fn stage16(col: &[f32], start: usize, len: usize) -> [f32; 16] {
264264
let mut buf = [0.0f32; 16];
265265
let end = (start + CHUNK).min(len);
266266
for (k, slot) in buf.iter_mut().enumerate().take(end - start) {
267-
*slot = col[start + k];
267+
// Bounds-safe: a short `col` (e.g. a depth_var shorter than batch.len)
268+
// reads 0.0, matching certify_batch_scalar's `.get().unwrap_or(0.0)` —
269+
// the SIMD path must not panic on a mismatched buffer.
270+
*slot = col.get(start + k).copied().unwrap_or(0.0);
268271
}
269272
buf
270273
}
@@ -413,7 +416,7 @@ pub fn certify_batch_simd(
413416
for k in 0..(end - start) {
414417
let idx = start + k;
415418
if batch.valid[idx] == 0 {
416-
out.push(ZERO_CERT);
419+
out.push(RenderDepthCertificate::ZERO);
417420
continue;
418421
}
419422
let cov_e = cov_a[k];
@@ -498,19 +501,23 @@ pub fn mesh_alignment(
498501
}
499502
}
500503

501-
/// Zeroed certificate for culled slots (shared by scalar + SIMD batch paths).
502-
const ZERO_CERT: RenderDepthCertificate = RenderDepthCertificate {
503-
min_depth: 0.0,
504-
max_depth: 0.0,
505-
depth_variance: 0.0,
506-
projected_radius_px: 0.0,
507-
occlusion_confidence: 0.0,
508-
ordering_uncertainty: 0.0,
509-
quantization_depth_error: 0.0,
510-
covariance_depth_error: 0.0,
511-
total_depth_error: 0.0,
512-
passed: false,
513-
};
504+
impl RenderDepthCertificate {
505+
/// Zeroed, **failed** certificate for culled (batch) or HEEL-rejected
506+
/// (cascade) slots. `passed = false` so a rejected/culled block can never
507+
/// be misread as certified.
508+
pub const ZERO: Self = Self {
509+
min_depth: 0.0,
510+
max_depth: 0.0,
511+
depth_variance: 0.0,
512+
projected_radius_px: 0.0,
513+
occlusion_confidence: 0.0,
514+
ordering_uncertainty: 0.0,
515+
quantization_depth_error: 0.0,
516+
covariance_depth_error: 0.0,
517+
total_depth_error: 0.0,
518+
passed: false,
519+
};
520+
}
514521

515522
#[cfg(test)]
516523
mod tests {
@@ -807,12 +814,43 @@ mod tests {
807814
assert!(approx(w.total_depth_error, g.total_depth_error, 1e-4), "i={i} total");
808815
if proj.valid[i] == 0 {
809816
culled += 1;
810-
assert_eq!(g, ZERO_CERT, "i={i} culled slot must be ZERO_CERT");
817+
assert_eq!(g, RenderDepthCertificate::ZERO, "i={i} culled slot must be ZERO_CERT");
811818
}
812819
}
813820
assert!(culled > 0, "test should exercise at least one culled slot");
814821
}
815822

823+
#[test]
824+
fn certify_batch_simd_short_depth_var_does_not_panic() {
825+
// Codex regression: a depth_var shorter than batch.len must not panic
826+
// in the SIMD path — it reads 0.0 for missing lanes, matching the
827+
// scalar path's `.get().unwrap_or(0.0)`.
828+
let mut batch = GaussianBatch::with_capacity(20);
829+
for i in 0..20 {
830+
let mut g = Gaussian3D::unit();
831+
g.mean = [0.0, 0.0, 1.0 + i as f32 * 0.1];
832+
g.scale = [0.1, 0.1, 0.1];
833+
g.quat = [1.0, 0.0, 0.0, 0.0];
834+
g.opacity = 1.0;
835+
batch.push(g);
836+
}
837+
let cam = Camera::identity_at_origin(256, 256);
838+
let mut proj = ProjectedBatch::with_capacity(batch.capacity);
839+
project_batch(&batch, &cam, &mut proj);
840+
let params = DepthCertParams::default();
841+
let short: Vec<f32> = Vec::new(); // deliberately too short (0 < batch.len)
842+
let mut simd = Vec::new();
843+
let mut scalar = Vec::new();
844+
certify_batch_simd(&proj, &short, &params, &mut simd);
845+
certify_batch_scalar(&proj, &short, &params, &mut scalar);
846+
assert_eq!(simd.len(), scalar.len());
847+
for (s, c) in simd.iter().zip(scalar.iter()) {
848+
assert_eq!(s.passed, c.passed);
849+
assert!(approx(s.depth_variance, c.depth_variance, 1e-6));
850+
assert!(approx(s.total_depth_error, c.total_depth_error, 1e-4));
851+
}
852+
}
853+
816854
// ── P4: mesh-anchor alignment + first demo ────────────────────────────────
817855

818856
#[test]

0 commit comments

Comments
 (0)