Regenerate the traceability matrix
The platform layer was never scanned, so every FR-PLAT-* and NFR-PORT-* tag in dr-plat was invisible. Coverage 51.4% -> 57.6%, almost all of it pre-existing tags that were simply not being counted.
This commit is contained in:
+168
-3
@@ -686,6 +686,22 @@ pub struct DevelopSession {
|
||||
/// open. One value rather than one per operation, for the same reason
|
||||
/// `curve_samples` is one polyline: the panel draws one curve.
|
||||
curve_channel: usize,
|
||||
/// TRACES: FR-DSP-8
|
||||
/// The space the canvas is encoded into, for the display now showing it.
|
||||
///
|
||||
/// **Not part of the edit, and not interface state either.** It is a fact
|
||||
/// about the glass in front of the photographer: the same graph on the
|
||||
/// same file composes differently on a wide-gamut second monitor, and
|
||||
/// neither the sidecar nor the undo stack has any business knowing about
|
||||
/// it. That is also why it lives here rather than on the `EditGraph` —
|
||||
/// `compose_for` deliberately takes the space per call because "the same
|
||||
/// edit goes to the screen in the display's space and to a file in
|
||||
/// whatever the export asks for, and neither is more authoritative".
|
||||
///
|
||||
/// sRGB until the application says otherwise, which is the same answer
|
||||
/// `dr_plat::display`'s fallback gives and means a session constructed in
|
||||
/// a test behaves exactly as it did before this existed.
|
||||
display_space: dr_types::ColourSpace,
|
||||
}
|
||||
|
||||
impl DevelopSession {
|
||||
@@ -756,6 +772,7 @@ impl DevelopSession {
|
||||
show_overlay: false,
|
||||
active_tab: None,
|
||||
curve_channel: 0,
|
||||
display_space: dr_types::ColourSpace::Srgb,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2624,6 +2641,30 @@ impl DevelopSession {
|
||||
Some((op.id, param.id))
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// Encode the canvas for a different display from now on.
|
||||
///
|
||||
/// Returns whether anything changed, so a caller polling for window moves
|
||||
/// can redraw only when the answer is genuinely different — the poll runs
|
||||
/// far more often than a monitor is changed, and a redraw per poll would
|
||||
/// undo the point of rendering on demand.
|
||||
///
|
||||
/// Nothing is invalidated here and nothing needs to be. The next
|
||||
/// [`Self::render`] composes against the new space, the pipeline cache
|
||||
/// distinguishes the two shaders by the structure hash the space enters,
|
||||
/// and the mask array — rasterised in source space, sampled through the
|
||||
/// framing — is unaffected because a colour space is not a geometry.
|
||||
pub fn set_display_space(&mut self, space: dr_types::ColourSpace) -> bool {
|
||||
let changed = self.display_space != space;
|
||||
self.display_space = space;
|
||||
changed
|
||||
}
|
||||
|
||||
/// The space the canvas is currently being encoded into.
|
||||
pub fn display_space(&self) -> dr_types::ColourSpace {
|
||||
self.display_space
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-1 | AC-8
|
||||
/// Render at the requested display size and hand back a Slint image.
|
||||
///
|
||||
@@ -2654,11 +2695,24 @@ impl DevelopSession {
|
||||
let (fw, fh) = self.graph.output_size(sw, sh);
|
||||
let (w, h) = fit(fw, fh, width.max(1), height.max(1));
|
||||
|
||||
let shader = self.graph.compose();
|
||||
// TRACES: FR-DSP-8 | FR-DSP-6
|
||||
// **Composed for the display that is showing this canvas**, not for
|
||||
// sRGB. This is the whole of FR-DSP-8's second half arriving at the
|
||||
// pipeline: a display change is a *recomposition* and nothing more,
|
||||
// because the output space was always a parameter of composition and
|
||||
// always entered the structure hash. Moving the window to a P3 panel
|
||||
// therefore costs one shader compile and no pipeline change at all.
|
||||
let space = self.display_space;
|
||||
let shader = self.graph.compose_for(space);
|
||||
|
||||
// Rasterise the masks first: the shader addresses array slices by
|
||||
// index, so the array has to describe *this* stack before it is bound.
|
||||
self.render_with_masks(&shader, w, h, dr_types::ColourSpace::Srgb)?;
|
||||
//
|
||||
// The same `space` to both, necessarily: where a detail stage exists
|
||||
// it is the *last* pass that performs the output transform, and two
|
||||
// halves composed for different spaces would encode the frame twice
|
||||
// or not at all.
|
||||
self.render_with_masks(&shader, w, h, space)?;
|
||||
let texture = self.adjust.output().ok_or("nothing was rendered")?;
|
||||
|
||||
// The import is fallible on format and usage only, and both are fixed
|
||||
@@ -2702,7 +2756,11 @@ impl DevelopSession {
|
||||
/// explicit about. It is in the output colour space, which is what
|
||||
/// FR-DSP-7 asks for — the levels counted are the levels the display will
|
||||
/// show, so a clipped bin means a highlight that is actually gone rather
|
||||
/// than one the transform might still recover. And when the view is zoomed
|
||||
/// than one the transform might still recover. Since FR-DSP-8 that is the
|
||||
/// space of *this display* rather than sRGB, which makes the reading more
|
||||
/// truthful and not less: a highlight that survives on a wide-gamut panel
|
||||
/// and clips on the laptop's screen genuinely is two different facts, and
|
||||
/// the histogram now reports whichever one the photographer is looking at. And when the view is zoomed
|
||||
/// or cropped it describes the visible region, not the whole file: a
|
||||
/// photographer inspecting a highlight at 4× is asking about *that*
|
||||
/// highlight, and a histogram of the parts of the frame off screen would
|
||||
@@ -5407,4 +5465,111 @@ mod tests {
|
||||
session.set_active_tab(99);
|
||||
assert_eq!(session.rows().len(), all);
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// Per-display colour (FR-DSP-8)
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
/// TRACES: FR-DSP-8 | FR-DSP-6
|
||||
/// The canvas is encoded for the display, not always for sRGB.
|
||||
///
|
||||
/// This is the assertion the requirement is actually about. Before it,
|
||||
/// `render` composed `ColourSpace::Srgb` unconditionally, and a second
|
||||
/// monitor with a different profile got sRGB pixels *labelled* as its own
|
||||
/// space by the compositor — the silent wrongness FR-DSP-8 calls a
|
||||
/// correctness defect. If someone re-hardcodes the space, these pixels
|
||||
/// stop differing and this fails.
|
||||
///
|
||||
/// A saturated red is the probe deliberately: it sits near the edge of
|
||||
/// sRGB's gamut, so re-encoding it into a wider one moves it a long way,
|
||||
/// where a mid grey would move by almost nothing in any of the four and
|
||||
/// the test would pass on a broken build.
|
||||
#[test]
|
||||
fn the_canvas_is_encoded_for_the_display_showing_it() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..64 * 64).flat_map(|_| [230u8, 20, 20, 255]).collect();
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 64, 64, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
assert_eq!(
|
||||
session.display_space(),
|
||||
dr_types::ColourSpace::Srgb,
|
||||
"a session starts on the fallback, so nothing changes for a \
|
||||
desktop whose display server will not say otherwise"
|
||||
);
|
||||
let on_srgb = read_back(&ctx, &session.render(64, 64).expect("render"));
|
||||
|
||||
assert!(session.set_display_space(dr_types::ColourSpace::AdobeRgb));
|
||||
let on_wide = read_back(&ctx, &session.render(64, 64).expect("render"));
|
||||
|
||||
assert_ne!(
|
||||
on_srgb, on_wide,
|
||||
"the same edit rendered for two displays produced the same pixels"
|
||||
);
|
||||
|
||||
// And back again, because a photographer dragging a window between
|
||||
// two monitors expects the first one to look as it did rather than to
|
||||
// accumulate a transform.
|
||||
assert!(session.set_display_space(dr_types::ColourSpace::Srgb));
|
||||
let returned = read_back(&ctx, &session.render(64, 64).expect("render"));
|
||||
assert_eq!(on_srgb, returned);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// A move that changes nothing reports nothing, so nothing is redrawn.
|
||||
///
|
||||
/// The window's position is polled twice a second and two displays often
|
||||
/// share a profile. A setter that reported a change every time it was
|
||||
/// called would turn that poll into a redraw loop.
|
||||
#[test]
|
||||
fn setting_the_same_display_space_twice_is_not_a_change() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..8 * 8).flat_map(|_| [128u8, 128, 128, 255]).collect();
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 8, 8, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
assert!(session.set_display_space(dr_types::ColourSpace::DisplayP3));
|
||||
assert!(!session.set_display_space(dr_types::ColourSpace::DisplayP3));
|
||||
assert_eq!(session.display_space(), dr_types::ColourSpace::DisplayP3);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8 | FR-EXP-2
|
||||
/// The display's space is the *canvas's*, and reaches nothing else.
|
||||
///
|
||||
/// A thumbnail goes into a shard that syncs between devices and an export
|
||||
/// claims the space the export dialogue asked for. Letting the monitor in
|
||||
/// front of the photographer decide either would write a file whose
|
||||
/// profile describes the desk it was made at.
|
||||
#[test]
|
||||
fn a_wide_gamut_monitor_does_not_reach_the_thumbnail_or_the_export() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..32 * 32).flat_map(|_| [230u8, 20, 20, 255]).collect();
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 32, 32, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
let thumb_before = session.render_thumbnail(16).expect("thumbnail");
|
||||
let export_before = session
|
||||
.render_for_export(dr_types::ColourSpace::Srgb)
|
||||
.expect("export")
|
||||
.rgba;
|
||||
|
||||
session.set_display_space(dr_types::ColourSpace::ProPhoto);
|
||||
|
||||
assert_eq!(
|
||||
session.render_thumbnail(16).expect("thumbnail"),
|
||||
thumb_before,
|
||||
"the grid's thumbnail followed the monitor"
|
||||
);
|
||||
assert_eq!(
|
||||
session
|
||||
.render_for_export(dr_types::ColourSpace::Srgb)
|
||||
.expect("export")
|
||||
.rgba,
|
||||
export_before,
|
||||
"an sRGB export followed the monitor"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,411 @@
|
||||
//! TRACES: FR-DSP-8
|
||||
//! Following the canvas from one display to the next.
|
||||
//!
|
||||
//! `dr_plat::display` establishes what each display *is*. This decides which
|
||||
//! of them is showing the canvas right now, keeps the develop session's output
|
||||
//! space pointed at it, and renders at that display's physical pixel size
|
||||
//! rather than its logical one.
|
||||
//!
|
||||
//! # Why a poll and not a callback
|
||||
//!
|
||||
//! Slint reports a window's size and its scale factor and does not report that
|
||||
//! it has moved — there is no `on_moved`, on any backend. What there is, is
|
||||
//! `Window::position()`, so this samples it. A sample is three integers
|
||||
//! compared against three integers and happens twice a second; a monitor is
|
||||
//! changed at human speed, and nothing here runs on the frame path.
|
||||
//!
|
||||
//! The re-survey is what costs something — a Wayland roundtrip pair, or an X11
|
||||
//! property fetch — so it is deliberately *not* done every tick. It runs when
|
||||
//! the geometry actually changed, which is also the moment a monitor could
|
||||
//! have been plugged in or unplugged: both of those move or rescale the window
|
||||
//! on every desktop that does anything sensible.
|
||||
|
||||
use std::cell::{Cell, RefCell};
|
||||
use std::rc::Rc;
|
||||
|
||||
use dr_plat::DisplaySurvey;
|
||||
use slint::ComponentHandle;
|
||||
|
||||
use crate::AppWindow;
|
||||
|
||||
/// How often the window's geometry is sampled.
|
||||
const POLL: std::time::Duration = std::time::Duration::from_millis(500);
|
||||
|
||||
/// The render-target size for a canvas that occupies `logical` logical pixels
|
||||
/// on a display scaled by `scale`.
|
||||
///
|
||||
/// # FR-DSP-8's scaling clause, in one multiplication
|
||||
///
|
||||
/// "Fractional and mixed DPI scaling are handled without resampling artefacts
|
||||
/// in the canvas." The canvas is a `wgpu::Texture` handed straight to the
|
||||
/// compositor (ARCH §6.1), which draws it into a box measured in *logical*
|
||||
/// pixels. Render 1600 logical pixels wide onto a display at 1.25 and the
|
||||
/// compositor has 1600 samples to fill 2000 device pixels, so every frame is
|
||||
/// resampled up by a quarter — a softness that reads like a bad demosaic
|
||||
/// rather than like a scaling bug, which is exactly why the requirement calls
|
||||
/// it out and why it is worth an explicit test.
|
||||
///
|
||||
/// Rendering the physical count instead makes the presentation 1:1. It costs
|
||||
/// what the extra pixels cost — 56% more work at 1.25 — and that cost is the
|
||||
/// requirement: the alternative is not cheaper, it is blurrier.
|
||||
///
|
||||
/// A `scale` of zero or worse is clamped rather than trusted. It arrives from
|
||||
/// the windowing system, and a zero would collapse the render target to one
|
||||
/// pixel and blank the canvas.
|
||||
pub(crate) fn physical(logical: (u32, u32), scale: f32) -> (u32, u32) {
|
||||
let scale = if scale.is_finite() && scale > 0.01 {
|
||||
scale
|
||||
} else {
|
||||
1.0
|
||||
};
|
||||
let convert = |v: u32| ((v as f32) * scale).round().max(1.0) as u32;
|
||||
(convert(logical.0), convert(logical.1))
|
||||
}
|
||||
|
||||
/// The geometry the window had when the display was last resolved.
|
||||
///
|
||||
/// Position *and* scale factor, because either changing can mean a different
|
||||
/// display: dragging across a seam moves the window, and a compositor that
|
||||
/// rescales in place — a fractional-scaling change, or a monitor unplugged
|
||||
/// from under a window — changes only the second.
|
||||
#[derive(Clone, Copy, PartialEq)]
|
||||
struct Geometry {
|
||||
x: i32,
|
||||
y: i32,
|
||||
scale_milli: u32,
|
||||
}
|
||||
|
||||
impl Geometry {
|
||||
fn of(window: &AppWindow) -> Self {
|
||||
let position = window.window().position();
|
||||
Self {
|
||||
x: position.x,
|
||||
y: position.y,
|
||||
// Compared as thousandths rather than as a float: this is an
|
||||
// equality test running twice a second, and a scale factor that
|
||||
// differs in its last bit must not read as a display change and
|
||||
// provoke a re-survey on every tick.
|
||||
scale_milli: (window.window().scale_factor().max(0.0) * 1000.0) as u32,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Everything the window needs to keep the canvas on the right display.
|
||||
pub(crate) struct DisplayWatch {
|
||||
survey: RefCell<DisplaySurvey>,
|
||||
/// Which entry of the survey is showing the canvas.
|
||||
current: Cell<usize>,
|
||||
seen: Cell<Option<Geometry>>,
|
||||
/// The canvas's size in *logical* pixels, as Slint last reported it.
|
||||
///
|
||||
/// Kept so that a scale-factor change can re-derive the physical size
|
||||
/// without waiting for a resize that may never come: moving a window
|
||||
/// between a 1× and a 2× display changes what to render and not how large
|
||||
/// the box is.
|
||||
logical_canvas: Cell<(u32, u32)>,
|
||||
}
|
||||
|
||||
impl DisplayWatch {
|
||||
/// Ask the platform once, at startup.
|
||||
pub(crate) fn probe() -> Rc<Self> {
|
||||
let survey = DisplaySurvey::probe();
|
||||
log::info!(
|
||||
"display server: {}, {} display(s)",
|
||||
survey.server.label(),
|
||||
survey.displays.len()
|
||||
);
|
||||
for display in &survey.displays {
|
||||
log::info!(" {}: {}", display.name, display.profile.describe());
|
||||
}
|
||||
Rc::new(Self {
|
||||
survey: RefCell::new(survey),
|
||||
current: Cell::new(0),
|
||||
seen: Cell::new(None),
|
||||
logical_canvas: Cell::new((1024, 768)),
|
||||
})
|
||||
}
|
||||
|
||||
/// The output space the canvas should be encoded into right now.
|
||||
pub(crate) fn space(&self) -> dr_types::ColourSpace {
|
||||
self.survey.borrow().profile(self.current.get()).space
|
||||
}
|
||||
|
||||
/// Record the canvas's logical size and return the size to render at.
|
||||
pub(crate) fn canvas_resized(&self, window: &AppWindow, logical: (u32, u32)) -> (u32, u32) {
|
||||
self.logical_canvas.set(logical);
|
||||
physical(logical, window.window().scale_factor())
|
||||
}
|
||||
|
||||
/// The size the canvas should be rendered at, from the last known logical
|
||||
/// size and the current scale factor.
|
||||
fn canvas_physical(&self, window: &AppWindow) -> (u32, u32) {
|
||||
physical(self.logical_canvas.get(), window.window().scale_factor())
|
||||
}
|
||||
|
||||
/// Push the About-page readouts.
|
||||
fn publish(&self, window: &AppWindow) {
|
||||
let Some(readouts) = readouts(&self.survey.borrow(), self.current.get()) else {
|
||||
return;
|
||||
};
|
||||
window.set_display_name(readouts.name.into());
|
||||
window.set_display_colour(readouts.colour.into());
|
||||
window.set_display_others(readouts.others.into());
|
||||
}
|
||||
|
||||
/// Re-ask the platform and re-resolve which display is showing the canvas.
|
||||
///
|
||||
/// Returns whether the output space changed, which is what decides a
|
||||
/// redraw. Nothing is pushed into the develop session from here: the
|
||||
/// session is asked for its space on the way into every render (see
|
||||
/// `render_now` in the crate root), so "the canvas is composed for the
|
||||
/// display showing it" is structural rather than something a callback has
|
||||
/// to remember to do. A session opened while the window sits on the second
|
||||
/// monitor is then right on its first frame, without this having to know
|
||||
/// that a photograph was opened at all.
|
||||
fn resolve(&self, window: &AppWindow) -> bool {
|
||||
let before = self.space();
|
||||
*self.survey.borrow_mut() = DisplaySurvey::probe();
|
||||
|
||||
// The window's *centre*, in the display server's own screen
|
||||
// coordinates. A window straddling a seam is showing more of itself on
|
||||
// one side, and its centre says which — where its top-left corner
|
||||
// would flip the transform as soon as one pixel crossed.
|
||||
let position = window.window().position();
|
||||
let size = window.window().size();
|
||||
let centre_x = position.x.saturating_add_unsigned(size.width / 2);
|
||||
let centre_y = position.y.saturating_add_unsigned(size.height / 2);
|
||||
|
||||
let index = self.survey.borrow().containing(centre_x, centre_y);
|
||||
self.current.set(index);
|
||||
self.publish(window);
|
||||
self.space() != before
|
||||
}
|
||||
}
|
||||
|
||||
/// The three strings the About page shows about the session's colour path.
|
||||
pub(crate) struct Readouts {
|
||||
pub name: String,
|
||||
pub colour: String,
|
||||
pub others: String,
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// Turn a survey into what the About page says.
|
||||
///
|
||||
/// A free function over the survey rather than a method that writes into the
|
||||
/// window, because the requirement's visibility clause is the part worth
|
||||
/// asserting: FR-DSP-8 asks for a fallback that is *defined*, and a reason
|
||||
/// that reaches the code but never the screen satisfies half of that. This is
|
||||
/// where a test can hold the strings.
|
||||
///
|
||||
/// `None` only for a survey with no displays at all, which `probe` does not
|
||||
/// produce.
|
||||
pub(crate) fn readouts(survey: &DisplaySurvey, index: usize) -> Option<Readouts> {
|
||||
// Clamped rather than indexed: a monitor unplugged between the survey and
|
||||
// this call leaves an index pointing past the end, and the page must
|
||||
// describe *a* display rather than nothing.
|
||||
let index = if index < survey.displays.len() {
|
||||
index
|
||||
} else {
|
||||
0
|
||||
};
|
||||
let here = survey.displays.get(index)?;
|
||||
|
||||
// The other monitors, listed rather than hidden. FR-DSP-8 is a
|
||||
// multi-monitor requirement and the failure it names — the second display
|
||||
// showing wrong colours — is invisible from the first, so a page that
|
||||
// described only the display it was being read on would report nothing
|
||||
// about the case that matters.
|
||||
let others: Vec<String> = survey
|
||||
.displays
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(i, _)| *i != index)
|
||||
.map(|(_, d)| format!("{}: {}", d.name, d.profile.describe()))
|
||||
.collect();
|
||||
|
||||
Some(Readouts {
|
||||
name: format!("{} ({})", here.name, survey.server.label()),
|
||||
colour: here.profile.describe(),
|
||||
others: others.join(" · "),
|
||||
})
|
||||
}
|
||||
|
||||
/// Start following the window's display, and answer once immediately.
|
||||
///
|
||||
/// `redraw` is the window's ordinary re-render, so a display change costs
|
||||
/// exactly one recomposition and one dispatch — the property the fused design
|
||||
/// was always going to give us here (`compose_with_framing`'s output space is
|
||||
/// a parameter, and enters the structure hash).
|
||||
pub(crate) fn attach(
|
||||
window: &AppWindow,
|
||||
watch: &Rc<DisplayWatch>,
|
||||
viewport: &Rc<RefCell<(u32, u32)>>,
|
||||
redraw: Rc<dyn Fn(&AppWindow)>,
|
||||
) {
|
||||
// The first answer, before any frame is drawn. Without it the canvas's
|
||||
// first render is sRGB on every desktop and corrects itself half a second
|
||||
// later, which on a wide-gamut panel is a visible flash of the wrong
|
||||
// colour on every image opened.
|
||||
watch.seen.set(Some(Geometry::of(window)));
|
||||
watch.resolve(window);
|
||||
|
||||
let timer = slint::Timer::default();
|
||||
let weak = window.as_weak();
|
||||
let watch = watch.clone();
|
||||
let viewport = viewport.clone();
|
||||
timer.start(slint::TimerMode::Repeated, POLL, move || {
|
||||
let Some(window) = weak.upgrade() else { return };
|
||||
let now = Geometry::of(&window);
|
||||
if watch.seen.get() == Some(now) {
|
||||
return;
|
||||
}
|
||||
watch.seen.set(Some(now));
|
||||
|
||||
// The physical size follows the scale factor, so a move onto a
|
||||
// differently-scaled display changes what to render as well as what
|
||||
// to render it into (FR-DSP-8's two halves arriving together).
|
||||
let size = watch.canvas_physical(&window);
|
||||
let resized = *viewport.borrow() != size;
|
||||
if resized {
|
||||
*viewport.borrow_mut() = size;
|
||||
}
|
||||
|
||||
if watch.resolve(&window) || resized {
|
||||
redraw(&window);
|
||||
}
|
||||
});
|
||||
// The timer stops when it is dropped, and it must outlive this function.
|
||||
// Leaked deliberately rather than threaded through the window's state:
|
||||
// it lives exactly as long as the process, and there is nothing that
|
||||
// could sensibly stop it.
|
||||
std::mem::forget(timer);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use dr_plat::{Bounds, DisplayInfo, DisplayProfile, DisplayServer, FallbackReason};
|
||||
|
||||
fn two_monitors() -> DisplaySurvey {
|
||||
DisplaySurvey {
|
||||
server: DisplayServer::X11,
|
||||
displays: vec![
|
||||
DisplayInfo {
|
||||
name: "eDP-1".to_string(),
|
||||
bounds: Some(Bounds {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 1920,
|
||||
height: 1200,
|
||||
}),
|
||||
profile: DisplayProfile::fallback(FallbackReason::NoWaylandProtocol),
|
||||
},
|
||||
DisplayInfo {
|
||||
name: "DP-2".to_string(),
|
||||
bounds: Some(Bounds {
|
||||
x: 1920,
|
||||
y: 0,
|
||||
width: 2560,
|
||||
height: 1440,
|
||||
}),
|
||||
profile: DisplayProfile {
|
||||
space: dr_types::ColourSpace::DisplayP3,
|
||||
source: dr_plat::ProfileSource::X11RootProperty(
|
||||
"_ICC_PROFILE_1".to_string(),
|
||||
),
|
||||
described_as: Some("EIZO CG279X".to_string()),
|
||||
approximated: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// The fallback is defined *and visible*, which is what the requirement
|
||||
/// asks for and the half that is easy to leave out.
|
||||
#[test]
|
||||
fn the_about_page_says_which_acquisition_path_the_session_is_on() {
|
||||
let survey = two_monitors();
|
||||
let shown = readouts(&survey, 0).expect("a display");
|
||||
|
||||
assert!(shown.name.contains("eDP-1"), "{}", shown.name);
|
||||
assert!(shown.name.contains("X11"), "{}", shown.name);
|
||||
// A photographer being shown sRGB because the compositor would not say
|
||||
// otherwise must be able to find that out rather than wonder.
|
||||
assert!(shown.colour.contains("sRGB"), "{}", shown.colour);
|
||||
assert!(shown.colour.contains("assumed"), "{}", shown.colour);
|
||||
assert!(
|
||||
shown.colour.contains("colour management"),
|
||||
"the reason for the fallback did not reach the page: {}",
|
||||
shown.colour
|
||||
);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// The second monitor is described on the page read on the first.
|
||||
///
|
||||
/// The requirement's whole subject is the display the reader is *not*
|
||||
/// looking at: "showing wrong colours on the second display is a
|
||||
/// correctness defect". A page that listed only the current one would say
|
||||
/// nothing about the case it exists for.
|
||||
#[test]
|
||||
fn the_other_monitor_is_named_on_the_page_read_from_this_one() {
|
||||
let survey = two_monitors();
|
||||
let shown = readouts(&survey, 0).expect("a display");
|
||||
assert!(shown.others.contains("DP-2"), "{}", shown.others);
|
||||
assert!(shown.others.contains("Display P3"), "{}", shown.others);
|
||||
// Approximated, and saying so. An honest approximation the user can
|
||||
// see is the whole trade made in `dr_plat::display`.
|
||||
assert!(shown.others.contains("nearest to"), "{}", shown.others);
|
||||
assert!(shown.others.contains("EIZO CG279X"), "{}", shown.others);
|
||||
|
||||
// And from the second monitor's point of view, the first is the other.
|
||||
let shown = readouts(&survey, 1).expect("a display");
|
||||
assert!(shown.colour.contains("Display P3"), "{}", shown.colour);
|
||||
assert!(shown.others.contains("eDP-1"), "{}", shown.others);
|
||||
}
|
||||
|
||||
/// A single-monitor desktop leaves the row empty rather than repeating
|
||||
/// itself, which is what makes the row conditional in `settings.slint`.
|
||||
#[test]
|
||||
fn one_display_has_no_others_to_list() {
|
||||
let survey = DisplaySurvey::assumed_srgb(FallbackReason::NoWaylandProtocol);
|
||||
let shown = readouts(&survey, 0).expect("a display");
|
||||
assert!(shown.others.is_empty(), "{}", shown.others);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// Fractional scaling renders more pixels, not the same pixels stretched.
|
||||
#[test]
|
||||
fn a_fractionally_scaled_canvas_is_rendered_at_its_physical_size() {
|
||||
// GNOME's fractional steps, against a canvas of a plausible size. The
|
||||
// failure being guarded is silent: at 1.25 the compositor is handed
|
||||
// 1600 samples for 2000 device pixels and the photograph goes soft.
|
||||
assert_eq!(physical((1600, 1000), 1.25), (2000, 1250));
|
||||
assert_eq!(physical((1600, 1000), 1.5), (2400, 1500));
|
||||
assert_eq!(physical((1600, 1000), 1.75), (2800, 1750));
|
||||
assert_eq!(physical((1600, 1000), 2.0), (3200, 2000));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unscaled_display_renders_exactly_what_it_is_asked_for() {
|
||||
// The other half of the same requirement: 1:1 must stay 1:1 rather
|
||||
// than acquiring a rounding error that resamples every frame by a
|
||||
// fraction of a pixel.
|
||||
assert_eq!(physical((1923, 1081), 1.0), (1923, 1081));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_nonsense_scale_factor_does_not_blank_the_canvas() {
|
||||
// It comes from the windowing system, and a zero would collapse the
|
||||
// render target to a single pixel with no error anywhere.
|
||||
assert_eq!(physical((800, 600), 0.0), (800, 600));
|
||||
assert_eq!(physical((800, 600), f32::NAN), (800, 600));
|
||||
assert_eq!(physical((800, 600), -2.0), (800, 600));
|
||||
// And a canvas that has been collapsed to nothing by a dragged
|
||||
// splitter still has to produce a renderable target.
|
||||
assert_eq!(physical((0, 0), 2.0), (1, 1));
|
||||
}
|
||||
}
|
||||
+43
-1
@@ -23,6 +23,7 @@ mod activity;
|
||||
mod collections_ui;
|
||||
mod derived_sync;
|
||||
mod develop;
|
||||
mod display_ui;
|
||||
mod export;
|
||||
pub mod faces;
|
||||
mod gradient;
|
||||
@@ -1377,8 +1378,20 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
let rows: Rc<slint::VecModel<ParamRow>> = Rc::new(slint::VecModel::default());
|
||||
window.set_adjust_rows(rows.clone().into());
|
||||
// Viewport size, tracked so a re-render after a slider move matches it.
|
||||
//
|
||||
// In *physical* pixels, which is what the render target wants and what
|
||||
// FR-DSP-8 means by handling fractional scaling without resampling. The
|
||||
// one caller that thinks in logical pixels is Slint's resize callback,
|
||||
// and it converts on the way in.
|
||||
let viewport = Rc::new(RefCell::new((1024u32, 768u32)));
|
||||
|
||||
// TRACES: FR-DSP-8
|
||||
// What the displays are, and which one the canvas is on. Probed once here
|
||||
// so that the About page can describe the session's colour path before any
|
||||
// photograph is opened — the acquisition path is a property of the desktop
|
||||
// and not of the image.
|
||||
let display = display_ui::DisplayWatch::probe();
|
||||
|
||||
// Re-render the current session into the canvas.
|
||||
//
|
||||
// Called on every slider change, so it must do no more than run the
|
||||
@@ -1393,10 +1406,26 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
let session = session.clone();
|
||||
let viewport = viewport.clone();
|
||||
let drawn_history = drawn_history.clone();
|
||||
let display = display.clone();
|
||||
Rc::new(move |window: &AppWindow, draft: bool| {
|
||||
let mut slot = session.borrow_mut();
|
||||
let Some(s) = slot.as_mut() else { return };
|
||||
|
||||
// TRACES: FR-DSP-8 | FR-DSP-6
|
||||
// **Every canvas render is encoded for the display showing it.**
|
||||
//
|
||||
// Set here rather than pushed from the display watch, and rather
|
||||
// than set once when a photograph is opened, because this is the
|
||||
// one path every frame takes. A session opened while the window
|
||||
// sits on the second monitor is then correct on its *first* frame
|
||||
// — where a push would leave it sRGB until the next poll, which is
|
||||
// a visible flash of the wrong colour on every image opened.
|
||||
//
|
||||
// Free when nothing has changed: the field is compared before it
|
||||
// is written, and an unchanged output space composes to the same
|
||||
// structure hash and the same cached pipeline.
|
||||
s.set_display_space(display.space());
|
||||
|
||||
// TRACES: FR-DEV-5
|
||||
// Whether undo has anywhere to go, pushed from here because every
|
||||
// edit ends in a redraw and nothing else is on all of their paths:
|
||||
@@ -2570,13 +2599,22 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
|
||||
// Track the canvas size so the adjust pass renders at viewport
|
||||
// resolution rather than sensor resolution (FR-DSP-1).
|
||||
//
|
||||
// TRACES: FR-DSP-8
|
||||
// Slint reports the canvas in *logical* pixels, which is the box the
|
||||
// compositor will draw into and not the number of device pixels it will
|
||||
// fill. `display_ui::physical` converts, so that a fractionally scaled
|
||||
// desktop is presented 1:1 rather than resampled — see that function for
|
||||
// why a soft canvas is the failure being avoided here.
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let viewport = viewport.clone();
|
||||
let redraw = redraw.clone();
|
||||
let display = display.clone();
|
||||
window.on_canvas_resized(move |w_px, h_px| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let size = (w_px.max(1) as u32, h_px.max(1) as u32);
|
||||
let logical = (w_px.max(1) as u32, h_px.max(1) as u32);
|
||||
let size = display.canvas_resized(&w, logical);
|
||||
if *viewport.borrow() == size {
|
||||
return;
|
||||
}
|
||||
@@ -2585,6 +2623,10 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
});
|
||||
}
|
||||
|
||||
// TRACES: FR-DSP-8 | FR-DSP-6
|
||||
// And which display that canvas is on, from now until the window closes.
|
||||
display_ui::attach(&window, &display, &viewport, redraw.clone());
|
||||
|
||||
// FR-UI-1: layout class from window width. Computed here rather than in
|
||||
// Slint because a property that both derives from and feeds the layout is
|
||||
// a binding loop.
|
||||
|
||||
+18
-5
@@ -275,6 +275,13 @@ export component AppWindow inherits Window {
|
||||
in property <string> adapter: "detecting…";
|
||||
in property <string> backend: "—";
|
||||
in property <int> fps: 0;
|
||||
/// TRACES: FR-DSP-8
|
||||
/// The display showing the canvas and the colour transform it is getting.
|
||||
/// Shown under ABOUT in Settings; see `settings.slint` for why the second
|
||||
/// one is a sentence rather than a name.
|
||||
in property <string> display-name: "detecting…";
|
||||
in property <string> display-colour: "detecting…";
|
||||
in property <string> display-others: "";
|
||||
/// TRACES: FR-UI-2
|
||||
/// Set from `CARGO_PKG_VERSION`, so the About line cannot disagree with
|
||||
/// the binary it is part of.
|
||||
@@ -1267,6 +1274,9 @@ in property <bool> panel-visible: true;
|
||||
fps: root.fps;
|
||||
layout-class: root.layout-class;
|
||||
app-version: root.app-version;
|
||||
display-name: root.display-name;
|
||||
display-colour: root.display-colour;
|
||||
display-others: root.display-others;
|
||||
|
||||
original-budget: root.settings-original-budget;
|
||||
original-unlimited: root.settings-original-unlimited;
|
||||
@@ -1700,11 +1710,14 @@ in property <bool> panel-visible: true;
|
||||
// that far. Below 1:1 it stays smooth, where filtering is
|
||||
// what keeps the image from aliasing.
|
||||
//
|
||||
// This matters even at moderate zoom on a HiDPI display:
|
||||
// the canvas is rendered at *logical* size and Slint scales
|
||||
// it up by the device pixel ratio, so the buffer is
|
||||
// resampled on its way to the screen whatever the pipeline
|
||||
// did.
|
||||
// TRACES: FR-DSP-8
|
||||
// The canvas is rendered at the *physical* pixel size of
|
||||
// the box this image occupies, so `contain` presents it
|
||||
// 1:1 and neither filter is reached at all until the
|
||||
// photographer zooms. That is the point: on a fractionally
|
||||
// scaled desktop the buffer used to be a logical-sized one
|
||||
// that the compositor stretched, and no choice of filter
|
||||
// recovers detail that was never rendered.
|
||||
image-rendering: root.magnified
|
||||
? ImageRendering.pixelated
|
||||
: ImageRendering.smooth;
|
||||
|
||||
@@ -117,6 +117,25 @@ export component SettingsPage inherits Rectangle {
|
||||
in property <string> layout-class;
|
||||
in property <string> app-version;
|
||||
|
||||
/// TRACES: FR-DSP-8
|
||||
/// The display showing the canvas, and the colour it is being given.
|
||||
///
|
||||
/// These are the same kind of thing as the two above — a fact about the
|
||||
/// session, not a setting — with one difference that earns them their own
|
||||
/// row rather than a line in a log. FR-DSP-8's fallback is *defined* to be
|
||||
/// sRGB where the display server will not say otherwise, and a photographer
|
||||
/// being shown sRGB because their compositor has no colour-management
|
||||
/// protocol has no other way to find that out. `display-colour` says which
|
||||
/// of the acquisition paths this session is on, in words.
|
||||
in property <string> display-name;
|
||||
in property <string> display-colour;
|
||||
/// The *other* monitors, where there are any. Empty on a single-display
|
||||
/// desktop, which is why the row below is conditional: FR-DSP-8 is about
|
||||
/// the second display, and the failure it names is invisible from the
|
||||
/// first, so this page has to be readable about a monitor the reader is
|
||||
/// not currently looking at.
|
||||
in property <string> display-others;
|
||||
|
||||
callback original-budget-changed(string);
|
||||
callback original-unlimited-toggled(bool);
|
||||
callback thumbnail-budget-changed(string);
|
||||
@@ -708,10 +727,46 @@ export component SettingsPage inherits Rectangle {
|
||||
Value { text: root.layout-class; horizontal-stretch: 1; }
|
||||
}
|
||||
|
||||
// TRACES: FR-DSP-8
|
||||
// Which display, and what colour it is being sent.
|
||||
HorizontalLayout {
|
||||
spacing: Theme.gap;
|
||||
Label { text: "Display"; }
|
||||
Value {
|
||||
text: root.display-name;
|
||||
horizontal-stretch: 1;
|
||||
overflow: elide;
|
||||
}
|
||||
}
|
||||
|
||||
HorizontalLayout {
|
||||
spacing: Theme.gap;
|
||||
Label { text: "Display colour"; }
|
||||
Value {
|
||||
text: root.display-colour;
|
||||
horizontal-stretch: 1;
|
||||
// Wraps rather than elides: this is the one
|
||||
// value on the page that is a sentence, and
|
||||
// eliding it would cut off the half that says
|
||||
// *why* — which is the half FR-DSP-8 asks for.
|
||||
wrap: word-wrap;
|
||||
}
|
||||
}
|
||||
|
||||
if root.display-others != "": HorizontalLayout {
|
||||
spacing: Theme.gap;
|
||||
Label { text: "Other displays"; }
|
||||
Value {
|
||||
text: root.display-others;
|
||||
horizontal-stretch: 1;
|
||||
wrap: word-wrap;
|
||||
}
|
||||
}
|
||||
|
||||
Caption {
|
||||
text: "Graphics and frame rate describe this session, "
|
||||
+ "not a setting — they are here so a bug report "
|
||||
+ "can quote them.";
|
||||
text: "Graphics, frame rate and display colour "
|
||||
+ "describe this session, not a setting — they "
|
||||
+ "are here so a bug report can quote them.";
|
||||
wrap: word-wrap;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user