#887: coalesce Scale (and similar) hot-text drags into one undo

A drag of Effect Controls / Properties numbers already used merge+begin, but
the press-start memory was only written on frames that emitted setParam.
egui drops unused temp data after a quiet frame, so a Scale scrub that
paused between percents began a new undo step for every 1%. Refresh the
drag session on idle frames while the button is down; still begin a new
step on the next press. Same keep-alive for graphics property drags.
This commit is contained in:
Vikram Bhaskaran committed 2026-10-11 13:05:18 +11:00
1 parent 4e2d0bc2d3
commit ba4dacdeff
2 files changed
+62 -9

No files matched your search

+50 -3
View File
@@ -963,21 +963,29 @@ pub fn properties_panel(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
/// Run the panel's actions. Parameter changes made while the mouse button is down (a drag) share
/// one undo step, which the first change of each press begins (#201); typed values and clicks stay
/// separate steps. A drag value only changes once the mouse moves, after the press frame, so the
/// press that already began a step is remembered by its start time.
/// press that already began a step is remembered by its start time. That memory is refreshed on
/// every frame the button stays down — including frames with no value change — so a Scale scrub
/// that pauses between percents is still one undo (#887).
fn run(app: &mut FilmcraftApp, ctx: &egui::Context, actions: Vec<(String, Value)>) {
let (down, press) = ctx.input(|i| (i.pointer.any_down(), i.pointer.press_start_time()));
let key = egui::Id::new("effect-controls-drag-step");
let begun = ctx.data(|d| d.get_temp::<Option<f64>>(key)).flatten();
let mut wrote = false;
for (cmd, mut p) in actions {
if cmd == "effects.setParam" && down {
let begun = ctx.data(|d| d.get_temp::<Option<f64>>(key)).flatten();
p["merge"] = json!(true);
p["begin"] = json!(begun != press);
ctx.data_mut(|d| d.insert_temp(key, press));
wrote = true;
}
if let Err(e) = app.session.execute(&cmd, p) {
app.ui.status = e.to_string();
}
}
if down && (wrote || begun.is_some()) {
ctx.data_mut(|d| d.insert_temp(key, press));
} else if !down {
ctx.data_mut(|d| d.remove::<Option<f64>>(key));
}
}
pub(crate) fn graph_key(clip: ClipId, idx: usize, pid: &str) -> String {
@@ -1253,6 +1261,45 @@ mod drag_undo_tests {
app.session.undo();
assert_eq!(opacity(&app), start, "and then the first");
}
/// #887: a Scale hot-text drag that pauses between percents (no `setParam` that frame) is
/// still one undo, not one step per 1%.
#[test]
fn a_scale_drag_with_idle_frames_is_one_undo_step() {
let mut s = filmcraft_engine::Session::default();
s.execute("file.openDemoProject", json!({})).unwrap();
let clip = s.active_sequence().unwrap().video_tracks[0].items[0].id.0;
let mut app = crate::FilmcraftApp::new(s);
let ctx = egui::Context::default();
let scale = |app: &crate::FilmcraftApp| {
let q = app.session.active_sequence().unwrap();
let it = q.find_item(filmcraft_project::ClipId(clip)).unwrap().1;
it.effects.iter().find(|e| e.effect == "motion").unwrap().params["scale"].value.as_f64().unwrap()
};
let start = scale(&app);
let pos = egui::pos2(10.0, 10.0);
let button = |pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
let mut time = 0.0;
let mut frame = |app: &mut crate::FilmcraftApp, events: Vec<egui::Event>, value: Option<f64>| {
time += 0.1;
let raw = egui::RawInput { time: Some(time), events, ..Default::default() };
let mut out = ctx.run_ui(raw, |ui| {
let actions = value.map(|v| ("effects.setParam".to_string(), json!({"clip": clip, "effect": "motion", "param": "scale", "value": v})));
super::run(app, ui.ctx(), actions.into_iter().collect());
});
out.textures_delta.clear();
};
frame(&mut app, vec![egui::Event::PointerMoved(pos), button(true)], None);
for v in [101.0, 150.0, 200.0, 249.0] {
frame(&mut app, vec![egui::Event::PointerMoved(pos)], Some(v));
frame(&mut app, vec![egui::Event::PointerMoved(pos)], None); // idle: mouse down, no 1% tick
}
frame(&mut app, vec![button(false)], None);
assert_eq!(scale(&app), 249.0);
app.session.undo();
assert_eq!(scale(&app), start, "one undo restores the scale from before the drag");
assert_eq!(app.session.undo(), None, "the drag must not have left extra 1% steps");
}
}
#[cfg(test)]
+12 -6
View File
@@ -1771,21 +1771,27 @@ pub fn properties(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) {
for (id, r, label) in autos {
app.auto.add(&id, r, &label);
}
let (down, press) = ui.ctx().input(|i| (i.pointer.any_down(), i.pointer.press_start_time()));
let key = egui::Id::new("gfx-props-drag-step");
let begun = ui.ctx().data(|d| d.get_temp::<Option<f64>>(key)).flatten();
let mut wrote = false;
for (cmd, mut p) in actions {
// a drag of a property is one undo step: the first change of each press begins a new one,
// the rest fold into it (like Effect Controls, #201)
if cmd == "graphics.set" && ui.ctx().input(|i| i.pointer.any_down()) {
let key = egui::Id::new("gfx-props-drag-step");
let press = ui.ctx().input(|i| i.pointer.press_start_time());
let begun = ui.ctx().data(|d| d.get_temp::<Option<f64>>(key)).flatten();
// the rest fold into it (like Effect Controls, #201 / #887)
if cmd == "graphics.set" && down {
p["merge"] = json!(true);
p["begin"] = json!(begun != press);
ui.ctx().data_mut(|d| d.insert_temp(key, press));
wrote = true;
}
if let Err(e) = app.session.execute(&cmd, p) {
app.ui.status = e.to_string();
}
}
if down && (wrote || begun.is_some()) {
ui.ctx().data_mut(|d| d.insert_temp(key, press));
} else if !down {
ui.ctx().data_mut(|d| d.remove::<Option<f64>>(key));
}
}
/// Text Properties (the wrench in the Text section): Text Layer Type and Text Styling.