diff --git a/crates/engine/src/cmd/shape_stroke.rs b/crates/engine/src/cmd/shape_stroke.rs index ecd3ed8e0..6bb09654b 100644 --- a/crates/engine/src/cmd/shape_stroke.rs +++ b/crates/engine/src/cmd/shape_stroke.rs @@ -205,7 +205,8 @@ const PATHS: [&str; 4] = ["rect", "ellipse", "star", "path"]; pub(crate) const MAX_DEPTH: usize = 64; /// The first Fill colour, Stroke colour and Stroke Width in `layer`'s contents at layer time -/// `lt` (what the toolbar shows for a selected shape layer). +/// `lt` (what the toolbar shows for a selected shape layer). Width comes from the first Stroke +/// or Gradient Stroke, independently of the first solid stroke's colour. pub fn first_paint(layer: &Layer, lt: Tick) -> (Option<[f64; 4]>, Option<[f64; 4]>, Option) { let (mut fill, mut stroke, mut width) = (None, None, None); if let Some(c) = layer.props.sub("contents") { @@ -216,10 +217,12 @@ pub fn first_paint(layer: &Layer, lt: Tick) -> (Option<[f64; 4]>, Option<[f64; 4 "fill" if fill.is_none() => fill = at("color").map(rgba), "stroke" if stroke.is_none() => { stroke = at("color").map(rgba); - width = at("width").map(|v| v.as_f64()); } _ => {} } + if width.is_none() && is_stroke(g) { + width = at("width").map(|v| v.as_f64()); + } }); } (fill, stroke, width) diff --git a/crates/engine/src/settings/prefs/dialog.rs b/crates/engine/src/settings/prefs/dialog.rs index d330ce190..c4ff1d546 100644 --- a/crates/engine/src/settings/prefs/dialog.rs +++ b/crates/engine/src/settings/prefs/dialog.rs @@ -87,6 +87,7 @@ pub fn pages() -> Vec { s("general.recentItems", "Recent Projects Shown", Kind::Int(1, 30, ""), true), Section("Options"), s("general.showToolTips", "Show Tool Tips", B, true), + s("general.viewerWheelScrubbing", "Scrub Composition Frames with Mouse Wheel", B, true), s("general.createLayersAtCompStart", "Create Layers at Composition Start Time", B, true), s("general.switchesAffectNestedComps", "Switches Affect Nested Comps", B, true), s("general.defaultSpatialLinear", "Default Spatial Interpolation to Linear", B, true), diff --git a/crates/engine/src/settings/prefs/sections.rs b/crates/engine/src/settings/prefs/sections.rs index 4cba41851..ac2fab1cc 100644 --- a/crates/engine/src/settings/prefs/sections.rs +++ b/crates/engine/src/settings/prefs/sections.rs @@ -31,6 +31,9 @@ page!(General { /// Path Point and Handle Size (px). path_point_size: u32 = 5, show_tool_tips: bool = true, + /// Step frames with an unmodified mouse wheel over the Composition viewer. + /// Off: the wheel zooms, which is what current After Effects does. + viewer_wheel_scrubbing: bool = false, /// Create New Layers at Composition Start Time (off: at the current time). create_layers_at_comp_start: bool = true, switches_affect_nested_comps: bool = true, diff --git a/crates/engine/src/tests/session.rs b/crates/engine/src/tests/session.rs index a29de9f1c..19bf93090 100644 --- a/crates/engine/src/tests/session.rs +++ b/crates/engine/src/tests/session.rs @@ -320,6 +320,53 @@ fn toolbar_fill_and_stroke_paint_selected_shape_layers() { let _ = s.render(cid, s.time(), Default::default()); } +/// The toolbar/query width follows the selected solid or gradient stroke, including animation. +#[test] +fn shape_toolbar_reads_solid_and_gradient_stroke_widths_at_the_current_time() { + for kind in ["solid", "linear", "radial"] { + for (stretch, expected) in [(100.0, 17.0), (200.0, 13.0)] { + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); + let layer = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.timeStretch", json!({"layer": layer, "percent": stretch})).unwrap(); + let configured = s.execute("shape.fillStroke", json!({"fill": false, "strokeType": kind, "strokeWidth": 9})).unwrap(); + assert_eq!(configured["strokeWidth"].as_f64(), Some(9.0), "configured {kind} stroke"); + let group = if kind == "solid" { "stroke" } else { "gstroke" }; + let path = format!("contents/group/contents/{group}/width"); + for (time, width) in [(0, 9), (1, 17)] { + s.execute("prop.addKey", json!({"layer": layer, "path": path, "time": time, "timeBase": "layer", "value": width})).unwrap(); + } + s.set_time(effectcraft_time::Tick::from_seconds_f64(1.0)); + let displayed = s.execute("shape.fillStroke", json!({})).unwrap(); + assert_eq!(displayed["strokeWidth"].as_f64(), Some(expected), "animated {kind} stroke at {stretch}% stretch"); + } + } +} + +/// A later solid stroke supplies color without overwriting the first gradient stroke's width. +#[test] +fn shape_toolbar_keeps_the_first_gradient_width_with_a_later_solid_stroke() { + for kind in ["linear", "radial"] { + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 400, "height": 300, "duration": 2})).unwrap(); + let layer = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("shape.fillStroke", json!({"strokeType": kind, "strokeWidth": 9})).unwrap(); + s.execute("layer.addShapeItem", json!({"layer": layer, "group": "contents/group", "kind": "stroke"})).unwrap(); + for (field, value) in [("color", json!([0.0, 1.0, 0.0, 1.0])), ("width", json!(99))] { + s.execute("prop.set", json!({"layer": layer, "path": format!("contents/group/contents/stroke/{field}"), "value": value})).unwrap(); + } + for (time, width) in [(0, 9), (1, 17)] { + s.execute("prop.addKey", json!({"layer": layer, "path": "contents/group/contents/gstroke/width", "time": time, "value": width})).unwrap(); + } + for (time, width) in [(0.0, 9.0), (1.0, 17.0)] { + s.set_time(effectcraft_time::Tick::from_seconds_f64(time)); + let displayed = s.execute("shape.fillStroke", json!({})).unwrap(); + assert_eq!(displayed["strokeWidth"].as_f64(), Some(width)); + assert_eq!(displayed["stroke"], json!([0.0, 1.0, 0.0, 1.0])); + } + } +} + /// A Wiggle Paths saved without Spatial Phase gets the property, at 0°, after Temporal Phase when /// the project opens. #[test] diff --git a/crates/render/src/cache.rs b/crates/render/src/cache.rs index b6f2e4076..cf1a84e82 100644 --- a/crates/render/src/cache.rs +++ b/crates/render/src/cache.rs @@ -527,7 +527,7 @@ pub fn input_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bo if layer.switches.adjustment || history_unkeyable(layer, Some(effects)) { return None; } - let base = key_any(ctx, layer, scale, draft, blur, true, Some(effects))?; + let base = key_any(ctx, layer, scale, draft, blur, true, EffectScope::First(effects))?; let mut h = KeyHasher(base ^ 0x5bd1_e995_7a3c_11d3); effects.hash(&mut h); Some(h.finish()) @@ -588,25 +588,39 @@ fn has_expression(g: &PropGroup) -> bool { }) } +/// How far the effect stack reaches into a cache key. [`EffectScope::Footprint`] keys an +/// adjustment layer's footprint — its source through its masks, without any effects — so the +/// stack must not reach the key at all (#687). +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) enum EffectScope { + /// The whole stack (the processed layer). + All, + /// Only the first `n` effects (a layer's input, see [`input_key`]). + First(usize), + /// None of it (an adjustment layer's footprint). + Footprint, +} + /// Cache key for an adjustment layer's footprint (its source through its masks, see /// `Renderer::adjustment_footprint`): the GPU compositor then uploads it once, not every frame. pub fn footprint_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool) -> Option { // The footprint is the source through the masks, without the effects: a time-dependent // effect (Noise on a finishing adjustment layer) must not re-key it every frame (on the GPU, - // a full-frame re-upload per frame). - Some(derive(key_any(ctx, layer, scale, draft, blur, false, Some(0))?, 0xf007_9417)) + // a full-frame re-upload per frame). Shutter, paint, the comp scene and effect dependencies + // stay out of this key too; a mask still does. + Some(derive(key_any(ctx, layer, scale, draft, blur, false, EffectScope::Footprint)?, 0xf007_9417)) } fn key_with(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool) -> Option { if layer.switches.adjustment { return None; } - key_any(ctx, layer, scale, draft, blur, footage, None) + key_any(ctx, layer, scale, draft, blur, footage, EffectScope::All) } -/// `blur`: the layer is motion blurred in this render (`Renderer::mb_on`). `effects`: key only -/// the first that many effects (a layer's input, see [`input_key`]). -fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool, effects: Option) -> Option { +/// `blur`: the layer is motion blurred in this render (`Renderer::mb_on`). `effects`: how far +/// the effect stack reaches into the key (see [`EffectScope`]). +fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, footage: bool, effects: EffectScope) -> Option { let mut h = KeyHasher(0xcbf2_9ce4_8422_2325); ctx.comp.frame_rate.as_f64().to_bits().hash(&mut h); ctx.comp.enable_frame_blending.hash(&mut h); @@ -651,8 +665,10 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo // (Audio, Lock and Shy don't change pixels.) hash_debug(&mut h, &layer.switches.pixels()); // Mask motion blur and effects that read the shutter depend on whether the layer is motion - // blurred in this render and on the comp's shutter. - if layer.masks().is_some_and(|m| !m.children.is_empty()) || layer.effects().is_some_and(|fx| !fx.children.is_empty()) { + // blurred in this render and on the comp's shutter. A footprint has no effects, so only its + // masks put the shutter in the key. + if layer.masks().is_some_and(|m| !m.children.is_empty()) || (effects != EffectScope::Footprint && layer.effects().is_some_and(|fx| !fx.children.is_empty())) + { blur.hash(&mut h); ctx.comp.enable_motion_blur.hash(&mut h); ctx.comp.shutter_angle.to_bits().hash(&mut h); @@ -666,23 +682,29 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo { continue; } + // A footprint is its source through its masks: the effect stack is not in it at all. // A layer's input runs only its first effects: the later ones (a Puppet whose pins are // dragged, #212) don't change it, so it stays cached. - if let (Some(n), Node::Group(g)) = (effects, c) + if let Node::Group(g) = c && g.match_id == "effects" { - let mut first = g.clone(); - let mut seen = 0; - first.children.retain(|e| match e { - Node::Prop(_) => true, - Node::Group(_) => { - seen += 1; - seen <= n - } - }); - hash_debug(&mut h, &first); - hash_values(&mut h, ctx, layer, &first); - continue; + if effects == EffectScope::Footprint { + continue; + } + if let EffectScope::First(n) = effects { + let mut first = g.clone(); + let mut seen = 0; + first.children.retain(|e| match e { + Node::Prop(_) => true, + Node::Group(_) => { + seen += 1; + seen <= n + } + }); + hash_debug(&mut h, &first); + hash_values(&mut h, ctx, layer, &first); + continue; + } } // Static structure: values, keyframes, expressions, enabled flags, effect ids, modes. hash_debug(&mut h, c); @@ -691,7 +713,9 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo } } // Paint strokes appear and disappear with their Duration spans (not property values). - if layer.switches.effects + // A footprint does not include them. + if effects != EffectScope::Footprint + && layer.switches.effects && let Some(fx) = layer.effects() { let lt = layer.layer_time(ctx.time).seconds(); @@ -704,14 +728,14 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo if mask_motion_blur(ctx, layer, blur) { layer.layer_time(ctx.time).0.hash(&mut h); } - if time_dependent(layer, effects != Some(0)) { + if time_dependent(layer, effects != EffectScope::Footprint) { layer.layer_time(ctx.time).0.hash(&mut h); // Effects also see the layer's time mapping. layer.start_time.0.hash(&mut h); layer.stretch.to_bits().hash(&mut h); } // Effects that see the comp camera or lights (#397) change when they or the layer move. - if reads_comp_scene(layer) { + if effects != EffectScope::Footprint && reads_comp_scene(layer) { hash_debug(&mut h, &crate::comp_scene(ctx, layer)); } Some(h.finish()) diff --git a/crates/render/src/cache/dependencies.rs b/crates/render/src/cache/dependencies.rs index 1a550f9c7..2019d858a 100644 --- a/crates/render/src/cache/dependencies.rs +++ b/crates/render/src/cache/dependencies.rs @@ -6,9 +6,15 @@ use crate::EvalCtx; use effectcraft_project::{ItemId, ItemKind, Layer, LayerId, LayerSource, LightKind, Node, PropGroup}; use std::hash::Hash; -pub(super) fn hash(h: &mut KeyHasher, ctx: &EvalCtx, layer: &Layer, effects: Option) -> Option<()> { +pub(super) fn hash(h: &mut KeyHasher, ctx: &EvalCtx, layer: &Layer, effects: super::EffectScope) -> Option<()> { + // A footprint runs no effects: nothing they read is in its pixels or its key. + let limit = match effects { + super::EffectScope::Footprint => return Some(()), + super::EffectScope::All => None, + super::EffectScope::First(n) => Some(n), + }; let mut deps = Dependencies { h, path: vec![(ctx.comp_id, layer.id)], remaining: 1024, other_times: super::reads_other_times(layer, None) }; - deps.effects(ctx, layer, effects) + deps.effects(ctx, layer, limit) } struct Dependencies<'a> { diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 3d2134e89..38beb2130 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -929,6 +929,54 @@ fn default_effect_points_sit_at_the_centre_of_a_shape_layer() { } } +/// #687: an adjustment layer's footprint (its source through its masks) must not be keyed by +/// its effect stack. Noise on a finishing adjustment changes every frame, and letting that +/// reach the key re-rendered and re-uploaded a full-frame buffer every frame. A moving mask +/// must still re-key it, and the cached buffer must follow the mask. +#[test] +fn adjustment_footprint_key_ignores_the_effect_stack() { + let (mut p, cid, comp) = setup(); + let mut adj = solid(&mut p, &comp, [1.0, 1.0, 1.0], 200, 100); + adj.switches.adjustment = true; + add_effect_200(&mut p, &mut adj, "ec.noise.noise", &[("amount", Value::Scalar(50.0))]); + let key = |p: &Project, t: f64| { + let c = p.comp(cid).unwrap(); + crate::cache::footprint_key(&crate::EvalCtx::new(p, cid, c, Tick::from_seconds_f64(t)), &c.layers[0], 1.0, false, false).unwrap() + }; + let mut next = p.next_id; + let mut m = build::mask(&mut Ids(&mut next), "Mask 1", ShapePath::rect([50.0, 50.0], 40.0, 40.0), MaskMode::Add, [255, 255, 0]); + p.next_id = next; + m.prop_mut("path").unwrap().keys = vec![ + Keyframe::new(Tick::ZERO, Value::Path(ShapePath::rect([50.0, 50.0], 40.0, 40.0))), + Keyframe::new(Tick::from_seconds_f64(1.0), Value::Path(ShapePath::rect([150.0, 50.0], 40.0, 40.0))), + ]; + // Effects alone must not put the shutter in the footprint key. Masks, added below, may. + p.comp_mut(cid).unwrap().layers.push(adj.clone()); + let bare = key(&p, 0.0); + p.comp_mut(cid).unwrap().shutter_angle = 90.0; + assert_eq!(bare, key(&p, 0.0), "the comp shutter must not re-key a footprint"); + p.comp_mut(cid).unwrap().shutter_angle = 180.0; + p.comp_mut(cid).unwrap().layers[0].props.sub_mut("masks").unwrap().children.push(m.into()); + let foot = |p: &Project, t: f64| { + let c = p.comp(cid).unwrap(); + let mut r = crate::Renderer::new(p, &crate::NoFootage, crate::RenderOpts::default()); + let cache = crate::LayerCache::default(); + r.cache = Some(&cache); + let layer = c.layers[0].clone(); + let b = r.adjustment_footprint(&crate::EvalCtx::new(p, cid, c, Tick::from_seconds_f64(t)), &layer).unwrap(); + b.img.get(50, 50) + }; + assert_ne!(key(&p, 0.0), key(&p, 0.5), "an animated mask path must re-key the footprint"); + assert_ne!(key(&p, 0.0), key(&p, 1.0)); + assert_ne!(foot(&p, 0.0), foot(&p, 1.0), "the moved mask must reach the cached footprint"); + assert_eq!(key(&p, 1.0), key(&p, 1.5), "a per-frame effect on the adjustment re-keyed its footprint"); + assert_eq!(key(&p, 1.0), key(&p, 3.5), "the footprint key followed the clock past the mask"); + assert_eq!(foot(&p, 1.0), foot(&p, 3.5)); + let before = key(&p, 0.0); + p.comp_mut(cid).unwrap().layers[0].props.prop_mut("effects/#1/amount").unwrap().value = Value::Scalar(90.0); + assert_eq!(before, key(&p, 0.0), "editing an effect parameter must not re-key the footprint"); +} + /// #659: a tiling effect on a text layer tiles across the layer's whole region. The glyphs /// were cut off at their own bounding box: the effect computed over the comp-sized layer rect /// but wrote into the painted (glyph-bounds) buffer alone. Tiling shrinks each copy, so the diff --git a/crates/ui-egui/src/a11y.rs b/crates/ui-egui/src/a11y.rs index 43a0a3fe4..e832d3b97 100644 --- a/crates/ui-egui/src/a11y.rs +++ b/crates/ui-egui/src/a11y.rs @@ -8,7 +8,7 @@ //! numeric value of a `Slider`). //! //! Names are the same translated strings the panel draws (`i18n::tr`), never hard-coded English. -//! `tests/ui_accessibility_audit.rs` walks the tree and fails on an unnamed control in the panels +//! `tests/it/ui_accessibility_audit.rs` walks the tree and fails on an unnamed control in the panels //! that have had their pass. use egui::accesskit::{HasPopup, Role, Toggled}; diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index c4a6f4d3a..4cc544c7c 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -135,8 +135,9 @@ pub(crate) fn project_drop_items(app: &EffectcraftApp, dragged: u64, target: Opt selected.iter().map(|id| id.0).collect() } else { // One unselected item goes through as is, so a folder or data file still gets the - // engine's error instead of a silent no-op. - return vec![dragged]; + // engine's error instead of a silent no-op. The composition the drop lands in is the + // one case that never reaches it, selected or not (#2589). + return if Some(ItemId(dragged)) == target { Vec::new() } else { vec![dragged] }; }; candidates .into_iter() @@ -189,6 +190,13 @@ pub(crate) struct ViewerScroll { /// Factor to zoom by around the pointer: a pinch, Ctrl/Cmd+scroll, a mouse wheel, or /// Alt + a trackpad scroll. pub zoom: f32, + /// The gesture came from a trackpad (or is still the smoothed tail of one). + pub trackpad: bool, + /// Unmodified mouse-wheel lines this frame. Zero for a trackpad, a point-unit wheel, or + /// Ctrl/Cmd/Alt+wheel. + pub wheel_lines: f32, + /// This frame has a Ctrl/Cmd/Alt mouse-wheel event, which keeps zooming. + pub wheel_modified: bool, } /// Splits the frame's scroll input into a viewer pan and a zoom: a mouse wheel zooms and a @@ -223,13 +231,32 @@ pub(crate) fn viewer_scroll(ctx: &egui::Context) -> ViewerScroll { let (scroll, pinch, alt) = ctx.input(|i| (i.smooth_scroll_delta, i.zoom_delta(), i.modifiers.alt)); let pinch = if pinch.is_finite() && pinch > 0.0 { pinch } else { 1.0 }; let scroll = if scroll.x.is_finite() && scroll.y.is_finite() { scroll } else { egui::Vec2::ZERO }; - if trackpad && !alt { - ViewerScroll { pan: scroll, zoom: pinch } + let (wheel_lines, wheel_modified) = ctx.input(|i| { + let mut lines = 0.0; + let mut modified = false; + if !trackpad { + for event in &i.events { + if let egui::Event::MouseWheel { unit: egui::MouseWheelUnit::Line, delta, modifiers, source, .. } = event + && !matches!(source, egui::MouseWheelSource::Trackpad | egui::MouseWheelSource::Momentum) + { + if modifiers.command || modifiers.ctrl || modifiers.alt { + modified = true; + } else { + lines += delta.y; + } + } + } + } + (lines, modified) + }); + let (pan, zoom) = if trackpad && !alt { + (scroll, pinch) } else if alt { - ViewerScroll { pan: egui::Vec2::ZERO, zoom: pinch * (scroll.y / 300.0).exp() } + (egui::Vec2::ZERO, pinch * (scroll.y / 300.0).exp()) } else { - ViewerScroll { pan: egui::vec2(scroll.x, 0.0), zoom: pinch * (scroll.y / 300.0).exp() } - } + (egui::vec2(scroll.x, 0.0), pinch * (scroll.y / 300.0).exp()) + }; + ViewerScroll { pan, zoom, trackpad, wheel_lines, wheel_modified } } pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: PanelKind, rect: Rect) { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 5c4272066..e3660bac1 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -870,6 +870,24 @@ fn phase_menu(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: &m resp } +/// Frames to step when wheel scrubbing is on, or `None` when this gesture should zoom or pan. +/// A scrub notch's smoothed tail stays a scrub after the wheel event itself is gone, so that +/// tail does not zoom or pan (#1218). +fn wheel_scrub_frames(ctx: &egui::Context, pref: bool, scroll: &super::ViewerScroll) -> Option { + let tail_id = egui::Id::unique("viewer-wheel-scrub-tail"); + let was_tail: bool = ctx.data(|d| d.get_temp(tail_id)).unwrap_or(false); + let moving = scroll.pan != egui::Vec2::ZERO || (scroll.zoom - 1.0).abs() > 3e-4; + let scrub = pref && !scroll.trackpad && !scroll.wheel_modified && (scroll.wheel_lines.abs() >= 0.01 || (was_tail && moving)); + ctx.data_mut(|d| { + if scrub { + d.insert_temp(tail_id, true); + } else { + d.remove::(tail_id); + } + }); + scrub.then_some((scroll.wheel_lines.round() as i64).clamp(-30, 30)) +} + fn phase_hover(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, resp: &egui::Response, v: &mut Vw<'_>) { let area = v.area; let zoom = v.zoom; @@ -951,12 +969,21 @@ fn phase_hover(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, resp }; ctx.set_cursor_icon(cursor); // The mouse wheel and a trackpad pinch zoom around the pointer; a two-finger trackpad - // scroll pans. + // scroll pans. With general.viewerWheelScrubbing, an unmodified wheel steps frames + // instead, including the smoothed tail of that notch so it does not zoom or pan (#1218). let scroll = super::viewer_scroll(&ctx); - if scroll.pan != egui::Vec2::ZERO && area.contains(hp) { + let scrub = wheel_scrub_frames(&ctx, app.session.prefs.general.viewer_wheel_scrubbing, &scroll); + if let Some(frames) = scrub { + if frames != 0 && area.contains(hp) { + if app.playback.playing { + let _ = crate::menus::invoke(app, &ctx, "playback.stop", json!({})); + } + let _ = app.session.execute("time.step", json!({"frames": frames})); + } + } else if scroll.pan != egui::Vec2::ZERO && area.contains(hp) { app.ui.viewer.pan = [app.ui.viewer.pan[0] + scroll.pan.x, app.ui.viewer.pan[1] + scroll.pan.y]; } - if (scroll.zoom - 1.0).abs() > 3e-4 && area.contains(hp) { + if scrub.is_none() && (scroll.zoom - 1.0).abs() > 3e-4 && area.contains(hp) { let nz = (zoom * scroll.zoom).clamp(0.01, 32.0); let before = map.to_comp(hp); let new_origin = hp - vec2(before[0] as f32 * nz, before[1] as f32 * nz); diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 180b56232..4fc34d9ff 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -5,6 +5,8 @@ mod native_menu_text_keys; mod render_queue_snapshot; mod shared_docking_ui; mod ui_3d; +mod ui_accessibility; +mod ui_accessibility_audit; mod ui_agent_audit; mod ui_angles; mod ui_audio_scrub; @@ -18,6 +20,7 @@ mod ui_character_stroke_order; mod ui_checkbox_drag; mod ui_color_hex; mod ui_comp_presets; +mod ui_contrast; mod ui_control_modifiers; mod ui_dialog_preview; mod ui_dock_resize; @@ -25,6 +28,7 @@ mod ui_drag_drop; mod ui_effect_controls; mod ui_effect_drop_highlight; mod ui_effect_names; +mod ui_effects_presets_keyboard; mod ui_env_light_gpu; mod ui_expression_values; mod ui_font_search; @@ -66,11 +70,13 @@ mod ui_project_rows; mod ui_quick_offset; mod ui_ram_preview; mod ui_render_queue; +mod ui_render_queue_keyboard; mod ui_script_console; mod ui_scriptui; mod ui_scrolling; mod ui_self_drop; mod ui_settings_info; +mod ui_settings_keyboard; mod ui_tabs; mod ui_tail; mod ui_text_edit; diff --git a/crates/ui-egui/tests/ui_accessibility.rs b/crates/ui-egui/tests/it/ui_accessibility.rs similarity index 100% rename from crates/ui-egui/tests/ui_accessibility.rs rename to crates/ui-egui/tests/it/ui_accessibility.rs diff --git a/crates/ui-egui/tests/ui_accessibility_audit.rs b/crates/ui-egui/tests/it/ui_accessibility_audit.rs similarity index 100% rename from crates/ui-egui/tests/ui_accessibility_audit.rs rename to crates/ui-egui/tests/it/ui_accessibility_audit.rs diff --git a/crates/ui-egui/tests/ui_contrast.rs b/crates/ui-egui/tests/it/ui_contrast.rs similarity index 100% rename from crates/ui-egui/tests/ui_contrast.rs rename to crates/ui-egui/tests/it/ui_contrast.rs diff --git a/crates/ui-egui/tests/ui_effects_presets_keyboard.rs b/crates/ui-egui/tests/it/ui_effects_presets_keyboard.rs similarity index 100% rename from crates/ui-egui/tests/ui_effects_presets_keyboard.rs rename to crates/ui-egui/tests/it/ui_effects_presets_keyboard.rs diff --git a/crates/ui-egui/tests/ui_render_queue_keyboard.rs b/crates/ui-egui/tests/it/ui_render_queue_keyboard.rs similarity index 100% rename from crates/ui-egui/tests/ui_render_queue_keyboard.rs rename to crates/ui-egui/tests/it/ui_render_queue_keyboard.rs diff --git a/crates/ui-egui/tests/it/ui_self_drop.rs b/crates/ui-egui/tests/it/ui_self_drop.rs index a371cc80b..f8a44de83 100644 --- a/crates/ui-egui/tests/it/ui_self_drop.rs +++ b/crates/ui-egui/tests/it/ui_self_drop.rs @@ -60,7 +60,8 @@ fn a_selection_holding_the_drop_target_does_not_fail_the_drop() { assert!(!h.state().ui.status.contains("contain itself"), "the drop did not fail: {}", h.state().ui.status); } -/// Dragging the composition's own row onto its Timeline adds nothing and reports nothing. +/// Dragging the composition's own row onto its Timeline adds nothing and reports nothing, whether +/// the row was selected first or the drag started on an unselected row (#2589). #[test] fn the_composition_dropped_on_itself_adds_nothing() { let (mut h, main, _clip) = harness(); @@ -72,4 +73,16 @@ fn the_composition_dropped_on_itself_adds_nothing() { drag(&mut h, from, row.center()); assert_eq!(stack(&h), before, "nothing is added"); assert!(!h.state().ui.status.contains("contain itself"), "no error: {}", h.state().ui.status); + + // The same row dragged while something else is selected: the early return for a single + // unselected item must not hand the composition to the engine either. + let (mut h, main, clip) = harness(); + h.state_mut().session.execute("project.select", json!({"items": [clip]})).unwrap(); + h.run_steps(2); + let before = stack(&h); + let from = rect(&h, &format!("project.item.{main}.name")).center(); + let row = rect(&h, &format!("timeline.layer.{}.row", layer_id(&h, "Bottom"))); + drag(&mut h, from, row.center()); + assert_eq!(stack(&h), before, "nothing is added from an unselected row either"); + assert!(!h.state().ui.status.contains("contain itself"), "no error: {}", h.state().ui.status); } diff --git a/crates/ui-egui/tests/ui_settings_keyboard.rs b/crates/ui-egui/tests/it/ui_settings_keyboard.rs similarity index 100% rename from crates/ui-egui/tests/ui_settings_keyboard.rs rename to crates/ui-egui/tests/it/ui_settings_keyboard.rs diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index 48bc36a44..0ab31237b 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -54,6 +54,65 @@ fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) { h.run_steps(2); } +/// #1218: frame stepping is optional, and consumes the whole wheel gesture including its +/// smoothed tail. Ctrl/Cmd+wheel still zooms. +#[test] +fn mouse_wheel_over_the_monitor_scrubs_frames() { + let mut h = harness(); + h.state_mut().session.execute("prefs.set", json!({"key": "general.viewerWheelScrubbing", "value": true})).unwrap(); + let p = rect(&h, "viewer.area").center(); + h.input_mut().events.push(Event::PointerMoved(p)); + h.step(); + let t0 = h.state().session.time(); + let line = |dy: f32, modifiers: egui::Modifiers| Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: vec2(0.0, dy), + modifiers, + phase: egui::TouchPhase::Move, + source: egui::MouseWheelSource::Wheel, + }; + let z0 = h.state().ui.viewer.zoom; + h.input_mut().events.push(line(1.0, egui::Modifiers::NONE)); + h.step(); + let t1 = h.state().session.time(); + for _ in 0..20 { + assert_eq!(h.state().ui.viewer.zoom, z0, "frame wheel must never zoom"); + h.step(); + } + assert!(t1 > t0, "wheel up advanced the playhead ({t0:?} -> {t1:?})"); + h.input_mut().events.push(line(-2.0, egui::Modifiers::NONE)); + h.step(); + assert!(h.state().session.time() < t1, "two lines back moves before the single line forward"); + let z0 = h.state().ui.viewer.zoom; + let t2 = h.state().session.time(); + let mods = egui::Modifiers { ctrl: true, command: true, ..Default::default() }; + h.input_mut().events.push(line(3.0, mods)); + h.step(); + assert_eq!(h.state().session.time(), t2, "Ctrl+wheel must not scrub"); + assert_ne!(h.state().ui.viewer.zoom, z0, "Ctrl+wheel still zooms"); +} + +#[test] +fn mouse_wheel_zooms_by_default_without_stepping_frames() { + let mut h = harness(); + assert!(!h.state().session.prefs.general.viewer_wheel_scrubbing); + let p = rect(&h, "viewer.area").center(); + h.input_mut().events.push(Event::PointerMoved(p)); + h.step(); + let t0 = h.state().session.time(); + let z0 = h.state().ui.viewer.zoom; + h.input_mut().events.push(Event::MouseWheel { + unit: egui::MouseWheelUnit::Line, + delta: vec2(0.0, 1.0), + modifiers: egui::Modifiers::NONE, + phase: egui::TouchPhase::Move, + source: egui::MouseWheelSource::Wheel, + }); + h.run_steps(20); + assert_eq!(h.state().session.time(), t0); + assert_ne!(h.state().ui.viewer.zoom, z0); +} + /// Comp pixel → screen point. fn screen(h: &Harness<'_, EffectcraftApp>, p: [f32; 2]) -> Pos2 { let c = rect(h, "viewer.comp"); diff --git a/docs/preferences.md b/docs/preferences.md index 6d692a936..2b80ffa0f 100644 --- a/docs/preferences.md +++ b/docs/preferences.md @@ -35,6 +35,7 @@ Commands (CLI, MCP, control channel): - `general.pathPointSize`: Path Point and Handle Size - `general.recentItems`: Recent Projects Shown - `general.showToolTips`: Show Tool Tips +- `general.viewerWheelScrubbing`: Scrub Composition Frames with Mouse Wheel (off by default). On: an unmodified mouse wheel over the Composition viewer steps frames, and the smoothed tail of that notch does not zoom or pan. Ctrl/Cmd+wheel and a trackpad still zoom and pan. - `general.createLayersAtCompStart`: Create Layers at Composition Start Time - `general.defaultSpatialLinear`: Default Spatial Interpolation to Linear - `startup.showHomeOnLaunch`: Show Home Screen When Launching