Raster keyframe timeline UI: display + explicit creation + no lazy create

Make raster layers behave like vector on the timeline.

- Timeline: draw a diamond per `RasterKeyframe` (mirrors the vector keyframe block).
- New Keyframe (K / menu): on a raster layer, insert a BLANK cel at the playhead via
  a new undoable `AddRasterKeyframeAction` (+ `RasterLayer::insert_blank_keyframe_at`
  / `remove_keyframe`). Vector path unchanged.
- Stop lazy creation: paint tools now edit the ACTIVE keyframe (at-or-before the
  playhead) instead of creating one. The brush captures the active keyframe's exact
  time; `RasterStroke`/`RasterFillAction` resolve via `keyframe_at_mut` (error if
  none); the tool-site `ensure_keyframe_at` blocks (brush/fill/bucket/wand/quick-
  select/floating-lift) are removed — each read already bails when no keyframe exists.
  New layers still seed a keyframe at the playhead, so there's normally one to paint
  into; painting before the first keyframe is now a no-op (as intended).

Next: onion skinning.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Skyler Lehmkuhl 2026-06-20 22:22:02 -04:00
parent 31e23b0fc7
commit 2cbaf67583
8 changed files with 176 additions and 79 deletions

View File

@ -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<Uuid>,
}
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()
}
}

View File

@ -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;

View File

@ -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;

View File

@ -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}"))
}

View File

@ -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<Uuid> {
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<RasterKeyframe> {
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())

View File

@ -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<uuid::Uuid> = 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<uuid::Uuid> = 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);
}
}
}
}

View File

@ -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) = {

View File

@ -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(
[