diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 0be35411f..93b278e79 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -203,6 +203,22 @@ pub(crate) fn modal_fit(ctx: &egui::Context, title: &str, size: egui::Vec2, t: & }); } +/// The buttons of a [`modal_fit`] body of `height`, kept in view: `footer` lays them out first, +/// at the bottom (bottom up: its first row is the lowest), and the rest of the body gets the room +/// above them. Returns that room (a left-to-right row, clipped to it, so a long or wide body +/// scrolls or clips under the buttons instead of pushing them out of the dialog) and its height. +pub(crate) fn above_footer(ui: &mut egui::Ui, height: f32, footer: impl FnOnce(&mut egui::Ui)) -> (egui::Ui, f32) { + let area = ui.allocate_space(vec2(ui.available_width(), height)).1; + let mut foot = ui.new_child(egui::UiBuilder::new().max_rect(area).layout(egui::Layout::bottom_up(egui::Align::Min))); + footer(&mut foot); + foot.add_space(8.0); + let body_h = (foot.min_rect().top() - area.top()).max(0.0); + let room = Rect::from_min_size(area.min, vec2(area.width(), body_h)); + let mut body = ui.new_child(egui::UiBuilder::new().max_rect(room).layout(egui::Layout::left_to_right(egui::Align::Min))); + body.set_clip_rect(room.intersect(body.clip_rect())); + (body, body_h) +} + pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context) { let Some(d) = app.dialog else { return }; let t = app.tokens; diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index 1366c2ae8..c0b03f77e 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/crates/ui-egui/src/panels/layer_styles_dialog.rs @@ -8,7 +8,7 @@ use crate::i18n::tr; use effectcraft_engine::keyframe::Value as KV; use effectcraft_engine::project::styles::{BLENDING, STYLES}; use effectcraft_engine::project::{Layer, LayerId, ParamUi, Property}; -use egui::{Align2, vec2}; +use egui::vec2; use serde_json::{Value, json}; use crate::theme::Tokens; @@ -138,36 +138,57 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let lt = l.layer_time(time); let mut actions: Vec<(String, Value)> = Vec::new(); let mut close: Option = None; - super::dialogs::modal(ctx, &format!("Layer Style — {}", l.name), vec2(720.0, 520.0), t, |ui| { - ui.horizontal_top(|ui| { + super::dialogs::modal_fit(ctx, &format!("Layer Style — {}", l.name), vec2(720.0, 520.0), t, |ui, height| { + // OK and Cancel stay at the bottom, in view; the list and the style's controls scroll + // above them (#1062). + let (mut body, body_h) = super::dialogs::above_footer(ui, height, |foot| { + foot.horizontal(|ui| { + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let ok = ui.button(tr("OK")); + app.auto.add("dialog.layerStyle.ok", ok.rect, "OK"); + if ok.clicked() { + close = Some(true); + } + let cancel = ui.button(tr("Cancel")); + app.auto.add("dialog.layerStyle.cancel", cancel.rect, "Cancel"); + if cancel.clicked() { + close = Some(false); + } + }); + }); + }); + { + let ui = &mut body; // Style list. ui.vertical(|ui| { ui.set_width(200.0); - let mut entries = vec![(BLENDING, "Blending Options")]; - entries.extend(STYLES.iter().copied()); - for (id, name) in entries { - ui.horizontal(|ui| { - if id != BLENDING { - let g = styles.as_ref().and_then(|s| s.sub(id)); - let mut on = g.is_some_and(|g| g.enabled); - if ui.checkbox(&mut on, "").changed() { - if g.is_none() { - actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); - } else { - actions.push(("layer.style.toggle".into(), json!({"layer": lid, "style": id, "value": on}))); + egui::ScrollArea::vertical().id_salt("ls-list").max_height(body_h).min_scrolled_height(0.0).show(ui, |ui| { + let mut entries = vec![(BLENDING, "Blending Options")]; + entries.extend(STYLES.iter().copied()); + for (id, name) in entries { + ui.horizontal(|ui| { + if id != BLENDING { + let g = styles.as_ref().and_then(|s| s.sub(id)); + let mut on = g.is_some_and(|g| g.enabled); + if ui.checkbox(&mut on, "").changed() { + if g.is_none() { + actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); + } else { + actions.push(("layer.style.toggle".into(), json!({"layer": lid, "style": id, "value": on}))); + } } + } else { + ui.add_space(24.0); } - } else { - ui.add_space(24.0); - } - let r = ui.selectable_label(sel == id, name); - if r.clicked() { - app.dialog_state.layer_style.selected = id.to_string(); - } - let rect = r.rect; - app.auto.add(&format!("dialog.layerStyle.list.{id}"), rect, name); - }); - } + let r = ui.selectable_label(sel == id, name); + if r.clicked() { + app.dialog_state.layer_style.selected = id.to_string(); + } + let rect = r.rect; + app.auto.add(&format!("dialog.layerStyle.list.{id}"), rect, name); + }); + } + }); }); ui.separator(); // Controls of the selected style. @@ -180,42 +201,39 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.label(egui::RichText::new(tr("Not applied: tick it in the list to add it.")).weak()); return; }; - egui::ScrollArea::vertical().max_height(400.0).show(ui, |ui| { - egui::Grid::new("ls-grid").num_columns(2).spacing(vec2(12.0, 6.0)).show(ui, |ui| { - g.walk("", &mut |_, p| { - if matches!(p.ui, ParamUi::Hidden) { - return; - } - let lbl = ui.label(&p.name); - let v = p.value_at(lt); - if let Some(nv) = control(ui, p, &v) { - actions.push(( - "prop.set".into(), - json!({"layer": lid, "prop": p.uid, "value": nv, "merge": format!("layer-style-dialog-{}", p.uid)}), - )); - } - let path = g.match_path_of(p.uid).unwrap_or_default(); - app.auto.add(&format!("dialog.layerStyle.prop.{sel}/{path}"), lbl.rect, &p.name); - ui.end_row(); + // Both ways, in the room left under the heading: in a narrow window (or at a high UI + // Scale) the controls scroll sideways rather than being cut off. + egui::ScrollArea::both() + .id_salt("ls-controls") + .max_width(ui.available_width().max(0.0)) + .max_height(ui.available_height().max(0.0)) + .min_scrolled_width(0.0) + .min_scrolled_height(0.0) + .auto_shrink([false, false]) + .show(ui, |ui| { + egui::Grid::new("ls-grid").num_columns(2).spacing(vec2(12.0, 6.0)).show(ui, |ui| { + g.walk("", &mut |_, p| { + if matches!(p.ui, ParamUi::Hidden) { + return; + } + let lbl = ui.label(&p.name); + let v = p.value_at(lt); + let edit = ui.scope(|ui| control(ui, p, &v)); + if let Some(nv) = edit.inner { + actions.push(( + "prop.set".into(), + json!({"layer": lid, "prop": p.uid, "value": nv, "merge": format!("layer-style-dialog-{}", p.uid)}), + )); + } + let path = g.match_path_of(p.uid).unwrap_or_default(); + app.auto.add(&format!("dialog.layerStyle.prop.{sel}/{path}"), lbl.rect, &p.name); + app.auto.add(&format!("dialog.layerStyle.value.{sel}/{path}"), edit.response.rect, &p.name); + ui.end_row(); + }); }); }); - }); }); - }); - ui.add_space(8.0); - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - let ok = ui.button(tr("OK")); - app.auto.add("dialog.layerStyle.ok", ok.rect, "OK"); - if ok.clicked() { - close = Some(true); - } - let cancel = ui.button(tr("Cancel")); - app.auto.add("dialog.layerStyle.cancel", cancel.rect, "Cancel"); - if cancel.clicked() { - close = Some(false); - } - let _ = Align2::LEFT_CENTER; - }); + } }); for (id, p) in actions { if let Err(e) = app.session.execute(&id, p) { diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 8cbb9b35a..b2d2ca435 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -10,7 +10,7 @@ use crate::i18n::{tr, tr_args}; use effectcraft_engine::prefs::{APPEARANCE_MODES, DARK_THEMES, Item, Kind, LIGHT_THEMES, Page, pages}; use effectcraft_engine::segment::Task; -use egui::{Color32, Rect, RichText, vec2}; +use egui::{Color32, RichText, vec2}; use serde_json::{Value, json}; use crate::theme::{ThemeKind, Tokens}; @@ -633,37 +633,31 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let mut acts: Vec = vec![]; let (mut ok, mut cancel) = (false, false); super::dialogs::modal_fit(ctx, "Settings", vec2(820.0, 600.0), t, |ui, height| { - // The buttons go first, from the bottom up, and the pages get the height they leave: at - // a high UI Scale the pages scroll and the buttons stay in view. - let area = ui.allocate_space(vec2(ui.available_width(), height)).1; - let mut foot = ui.new_child(egui::UiBuilder::new().max_rect(area).layout(egui::Layout::bottom_up(egui::Align::Min))); - foot.horizontal(|ui| { - let r = ui.add_enabled(idx > 0, egui::Button::new(tr("Previous"))); - app.auto.add("settings.previous", r.rect, "Previous"); - if r.clicked() { - page = all[idx - 1].id.to_string(); - } - let r = ui.add_enabled(idx + 1 < all.len(), egui::Button::new(tr("Next"))); - app.auto.add("settings.next", r.rect, "Next"); - if r.clicked() { - page = all[idx + 1].id.to_string(); - } - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - let r = ui.add(egui::Button::new(RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)); - app.auto.add("settings.ok", r.rect, "OK"); - ok |= r.clicked(); - let r = ui.button(tr("Cancel")); - app.auto.add("settings.cancel", r.rect, "Cancel"); - cancel |= r.clicked(); + // The buttons go first and the pages get the height they leave: at a high UI Scale the + // pages scroll and the buttons stay in view. + let (mut body, body_h) = super::dialogs::above_footer(ui, height, |foot| { + foot.horizontal(|ui| { + let r = ui.add_enabled(idx > 0, egui::Button::new(tr("Previous"))); + app.auto.add("settings.previous", r.rect, "Previous"); + if r.clicked() { + page = all[idx - 1].id.to_string(); + } + let r = ui.add_enabled(idx + 1 < all.len(), egui::Button::new(tr("Next"))); + app.auto.add("settings.next", r.rect, "Next"); + if r.clicked() { + page = all[idx + 1].id.to_string(); + } + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let r = ui.add(egui::Button::new(RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)); + app.auto.add("settings.ok", r.rect, "OK"); + ok |= r.clicked(); + let r = ui.button(tr("Cancel")); + app.auto.add("settings.cancel", r.rect, "Cancel"); + cancel |= r.clicked(); + }); }); + foot.separator(); }); - foot.separator(); - foot.add_space(8.0); - let body_h = (foot.min_rect().top() - area.top()).max(0.0); - // (A child, clipped to its share, so a wide page can't widen the dialog under the buttons.) - let pages = Rect::from_min_size(area.min, vec2(area.width(), body_h)); - let mut body = ui.new_child(egui::UiBuilder::new().max_rect(pages).layout(egui::Layout::left_to_right(egui::Align::Min))); - body.set_clip_rect(pages.intersect(body.clip_rect())); { let ui = &mut body; // Page list (left, like After Effects). diff --git a/crates/ui-egui/tests/ui_tail.rs b/crates/ui-egui/tests/ui_tail.rs index aa4128cbf..aed59e56d 100644 --- a/crates/ui-egui/tests/ui_tail.rs +++ b/crates/ui-egui/tests/ui_tail.rs @@ -8,10 +8,14 @@ use egui_kittest::Harness; use serde_json::json; fn harness(w: f32) -> Harness<'static, EffectcraftApp> { + harness_at(vec2(w, 900.0)) +} + +fn harness_at(size: egui::Vec2) -> Harness<'static, EffectcraftApp> { let mut s = Session::default(); s.execute("comp.new", json!({"name": "Tail", "width": 640, "height": 360, "duration": 4})).unwrap(); s.execute("layer.newSolid", json!({"name": "Plate", "color": "#406080", "width": 640, "height": 360})).unwrap(); - let mut h = Harness::builder().with_size(egui::vec2(w, 900.0)).build_eframe(|_| EffectcraftApp::new(s)); + let mut h = Harness::builder().with_size(size).build_eframe(|_| EffectcraftApp::new(s)); h.run_steps(3); h } @@ -60,6 +64,16 @@ fn timeline_outline_columns_scroll_horizontally() { assert!(h.state().auto.find("timeline.outlineScroll").is_none()); } +fn click(h: &mut Harness<'_, EffectcraftApp>, at: Pos2) { + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerMoved(at)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + h.step(); + } + h.run_steps(2); +} + fn styles(h: &mut Harness<'_, EffectcraftApp>) -> Vec { let v = h.state_mut().session.execute("layer.style.list", json!({})).unwrap(); v["styles"].as_array().unwrap().iter().map(|s| s["style"].as_str().unwrap().to_string()).collect() @@ -103,6 +117,96 @@ fn layer_style_dialog_adds_previews_and_cancels() { assert_eq!(styles(&mut h), ["stroke"]); } +/// The Layer Style dialog on the solid, opened from Layer ▸ Layer Styles ▸ Drop Shadow (which +/// adds the style) at `scale` % UI Scale. Returns the layer. +fn open_drop_shadow(h: &mut Harness<'_, EffectcraftApp>, scale: u32) -> u64 { + let lid = h.state().session.active_comp().unwrap().layers[0].id.0; + h.state_mut().session.execute("layer.select", json!({"layers": [lid]})).unwrap(); + h.state_mut().set_pref("appearance.uiScale", json!(scale)).unwrap(); + h.run_steps(3); + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.style.dropShadow", json!({})).unwrap(); + // (Its layout settles a few frames after it opens.) + h.run_steps(5); + lid +} + +/// `id` whole: inside the window, and painted with none of it clipped away. +fn in_view(h: &Harness<'_, EffectcraftApp>, id: &str) -> Pos2 { + let r = rect(h, id); + // (A shape it paints: the topmost lying inside it, not the panels behind the dialog.) + let clip = h.output().shapes.iter().rev().find(|c| matches!(&c.shape, egui::Shape::Rect(s) if r.expand(1.0).contains_rect(s.rect))).map(|c| c.clip_rect); + assert!(h.ctx.content_rect().contains_rect(r) && clip.is_some_and(|c| c.contains_rect(r)), "{id} is cut off: {r:?} in {clip:?}"); + r.center() +} + +fn style_value(h: &mut Harness<'_, EffectcraftApp>, lid: u64, path: &str) -> f64 { + h.state_mut().session.execute("prop.get", json!({"layer": lid, "path": format!("layerStyles/dropShadow/{path}")})).unwrap()["value"].as_f64().unwrap() +} + +/// #1062: the Layer Style dialog's OK and Cancel stay in view below the list and the style's +/// controls, at 100 % and 200 % UI Scale (they were laid out past its bottom edge, under the fold +/// of its scroll area, so a click on them did nothing until the dialog was scrolled). OK keeps an +/// edit; Cancel puts the project and its undo history back as they were before the dialog. +#[test] +fn layer_style_buttons_stay_in_view() { + for scale in [100, 200] { + for ok in [true, false] { + let mut h = harness(1400.0); + let before = (h.state().session.project.clone(), h.state().session.history.undo.len()); + let lid = open_drop_shadow(&mut h, scale); + let buttons = [in_view(&h, "dialog.layerStyle.ok"), in_view(&h, "dialog.layerStyle.cancel")]; + // An edit, as the dialog's controls make them. + h.state_mut() + .session + .execute("prop.set", json!({"layer": lid, "path": "layerStyles/dropShadow/distance", "value": 40, "merge": "layer-style-dialog"})) + .unwrap(); + h.run_steps(2); + click(&mut h, buttons[if ok { 0 } else { 1 }]); + let case = format!("{scale} %, {}", if ok { "OK" } else { "Cancel" }); + assert_eq!(h.state().dialog, None, "{case} closed the dialog"); + if ok { + assert_eq!(style_value(&mut h, lid, "distance"), 40.0, "{case} kept the edit"); + } else { + let after = (&h.state().session.project, h.state().session.history.undo.len()); + assert!(after.0.same_content(&before.0) && after.1 == before.1, "{case} put back the project and its history"); + } + } + } +} + +/// #1062: with the buttons kept at the bottom, the style's controls still scroll both ways, so +/// every one of them can be reached in the smallest window (960×600) at 200 % UI Scale, and in a +/// dialog opened in a large window that then narrows: here Noise, near the end of Drop Shadow, +/// is scrolled to, seen whole and dragged. +#[test] +fn layer_style_controls_stay_reachable_in_a_small_window() { + for narrowed in [false, true] { + let mut h = harness_at(if narrowed { vec2(1600.0, 1000.0) } else { vec2(960.0, 600.0) }); + let lid = open_drop_shadow(&mut h, if narrowed { 100 } else { 200 }); + if narrowed { + h.set_size(vec2(960.0, 600.0)); + h.state_mut().set_pref("appearance.uiScale", json!(200)).unwrap(); + h.run_steps(5); + } + // Scroll the controls (under their heading) to their far end, both ways. + let over = rect(&h, "dialog.layerStyle.page").left_bottom() + vec2(10.0, 16.0); + h.input_mut().events.push(Event::PointerMoved(over)); + h.input_mut().events.push(Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(-2000.0, -2000.0), + modifiers: Default::default(), + phase: egui::TouchPhase::Move, + }); + h.run_steps(5); + let at = in_view(&h, "dialog.layerStyle.value.dropShadow/noise"); + let slider = rect(&h, "dialog.layerStyle.value.dropShadow/noise").left_center(); + drag(&mut h, slider + vec2(8.0, 0.0), slider + vec2(60.0, 0.0)); + assert!(style_value(&mut h, lid, "noise") > 0.0, "narrowed {narrowed}: Noise was dragged (at {at:?})"); + assert_eq!(h.state().dialog, Some(effectcraft_ui_egui::Dialog::LayerStyles)); + } +} + #[test] fn project_panel_columns_scroll_horizontally() { let mut h = harness(1400.0);