From 238fe6cd2a23e2edd15a0daa1f6f11ba0fb248ab Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 04:55:22 +0100 Subject: [PATCH 1/8] #1342: preserve gradient stroke widths in the toolbar --- crates/engine/src/cmd/shape_stroke.rs | 7 +++++-- crates/engine/src/tests/session.rs | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) 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/tests/session.rs b/crates/engine/src/tests/session.rs index a29de9f1c..f2dcca49c 100644 --- a/crates/engine/src/tests/session.rs +++ b/crates/engine/src/tests/session.rs @@ -320,6 +320,26 @@ 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"] { + 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(); + 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, "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(17.0), "animated {kind} stroke"); + } +} + /// A Wiggle Paths saved without Spatial Phase gets the property, at 0°, after Temporal Phase when /// the project opens. #[test] From ceb6cc52778e420d899d058869c9350104067baf Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Fri, 30 Oct 2026 15:39:00 +1100 Subject: [PATCH 2/8] Keep an adjustment footprint's cache key free of its effect stack. Noise, paint, the shutter, and the comp scene were still reaching the footprint key, so a finishing adjustment re-uploaded a full frame every frame. The key now uses EffectScope::Footprint and ignores that stack, while a moving mask still re-keys it. --- crates/render/src/cache.rs | 74 ++++++++++++++++--------- crates/render/src/cache/dependencies.rs | 10 +++- crates/render/src/tests.rs | 48 ++++++++++++++++ 3 files changed, 105 insertions(+), 27 deletions(-) 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 488f38ec7..9ab94ebfb 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -903,6 +903,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 From 85d197e0b9de91087617e72a029ff66bb4bf57ca Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Fri, 30 Oct 2026 16:07:37 +1100 Subject: [PATCH 3/8] Make wheel frame-stepping an opt-in that consumes the smoothed tail. Main routes viewer scrolling through viewer_scroll, so a raw-event scrub fought the wheel zoom. With general.viewerWheelScrubbing on, an unmodified wheel steps frames and the rest of that notch does not zoom or pan. The preference stays off, so the wheel still zooms. --- crates/engine/src/settings/prefs/dialog.rs | 1 + crates/engine/src/settings/prefs/sections.rs | 3 + crates/ui-egui/src/panels.rs | 36 ++++++++++-- crates/ui-egui/src/panels/viewer.rs | 33 ++++++++++- crates/ui-egui/tests/it/ui_viewer.rs | 59 ++++++++++++++++++++ docs/preferences.md | 1 + 6 files changed, 125 insertions(+), 8 deletions(-) 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/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index f6a5b5321..47e36315a 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -184,6 +184,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 @@ -218,13 +225,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 d084dd006..86a3cbe64 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/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index 48bc36a44..e5f3590e6 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -55,6 +55,65 @@ fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) { } /// Comp pixel → screen point. +/// #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); +} + fn screen(h: &Harness<'_, EffectcraftApp>, p: [f32; 2]) -> Pos2 { let c = rect(h, "viewer.comp"); let z = c.width() / 640.0; 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 From f3aad4829adf0d68cf4ce8da910936e7a85a0798 Mon Sep 17 00:00:00 2001 From: zitann2020 Date: Fri, 30 Oct 2026 14:56:28 +0900 Subject: [PATCH 4/8] ui: the composition the drop lands in is left out even when unselected (#2589) Co-Authored-By: DeepSeek V4.1 Flash --- crates/ui-egui/src/panels.rs | 5 +++-- crates/ui-egui/tests/it/ui_self_drop.rs | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index c4a6f4d3a..230ac14fe 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() 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); } From c8a40733cc0cfece895a7156fb7be1a6a1c5d15f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 30 Oct 2026 01:49:46 -0400 Subject: [PATCH 5/8] UPD.1: honor app pretend versions in update checks --- Cargo.lock | 2 +- Cargo.toml | 4 ++-- contributors/people.toml | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ebd53b78..07564b7df 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1121,7 +1121,7 @@ dependencies = [ [[package]] name = "craft-update" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1065653e75854e9c17e1ff563acdd344c1b185f0#1065653e75854e9c17e1ff563acdd344c1b185f0" +source = "git+https://github.com/storytold/craft-libs?rev=04aa1703b32d299e54f78fcde4182f2b393a9116#04aa1703b32d299e54f78fcde4182f2b393a9116" dependencies = [ "chrono", "dunce", diff --git a/Cargo.toml b/Cargo.toml index 772f62ee4..afc59257b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,8 +14,8 @@ repository = "https://github.com/storytold/effectcraft" craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0", version = "=0.1.0", default-features = false, features = ["read"] } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0" } craft-quit = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0" } -# craft-update with storytold/craft-libs#434: the update prompt shows the app's icon, accent and developer. -craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "1065653e75854e9c17e1ff563acdd344c1b185f0", features = ["egui"] } +# craft-update: app-specific pretend versions apply to startup and manual checks. +craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "04aa1703b32d299e54f78fcde4182f2b393a9116", features = ["egui"] } # internal effectcraft-time = { path = "crates/time" } effectcraft-geom = { path = "crates/geom" } diff --git a/contributors/people.toml b/contributors/people.toml index 59365207a..e0915f09c 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -114,3 +114,6 @@ sync_github_name = false [people.ashokram-si] real_name = "Ashok Ram" + +[people.dylanmtaylor] +sync_github_name = true From 8fde08d269be65b40a3d7986409206b95adc8fe9 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Fri, 30 Oct 2026 13:34:05 +0700 Subject: [PATCH 6/8] Keep the screen() helper's doc comment on screen() in ui_viewer tests Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/it/ui_viewer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/it/ui_viewer.rs b/crates/ui-egui/tests/it/ui_viewer.rs index e5f3590e6..0ab31237b 100644 --- a/crates/ui-egui/tests/it/ui_viewer.rs +++ b/crates/ui-egui/tests/it/ui_viewer.rs @@ -54,7 +54,6 @@ fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) { h.run_steps(2); } -/// Comp pixel → screen point. /// #1218: frame stepping is optional, and consumes the whole wheel gesture including its /// smoothed tail. Ctrl/Cmd+wheel still zooms. #[test] @@ -114,6 +113,7 @@ fn mouse_wheel_zooms_by_default_without_stepping_frames() { 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"); let z = c.width() / 640.0; From 697556f252d542abfaba447f71c1456152ffbfa8 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Fri, 30 Oct 2026 14:04:01 +0700 Subject: [PATCH 7/8] ui-egui: the accessibility and keyboard tests join the one test binary #1339 was written before ui-egui's integration tests became modules of one binary (`tests/it/main.rs`), so its six test files landed at the top of `tests/`, each its own crate and link. They move into `tests/it/` as modules; the a11y module's doc names the new path. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/a11y.rs | 2 +- crates/ui-egui/tests/it/main.rs | 6 ++++++ crates/ui-egui/tests/{ => it}/ui_accessibility.rs | 0 crates/ui-egui/tests/{ => it}/ui_accessibility_audit.rs | 0 crates/ui-egui/tests/{ => it}/ui_contrast.rs | 0 .../ui-egui/tests/{ => it}/ui_effects_presets_keyboard.rs | 0 crates/ui-egui/tests/{ => it}/ui_render_queue_keyboard.rs | 0 crates/ui-egui/tests/{ => it}/ui_settings_keyboard.rs | 0 8 files changed, 7 insertions(+), 1 deletion(-) rename crates/ui-egui/tests/{ => it}/ui_accessibility.rs (100%) rename crates/ui-egui/tests/{ => it}/ui_accessibility_audit.rs (100%) rename crates/ui-egui/tests/{ => it}/ui_contrast.rs (100%) rename crates/ui-egui/tests/{ => it}/ui_effects_presets_keyboard.rs (100%) rename crates/ui-egui/tests/{ => it}/ui_render_queue_keyboard.rs (100%) rename crates/ui-egui/tests/{ => it}/ui_settings_keyboard.rs (100%) 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/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/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 From f0b5e4eeff4eed5a9238d8875c12373267762b4f Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 08:23:02 +0100 Subject: [PATCH 8/8] #2668: cover mixed strokes and stretched layer timing --- crates/engine/src/tests/session.rs | 43 ++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/tests/session.rs b/crates/engine/src/tests/session.rs index f2dcca49c..19bf93090 100644 --- a/crates/engine/src/tests/session.rs +++ b/crates/engine/src/tests/session.rs @@ -324,19 +324,46 @@ fn toolbar_fill_and_stroke_paint_selected_shape_layers() { #[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(); - 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"); + 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": path, "time": time, "value": width})).unwrap(); + 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])); } - 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(17.0), "animated {kind} stroke"); } }