Follow-up to #2841: Composition Settings settles after a width change

- `dialog.comp.view` is the scroll area's viewport (its clip rect). Cut
  by the content's max rect, it lost the scrolled amount at the bottom.
- When the dialog's width changes, the pass laid out at the old width is
  discarded and a repaint requested.
- The measured width keeps room for a vertical scroll bar. Without it,
  switching the language with the dialog open left the two scroll bars
  trading places every frame: the UI never settled and OK was clipped
  every other frame.
- `content_width` becomes `dialog_width`, which returns the dialog's
  width, so the 600-point rule is in one place.

Tests: the view keeps its place when the dialog scrolls, and switching
to French and back with the dialog open settles with OK inside it.

crates/ui-egui/src/panels/comp_settings.rs

@@ -478,15 +478,17 @@ fn tab_size(ui: &mut egui::Ui, tab: Tab, d: &CompDraft, t: &Tokens, custom: &[Co
     vec2(child.min_rect().width(), child.cursor().top() - top)
 }
 
+/// Where the width of the dialog's content, measured when it was last shown, is kept.
 fn content_width_id() -> egui::Id {
     egui::Id::unique("comp-settings-content-width")
 }
 
-/// The width the dialog's content took when it was last shown: its widest row on any tab. A
-/// translation longer than the English (French, #2841) is wider than the 600-point dialog, so the
-/// dialog grows to it rather than hiding OK past its right edge.
-pub fn content_width(ctx: &egui::Context) -> f32 {
-    ctx.data(|d| d.get_temp::<f32>(content_width_id())).unwrap_or(0.0)
+/// The dialog's width: 600 points, or more when the widest row on any of its tabs (as last shown),
+/// a scroll bar and the modal's two 18-point margins need it. A translation longer than the English (French,
+/// #2841) widens the dialog rather than hiding OK past its right edge.
+pub fn dialog_width(ctx: &egui::Context) -> f32 {
+    let content = ctx.data(|d| d.get_temp::<f32>(content_width_id())).unwrap_or(0.0);
+    (content + 36.0).max(600.0)
 }
 
 /// Draw the dialog. Returns `Some(params)` when OK is pressed, `Some(Null)` on Cancel. `custom`
@@ -501,8 +503,9 @@ pub fn show(
     preview: Option<&mut bool>,
 ) -> Option<serde_json::Value> {
     let mut out = None;
-    // The dialog's visible area, which Cancel and OK stay inside.
-    auto.add("dialog.comp.view", ui.clip_rect().intersect(ui.max_rect()), "Composition Settings");
+    // The dialog's visible area, which Cancel and OK stay inside: the scroll area's viewport,
+    // which stays put when the content scrolls.
+    auto.add("dialog.comp.view", ui.clip_rect(), "Composition Settings");
     let name_row = ui.horizontal(|ui| {
         ui.label(tr("Composition Name:"));
         let r = ui.add(egui::TextEdit::singleline(&mut d.name).desired_width(300.0));
@@ -529,7 +532,19 @@ pub fn show(
     ui.add_space((tallest - shown).max(0.0));
     // Every tab takes the width of the widest one too, so the dialog doesn't change size with them.
     let widest = sizes.iter().fold(name_row.response.rect.width().max(tab_row.response.rect.width()), |w, s| w.max(s.x));
-    ui.ctx().data_mut(|m| m.insert_temp(content_width_id(), widest.ceil()));
+    // Room for a vertical scroll bar too: without it, a bar shown while the dialog settles narrows
+    // the content into a horizontal bar, which takes the height back from it, and the two bars
+    // trade places every frame.
+    let needed = widest + ui.spacing().scroll.allocated_width();
+    let ctx = ui.ctx().clone();
+    let before = dialog_width(&ctx);
+    ctx.data_mut(|m| m.insert_temp(content_width_id(), needed.ceil()));
+    if dialog_width(&ctx) != before {
+        // This pass was laid out at the old width: lay it out again at the new one instead of
+        // showing it, and paint again in case egui declines the extra pass.
+        ctx.request_discard("Composition Settings width");
+        ctx.request_repaint();
+    }
     ui.add_space(14.0);
     ui.horizontal(|ui| {
         ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {

crates/ui-egui/src/panels/dialogs.rs

@@ -446,9 +446,7 @@ fn comp_settings(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens, exis
     let live = existing && super::live_preview::active(app);
     let place = super::live_preview::place(app);
     let mut preview = app.dialog_state.live.on;
-    // 600 points wide, or as wide as the content took last time (with the 18-point margins).
-    let width = (super::comp_settings::content_width(ctx) + 36.0).max(600.0);
-    modal_placed(ctx, "Composition Settings", vec2(width, 470.0), t, place, |ui| {
+    modal_placed(ctx, "Composition Settings", vec2(super::comp_settings::dialog_width(ctx), 470.0), t, place, |ui| {
         result = super::comp_settings::show(ui, &mut d, t, &mut app.auto, &custom, &mut cmds, live.then_some(&mut preview));
     });
     app.dialog_state.live.on = preview;

crates/ui-egui/tests/it/ui_comp_presets.rs

@@ -136,3 +136,63 @@ fn composition_settings_buttons_fit_every_language() {
         assert!(oks.windows(2).all(|w| w[0] == w[1]), "{language}: OK moves between tabs: {oks:?}");
     }
 }
+
+fn rect_of(h: &Harness<'_, EffectcraftApp>, id: &str) -> egui::Rect {
+    let r = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).rect;
+    egui::Rect::from_min_size(pos2(r[0], r[1]), egui::vec2(r[2], r[3]))
+}
+
+/// In a window too short for it, the dialog scrolls: its registered view stays the scroll
+/// area's viewport (not a part of it shifted by the scroll), and OK scrolls into it.
+#[test]
+fn composition_settings_view_stays_put_when_it_scrolls() {
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"name": "Edit", "width": 2560, "height": 1440, "frameRate": 60, "duration": 4})).unwrap();
+    let mut h = Harness::builder().with_size(egui::vec2(1600.0, 520.0)).with_step_dt(1.0 / 60.0).build_eframe(|_| EffectcraftApp::new(s));
+    h.run_steps(3);
+    open_settings(&mut h);
+    let (view, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok"));
+    assert!(ok.bottom() > view.bottom(), "the dialog must be taller than the window for this test: OK {ok:?}, view {view:?}");
+    h.input_mut().events.push(Event::PointerMoved(view.center()));
+    h.input_mut().events.push(Event::MouseWheel {
+        unit: egui::MouseWheelUnit::Point,
+        delta: egui::vec2(0.0, -2000.0),
+        modifiers: Modifiers::NONE,
+        phase: egui::TouchPhase::Move,
+        source: egui::MouseWheelSource::Wheel,
+    });
+    h.run_steps(30);
+    let (scrolled, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok"));
+    // The scroll bar may take a few points on the right while it is shown.
+    assert_eq!((scrolled.y_range(), scrolled.left()), (view.y_range(), view.left()), "the view is the viewport, wherever the content is scrolled to");
+    assert!(scrolled.contains_rect(ok), "OK {ok:?} scrolls into the view {scrolled:?}");
+}
+
+/// Changing the interface language with the dialog open refits it to the new text and then
+/// settles, with OK inside it: its scroll bars don't trade places every frame while it grows.
+/// Switching back to English returns it to 600 points.
+#[test]
+fn composition_settings_refits_when_the_language_changes() {
+    let mut s = Session::default();
+    s.execute("comp.new", json!({"name": "Edit", "width": 2560, "height": 1440, "frameRate": 60, "duration": 4})).unwrap();
+    let mut h = Harness::builder()
+        .with_size(egui::vec2(1600.0, 1000.0))
+        .with_step_dt(1.0 / 60.0)
+        .with_max_steps(30)
+        // `run` checks accessibility after settling; that is not what this test is about.
+        .with_accessibility_check(false)
+        .build_eframe(|_| EffectcraftApp::new(s));
+    h.run_steps(3);
+    open_settings(&mut h);
+    click(&mut h, "dialog.comp.tab.advanced");
+    h.run_ok().expect("the open dialog settles");
+    assert_eq!(rect_of(&h, "dialog.comp.view").width(), 600.0 - 36.0);
+    for (language, wider) in [("fr", true), ("en", false)] {
+        h.state_mut().session.prefs.general.language = language.into();
+        // Steps while anything asks for a repaint, and fails if that never stops.
+        let steps = h.try_run().unwrap_or_else(|e| panic!("{language}: {e}"));
+        let (view, ok) = (rect_of(&h, "dialog.comp.view"), rect_of(&h, "dialog.comp.ok"));
+        assert!(view.contains_rect(ok), "{language}: after {steps} steps OK {ok:?} is outside the dialog {view:?}");
+        assert_eq!(view.width() > 600.0 - 36.0, wider, "{language}: the dialog is {} wide", view.width());
+    }
+}