Motion tweens: implement Group + Convert to Movie Clip for DCEL geometry

Both actions were DCEL-stubs (no-ops). They now extract the selected geometry from a
vector layer's active keyframe into a new VectorClip (group vs movie clip) and place a
ClipInstance in its place (identity transform → renders where the geometry was), which
the existing transform-animation system can motion-tween.

- Shared `clip_from_geometry::extract_geometry_to_clip` (+ undo) does the work; the
  actions are thin wrappers. Undo snapshots the source graph + removes the clip/instance.
- `extract_subgraph` now DERIVES shared-fill boundary edges internally (an inside edge
  still used by a non-extracted fill must be duplicated, not moved) and unions them with
  the caller's `explicit_boundary` — so a plain geometry selection needs no boundary
  analysis (the selection already includes fill boundary edges via `select_fill`). The
  region-select caller keeps passing its lasso boundary.
- Handlers: Group / Convert to Movie Clip on a geometry selection now build these actions
  from `selected_fills`/`selected_edges`.

Next: shape tweens (same-topology lerp).

lightningbeam-ui/lightningbeam-core/src/actions/clip_from_geometry.rs

@@ -0,0 +1,98 @@
+//! Shared logic for the "Group" and "Convert to Movie Clip" actions: extract the
+//! selected DCEL geometry from a vector layer's active keyframe into a new `VectorClip`
+//! and drop a `ClipInstance` in its place (so it can then be motion-tweened).
+//!
+//! A *group* (`is_group = true`) is a static container; a *movie clip* (`is_group =
+//! false`) has its own timeline. Both are tweenable via the clip instance's transform.
+
+use std::collections::HashSet;
+
+use crate::clip::{ClipInstance, VectorClip};
+use crate::document::Document;
+use crate::layer::{AnyLayer, ShapeKeyframe, VectorLayer};
+use crate::vector_graph::{EdgeId, FillId, VectorGraph};
+use uuid::Uuid;
+
+/// Extract the selected geometry into a new clip + place a `ClipInstance`. Returns the
+/// pre-extraction graph snapshot for undo. `clip_id`/`instance_id` are caller-provided
+/// so undo/redo is stable. The selection sets come straight from the editor selection
+/// (`select_fill` already includes each fill's boundary edges); `extract_subgraph`
+/// derives which of those edges are shared with non-selected shapes.
+pub fn extract_geometry_to_clip(
+    document: &mut Document,
+    layer_id: Uuid,
+    time: f64,
+    fills: &HashSet<FillId>,
+    edges: &HashSet<EdgeId>,
+    clip_id: Uuid,
+    instance_id: Uuid,
+    is_group: bool,
+    clip_name: &str,
+) -> Result<VectorGraph, String> {
+    if fills.is_empty() && edges.is_empty() {
+        return Err("No geometry selected".to_string());
+    }
+    let (doc_w, doc_h, doc_dur) = (document.width, document.height, document.duration.max(1.0));
+
+    // 1. Extract from the source graph (extract_subgraph removes the moved geometry).
+    let (graph_before, sub_graph) = {
+        let layer = document.get_layer_mut(&layer_id).ok_or("Layer not found")?;
+        let vl = match layer {
+            AnyLayer::Vector(vl) => vl,
+            _ => return Err("Not a vector layer".to_string()),
+        };
+        let graph = vl.graph_at_time_mut(time).ok_or("No keyframe at time")?;
+        let before = graph.clone();
+        // No explicit cut boundary — extract_subgraph derives shared-fill boundaries.
+        let (sub, _, _) = graph.extract_subgraph(edges, fills, &HashSet::new());
+        (before, sub)
+    };
+
+    // 2. Build the clip: a vector layer whose single keyframe holds the extracted graph
+    //    (in the source's coordinate space, so identity placement renders it in place).
+    let mut inner = VectorLayer::new("Layer 1");
+    let mut kf = ShapeKeyframe::new(0.0);
+    kf.graph = sub_graph;
+    inner.keyframes.push(kf);
+    let mut clip = VectorClip::with_id(clip_id, clip_name, doc_w, doc_h, doc_dur);
+    clip.is_group = is_group;
+    clip.layers.add_root(AnyLayer::Vector(inner));
+    document.add_vector_clip(clip);
+
+    // 3. Place a ClipInstance (identity transform → geometry stays put).
+    let instance = ClipInstance::with_id(instance_id, clip_id);
+    if let Some(AnyLayer::Vector(vl)) = document.get_layer_mut(&layer_id) {
+        // Groups gate visibility by the active keyframe's clip_instance_ids; movie
+        // clips render unconditionally.
+        if is_group {
+            if let Some(kf) = vl.keyframe_at_mut(time) {
+                kf.clip_instance_ids.push(instance_id);
+            }
+        }
+        vl.clip_instances.push(instance);
+    }
+
+    Ok(graph_before)
+}
+
+/// Reverse `extract_geometry_to_clip`: remove the clip + instance and restore the graph.
+pub fn undo_extract_geometry(
+    document: &mut Document,
+    layer_id: Uuid,
+    time: f64,
+    clip_id: Uuid,
+    instance_id: Uuid,
+    graph_before: &VectorGraph,
+) {
+    document.vector_clips.remove(&clip_id);
+    document.rebuild_layer_to_clip_map();
+    if let Some(AnyLayer::Vector(vl)) = document.get_layer_mut(&layer_id) {
+        vl.clip_instances.retain(|ci| ci.id != instance_id);
+        if let Some(kf) = vl.keyframe_at_mut(time) {
+            kf.clip_instance_ids.retain(|id| *id != instance_id);
+        }
+        if let Some(graph) = vl.graph_at_time_mut(time) {
+            *graph = graph_before.clone();
+        }
+    }
+}

lightningbeam-ui/lightningbeam-core/src/actions/convert_to_movie_clip.rs

@@ -1,55 +1,57 @@
-//! Convert to Movie Clip action — STUB: needs DCEL rewrite
+//! Convert to Movie Clip — extract selected geometry into a movie-clip `VectorClip`
+//! (its own timeline) + a `ClipInstance` that can be motion-tweened.
+
+use std::collections::HashSet;
 
 use crate::action::Action;
-use crate::clip::ClipInstance;
+use crate::actions::clip_from_geometry::{extract_geometry_to_clip, undo_extract_geometry};
 use crate::document::Document;
+use crate::vector_graph::{EdgeId, FillId, VectorGraph};
 use uuid::Uuid;
 
-/// Action that converts selected items to a Movie Clip
-/// TODO: Rewrite for DCEL
-#[allow(dead_code)]
 pub struct ConvertToMovieClipAction {
     layer_id: Uuid,
     time: f64,
-    shape_ids: Vec<Uuid>,
-    clip_instance_ids: Vec<Uuid>,
+    fills: Vec<FillId>,
+    edges: Vec<EdgeId>,
+    clip_id: Uuid,
     instance_id: Uuid,
-    created_clip_id: Option<Uuid>,
-    removed_clip_instances: Vec<ClipInstance>,
+    graph_before: Option<VectorGraph>,
 }
 
 impl ConvertToMovieClipAction {
     pub fn new(
         layer_id: Uuid,
         time: f64,
-        shape_ids: Vec<Uuid>,
-        clip_instance_ids: Vec<Uuid>,
+        fills: Vec<FillId>,
+        edges: Vec<EdgeId>,
+        clip_id: Uuid,
         instance_id: Uuid,
     ) -> Self {
-        Self {
-            layer_id,
-            time,
-            shape_ids,
-            clip_instance_ids,
-            instance_id,
-            created_clip_id: None,
-            removed_clip_instances: Vec::new(),
-        }
+        Self { layer_id, time, fills, edges, clip_id, instance_id, graph_before: None }
     }
 }
 
 impl Action for ConvertToMovieClipAction {
-    fn execute(&mut self, _document: &mut Document) -> Result<(), String> {
-        let _ = (&self.layer_id, self.time, &self.shape_ids, &self.clip_instance_ids, self.instance_id);
+    fn execute(&mut self, document: &mut Document) -> Result<(), String> {
+        let fills: HashSet<FillId> = self.fills.iter().copied().collect();
+        let edges: HashSet<EdgeId> = self.edges.iter().copied().collect();
+        let before = extract_geometry_to_clip(
+            document, self.layer_id, self.time, &fills, &edges,
+            self.clip_id, self.instance_id, false, "Movie Clip",
+        )?;
+        self.graph_before = Some(before);
         Ok(())
     }
 
-    fn rollback(&mut self, _document: &mut Document) -> Result<(), String> {
+    fn rollback(&mut self, document: &mut Document) -> Result<(), String> {
+        if let Some(before) = &self.graph_before {
+            undo_extract_geometry(document, self.layer_id, self.time, self.clip_id, self.instance_id, before);
+        }
         Ok(())
     }
 
     fn description(&self) -> String {
-        let count = self.shape_ids.len() + self.clip_instance_ids.len();
-        format!("Convert {} object(s) to Movie Clip", count)
+        "Convert to Movie Clip".to_string()
     }
 }

lightningbeam-ui/lightningbeam-core/src/actions/group_shapes.rs

@@ -1,56 +1,58 @@
-//! Group action — STUB: needs DCEL rewrite
+//! Group action — extract selected geometry into a group `VectorClip` + a `ClipInstance`.
+
+use std::collections::HashSet;
 
 use crate::action::Action;
-use crate::clip::ClipInstance;
+use crate::actions::clip_from_geometry::{extract_geometry_to_clip, undo_extract_geometry};
 use crate::document::Document;
+use crate::vector_graph::{EdgeId, FillId, VectorGraph};
 use uuid::Uuid;
 
-/// Action that groups selected shapes and/or clip instances into a VectorClip
-/// TODO: Rewrite for DCEL (group DCEL faces/edges into a sub-clip)
-#[allow(dead_code)]
+/// Groups the selected DCEL geometry (fills/edges) of a vector layer's active keyframe
+/// into a new group clip, placing a clip instance in its place (which can be tweened).
 pub struct GroupAction {
     layer_id: Uuid,
     time: f64,
-    shape_ids: Vec<Uuid>,
-    clip_instance_ids: Vec<Uuid>,
+    fills: Vec<FillId>,
+    edges: Vec<EdgeId>,
+    clip_id: Uuid,
     instance_id: Uuid,
-    created_clip_id: Option<Uuid>,
-    removed_clip_instances: Vec<ClipInstance>,
+    graph_before: Option<VectorGraph>,
 }
 
 impl GroupAction {
     pub fn new(
         layer_id: Uuid,
         time: f64,
-        shape_ids: Vec<Uuid>,
-        clip_instance_ids: Vec<Uuid>,
+        fills: Vec<FillId>,
+        edges: Vec<EdgeId>,
+        clip_id: Uuid,
         instance_id: Uuid,
     ) -> Self {
-        Self {
-            layer_id,
-            time,
-            shape_ids,
-            clip_instance_ids,
-            instance_id,
-            created_clip_id: None,
-            removed_clip_instances: Vec::new(),
-        }
+        Self { layer_id, time, fills, edges, clip_id, instance_id, graph_before: None }
     }
 }
 
 impl Action for GroupAction {
-    fn execute(&mut self, _document: &mut Document) -> Result<(), String> {
-        let _ = (&self.layer_id, self.time, &self.shape_ids, &self.clip_instance_ids, self.instance_id);
-        // TODO: Implement DCEL-aware grouping
+    fn execute(&mut self, document: &mut Document) -> Result<(), String> {
+        let fills: HashSet<FillId> = self.fills.iter().copied().collect();
+        let edges: HashSet<EdgeId> = self.edges.iter().copied().collect();
+        let before = extract_geometry_to_clip(
+            document, self.layer_id, self.time, &fills, &edges,
+            self.clip_id, self.instance_id, true, "Group",
+        )?;
+        self.graph_before = Some(before);
         Ok(())
     }
 
-    fn rollback(&mut self, _document: &mut Document) -> Result<(), String> {
+    fn rollback(&mut self, document: &mut Document) -> Result<(), String> {
+        if let Some(before) = &self.graph_before {
+            undo_extract_geometry(document, self.layer_id, self.time, self.clip_id, self.instance_id, before);
+        }
         Ok(())
     }
 
     fn description(&self) -> String {
-        let count = self.shape_ids.len() + self.clip_instance_ids.len();
-        format!("Group {} objects", count)
+        "Group".to_string()
     }
 }

lightningbeam-ui/lightningbeam-core/src/actions/mod.rs

@@ -34,6 +34,7 @@ pub mod convert_to_movie_clip;
 pub mod region_split;
 pub mod toggle_group_expansion;
 pub mod group_layers;
+pub mod clip_from_geometry;
 pub mod raster_diff;
 pub mod raster_stroke;
 pub mod raster_fill;

lightningbeam-ui/lightningbeam-core/src/vector_graph/mod.rs

@@ -1458,11 +1458,14 @@ impl VectorGraph {
 
     // ── Region selection: extract / merge subgraph ──────────────────────
 
-    /// Extract a subgraph containing `inside_edges` and `inside_fills`.
+    /// Extract a subgraph containing `inside_edges` and `inside_fills` (typically a
+    /// geometry selection — `select_fill` already includes each fill's boundary edges).
     ///
-    /// Boundary edges (`boundary_edge_ids`) are **duplicated** — they exist in
-    /// both the returned graph and `self`, so both sides have closed fill
-    /// boundaries when the selection is moved.
+    /// **Boundary edges** are *duplicated* (copied into the returned graph but kept in
+    /// `self`, so remaining shapes keep closed boundaries). They are `explicit_boundary`
+    /// (a cut the caller knows about, e.g. a lasso region — pass an empty set if none)
+    /// UNION any inside edge still shared with a non-extracted fill (derived here, so a
+    /// plain geometry selection needs no boundary analysis from the caller).
     ///
     /// Returns `(new_graph, vertex_map, edge_map)` where the maps go from
     /// old (self) IDs to new (returned graph) IDs.
@@ -1470,17 +1473,32 @@ impl VectorGraph {
         &mut self,
         inside_edges: &HashSet<EdgeId>,
         inside_fills: &HashSet<FillId>,
-        boundary_edge_ids: &HashSet<EdgeId>,
+        explicit_boundary: &HashSet<EdgeId>,
     ) -> (VectorGraph, HashMap<VertexId, VertexId>, HashMap<EdgeId, EdgeId>) {
         let mut new_graph = VectorGraph::new();
         let mut vtx_map: HashMap<VertexId, VertexId> = HashMap::new();
         let mut edge_map: HashMap<EdgeId, EdgeId> = HashMap::new();
 
-        // Collect all edge IDs we need to copy into the new graph
-        let edges_to_copy: HashSet<EdgeId> = inside_edges
-            .union(boundary_edge_ids)
-            .copied()
-            .collect();
+        // Boundary = `explicit_boundary` (e.g. a region/lasso cut the caller knows about)
+        // UNION any inside edge still referenced by a fill we're NOT extracting (a shared
+        // DCEL edge — must be duplicated, not moved, or that fill dangles). Deriving the
+        // latter here means a plain geometry selection needs no boundary analysis.
+        let mut boundary_edge_ids: HashSet<EdgeId> = explicit_boundary.clone();
+        for (i, fill) in self.fills.iter().enumerate() {
+            if fill.deleted || inside_fills.contains(&FillId(i as u32)) {
+                continue;
+            }
+            for &(eid, _) in &fill.boundary {
+                if !eid.is_none() && inside_edges.contains(&eid) {
+                    boundary_edge_ids.insert(eid);
+                }
+            }
+        }
+        let boundary_edge_ids = &boundary_edge_ids;
+
+        // Copy all inside edges + any boundary edges (the explicit ones may not be in
+        // inside_edges); boundary edges are kept in self below.
+        let edges_to_copy: HashSet<EdgeId> = inside_edges.union(boundary_edge_ids).copied().collect();
 
         // Collect all vertices referenced by edges we're copying
         let mut referenced_vids: HashSet<VertexId> = HashSet::new();
@@ -1566,9 +1584,10 @@ impl VectorGraph {
             new_graph.fills[new_fid.idx()].image_fill = fill.image_fill;
         }
 
-        // Remove inside_edges from self (but NOT boundary edges — those are duplicated)
+        // Remove inside_edges from self, EXCEPT boundary edges (those are duplicated —
+        // a non-extracted fill still needs them).
         for &eid in inside_edges {
-            if !eid.is_none() && !self.edges[eid.idx()].deleted {
+            if !eid.is_none() && !boundary_edge_ids.contains(&eid) && !self.edges[eid.idx()].deleted {
                 self.free_edge(eid);
             }
         }

lightningbeam-ui/lightningbeam-editor/src/main.rs

@@ -3471,26 +3471,24 @@ impl EditorApp {
                         self.pending_node_group = true;
                     }
                     _ => {
-                        // Existing clip instance grouping fallback (stub)
+                        // Group selected geometry into a group clip + clip instance.
                         if let Some(layer_id) = self.active_layer_id {
                             if self.selection.has_geometry_selection() {
-                                // TODO: DCEL group deferred to Phase 2
-                            } else {
-                                let clip_ids: Vec<uuid::Uuid> = self.selection.clip_instances().to_vec();
-                                if clip_ids.len() >= 2 {
-                                    let instance_id = uuid::Uuid::new_v4();
-                                    let action = lightningbeam_core::actions::GroupAction::new(
-                                        layer_id, self.playback_time, Vec::new(), clip_ids, instance_id,
-                                    );
-                                    if let Err(e) = self.action_executor.execute(Box::new(action)) {
-                                        eprintln!("Failed to group: {}", e);
-                                    } else {
-                                        self.selection.clear();
-                                        self.selection.add_clip_instance(instance_id);
-                                    }
+                                let fills: Vec<_> = self.selection.selected_fills().iter().copied().collect();
+                                let edges: Vec<_> = self.selection.selected_edges().iter().copied().collect();
+                                let clip_id = uuid::Uuid::new_v4();
+                                let instance_id = uuid::Uuid::new_v4();
+                                let action = lightningbeam_core::actions::GroupAction::new(
+                                    layer_id, self.playback_time, fills, edges, clip_id, instance_id,
+                                );
+                                if let Err(e) = self.action_executor.execute(Box::new(action)) {
+                                    eprintln!("Failed to group: {}", e);
+                                } else {
+                                    self.selection.clear();
+                                    self.selection.add_clip_instance(instance_id);
                                 }
                             }
-                            let _ = layer_id;
+                            // (Grouping existing clip instances is a separate op, not wired.)
                         }
                     }
                 }
@@ -3498,24 +3496,18 @@ impl EditorApp {
             MenuAction::ConvertToMovieClip => {
                 if let Some(layer_id) = self.active_layer_id {
                     if self.selection.has_geometry_selection() {
-                        // TODO: DCEL convert-to-movie-clip deferred to Phase 2
-                    } else {
-                        let clip_ids: Vec<uuid::Uuid> = self.selection.clip_instances().to_vec();
-                        if clip_ids.len() >= 1 {
-                            let instance_id = uuid::Uuid::new_v4();
-                            let action = lightningbeam_core::actions::ConvertToMovieClipAction::new(
-                                layer_id,
-                                self.playback_time,
-                                Vec::new(),
-                                clip_ids,
-                                instance_id,
-                            );
-                            if let Err(e) = self.action_executor.execute(Box::new(action)) {
-                                eprintln!("Failed to convert to movie clip: {}", e);
-                            } else {
-                                self.selection.clear();
-                                self.selection.add_clip_instance(instance_id);
-                            }
+                        let fills: Vec<_> = self.selection.selected_fills().iter().copied().collect();
+                        let edges: Vec<_> = self.selection.selected_edges().iter().copied().collect();
+                        let clip_id = uuid::Uuid::new_v4();
+                        let instance_id = uuid::Uuid::new_v4();
+                        let action = lightningbeam_core::actions::ConvertToMovieClipAction::new(
+                            layer_id, self.playback_time, fills, edges, clip_id, instance_id,
+                        );
+                        if let Err(e) = self.action_executor.execute(Box::new(action)) {
+                            eprintln!("Failed to convert to movie clip: {}", e);
+                        } else {
+                            self.selection.clear();
+                            self.selection.add_clip_instance(instance_id);
                         }
                     }
                 }