dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Merge branch 'zero-copy-display'

Browse Source 
# Conflicts:
#	core/dr-gpu/src/adjust.rs
#	ui/dr-ui/src/develop.rs
This commit is contained in:
dtourolle
2026-08-17 10:04:14 +02:00
parent 9fc8721fa8 cf8f5b632f
commit cfff6a3302
11 changed files with 564 additions and 139 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/develop.rs
+176 -18
View File
@@ -529,15 +529,25 @@ impl DevelopSession {
Some((op.id, param.id))
}
/// TRACES: FR-DSP-1 | AC-8
/// Render at the requested display size and hand back a Slint image.
///
/// Renders at *viewport* resolution rather than sensor resolution, which
/// is what keeps slider interaction inside the frame budget on a 24 MP
/// file (FR-DSP-1).
///
/// The readback at the end is the temporary bridge documented on
/// `AdjustPass::read_output`: ARCH §6.1 forbids it, and spike S1 removes
/// it by importing the texture into Slint directly.
/// **The image is the texture, not a copy of it.** This used to end in a
/// `read_output` into a `SharedPixelBuffer` — the GPU→CPU→GPU round-trip
/// ARCH §6.1 forbids and AC-8 asserts against, measured at ~7 ms at 4K
/// against a 0.28 ms compute pass. Spike S1 replaced it with
/// `slint::Image::try_from`, which wraps the texture where it already is.
/// The `clone` below is a refcount on the wgpu handle, not on the pixels.
///
/// This only works because the compositor is drawing with the same device
/// the pass wrote with; see `shared_gpu` in the crate root for how that is
/// arranged, and note that nothing here can detect it having gone wrong —
/// a texture from a foreign device is a runtime fault on a real screen,
/// which is why the arrangement is made once at startup and never again.
pub fn render(&mut self, width: u32, height: u32) -> Result<slint::Image, String> {
// Fit the render to the viewport while preserving aspect, so the
// pass does no work on pixels the view will letterbox away.
@@ -550,14 +560,17 @@ impl DevelopSession {
let (w, h) = fit(fw, fh, width.max(1), height.max(1));
let shader = self.graph.compose();
self.adjust
let texture = self
.adjust
.render(&self.demosaiced, &shader, w, h)
.map_err(|e| e.to_string())?;
let (pixels, rw, rh) = self.adjust.read_output().map_err(|e| e.to_string())?;
let buffer =
slint::SharedPixelBuffer::<slint::Rgba8Pixel>::clone_from_slice(&pixels, rw, rh);
Ok(slint::Image::from_rgba8(buffer))
// The import is fallible on format and usage only, and both are fixed
// in `AdjustPass`'s texture descriptor — so a failure here is a
// descriptor that drifted, not anything the caller did. Say that,
// rather than surfacing "InvalidUsage" to a photographer.
slint::Image::try_from(texture.clone())
.map_err(|e| format!("the render target is not importable by the compositor: {e}"))
}
/// TRACES: FR-DSP-7
@@ -647,13 +660,17 @@ impl DevelopSession {
/// screen-sized file. This renders the framed output size instead, so the
/// export is the full-quality path FR-EXP-9 requires.
///
/// The readback here is `export_pixels`, not the display bridge: a file
/// is made of bytes on the CPU and there is no path to one that avoids
/// the transfer. See the note on that method for why the two are separate.
/// This reads pixels back and [`Self::render`] does not, and that is the
/// whole distinction AC-8 draws: a file is made of bytes on the CPU and
/// there is no path to one that avoids the transfer, whereas a frame on
/// screen had no business making the trip. See `AdjustPass::export_pixels`
/// for the longer version.
///
/// Leaves the pass holding a full-resolution target, so the caller should
/// expect the next display render to reallocate. Cheaper than keeping a
/// second pass alive for the exports a session rarely performs.
/// Leaves one of the pass's two targets at full resolution; it is dropped
/// and reallocated on the second display render after this, since the
/// other target still holds a viewport-sized texture and comes up first.
/// Cheaper than keeping a second pass alive for the exports a session
/// rarely performs.
///
/// `space` is the output colour space the file will claim. It is chosen
/// here rather than at encode time because the conversion happens in the
@@ -1106,6 +1123,145 @@ mod tests {
use super::*;
use dr_pipeline::EditGraph;
/// TRACES: FR-DSP-1 | AC-8
/// Copy a displayed frame back to the CPU, for assertions and nothing else.
///
/// The library has no such function on purpose: S1 removed the display
/// readback, and AC-8 is the assertion that it stayed removed. A test that
/// wants to look at the pixels therefore has to do the copy itself, which
/// is exactly the right shape — the round-trip lives in the test binary
/// and cannot be reached from a shipping one.
///
/// Doubles as the proof: this only compiles because the image *is* a wgpu
/// texture. Hand it a `SharedPixelBuffer`-backed image and it panics.
fn read_back(ctx: &GpuContext, image: &slint::Image) -> Vec<u8> {
let texture = image
.to_wgpu_29_texture()
.expect("the develop canvas must be a GPU texture, not a pixel buffer");
let (w, h) = (texture.width(), texture.height());
// Buffer rows must be aligned to COPY_BYTES_PER_ROW_ALIGNMENT.
let unpadded = w * 4;
let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT;
let padded = unpadded.div_ceil(align) * align;
let buf = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: Some("test-readback"),
size: u64::from(padded * h),
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ,
mapped_at_creation: false,
});
let mut enc = ctx.device.create_command_encoder(&Default::default());
enc.copy_texture_to_buffer(
wgpu::TexelCopyTextureInfo {
texture: &texture,
mip_level: 0,
origin: wgpu::Origin3d::ZERO,
aspect: wgpu::TextureAspect::All,
},
wgpu::TexelCopyBufferInfo {
buffer: &buf,
layout: wgpu::TexelCopyBufferLayout {
offset: 0,
bytes_per_row: Some(padded),
rows_per_image: Some(h),
},
},
wgpu::Extent3d {
width: w,
height: h,
depth_or_array_layers: 1,
},
);
ctx.queue.submit(Some(enc.finish()));
let slice = buf.slice(..);
let (tx, rx) = std::sync::mpsc::channel();
slice.map_async(wgpu::MapMode::Read, move |r| {
let _ = tx.send(r);
});
ctx.device
.poll(wgpu::PollType::wait_indefinitely())
.expect("poll");
rx.recv().expect("map").expect("map");
let data = slice.get_mapped_range();
let mut out = Vec::with_capacity((unpadded * h) as usize);
for row in 0..h {
let start = (row * padded) as usize;
out.extend_from_slice(&data[start..start + unpadded as usize]);
}
drop(data);
buf.unmap();
out
}
/// TRACES: FR-DSP-1 | AC-8
#[test]
fn the_displayed_frame_is_a_texture_and_not_a_pixel_buffer() {
// The acceptance criterion itself, asserted from the side that would
// notice it regressing. `to_rgba8` returning `Some` would mean the
// frame had come back through system memory to be looked at, which is
// the ~7 ms per frame at 4K that ARCH §6.1 forbids; `to_wgpu_29_texture`
// returning `Some` means the compositor got the texture where it lay.
//
// Note this passes without a display: the import is a wrapper, and it
// is the *compositor* adopting the device that needs a screen. What
// cannot be proved here is that the picture arrives; what can be
// proved is that no copy was made on the way.
let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else {
log::warn!("no GPU adapter; skipping");
return;
};
let rgba = vec![128u8; 32 * 32 * 4];
let mut session =
DevelopSession::open_rgb(&ctx, &rgba, 32, 32, dr_types::Orientation::NORMAL)
.expect("session");
let frame = session.render(32, 32).expect("render");
assert!(
frame.to_rgba8().is_none(),
"the canvas has CPU pixels, so something copied them there"
);
let texture = frame
.to_wgpu_29_texture()
.expect("the canvas is neither a texture nor a pixel buffer");
assert_eq!((texture.width(), texture.height()), (32, 32));
}
/// TRACES: FR-DSP-1 | AC-8
#[test]
fn consecutive_frames_look_different_to_the_property_system() {
// The catch that comes free with handing over a texture instead of a
// buffer. Slint repaints when the image property *changes*, and it
// decides that with `PartialEq` — which for two images over one
// `wgpu::Texture` says "unchanged". A pass that reused a single target
// would therefore render every slider move correctly and show none of
// them.
//
// `AdjustPass` alternates between two targets to prevent it. This
// asserts the consequence in the terms Slint actually uses, so it
// would still catch the regression if the mechanism were replaced.
let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else {
log::warn!("no GPU adapter; skipping");
return;
};
let rgba = vec![128u8; 32 * 32 * 4];
let mut session =
DevelopSession::open_rgb(&ctx, &rgba, 32, 32, dr_types::Orientation::NORMAL)
.expect("session");
let first = session.render(32, 32).expect("first render");
let second = session.render(32, 32).expect("second render");
assert_ne!(
first, second,
"the canvas property would not change, so the frame would never be shown"
);
}
/// The whole scroll-to-zoom path, end to end, in the order the user drives
/// it: show the image fitted, *then* turn the wheel.
///
@@ -1142,12 +1298,14 @@ mod tests {
assert!(session.is_zoomed(), "the session did not register the zoom");
let zoomed = session.render(64, 64).expect("zoomed render");
let before = fitted.to_rgba8().expect("fitted pixels");
let after = zoomed.to_rgba8().expect("zoomed pixels");
// Both images are still readable here because consecutive frames go to
// alternating textures; see `AdjustPass::targets`. Holding two frames
// at once would be meaningless against a single reused target.
let before = read_back(&ctx, &fitted);
let after = read_back(&ctx, &zoomed);
let differing = before
.as_bytes()
.iter()
.zip(after.as_bytes().iter())
.zip(after.iter())
.filter(|(a, b)| a != b)
.count();
ui/dr-ui/src/lib.rs
+90 -26
View File
@@ -3,12 +3,17 @@
//! A viewer with a develop panel: open a folder of RAW files, decode and
//! demosaic on the GPU, and adjust.
//!
//! **Read before assuming A1 is proven.** Slint's public API for adopting an
//! externally created wgpu texture is not wired up here; this build uploads
//! through `SharedPixelBuffer`, which *is* a CPU round-trip — explicitly the
//! thing ARCH §6.1 forbids in production. Spike S1 replaces it. Until then A1
//! is unvalidated, and the develop path pays a readback per frame that the
//! finished one will not.
//! **The develop frame never leaves the GPU** (ARCH §6.1, AC-8). Spike S1
//! wired Slint's texture import: [`shared_gpu`] opens one wgpu device and
//! gives it to *both* the compute passes and Slint's renderer, and
//! `DevelopSession::render` then hands the compositor the very texture the
//! adjust pass wrote. What used to be a readback and an upload per frame is
//! now a refcount.
//!
//! `SharedPixelBuffer` still appears in this file and in the library grid, and
//! that is not a relapse: an embedded JPEG preview and a thumbnail are decoded
//! on the CPU and have no texture to hand over. AC-8 is about pixels that were
//! *computed on the GPU* travelling to the CPU and back to be looked at.
//!
//! **The develop panel is generated, not written.** [`develop`] asks the
//! pipeline what parameters it has and builds a control per answer; no code
@@ -587,6 +592,64 @@ enum PointsUpdate {
Unchanged,
}
/// TRACES: FR-DSP-1 | AC-8
/// Open the one wgpu device the compute passes and the compositor share.
///
/// **This is the whole of the zero-copy display path, and it is four lines of
/// configuration.** A `wgpu::Texture` belongs to the device that allocated it;
/// handing one to a compositor drawing on a *different* device is meaningless,
/// and the two would have to meet through system memory — which is the round
/// trip ARCH §6.1 forbids. So there is exactly one device, made here, before
/// anything else needs it.
///
/// **Called before the window exists, and it must be.** `BackendSelector`
/// installs the Slint platform, and Slint installs a default one the first
/// time a window is created; selecting afterwards is too late. That is why the
/// GPU is opened at the top of [`run`] rather than beside the other
/// controllers, where it used to sit.
///
/// `None` means develop is unavailable and the viewer falls back to embedded
/// previews — the same degradation as a machine with no adapter at all.
fn shared_gpu() -> Option<dr_gpu::GpuContext> {
let shared = match pollster::block_on(dr_gpu::GpuContext::new_shared()) {
Ok(shared) => shared,
Err(e) => {
log::warn!("no shareable GPU: {e}");
return None;
}
};
let dr_gpu::SharedGpu {
ctx,
instance,
adapter,
} = shared;
// `Manual` is the variant that means "render with these, do not open your
// own". The two clones are of wgpu handles, which are refcounts over the
// one device and the one queue — not copies of either.
let configuration = slint::wgpu_29::WGPUConfiguration::Manual {
instance,
adapter,
device: (*ctx.device).clone(),
queue: (*ctx.queue).clone(),
};
if let Err(e) = slint::BackendSelector::new()
.require_wgpu_29(configuration)
.select()
{
// Dropping the context rather than keeping it: Slint has fallen back
// to a renderer that did not adopt our device, so every texture this
// context produces is one the compositor cannot sample. A disabled
// develop panel is a visible, explicable failure; a texture handed
// across devices is undefined behaviour on a good day.
log::warn!("Slint would not adopt the GPU device, develop disabled: {e}");
return None;
}
Some(ctx)
}
/// TRACES: M-13 | M-14
/// Build and run the viewer.
pub fn run(paths: Vec<PathBuf>) -> Result<()> {
@@ -598,7 +661,21 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
let entries = Rc::new(RefCell::new(collect(&paths)));
log::info!("{} image(s) to browse", entries.borrow().len());
// Before the window, and it has to be: this selects the Slint backend, and
// creating a window selects one for us. See `shared_gpu`. The device is
// shared by demosaic, the adjust pass and the compositor; without one the
// app still browses through the preview path, just without develop.
let gpu = shared_gpu();
let window = AppWindow::new()?;
match &gpu {
Some(ctx) => {
log::info!("adapter: {} ({:?})", ctx.adapter_name(), ctx.backend());
window.set_adapter(ctx.adapter_name().into());
window.set_backend(format!("{:?}", ctx.backend()).to_uppercase().into());
}
None => window.set_backend("NO GPU".into()),
}
// Every background job reports here, and this draws the bar across the top
// of the shell and fills the settings page's list. Built before the
@@ -924,22 +1001,6 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
});
}
// The device is shared by demosaic and the adjust pass. Without one the
// app still browses through the preview path, just without develop.
let gpu = match pollster::block_on(dr_gpu::GpuContext::new_headless()) {
Ok(ctx) => {
log::info!("adapter: {} ({:?})", ctx.adapter_name(), ctx.backend());
window.set_adapter(ctx.adapter_name().into());
window.set_backend(format!("{:?}", ctx.backend()).to_uppercase().into());
Some(ctx)
}
Err(e) => {
log::warn!("no GPU adapter: {e}");
window.set_backend("NO GPU".into());
None
}
};
window.set_total(entries.borrow().len() as i32);
let index = Rc::new(RefCell::new(0usize));
// The current develop session, if the file yielded sensor data.
@@ -985,9 +1046,11 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
// **Half resolution while the gesture is still moving.**
//
// The adjust pass and the readback both scale with pixel count, so
// halving each edge is roughly a quarter of the work — the
// difference between keeping up with a drag and lagging behind it.
// The adjust pass scales with pixel count, so halving each edge is
// roughly a quarter of the work — the difference between keeping
// up with a drag and lagging behind it. Less dramatic since S1
// removed the readback that scaled the same way and cost far more,
// but a dispatch is still not free at 4K.
// A draft frame is visible for one gesture and is replaced by a
// full-resolution one the moment motion stops, so the cost is a
// little softness exactly while the image is moving too fast to
@@ -1055,7 +1118,8 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
// **Rendering is decoupled from input, and this is why.**
//
// A render is a blocking GPU round-trip (see `AdjustPass::read_output`).
// A render used to be a blocking GPU round-trip — S1 removed the block,
// but not the reason for this, so read it as history that still applies.
// Running one straight from a `moved` handler put that stall *inside* the
// gesture: touch events arrive far faster than a render completes, so the
// input queue backed up, positions arrived stale, and Android — seeing the