Merge pull request #2270 from mypenc/altfix

Viewer: Alt/Option-drop of a Project item replaces the layer under the pointer

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

@@ -1557,7 +1557,7 @@ fn replaceable_item(it: &effectcraft_engine::project::Item) -> bool {
 
 /// The Alt/Option-drag target: the layer under the pointer can take this item as its source, and
 /// the item wouldn't put the composition inside itself.
-fn replace_source_target(app: &EffectcraftApp, cid: ItemId, item: ItemId, lid: LayerId, comp: &Comp) -> bool {
+pub(crate) fn replace_source_target(app: &EffectcraftApp, cid: ItemId, item: ItemId, lid: LayerId, comp: &Comp) -> bool {
     app.session.project.item(item).is_some_and(replaceable_item)
         && !app.session.project.comp_contains(item, cid)
         && comp

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

@@ -1821,7 +1821,7 @@ use geometry::{
 pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad};
 use gesture::{Gesture, HANDLE, ROTO_HINT, pen_id, roi_drag, roi_handles, vertex_grab_radius};
 use shapes::{create_shape, draw_shape_ghost, shape_drag};
-pub(crate) use shapes::{layer_at, layers_at, pick, selectable_layers};
+pub(crate) use shapes::{layer_at, layer_at_including_locked, layers_at, pick, selectable_layers};
 #[cfg(test)]
 use tools::constrain_45;
 use tools::{add_motion_key, axis_at, axis_drag, drag_axes, draw_rigs, parent_world_inverse, pen_cursor, pen_cursor_icon, pen_press, select_motion_key};

crates/ui-egui/src/panels/viewer/shapes.rs

@@ -4,14 +4,17 @@ use super::{
     pos2,
 };
 
-/// Visible, unlocked layers eligible for viewer hit tests, in front-to-back order.
+/// Visible layers eligible for viewer hit tests, locked or not, in front-to-back order.
 /// Solo scope matches the renderer: any soloed AV layer activates it at every time (as in After
 /// Effects), even when hidden, locked or outside its in-out range.
-pub(crate) fn selectable_layers(comp: &Comp, time: Tick) -> impl Iterator<Item = &Layer> {
+fn hit_testable_layers(comp: &Comp, time: Tick) -> impl Iterator<Item = &Layer> {
     let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av());
-    comp.layers
-        .iter()
-        .filter(move |l| l.is_active_at(time) && l.switches.video && !l.switches.locked && !l.is_camera() && !l.is_light() && (!any_solo || l.switches.solo))
+    comp.layers.iter().filter(move |l| l.is_active_at(time) && l.switches.video && !l.is_camera() && !l.is_light() && (!any_solo || l.switches.solo))
+}
+
+/// Visible, unlocked layers eligible for viewer hit tests, in front-to-back order.
+pub(crate) fn selectable_layers(comp: &Comp, time: Tick) -> impl Iterator<Item = &Layer> {
+    hit_testable_layers(comp, time).filter(|l| !l.switches.locked)
 }
 
 /// Selectable layers under a comp point, front to back.
@@ -24,6 +27,18 @@ pub(crate) fn layer_at<'a>(ectx: &EvalCtx<'a>, cpt: [f64; 2]) -> Option<&'a Laye
     layers_at(ectx, cpt).next()
 }
 
+/// Topmost visible layer under a comp point, locked or not. An Alt-drop uses it so a locked layer in
+/// front is refused rather than skipped, which would replace the layer behind it.
+pub(crate) fn layer_at_including_locked<'a>(ectx: &EvalCtx<'a>, cpt: [f64; 2]) -> Option<&'a Layer> {
+    // A layer scaled to nothing has a degenerate quad that `point_in_quad` accepts everywhere.
+    hit_testable_layers(ectx.comp, ectx.time).find(|l| layer_quad(ectx, l).is_some_and(|(_, q, _)| quad_area(&q) > 1e-9 && point_in_quad(cpt, &q)))
+}
+
+/// Area of a quad (shoelace formula); zero when its corners coincide or are collinear.
+fn quad_area(q: &[[f64; 2]; 4]) -> f64 {
+    (0..4).map(|i| q[i][0] * q[(i + 1) % 4][1] - q[(i + 1) % 4][0] * q[i][1]).sum::<f64>().abs() / 2.0
+}
+
 /// The layer a press at a comp point acts on, selected (Shift toggles the topmost layer). As in
 /// After Effects, a selected layer under the point comes before the layers in front of it, so it
 /// can be dragged where they cover it; otherwise the topmost layer (#230).

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

@@ -1,7 +1,8 @@
 //! Dropping onto the Composition viewer: footage, comps and solids from the
 //! Project panel and files from the Media Browser become layers centred where they are dropped
 //! (#85), and an effect or animation preset from Effects & Presets goes on the layer under the
-//! pointer, outlined while it is dragged (#88). Agents do the same with `layer.addItem {position}`,
+//! pointer, outlined while it is dragged (#88). Alt/Option-dropping a Project item replaces the source of
+//! the layer under the pointer instead, as in the Timeline (#2173). Agents do the same with `layer.addItem {position}`,
 //! `mediaBrowser.import {position}`, `effect.apply {layers}` and the preset commands.
 
 use effectcraft_engine::project::{ItemId, ItemKind};
@@ -10,7 +11,7 @@ use egui::{Rect, Stroke, StrokeKind, vec2};
 use serde_json::{Value, json};
 
 use super::DragPayload;
-use super::viewer::{ViewerMap, layer_at, layer_quad};
+use super::viewer::{ViewerMap, layer_at, layer_at_including_locked, layer_quad};
 use crate::EffectcraftApp;
 
 /// While something is dragged over the viewer, show where it goes; drop it on release.
@@ -23,6 +24,9 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &egui::Ui, painter: &egui::Pain
     let Some(ptr) = ctx.input(|i| i.pointer.hover_pos()).filter(|_| ui.rect_contains_pointer(map.area)) else { return };
     let stroke = Stroke::new(2.0, app.tokens.accent);
     let at = map.to_comp(ptr);
+    // Alt/Option-drop of a Project item replaces the source of the layer under the pointer (After
+    // Effects), as in the Timeline; it never adds a layer.
+    let alt_item = ctx.input(|i| i.modifiers.alt) && matches!(payload.as_ref(), DragPayload::Item(_));
     let actions: Vec<(&str, Value)> = match payload.as_ref() {
         DragPayload::Effect(effect) => layer_at(ectx, at)
             .map(|l| {
@@ -42,6 +46,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &egui::Ui, painter: &egui::Pain
             })
             .into_iter()
             .collect(),
+        DragPayload::Item(item) if alt_item => alt_replace(app, ectx, painter, map, stroke, at, *item).into_iter().collect(),
         DragPayload::Item(item) => {
             // The layer's frame, centred on the pointer.
             if let Some([w, h]) = app.session.project.item(ItemId(*item)).and_then(|it| frame_size(&it.kind, ectx.comp.pixel_aspect)) {
@@ -55,7 +60,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &egui::Ui, painter: &egui::Pain
         DragPayload::Property { .. } => vec![],
     };
     // Layer drops outline the target layer; the others outline the whole viewer.
-    let layer_drop = matches!(payload.as_ref(), DragPayload::Effect(_) | DragPayload::Preset { .. });
+    let layer_drop = alt_item || matches!(payload.as_ref(), DragPayload::Effect(_) | DragPayload::Preset { .. });
     if !layer_drop && !actions.is_empty() {
         painter.rect_stroke(map.area, 0.0, stroke, StrokeKind::Inside);
     }
@@ -80,3 +85,21 @@ fn frame_size(kind: &ItemKind, comp_pixel_aspect: f64) -> Option<[f32; 2]> {
     let stretch = if comp_pixel_aspect > 0.0 && pixel_aspect > 0.0 { pixel_aspect / comp_pixel_aspect } else { 1.0 };
     (w > 0 && h > 0).then_some([(w as f64 * stretch) as f32, h as f32])
 }
+
+/// The `layer.replaceSource` for an Alt/Option-drop of Project item `item` at comp point `at`: the
+/// topmost layer there, outlined, if it can take the item as its source (a locked one cannot).
+fn alt_replace(
+    app: &EffectcraftApp,
+    ectx: &EvalCtx,
+    painter: &egui::Painter,
+    map: &ViewerMap,
+    stroke: Stroke,
+    at: [f64; 2],
+    item: u64,
+) -> Option<(&'static str, Value)> {
+    let layer = layer_at_including_locked(ectx, at).filter(|l| super::timeline::replace_source_target(app, ectx.comp_id, ItemId(item), l.id, ectx.comp))?;
+    if let Some((_, q, _)) = layer_quad(ectx, layer) {
+        painter.add(egui::Shape::closed_line(q.iter().map(|c| map.to_screen(*c)).collect(), stroke));
+    }
+    Some(("layer.replaceSource", json!({"layers": [layer.id.0], "item": item, "fixExpressions": false})))
+}

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

@@ -124,6 +124,44 @@ fn alt_dragging_a_project_item_replaces_the_layer_source() {
     assert_eq!(layer.name, "Top", "the layer keeps its name");
 }
 
+/// Alt/Option-dropping a Project item on the Composition viewer replaces the source of the layer
+/// under the pointer, as in the Timeline, instead of adding a layer (#2173).
+#[test]
+fn alt_dropping_a_project_item_on_the_viewer_replaces_the_layer_source() {
+    use effectcraft_engine::project::{ItemId, LayerId, LayerSource};
+    let (mut h, clip) = harness();
+    let item = rect(&h, &format!("project.item.{clip}.name")).center();
+    let to = comp_to_screen(&h.ctx, [160.0, 90.0]).unwrap();
+    // The three solids overlap completely, so the topmost one is under the pointer.
+    let (top, middle, bottom) = (layer_id(&h, "Top"), layer_id(&h, "Middle"), layer_id(&h, "Bottom"));
+    let source = |h: &Harness<'_, EffectcraftApp>, id: u64| h.state().session.active_comp().unwrap().layer(LayerId(id)).unwrap().source.clone();
+    let (middle_before, bottom_before) = (source(&h, middle), source(&h, bottom));
+    let before = stack(&h);
+    drag(&mut h, item, to, Modifiers::ALT);
+    assert_eq!(stack(&h), before, "no layer is added");
+    assert_eq!(source(&h, top), LayerSource::Comp { item: ItemId(clip) }, "the layer under the pointer takes the new source");
+    assert_eq!(source(&h, middle), middle_before, "the layer below it keeps its source");
+    assert_eq!(source(&h, bottom), bottom_before, "the bottom layer keeps its source");
+}
+
+/// A locked layer under the pointer refuses the Alt-drop; the layer behind it is not replaced
+/// instead (#2173).
+#[test]
+fn alt_dropping_on_a_locked_layer_in_the_viewer_replaces_nothing() {
+    use effectcraft_engine::project::LayerId;
+    let (mut h, clip) = harness();
+    let item = rect(&h, &format!("project.item.{clip}.name")).center();
+    let to = comp_to_screen(&h.ctx, [160.0, 90.0]).unwrap();
+    let top = layer_id(&h, "Top");
+    h.state_mut().session.execute("layer.setSwitch", json!({"layers": [top], "switch": "locked", "value": true})).unwrap();
+    h.run_steps(3);
+    let sources = |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().layers.iter().map(|l| l.source.clone()).collect::<Vec<_>>();
+    let before = sources(&h);
+    drag(&mut h, item, to, Modifiers::ALT);
+    assert_eq!(sources(&h), before, "no layer takes the new source");
+    assert!(h.state().session.active_comp().unwrap().layer(LayerId(top)).unwrap().switches.locked);
+}
+
 /// Ctrl+Alt+/ (Cmd+Opt+/ on macOS) replaces every selected layer's source with the Project panel
 /// selection (#991).
 #[test]