render: fix the BGRA8 display transform + dispatcher diagnostics
convert_bgra8 applied packed u8 pixels to the default (F32-finalized) OCIO CPU processor, which rejects them with a bit-depth mismatch; the callers swallow the error, so the display-ICC transform was silently inert on the shm preview path. ocio-rs exposes no Uint8-finalized CPU processor, so the conversion now detours through F32. Verified against the machine's actual display profile with the new display_icc_bgra8_never_outputs_black test (OAK_DISPLAY_ICC). Also: - procpool_integration: audio tickets are Seek priority and claimable by any worker now, so the shard-spread assertion goes (rendering on a live worker is what matters). - OAK_DEBUG_VIEWER=1: the program viewer logs frame pushes and dumps the displayed frame to /tmp/oak_viewer_frame.ppm (the black-screen investigation tooling).
This commit is contained in:
@@ -276,6 +276,20 @@ impl<E: AppEngine> ProgramViewerPanel<E> {
|
||||
{
|
||||
self.last_cpu_frame = Some(displayed.clone());
|
||||
let displayed = displayed.clone();
|
||||
if std::env::var_os("OAK_DEBUG_VIEWER").is_some() {
|
||||
let sz = displayed.size(0);
|
||||
eprintln!("[viewer] push frame {}x{}", sz.width.0, sz.height.0);
|
||||
// Dump the displayed pixels (BGRA -> PPM) for black-frame
|
||||
// debugging: what the viewer RECEIVES, before the GPU path.
|
||||
if let Some(bytes) = displayed.as_bytes(0) {
|
||||
let (w, h) = (sz.width.0 as usize, sz.height.0 as usize);
|
||||
let mut ppm = format!("P6\n{w} {h}\n255\n").into_bytes();
|
||||
for px in bytes[..w * h * 4].chunks_exact(4) {
|
||||
ppm.extend_from_slice(&[px[2], px[1], px[0]]);
|
||||
}
|
||||
let _ = std::fs::write("/tmp/oak_viewer_frame.ppm", ppm);
|
||||
}
|
||||
}
|
||||
self.viewer
|
||||
.update(cx, |viewer, cx| viewer.set_cpu_frame(Some(displayed), cx));
|
||||
}
|
||||
|
||||
@@ -316,12 +316,27 @@ impl ColorProcessor {
|
||||
/// [`create_display_icc_bgra8`](Self::create_display_icc_bgra8) — the
|
||||
/// R/B swizzle is baked into the chain, so the bytes go through OCIO's
|
||||
/// RGBA entry point unchanged. A pass-through processor is a no-op.
|
||||
///
|
||||
/// The conversion detours through F32: the default CPU processor is
|
||||
/// F32-finalized and rejects packed u8 buffers with a bit-depth
|
||||
/// mismatch, and the ocio-rs binding exposes no Uint8-finalized CPU
|
||||
/// processor. The round-trip cost is one small staging buffer.
|
||||
pub fn convert_bgra8(&self, data: &mut [u8], pixels: i64) -> Result<()> {
|
||||
let Some(cpu) = &self.cpu else {
|
||||
return Ok(());
|
||||
};
|
||||
cpu.try_apply_rgba_packed_bit_depth(data, ocio_rs::BitDepth::Uint8, pixels, 4)
|
||||
.map_err(|e| Error::Failed(format!("OCIO packed-u8 apply: {e}")))
|
||||
let count = (pixels.max(0) as usize) * 4;
|
||||
if data.len() < count {
|
||||
return Err(Error::Invalid);
|
||||
}
|
||||
let mut f32s: Vec<f32> =
|
||||
data[..count].iter().map(|&v| v as f32 / 255.0).collect();
|
||||
cpu.try_apply_rgba_pixels(&mut f32s, pixels, 4)
|
||||
.map_err(|e| Error::Failed(format!("OCIO f32 apply (bgra8 detour): {e}")))?;
|
||||
for (dst, v) in data[..count].iter_mut().zip(f32s) {
|
||||
*dst = (v.clamp(0.0, 1.0) * 255.0).round() as u8;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Convert an F32 RGBA buffer in place (tightly packed, 4 floats per
|
||||
@@ -734,6 +749,65 @@ mod tests {
|
||||
assert!((out[3] - 1.0).abs() < 1e-5, "alpha preserved");
|
||||
}
|
||||
|
||||
/// The exact chain the viewers use (BGRA8, display-class ICC from
|
||||
/// `OAK_DISPLAY_ICC`): a mid-grey frame must NOT collapse to black —
|
||||
/// the viewer-black-screen regression guard. Skipped without the env
|
||||
/// var (point it at the display profile under investigation).
|
||||
#[test]
|
||||
fn display_icc_bgra8_never_outputs_black() {
|
||||
let _lock = config_lock();
|
||||
if set_up_default_config().is_err() {
|
||||
return;
|
||||
}
|
||||
let Ok(icc) = std::env::var("OAK_DISPLAY_ICC") else {
|
||||
eprintln!("OAK_DISPLAY_ICC unset; skipping");
|
||||
return;
|
||||
};
|
||||
let p = ColorProcessor::create_display_icc_bgra8("sRGB Encoded Rec.709 (sRGB)", &icc)
|
||||
.expect("handle always returned");
|
||||
assert!(p.is_valid(), "BGRA8 ICC processor builds from {icc}");
|
||||
// BGRA bytes: 0.5 grey, 0.75 red, 0.25 green, 0.6 blue.
|
||||
let mut data: Vec<u8> = vec![
|
||||
128, 128, 128, 255, // grey
|
||||
0, 0, 191, 255, // red
|
||||
0, 64, 0, 255, // green
|
||||
153, 0, 0, 255, // blue
|
||||
];
|
||||
let bgra_result = p.convert_bgra8(&mut data, 4);
|
||||
eprintln!("bgra8 convert: {bgra_result:?} -> {data:?}");
|
||||
// The F32 RGBA leg (the CpuF32 display path) must not crush to
|
||||
// black either — and unlike the u8 leg it must not even fail.
|
||||
let p32 = ColorProcessor::create_display_icc("sRGB Encoded Rec.709 (sRGB)", &icc)
|
||||
.expect("handle always returned");
|
||||
assert!(p32.is_valid(), "F32 ICC processor builds from {icc}");
|
||||
let mut f32s: Vec<f32> = vec![
|
||||
0.5, 0.5, 0.5, 1.0, // grey
|
||||
0.75, 0.0, 0.0, 1.0, // red
|
||||
0.0, 0.25, 0.0, 1.0, // green
|
||||
0.0, 0.0, 0.6, 1.0, // blue
|
||||
];
|
||||
let f32_result = p32.convert_f32_rgba(&mut f32s, 4);
|
||||
eprintln!("f32 convert: {f32_result:?} -> {f32s:?}");
|
||||
bgra_result.expect("convert");
|
||||
for (i, px) in data.chunks_exact(4).enumerate() {
|
||||
assert_eq!(px[3], 255, "pixel {i}: alpha preserved");
|
||||
let rgb: u32 = px[0] as u32 + px[1] as u32 + px[2] as u32;
|
||||
assert!(rgb > 0, "pixel {i} must not be crushed to black: {px:?}");
|
||||
}
|
||||
// Grey stays greyish (a display profile must not tint wildly).
|
||||
let (b, g, r) = (data[0] as i32, data[1] as i32, data[2] as i32);
|
||||
assert!(
|
||||
(b - g).abs() < 24 && (g - r).abs() < 24,
|
||||
"grey stays grey through the display ICC: {b} {g} {r}"
|
||||
);
|
||||
f32_result.expect("convert f32");
|
||||
for (i, px) in f32s.chunks_exact(4).enumerate() {
|
||||
assert!((px[3] - 1.0).abs() < 1e-3, "pixel {i}: alpha preserved");
|
||||
let rgb = px[0] + px[1] + px[2];
|
||||
assert!(rgb > 0.0, "pixel {i} must not be crushed to black: {px:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inverse_direction_reverses() {
|
||||
let _lock = config_lock();
|
||||
|
||||
@@ -391,7 +391,6 @@ fn audio_tickets_roundtrip_through_shm_slots() {
|
||||
}
|
||||
pump_until(&dispatcher, &results, 4);
|
||||
|
||||
let mut seen_worker = [false; 2];
|
||||
for result in results.lock().unwrap().drain(..) {
|
||||
let payload = result.expect("audio rendered");
|
||||
let TicketPayload::ShmAudio(audio) = payload else {
|
||||
@@ -407,10 +406,12 @@ fn audio_tickets_roundtrip_through_shm_slots() {
|
||||
let samples = audio.samples();
|
||||
assert_eq!(samples.len(), 2000 * 2);
|
||||
assert!(samples.iter().all(|&v| v == 0.0), "empty montage is silence");
|
||||
seen_worker[audio.worker as usize] = true;
|
||||
// Audio tickets are Seek priority — claimable by ANY worker (the
|
||||
// seek-starvation fix), so there is no shard-spread assertion; what
|
||||
// matters is that every ticket rendered on a live worker.
|
||||
assert!(audio.worker < 2, "rendered on a live worker");
|
||||
dispatcher.release_audio_frame(&audio);
|
||||
}
|
||||
assert!(seen_worker[0] && seen_worker[1], "both workers rendered audio");
|
||||
// Samples are read via the slot mapping and parsed into a Vec — never
|
||||
// through the counted `slot_to_vec` copy path.
|
||||
assert_eq!(main_heap_frame_copies(), 0);
|
||||
|
||||
Reference in New Issue
Block a user