collective
Browse / github.com/storytold/filmcraft / patch
Preserve Reverse Speed when reopening the clip dialog
Preserve Reverse Speed when reopening the clip dialog Reopening Speed/Duration on a reversed clip initializes Reverse Speed from `speed < 0`, although speed is stored as a positive magnitude and direction is a separate flag. The checkbox therefore starts unchecked and accepting unchanged settings silently switches playback forward. Read the stored reverse flag instead. Refs #3238 for this demonstrated reopening/reset subcase; this does not claim to explain every reported first-click symptom. The headless dialog regression fails before the fix, then verifies the displayed direction, Cancel, unchanged acceptance, retained speed and explicit return
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- jnohclee-rgb (imported from pull request #3247, open)
- Base
a504f3c5adc4cd60d9c1120916cddb50c9b24b86- Size
- +26 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
00b9b118999976ce62711b9d
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/panels/clip_dialogs.rs crates/ui-egui/tests/it/clip_dialogs_ui.rs
@@ -224,7 +224,7 @@ fn defaults(app: &FilmcraftApp, id: &str) -> (Value, Value) { let speed = it.as_ref().map_or(1.0, |i| i.speed); let interp = it.as_ref().map_or(filmcraft_project::TimeInterpolation::default(), |i| i.time_interpolation); (- json!({"speed": (speed.abs() * 100.0 * 100.0).round() / 100.0, "reverse": speed < 0.0, "ripple": false, "interpolation": interp.name()}),+ json!({"speed": (speed.abs() * 100.0 * 100.0).round() / 100.0, "reverse": it.as_ref().is_some_and(|i| i.reverse), "ripple": false, "interpolation": interp.name()}), json!({"duration": it.as_ref().map_or(0, |i| i.duration.0), "speed": speed.abs(), "fps": fps}), ) }@@ -289,6 +289,31 @@ fn speed_duration_from_the_clip_menu_opens_the_dialog() { assert_eq!(got.time_interpolation, filmcraft_project::TimeInterpolation::FrameBlending); } +#[test]+fn speed_duration_reopens_with_the_clips_reverse_state() {+ let mut d = Driver::new();+ let c = d.v1(0);+ d.exec("timeline.select", json!({"clips": [c.id.0]}));+ d.exec("clip.speedDuration", json!({"speed": 200, "reverse": true}));+ assert!(d.v1(0).reverse);++ d.menu("clip.speedDuration");+ let draft = d.app().ui.clip_dialog.as_ref().expect("the dialog opened");+ assert_eq!(draft.params["reverse"], true, "uses the stored direction, not the positive speed magnitude");+ assert_eq!(draft.params["speed"], 200.0);+ d.click("speedDuration.cancel");+ assert!(d.v1(0).reverse, "Cancel retains reverse playback");++ d.menu("clip.speedDuration");+ d.click("speedDuration.ok");+ assert!(d.v1(0).reverse, "accepting unchanged settings retains reverse playback");+ assert_eq!(d.v1(0).speed, 2.0);+ d.menu("clip.speedDuration");+ d.click("speedDuration.reverse");+ d.click("speedDuration.ok");+ assert!(!d.v1(0).reverse, "the checkbox can explicitly restore forward playback");+}+ /// #786: the clip's right-click Label menu lists the labels by the user's names (with their /// colours, as Edit ▸ Label does) and each choice labels the clip. #[test]