diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/add_raster_keyframe.rs b/lightningbeam-ui/lightningbeam-core/src/actions/add_raster_keyframe.rs new file mode 100644 index 0000000..d05d6d4 --- /dev/null +++ b/lightningbeam-ui/lightningbeam-core/src/actions/add_raster_keyframe.rs @@ -0,0 +1,52 @@ +//! Add a blank raster keyframe at the playhead (the explicit "New Keyframe" command +//! for raster layers — mirrors `SetKeyframeAction` for vector, but inserts an empty +//! cel rather than copying the current graph). + +use crate::action::Action; +use crate::document::Document; +use crate::layer::AnyLayer; +use uuid::Uuid; + +pub struct AddRasterKeyframeAction { + layer_id: Uuid, + time: f64, + width: u32, + height: u32, + /// Id of the keyframe created by the last `execute` (so `rollback` can remove + /// exactly that one). `None` if a keyframe already existed at `time` (no-op). + created_id: Option, +} + +impl AddRasterKeyframeAction { + pub fn new(layer_id: Uuid, time: f64, width: u32, height: u32) -> Self { + Self { layer_id, time, width, height, created_id: None } + } +} + +impl Action for AddRasterKeyframeAction { + fn execute(&mut self, document: &mut Document) -> Result<(), String> { + let layer = document + .get_layer_mut(&self.layer_id) + .ok_or_else(|| format!("Layer {} not found", self.layer_id))?; + let rl = match layer { + AnyLayer::Raster(rl) => rl, + _ => return Err("Not a raster layer".to_string()), + }; + // Inserts a blank cel only if one doesn't already exist at this time. + self.created_id = rl.insert_blank_keyframe_at(self.time, self.width, self.height); + Ok(()) + } + + fn rollback(&mut self, document: &mut Document) -> Result<(), String> { + if let Some(id) = self.created_id.take() { + if let Some(AnyLayer::Raster(rl)) = document.get_layer_mut(&self.layer_id) { + rl.remove_keyframe(id); + } + } + Ok(()) + } + + fn description(&self) -> String { + "New keyframe".to_string() + } +} diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs b/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs index 2e22a15..cc23c0c 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs @@ -37,6 +37,7 @@ pub mod group_layers; pub mod raster_diff; pub mod raster_stroke; pub mod raster_fill; +pub mod add_raster_keyframe; pub mod move_layer; pub mod set_fill_paint; @@ -71,6 +72,7 @@ pub use toggle_group_expansion::ToggleGroupExpansionAction; pub use group_layers::GroupLayersAction; pub use raster_stroke::RasterStrokeAction; pub use raster_fill::RasterFillAction; +pub use add_raster_keyframe::AddRasterKeyframeAction; pub use move_layer::MoveLayerAction; pub use set_fill_paint::SetFillPaintAction; pub use change_bpm::ChangeBpmAction; diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/raster_fill.rs b/lightningbeam-ui/lightningbeam-core/src/actions/raster_fill.rs index 8bab747..92c0dd0 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/raster_fill.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/raster_fill.rs @@ -46,7 +46,10 @@ impl Action for RasterFillAction { AnyLayer::Raster(rl) => rl, _ => return Err("Not a raster layer".to_string()), }; - let kf = raster.ensure_keyframe_at(self.time, self.width, self.height); + let _ = (self.width, self.height); + let kf = raster + .keyframe_at_mut(self.time) + .ok_or_else(|| format!("No raster keyframe at/before t={}", self.time))?; if let Some(full) = self.full_after.take() { kf.raw_pixels = full; } else { @@ -64,7 +67,10 @@ impl Action for RasterFillAction { AnyLayer::Raster(rl) => rl, _ => return Err("Not a raster layer".to_string()), }; - let kf = raster.ensure_keyframe_at(self.time, self.width, self.height); + let _ = (self.width, self.height); + let kf = raster + .keyframe_at_mut(self.time) + .ok_or_else(|| format!("No raster keyframe at/before t={}", self.time))?; self.diff.apply_before(&mut kf.raw_pixels); kf.texture_dirty = true; kf.dirty = true; diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/raster_stroke.rs b/lightningbeam-ui/lightningbeam-core/src/actions/raster_stroke.rs index 68c4fa4..06517b9 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/raster_stroke.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/raster_stroke.rs @@ -99,5 +99,10 @@ fn get_keyframe_mut<'a>( AnyLayer::Raster(rl) => rl, _ => return Err("Not a raster layer".to_string()), }; - Ok(raster.ensure_keyframe_at(time, width, height)) + let _ = (width, height); + // Edit the ACTIVE keyframe (at-or-before `time`); never create one — keyframes + // are made explicitly via "New Keyframe". + raster + .keyframe_at_mut(time) + .ok_or_else(|| format!("No raster keyframe at/before t={time}")) } diff --git a/lightningbeam-ui/lightningbeam-core/src/raster_layer.rs b/lightningbeam-ui/lightningbeam-core/src/raster_layer.rs index 2bb045c..b4c858a 100644 --- a/lightningbeam-ui/lightningbeam-core/src/raster_layer.rs +++ b/lightningbeam-ui/lightningbeam-core/src/raster_layer.rs @@ -235,6 +235,35 @@ impl RasterLayer { &mut self.keyframes[insert_idx] } + /// Insert a blank keyframe at `time` if none exists there (within tolerance). + /// Returns the new keyframe's id if one was created, `None` if a keyframe already + /// existed. Used by the explicit "New Keyframe" command (blank cel). + pub fn insert_blank_keyframe_at(&mut self, time: f64, width: u32, height: u32) -> Option { + if self.keyframe_index_at_exact(time, 0.001).is_some() { + return None; + } + let (w, h) = if width == 0 || height == 0 { + self.keyframe_at(time) + .map(|kf| (kf.width, kf.height)) + .unwrap_or((1920, 1080)) + } else { + (width, height) + }; + let insert_idx = self.keyframes.partition_point(|kf| kf.time < time); + let kf = RasterKeyframe::new(time, w, h); + let id = kf.id; + self.keyframes.insert(insert_idx, kf); + Some(id) + } + + /// Remove the keyframe with the given id, returning it if found. + pub fn remove_keyframe(&mut self, id: Uuid) -> Option { + self.keyframes + .iter() + .position(|kf| kf.id == id) + .map(|pos| self.keyframes.remove(pos)) + } + /// Return the ZIP-relative PNG path for the active keyframe at `time`, or `None`. pub fn buffer_path_at_time(&self, time: f64) -> Option<&str> { self.keyframe_at(time).map(|kf| kf.media_path.as_str()) diff --git a/lightningbeam-ui/lightningbeam-editor/src/main.rs b/lightningbeam-ui/lightningbeam-editor/src/main.rs index a33b557..99b586b 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/main.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/main.rs @@ -3730,27 +3730,39 @@ impl EditorApp { MenuAction::NewKeyframe | MenuAction::AddKeyframeAtPlayhead => { if let Some(layer_id) = self.active_layer_id { let document = self.action_executor.document(); - // Determine which selected objects are shape instances vs clip instances - let _shape_ids: Vec = Vec::new(); - let mut clip_ids = Vec::new(); - if let Some(AnyLayer::Vector(vl)) = document.get_layer(&layer_id) { - // TODO: DCEL - shape instance lookup disabled during migration - // (was: get_shape_in_keyframe to check which selected objects are shapes) - for &id in self.selection.clip_instances() { - if vl.clip_instances.iter().any(|ci| ci.id == id) { - clip_ids.push(id); - } - } - } - // For vector layers, always create a shape keyframe (even without clip selection) - if document.get_layer(&layer_id).map_or(false, |l| matches!(l, AnyLayer::Vector(_))) || !clip_ids.is_empty() { - let action = lightningbeam_core::actions::SetKeyframeAction::new( - layer_id, - self.playback_time, - clip_ids, + let is_raster = matches!(document.get_layer(&layer_id), Some(AnyLayer::Raster(_))); + if is_raster { + // Raster layers: insert a blank cel at the playhead. + let (doc_w, doc_h) = (document.width as u32, document.height as u32); + let action = lightningbeam_core::actions::AddRasterKeyframeAction::new( + layer_id, self.playback_time, doc_w, doc_h, ); if let Err(e) = self.action_executor.execute(Box::new(action)) { - eprintln!("Failed to set keyframe: {}", e); + eprintln!("Failed to add raster keyframe: {}", e); + } + } else { + // Determine which selected objects are shape instances vs clip instances + let _shape_ids: Vec = Vec::new(); + let mut clip_ids = Vec::new(); + if let Some(AnyLayer::Vector(vl)) = document.get_layer(&layer_id) { + // TODO: DCEL - shape instance lookup disabled during migration + // (was: get_shape_in_keyframe to check which selected objects are shapes) + for &id in self.selection.clip_instances() { + if vl.clip_instances.iter().any(|ci| ci.id == id) { + clip_ids.push(id); + } + } + } + // For vector layers, always create a shape keyframe (even without clip selection) + if document.get_layer(&layer_id).map_or(false, |l| matches!(l, AnyLayer::Vector(_))) || !clip_ids.is_empty() { + let action = lightningbeam_core::actions::SetKeyframeAction::new( + layer_id, + self.playback_time, + clip_ids, + ); + if let Err(e) = self.action_executor.execute(Box::new(action)) { + eprintln!("Failed to set keyframe: {}", e); + } } } } diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/stage.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/stage.rs index ecbdaf4..b5d93bb 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/stage.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/stage.rs @@ -5624,15 +5624,8 @@ impl StagePane { (doc.width as u32, doc.height as u32) }; - // Ensure the keyframe exists before reading its ID. - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&layer_id) { - rl.ensure_keyframe_at(time, doc_w, doc_h); - } else { - return None; // not a raster layer - } - } + // Don't create a keyframe — explicit "New Keyframe" only. The read below + // returns None (no workspace) if there's no active keyframe to lift from. // Read keyframe id and pixels. let (kf_id, w, h, pixels) = { @@ -6066,21 +6059,12 @@ impl StagePane { (doc.width as u32, doc.height as u32) }; - // Ensure the keyframe exists BEFORE reading its ID, so we always get - // the real UUID. Previously we read the ID first and fell back to a - // randomly-generated UUID when no keyframe existed; that fake UUID was - // stored in painting_canvas but subsequent drag frames used the real UUID - // from keyframe_at(), causing the GPU canvas to be a different object from - // the one being composited. - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&active_layer_id) { - rl.ensure_keyframe_at(*shared.playback_time, doc_width, doc_height); - } - } - - // Now read the guaranteed-to-exist keyframe to get the real UUID. - let (keyframe_id, canvas_width, canvas_height, buffer_before, initial_pixels) = { + // Paint into the ACTIVE keyframe (the one at-or-before the playhead) — + // do NOT create one. Keyframes are made explicitly via "New Keyframe" + // (a new layer already seeds one). If none exists at-or-before the + // playhead, there's nothing to paint into; bail. + let _ = (doc_width, doc_height); + let (keyframe_id, kf_time, canvas_width, canvas_height, buffer_before, initial_pixels) = { let doc = shared.action_executor.document(); if let Some(AnyLayer::Raster(rl)) = doc.get_layer(&active_layer_id) { if let Some(kf) = rl.keyframe_at(*shared.playback_time) { @@ -6090,9 +6074,9 @@ impl StagePane { } else { raw.clone() }; - (kf.id, kf.width, kf.height, raw, init) + (kf.id, kf.time, kf.width, kf.height, raw, init) } else { - return; // shouldn't happen after ensure_keyframe_at + return; // no keyframe at/before the playhead — nothing to paint } } else { return; @@ -6124,7 +6108,7 @@ impl StagePane { self.painting_canvas = Some((active_layer_id, keyframe_id)); self.pending_undo_before = Some(( active_layer_id, - *shared.playback_time, + kf_time, canvas_width, canvas_height, buffer_before, @@ -6132,7 +6116,7 @@ impl StagePane { self.pending_raster_dabs = Some(PendingRasterDabs { keyframe_id, layer_id: active_layer_id, - time: *shared.playback_time, + time: kf_time, canvas_width, canvas_height, initial_pixels: Some(initial_pixels), @@ -6142,7 +6126,7 @@ impl StagePane { }); self.raster_stroke_state = Some(( active_layer_id, - *shared.playback_time, + kf_time, stroke_state, Vec::new(), // buffer_before now lives in pending_undo_before )); @@ -6368,17 +6352,13 @@ impl StagePane { let time = *shared.playback_time; // Canvas dimensions (to create keyframe if needed). - let (doc_w, doc_h) = { + let (_doc_w, _doc_h) = { let doc = shared.action_executor.document(); (doc.width as u32, doc.height as u32) }; // Ensure a keyframe exists at the current time. - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&layer_id) { - rl.ensure_keyframe_at(time, doc_w, doc_h); - } - } + // Don't create a keyframe — keyframes are made explicitly via "New Keyframe". + // The snapshot below edits the active keyframe and bails if none exists. // Snapshot the pixel buffer before drawing. let (buffer_before, w, h) = { let doc = shared.action_executor.document(); @@ -6721,16 +6701,12 @@ impl StagePane { let time = *shared.playback_time; // Ensure a keyframe exists at the current time. - let (doc_w, doc_h) = { + let (_doc_w, _doc_h) = { let doc = shared.action_executor.document(); (doc.width as u32, doc.height as u32) }; - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&layer_id) { - rl.ensure_keyframe_at(time, doc_w, doc_h); - } - } + // Don't create a keyframe — keyframes are made explicitly via "New Keyframe". + // The snapshot below edits the active keyframe and bails if none exists. // Snapshot current pixels. let (buffer_before, width, height) = { @@ -6802,16 +6778,12 @@ impl StagePane { let time = *shared.playback_time; // Ensure keyframe exists. - let (doc_w, doc_h) = { + let (_doc_w, _doc_h) = { let doc = shared.action_executor.document(); (doc.width as u32, doc.height as u32) }; - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&layer_id) { - rl.ensure_keyframe_at(time, doc_w, doc_h); - } - } + // Don't create a keyframe — keyframes are made explicitly via "New Keyframe". + // The snapshot below edits the active keyframe and bails if none exists. let (pixels, width, height) = { let doc = shared.action_executor.document(); @@ -6888,16 +6860,11 @@ impl StagePane { Self::commit_raster_floating_now(shared); // Ensure the keyframe exists. - let (doc_w, doc_h) = { + let (_doc_w, _doc_h) = { let doc = shared.action_executor.document(); (doc.width as u32, doc.height as u32) }; - { - let doc = shared.action_executor.document_mut(); - if let Some(AnyLayer::Raster(rl)) = doc.get_layer_mut(&layer_id) { - rl.ensure_keyframe_at(time, doc_w, doc_h); - } - } + // Don't create a keyframe — explicit "New Keyframe" only; bail below if none. // Snapshot canvas pixels. let (pixels, width, height) = { diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs index 882c40b..8c8577e 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs @@ -3976,6 +3976,30 @@ impl TimelinePane { } } + // Draw keyframe markers for raster layers (same diamond as vector). + if let lightningbeam_core::layer::AnyLayer::Raster(rl) = layer { + for kf in &rl.keyframes { + let x = self.time_to_x(kf.time); + if x >= 0.0 && x <= rect.width() { + let cx = rect.min.x + x; + let cy = y + LAYER_HEIGHT - 8.0; + let size = 5.0; + let diamond = [ + egui::pos2(cx, cy - size), + egui::pos2(cx + size, cy), + egui::pos2(cx, cy + size), + egui::pos2(cx - size, cy), + ]; + let color = theme.bg_color(&["#timeline", ".keyframe-diamond"], ui.ctx(), egui::Color32::from_rgb(255, 220, 100)); + painter.add(egui::Shape::convex_polygon( + diamond.to_vec(), + color, + egui::Stroke::new(1.0, theme.border_color(&["#timeline", ".keyframe-diamond"], ui.ctx(), egui::Color32::from_rgb(180, 150, 50))), + )); + } + } + } + // Separator line at bottom painter.line_segment( [