Move the instrumentation off the header and into About
The render backend, the layout class and the frame rate sat permanently in a 44px strip that also carries the only way out of develop, the undo pair, the panel toggle and the export button. They cost about 200 logical pixels, and a `HorizontalLayout` given less width than its children's minimums does not shrink them — it runs off the end. There was already a breakpoint hiding them below 820px, which is why a tablet in portrait (768) looked fine and landscape (1200) did not: above the breakpoint the readouts came back and pushed the header off the right-hand edge. A breakpoint that hides a problem at one size and not another is a workaround, and this removes the reason for it rather than moving it. They are diagnostics — read once when something looks wrong, and then not again — so they are in Settings under ABOUT, beside the version, which is where someone goes when they have a bug to report rather than a photograph to edit. The version is there for the same reason and comes from `CARGO_PKG_VERSION`, so the line cannot disagree with the binary showing it. The frame rate keeps its warning hue. It is the number that says whether the zero-copy display path is holding up, which is the assumption the whole display design rests on, and that is worth colour wherever it is shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -764,6 +764,10 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
|||||||
let gpu = shared_gpu();
|
let gpu = shared_gpu();
|
||||||
|
|
||||||
let window = AppWindow::new()?;
|
let window = AppWindow::new()?;
|
||||||
|
// The version the About page shows. Taken from the crate rather than
|
||||||
|
// passed in, so it is the version of the code that is running and cannot
|
||||||
|
// be set to something else by a caller.
|
||||||
|
window.set_app_version(env!("CARGO_PKG_VERSION").into());
|
||||||
match &gpu {
|
match &gpu {
|
||||||
Some(ctx) => {
|
Some(ctx) => {
|
||||||
log::info!("adapter: {} ({:?})", ctx.adapter_name(), ctx.backend());
|
log::info!("adapter: {} ({:?})", ctx.adapter_name(), ctx.backend());
|
||||||
|
|||||||
+27
-50
@@ -104,64 +104,31 @@ component StatusBar inherits Rectangle {
|
|||||||
|
|
||||||
Rectangle { horizontal-stretch: 1; }
|
Rectangle { horizontal-stretch: 1; }
|
||||||
|
|
||||||
// --- instrumentation, and only where there is room for it ---------
|
// --- instrumentation lives in Settings now ------------------------
|
||||||
//
|
//
|
||||||
// The render backend, the layout class and the frame rate are all
|
// The render backend, the layout class and the frame rate are
|
||||||
// developer readouts. They cost about 200 logical pixels of a strip
|
// developer readouts. They cost about 200 logical pixels of a strip
|
||||||
// that also carries the only way out of develop, the undo pair, the
|
// that also carries the only way out of develop, the undo pair, the
|
||||||
// panel toggle and the export button — and a `HorizontalLayout` given
|
// panel toggle and the export button — and a `HorizontalLayout` given
|
||||||
// less width than its children's minimums does not shrink them, it
|
// less width than its children's minimums does not shrink them, it
|
||||||
// runs off the end.
|
// runs off the end.
|
||||||
//
|
//
|
||||||
// A tablet in portrait is 768 logical pixels, so that is exactly what
|
// A tablet in portrait is 768 logical pixels and was already below the
|
||||||
// happened: everything from the panel toggle rightwards was pushed off
|
// breakpoint that hid them, so it kept its header. Landscape is 1200
|
||||||
// the right-hand edge. The develop column is *also* closed by default
|
// and is not, so the readouts came back and the header ran off the
|
||||||
// below the breakpoint, so the toggle being unreachable meant the
|
// right-hand edge there instead — a breakpoint that hides a problem at
|
||||||
// column could not be opened at all — and with it went copy and paste,
|
// one size and not another is a workaround, not a fix.
|
||||||
// which live in it. Reported as "I have no idea how to copy a setting
|
|
||||||
// on Android and apply it to other images", and the answer was that
|
|
||||||
// there was no way to.
|
|
||||||
//
|
//
|
||||||
// Collapsed to zero width rather than wrapped in an `if`: this strip
|
// The backend, the frame rate and the layout class used to live here.
|
||||||
// is inside the layout that `expanded` feeds, and a conditional child
|
// They are diagnostics — read once when something looks wrong, and
|
||||||
// here is the shape that has already caused binding loops in this file
|
// never again — and as permanent furniture in a 44px strip they cost
|
||||||
// (see the panel below). A `visible: false` child still takes its slot
|
// three slots that the controls beside them actually needed. On a
|
||||||
// in a layout, so the width has to go as well.
|
// tablet in landscape the header ran off the right-hand edge, and this
|
||||||
Rectangle {
|
// was a third of the reason.
|
||||||
width: root.expanded ? diagnostics.preferred-width : 0px;
|
//
|
||||||
visible: root.expanded;
|
// They are in Settings under ABOUT now, next to the version, which is
|
||||||
clip: true;
|
// where someone goes when they have a bug to report rather than a
|
||||||
|
// photograph to edit.
|
||||||
diagnostics := HorizontalLayout {
|
|
||||||
spacing: Theme.gap;
|
|
||||||
|
|
||||||
// The backend is an identifying label, not a state — it says
|
|
||||||
// which path is in use, and it says the same thing whether
|
|
||||||
// that path is fast or slow. The accent it used to carry made
|
|
||||||
// every frame look like an alert; `fps` below is the thing
|
|
||||||
// here that can go wrong.
|
|
||||||
Label {
|
|
||||||
text: root.backend;
|
|
||||||
vertical-alignment: center;
|
|
||||||
}
|
|
||||||
|
|
||||||
Caption {
|
|
||||||
text: root.layout-class;
|
|
||||||
vertical-alignment: center;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Degraded performance, so this is a caution rather than an
|
|
||||||
// active state: the frame rate has fallen below what the
|
|
||||||
// compositing path is supposed to sustain, which is exactly
|
|
||||||
// the assumption A1 exists to test. `warn` is the one hue left
|
|
||||||
// in the palette and this earns it.
|
|
||||||
Caption {
|
|
||||||
text: root.fps + " fps";
|
|
||||||
warn: root.fps < 55;
|
|
||||||
vertical-alignment: center;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Undo and redo, in the strip rather than in the develop column: the
|
// Undo and redo, in the strip rather than in the develop column: the
|
||||||
// panel can be put away, and the one control that takes back a
|
// panel can be put away, and the one control that takes back a
|
||||||
@@ -292,6 +259,10 @@ export component AppWindow inherits Window {
|
|||||||
in property <string> adapter: "detecting…";
|
in property <string> adapter: "detecting…";
|
||||||
in property <string> backend: "—";
|
in property <string> backend: "—";
|
||||||
in property <int> fps: 0;
|
in property <int> fps: 0;
|
||||||
|
/// TRACES: FR-UI-2
|
||||||
|
/// Set from `CARGO_PKG_VERSION`, so the About line cannot disagree with
|
||||||
|
/// the binary it is part of.
|
||||||
|
in property <string> app-version: "unknown";
|
||||||
|
|
||||||
// Current image, for the status strip and empty state.
|
// Current image, for the status strip and empty state.
|
||||||
in property <string> filename: "";
|
in property <string> filename: "";
|
||||||
@@ -1143,6 +1114,12 @@ in property <bool> panel-visible: true;
|
|||||||
width: 100%;
|
width: 100%;
|
||||||
height: 100%;
|
height: 100%;
|
||||||
|
|
||||||
|
adapter: root.adapter;
|
||||||
|
backend: root.backend;
|
||||||
|
fps: root.fps;
|
||||||
|
layout-class: root.layout-class;
|
||||||
|
app-version: root.app-version;
|
||||||
|
|
||||||
original-budget: root.settings-original-budget;
|
original-budget: root.settings-original-budget;
|
||||||
original-unlimited: root.settings-original-unlimited;
|
original-unlimited: root.settings-original-unlimited;
|
||||||
thumbnail-budget: root.settings-thumbnail-budget;
|
thumbnail-budget: root.settings-thumbnail-budget;
|
||||||
|
|||||||
@@ -90,6 +90,20 @@ export component SettingsPage inherits Rectangle {
|
|||||||
/// What the cache currently holds. Empty hides the line.
|
/// What the cache currently holds. Empty hides the line.
|
||||||
in property <string> cache-usage;
|
in property <string> cache-usage;
|
||||||
|
|
||||||
|
/// TRACES: FR-UI-2
|
||||||
|
/// What is drawing, and how fast. These used to sit in the window's top
|
||||||
|
/// strip, where they were three items of permanent furniture answering a
|
||||||
|
/// question almost nobody asks twice — and on a tablet in landscape they
|
||||||
|
/// were part of why the header ran off the screen.
|
||||||
|
///
|
||||||
|
/// They are diagnostics, so they belong where a person goes to look
|
||||||
|
/// something up, not where they are looked at all day.
|
||||||
|
in property <string> adapter;
|
||||||
|
in property <string> backend;
|
||||||
|
in property <int> fps;
|
||||||
|
in property <string> layout-class;
|
||||||
|
in property <string> app-version;
|
||||||
|
|
||||||
callback original-budget-changed(string);
|
callback original-budget-changed(string);
|
||||||
callback original-unlimited-toggled(bool);
|
callback original-unlimited-toggled(bool);
|
||||||
callback thumbnail-budget-changed(string);
|
callback thumbnail-budget-changed(string);
|
||||||
@@ -551,6 +565,65 @@ export component SettingsPage inherits Rectangle {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- about and diagnostics -------------------------------
|
||||||
|
Rectangle {
|
||||||
|
width: content.column;
|
||||||
|
height: about-panel.preferred-height;
|
||||||
|
|
||||||
|
about-panel := Panel {
|
||||||
|
width: 100%;
|
||||||
|
spacing: Theme.gap;
|
||||||
|
|
||||||
|
PanelHeading { text: "ABOUT"; }
|
||||||
|
|
||||||
|
// The version first, because it is the one line anyone
|
||||||
|
// is ever asked to quote. A bug report that names a
|
||||||
|
// version names a commit.
|
||||||
|
HorizontalLayout {
|
||||||
|
spacing: Theme.gap;
|
||||||
|
Label { text: "Version"; }
|
||||||
|
Value { text: root.app-version; horizontal-stretch: 1; overflow: elide; }
|
||||||
|
}
|
||||||
|
|
||||||
|
HorizontalLayout {
|
||||||
|
spacing: Theme.gap;
|
||||||
|
Label { text: "Graphics"; }
|
||||||
|
Value {
|
||||||
|
text: root.adapter + " (" + root.backend + ")";
|
||||||
|
horizontal-stretch: 1;
|
||||||
|
overflow: elide;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
HorizontalLayout {
|
||||||
|
spacing: Theme.gap;
|
||||||
|
Label { text: "Frame rate"; }
|
||||||
|
// Still earns the warning hue here. It is the
|
||||||
|
// number that says whether the zero-copy path is
|
||||||
|
// holding up, which is the assumption the whole
|
||||||
|
// display design rests on.
|
||||||
|
// `Caption` rather than `Value` here alone: it is
|
||||||
|
// the one that carries `warn`, and the frame rate
|
||||||
|
// is the one reading on this page that can be bad
|
||||||
|
// news rather than merely a fact.
|
||||||
|
Caption { text: root.fps + " fps"; warn: root.fps < 55; horizontal-stretch: 1; }
|
||||||
|
}
|
||||||
|
|
||||||
|
HorizontalLayout {
|
||||||
|
spacing: Theme.gap;
|
||||||
|
Label { text: "Layout"; }
|
||||||
|
Value { text: root.layout-class; horizontal-stretch: 1; }
|
||||||
|
}
|
||||||
|
|
||||||
|
Caption {
|
||||||
|
text: "Graphics and frame rate describe this session, "
|
||||||
|
+ "not a setting — they are here so a bug report "
|
||||||
|
+ "can quote them.";
|
||||||
|
wrap: word-wrap;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// --- naming and destination ------------------------------
|
// --- naming and destination ------------------------------
|
||||||
//
|
//
|
||||||
// Its own panel rather than more of the export one: format and
|
// Its own panel rather than more of the export one: format and
|
||||||
|
|||||||
Reference in New Issue
Block a user