mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-24 09:57:08 -06:00
* Curves: the graph grows with the Properties panel and a resizable dialog (#8984) The graph was fixed at 300 points. It now takes the width it is given, no taller than the window leaves room for beside the other rows. The Image > Adjustments > Curves dialog gets a resize grip and reopens at the size it was left at. * Curves: the dialog stops widening where the graph stops; a squeezed size isn't remembered (#8984) The dialog's resize grip goes no wider than the graph can grow in this window, so no blank column. A size at the window's limit no longer replaces the size the user left the dialog at. * test(ui): keep the Issue #43 doc comment on its own test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Brandon Thomas <echelon@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
Brandon Thomas
parent
29e4039c72
commit
427fb34042
@@ -629,6 +629,84 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// #8984: the Curves dialog has a resize grip; its graph grows with the dialog, the controls
|
||||
/// stay inside it, and it reopens at the size it was left at.
|
||||
#[test]
|
||||
fn curves_dialog_resizes_with_its_graph_and_reopens_at_that_size() {
|
||||
let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1100.0)).build_ui_state(|ui, app| crate::dialogs::show(app, ui.ctx()), app_with_image());
|
||||
PhotocraftApp::setup_context(&h.ctx, Default::default());
|
||||
let graph = |h: &Harness<'_, PhotocraftApp>| h.ctx.data(|d| d.get_temp::<egui::Rect>(dialog_mem("curves").with("curves-graph"))).unwrap();
|
||||
let window = |h: &Harness<'_, PhotocraftApp>, id: u64| h.ctx.memory(|m| m.area_rect(egui::Id::new(("dialog", id)))).unwrap();
|
||||
let id = open(h.state_mut(), "image.adjustments.curves").unwrap();
|
||||
h.run_steps(4);
|
||||
let (opened, small) = (window(&h, id), graph(&h));
|
||||
assert!(small.width() > 270.0 && small.width() < 300.0, "opens at the 300-point graph: {small:?}");
|
||||
// Drag the grip (bottom-right corner) 300 points right and 200 down.
|
||||
let corner = opened.max - egui::vec2(12.0, 12.0);
|
||||
let by = egui::vec2(300.0, 200.0);
|
||||
h.event(egui::Event::PointerMoved(corner));
|
||||
h.step();
|
||||
h.event(egui::Event::PointerButton { pos: corner, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE });
|
||||
h.step();
|
||||
for i in 1..=4 {
|
||||
h.event(egui::Event::PointerMoved(corner + by * (i as f32 / 4.0)));
|
||||
h.step();
|
||||
}
|
||||
h.event(egui::Event::PointerButton { pos: corner + by, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE });
|
||||
h.run_steps(6);
|
||||
let (wide, big) = (window(&h, id), graph(&h));
|
||||
assert!(big.width() > small.width() + 250.0, "the graph follows the dialog: {small:?} → {big:?}");
|
||||
assert!((big.width() - big.height()).abs() < 1.0, "still square: {big:?}");
|
||||
assert!(wide.contains_rect(big) && wide.right() - big.right() < 40.0, "no blank column: window {wide:?}, graph {big:?}");
|
||||
for label in ["Channel:", "Input:", "Output:", "Preview", "OK", "Cancel"] {
|
||||
assert!(wide.contains_rect(h.get_by_label(crate::i18n::t(label)).rect()), "{label} inside the window");
|
||||
}
|
||||
// Cancel, open again: the size it was left at.
|
||||
h.get_by_label("Cancel").click();
|
||||
h.run_steps(3);
|
||||
assert!(h.state().ui.dialogs.is_empty());
|
||||
let again = open(h.state_mut(), "image.adjustments.curves").unwrap();
|
||||
h.run_steps(4);
|
||||
assert!((graph(&h).width() - big.width()).abs() < 2.0, "reopened at {:?}, left at {big:?}", graph(&h));
|
||||
assert!((window(&h, again).width() - wide.width()).abs() < 2.0);
|
||||
}
|
||||
|
||||
/// #8984: however tall the graph's width would make it, the dialog still fits the window.
|
||||
#[test]
|
||||
fn curves_graph_never_pushes_the_dialog_off_a_short_window() {
|
||||
let mut h = Harness::builder().with_size(egui::vec2(1600.0, 700.0)).build_ui_state(|ui, app| crate::dialogs::show(app, ui.ctx()), app_with_image());
|
||||
PhotocraftApp::setup_context(&h.ctx, Default::default());
|
||||
h.ctx.data_mut(|m| m.insert_persisted(egui::Id::new(("dialog-place", "image.adjustments.curves")).with("size"), egui::vec2(1200.0, 1200.0)));
|
||||
let id = open(h.state_mut(), "image.adjustments.curves").unwrap();
|
||||
h.run_steps(5);
|
||||
let graph = h.ctx.data(|d| d.get_temp::<egui::Rect>(dialog_mem("curves").with("curves-graph"))).unwrap();
|
||||
let window = h.ctx.memory(|m| m.area_rect(egui::Id::new(("dialog", id)))).unwrap();
|
||||
assert!(graph.height() <= 700.0 - 360.0, "{graph:?}");
|
||||
assert!(window.height() <= 700.0, "{window:?}");
|
||||
assert!(window.contains_rect(h.get_by_label("OK").rect()));
|
||||
// The size the window squeezed it to is not remembered: a larger window gets 1200 again.
|
||||
let kept = h.ctx.data_mut(|m| m.get_persisted::<egui::Vec2>(egui::Id::new(("dialog-place", "image.adjustments.curves")).with("size")));
|
||||
assert_eq!(kept, Some(egui::vec2(1200.0, 1200.0)));
|
||||
// Dragged far wider, the dialog stops where the graph stops: no blank column.
|
||||
let corner = window.max - egui::vec2(12.0, 12.0);
|
||||
h.event(egui::Event::PointerMoved(corner));
|
||||
h.step();
|
||||
h.event(egui::Event::PointerButton { pos: corner, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE });
|
||||
h.step();
|
||||
h.event(egui::Event::PointerMoved(corner + egui::vec2(600.0, 0.0)));
|
||||
h.step();
|
||||
h.event(egui::Event::PointerButton {
|
||||
pos: corner + egui::vec2(600.0, 0.0),
|
||||
button: egui::PointerButton::Primary,
|
||||
pressed: false,
|
||||
modifiers: egui::Modifiers::NONE,
|
||||
});
|
||||
h.run_steps(5);
|
||||
let graph = h.ctx.data(|d| d.get_temp::<egui::Rect>(dialog_mem("curves").with("curves-graph"))).unwrap();
|
||||
let window = h.ctx.memory(|m| m.area_rect(egui::Id::new(("dialog", id)))).unwrap();
|
||||
assert!(window.right() - graph.right() < 40.0, "no blank column: window {window:?}, graph {graph:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancel_leaves_the_document_untouched() {
|
||||
let mut harness =
|
||||
|
||||
@@ -357,6 +357,27 @@ fn gradient_bar(p: &egui::Painter, r: Rect, from: Color32, to: Color32, vertical
|
||||
const CURVE_HANDLE_SIZE: f32 = 7.0;
|
||||
const CURVE_GRAPH_PADDING: f32 = 5.0;
|
||||
|
||||
/// The room left on screen for the Curves dialog's other rows (title, channel and mode, Input and
|
||||
/// Output, eyedroppers, buttons) when its graph is as tall as it can be.
|
||||
const CURVES_CHROME: f32 = 360.0;
|
||||
/// The smallest and the default largest side of the Curves graph: 300 points was the fixed
|
||||
/// size until #8984.
|
||||
const CURVE_SIDE: (f32, f32) = (120.0, 300.0);
|
||||
|
||||
/// The Curves graph's side (#8984): the width it is given, in the Properties panel and in the
|
||||
/// resizable dialog, and no taller than the window leaves room for beside the other rows.
|
||||
fn curve_side(ui: &egui::Ui) -> f32 {
|
||||
let tallest = curve_side_max(ui.ctx());
|
||||
let width = ui.available_width();
|
||||
if width.is_finite() { width.clamp(CURVE_SIDE.0, tallest) } else { CURVE_SIDE.1 }
|
||||
}
|
||||
|
||||
/// The largest the Curves graph gets in this window: as tall as the window leaves room for beside
|
||||
/// the dialog's other rows, and never under the old 300 points. The Curves dialog is no wider.
|
||||
pub(crate) fn curve_side_max(ctx: &egui::Context) -> f32 {
|
||||
(ctx.content_rect().height() - CURVES_CHROME).max(CURVE_SIDE.1)
|
||||
}
|
||||
|
||||
fn curve_graph(full: Rect, side: f32) -> Rect {
|
||||
let base = Rect::from_min_size(full.min + vec2(14.0, 0.0), vec2(side - 14.0, side - 14.0));
|
||||
Rect::from_min_max(base.min + vec2(CURVE_GRAPH_PADDING, CURVE_GRAPH_PADDING), base.max - vec2(CURVE_GRAPH_PADDING, CURVE_GRAPH_PADDING))
|
||||
@@ -461,7 +482,7 @@ fn curves(ui: &mut egui::Ui, v: &mut Value, cx: &EditorCx) -> Edit {
|
||||
let pencil = v.get("pencil").and_then(Value::as_bool).unwrap_or(false);
|
||||
let mut pts = read_curve(v, key);
|
||||
let mut map = read_map(v, key);
|
||||
let side = ui.available_width().clamp(120.0, 300.0);
|
||||
let side = curve_side(ui);
|
||||
ui.add_space(4.0);
|
||||
let (full, resp) = ui.allocate_exact_size(vec2(side, side + 14.0), Sense::click_and_drag());
|
||||
let graph = curve_graph(full, side);
|
||||
|
||||
@@ -206,6 +206,27 @@ fn curve_of(a: &Adjustment) -> Vec<[f32; 2]> {
|
||||
}
|
||||
}
|
||||
|
||||
/// #8984: the Curves graph in Properties grows with the panel instead of stopping at 300
|
||||
/// points, stays square, and edits where it is clicked at its new size.
|
||||
#[test]
|
||||
fn curves_graph_grows_with_the_properties_panel() {
|
||||
let mut h = app_harness("curves", "rgb", 8);
|
||||
let (id, _) = layer(&h);
|
||||
let narrow = curves_graph(&h, id);
|
||||
assert!(narrow.width() < 300.0, "{narrow:?}");
|
||||
crate::panels::request_dock_width(&h.ctx, 760.0);
|
||||
h.run_steps(8);
|
||||
let wide = curves_graph(&h, id);
|
||||
// 760 points of panel, but the 900-point window leaves room for 540 beside the other rows.
|
||||
assert!(wide.width() > narrow.width() + 250.0 && wide.height() <= 900.0 - 360.0, "{narrow:?} → {wide:?}");
|
||||
assert!((wide.width() - wide.height()).abs() < 1.0, "square: {wide:?}");
|
||||
// A point added at the graph's centre lands at 128, 128 of the curve.
|
||||
click(&mut h, wide.center());
|
||||
let c = curve_of(&layer(&h).1);
|
||||
assert_eq!(c.len(), 3, "{c:?}");
|
||||
assert!((c[1][0] - 127.5).abs() < 2.0 && (c[1][1] - 127.5).abs() < 2.0, "{c:?}");
|
||||
}
|
||||
|
||||
/// Issue #43: pressing on a point selects it (no new point), Delete/Backspace and ⌘/Ctrl-click
|
||||
/// remove points, dragging off the graph removes one, endpoints stay.
|
||||
#[test]
|
||||
|
||||
@@ -181,16 +181,47 @@ fn resizable_dialog(fields: &serde_json::Map<String, Value>) -> Option<egui::Vec
|
||||
Some(crate::prefs_ui::PREFERENCES_SIZE)
|
||||
} else if fields.contains_key("__web") {
|
||||
Some(egui::vec2(960.0, 900.0))
|
||||
} else if is_curves(fields) {
|
||||
Some(CURVES_SIZE)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether `fields` are the Image › Adjustments › Curves dialog's.
|
||||
fn is_curves(fields: &serde_json::Map<String, Value>) -> bool {
|
||||
fields.get("__adjust").and_then(Value::as_str) == Some("curves")
|
||||
}
|
||||
|
||||
/// The size the Curves dialog first opens at: the 300-point graph it had before it could be
|
||||
/// resized (#5113); it grows to fit its controls below the graph.
|
||||
const CURVES_SIZE: egui::Vec2 = egui::vec2(300.0, 420.0);
|
||||
/// The smallest a resizable dialog is dragged to: Curves keeps a usable graph (#8984), the others
|
||||
/// their two-column layouts.
|
||||
fn resizable_min(fields: &serde_json::Map<String, Value>) -> egui::Vec2 {
|
||||
if is_curves(fields) { egui::vec2(300.0, 420.0) } else { egui::vec2(560.0, 400.0) }
|
||||
}
|
||||
|
||||
/// Where the size the user last gave a resizable dialog is kept (#8984), beside its place.
|
||||
fn size_id(place: egui::Id) -> egui::Id {
|
||||
place.with("size")
|
||||
}
|
||||
|
||||
/// The largest a resizable dialog of `fields` with smallest size `min` is dragged to: the window
|
||||
/// less a margin, and for Curves no wider than its graph can grow (#8984), so no blank column.
|
||||
fn resizable_max(ctx: &egui::Context, fields: &serde_json::Map<String, Value>, min: egui::Vec2) -> egui::Vec2 {
|
||||
let mut max = (ctx.content_rect().size() - egui::vec2(40.0, 40.0)).max(min);
|
||||
if is_curves(fields) {
|
||||
max.x = max.x.min(crate::adjust_editors::curve_side_max(ctx)).max(min.x);
|
||||
}
|
||||
max
|
||||
}
|
||||
|
||||
fn show_modal<T>(
|
||||
modal: egui::Modal,
|
||||
ctx: &egui::Context,
|
||||
id: egui::Id,
|
||||
resizable: Option<egui::Vec2>,
|
||||
resizable: Option<(egui::Vec2, egui::Vec2, egui::Vec2)>,
|
||||
interactive: bool,
|
||||
contents: impl FnOnce(&mut egui::Ui) -> T,
|
||||
) -> egui::ModalResponse<T> {
|
||||
@@ -199,17 +230,10 @@ fn show_modal<T>(
|
||||
if !interactive {
|
||||
ui.disable();
|
||||
}
|
||||
if let Some(size) = resizable {
|
||||
let max = (ctx.content_rect().size() - egui::vec2(40.0, 40.0)).max(egui::vec2(560.0, 400.0));
|
||||
if let Some((size, min, max)) = resizable {
|
||||
// No stroke: Resize outlines its contents 2 points out, an inner box inside the
|
||||
// dialog's frame that left the title and buttons flush against it (#8199).
|
||||
egui::Resize::default()
|
||||
.id(id.with("resize"))
|
||||
.with_stroke(false)
|
||||
.default_size(size.min(max))
|
||||
.min_size(egui::vec2(560.0, 400.0))
|
||||
.max_size(max)
|
||||
.show(ui, contents)
|
||||
egui::Resize::default().id(id.with("resize")).with_stroke(false).default_size(size.min(max)).min_size(min).max_size(max).show(ui, contents)
|
||||
} else {
|
||||
contents(ui)
|
||||
}
|
||||
@@ -289,14 +313,22 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
|
||||
let web = fields.contains_key("__web");
|
||||
let web_popup_open = web && egui::Popup::is_any_open(ctx);
|
||||
let resizable = resizable_dialog(&fields);
|
||||
let modal = show_modal(modal, ctx, id, resizable, interactive, |ui| {
|
||||
// Curves reopens at the size it was left at (#8984).
|
||||
let remembered = if is_curves(&fields) { ctx.data_mut(|m| m.get_persisted::<egui::Vec2>(size_id(place))) } else { None };
|
||||
let resize = resizable.map(|size| {
|
||||
let min = resizable_min(&fields);
|
||||
(remembered.unwrap_or(size), min, resizable_max(ctx, &fields, min))
|
||||
});
|
||||
let mut given = None;
|
||||
let modal = show_modal(modal, ctx, id, resize, interactive, |ui| {
|
||||
sizing = ui.is_sizing_pass();
|
||||
let available = ui.available_size();
|
||||
given = Some(available);
|
||||
// A dialog of a few short fields (`__compact`, Fit Image) is narrower (#4673): the title
|
||||
// rule and the button row fill the width they are given, so it needs its own maximum too.
|
||||
let compact = d.fields.contains_key("__compact");
|
||||
// The curve editor caps its graph allocation at 300 points (#5113).
|
||||
let curves = fields.get("__adjust").and_then(Value::as_str) == Some("curves");
|
||||
// The curve editor's graph is as wide as the dialog lets it be (#5113, #8984).
|
||||
let curves = is_curves(&fields);
|
||||
if !compact {
|
||||
ui.set_min_width(if curves { 300.0 } else { 380.0 });
|
||||
}
|
||||
@@ -323,7 +355,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
|
||||
} else if compact {
|
||||
260.0
|
||||
} else if curves {
|
||||
300.0
|
||||
available.x.max(300.0)
|
||||
} else {
|
||||
440.0
|
||||
}));
|
||||
@@ -579,6 +611,17 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
|
||||
});
|
||||
// The picker has a larger frame margin; include that whole frame in the canvas hit guard.
|
||||
shown.push(if color_picker { modal.response.rect } else { modal.inner });
|
||||
// The size Curves was given (dragged or grown to fit) is the one it reopens at (#8984).
|
||||
// A size at the window's limit is the window's, not the user's: a large remembered size
|
||||
// squeezed into a small window is kept for a larger one.
|
||||
if let (false, true, Some(size), Some((opened, _, max))) = (sizing, is_curves(&fields), given.filter(|g| g.is_finite()), resize)
|
||||
&& size != opened.min(max)
|
||||
&& size.x < max.x - 0.5
|
||||
&& size.y < max.y - 0.5
|
||||
&& remembered != Some(size)
|
||||
{
|
||||
ctx.data_mut(|m| m.insert_persisted(size_id(place), size));
|
||||
}
|
||||
// Pin once laid out at its real size (the first frame is an invisible sizing pass).
|
||||
if !sizing && (web || pinned.is_none() || drag != egui::Vec2::ZERO) {
|
||||
let screen = ctx.content_rect();
|
||||
|
||||
Reference in New Issue
Block a user