diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index 82bbf5eba..4fcbcc83e 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -25,13 +25,23 @@ fn add_styles(s: &mut Session, p: &Value, cmd: &str, styles: &[&str], enabled: b if ids.is_empty() { return Err(bad(cmd, "no layer that takes layer styles (footage, solid, precomp, text or shape layer)")); } - let uids = s.edit(label, None, |proj, state| { + let uids = s.edit(label, super::merge_p(p), |proj, state| { let gl = proj.comp(cid).ok_or(EngineError::NoComp)?.global_light.clone(); let mut out = vec![]; let mut sel = vec![]; for lid in &ids { let mut next = proj.next_id; let l = layer_mut(proj, cid, *lid)?; + // No style: Blending Options alone (the Layer Style dialog's first change on a layer + // without styles). + if styles.is_empty() { + let uid = st::add_blending_options(l, &mut Ids(&mut next), &gl).ok_or_else(|| bad(cmd, "no Blending Options"))?; + if let Some(g) = l.props.sub_mut(GROUP) { + g.enabled = true; + } + out.push(json!({"layer": lid.0, "style": st::BLENDING, "uid": uid})); + sel.push((*lid, uid)); + } for id in styles { let had = l.layer_styles().and_then(|g| g.sub(id)).is_some(); let uid = st::add_style(l, &mut Ids(&mut next), id, &gl, enabled).ok_or_else(|| bad(cmd, format!("unknown style `{id}`")))?; @@ -58,6 +68,9 @@ fn add_styles(s: &mut Session, p: &Value, cmd: &str, styles: &[&str], enabled: b fn add(s: &mut Session, p: &Value) -> Result { let name = str_p(p, "style").ok_or_else(|| bad("layer.style.add", "missing `style` (e.g. dropShadow or \"Drop Shadow\")"))?; + if name.eq_ignore_ascii_case(st::BLENDING) || name.eq_ignore_ascii_case("Blending Options") { + return add_styles(s, p, "layer.style.add", &[], true, "Blending Options"); + } let id = st::style_id(name) .ok_or_else(|| bad("layer.style.add", format!("unknown style `{name}`; one of {}", STYLES.iter().map(|s| s.0).collect::>().join(", "))))?; let label = STYLES.iter().find(|s| s.0 == id).map(|s| s.1).unwrap_or(id); @@ -139,8 +152,9 @@ fn remove(s: &mut Session, p: &Value) -> Result { return Err(bad("layer.style.remove", "Blending Options can't be removed")); } g.children.retain(|c| c.uid() != uid); - // The last style gone removes the Layer Styles group, as in AE. - if g.children.iter().all(|c| c.match_id() == st::BLENDING) { + // The last style gone removes the Layer Styles group, as in AE, unless its Blending + // Options were changed (they can be edited with no style since #1389). + if g.children.iter().all(|c| c.match_id() == st::BLENDING) && g.sub(st::BLENDING).is_none_or(st::blending_is_default) { l.props.children.retain(|c| c.match_id() != GROUP); } } @@ -224,7 +238,7 @@ pub fn specs() -> Vec { "Add Layer Style", [], None, - "{style: dropShadow|innerShadow|outerGlow|innerGlow|bevelEmboss|satin|colorOverlay|gradientOverlay|stroke (or display name), layers?}", + "{style: dropShadow|innerShadow|outerGlow|innerGlow|bevelEmboss|satin|colorOverlay|gradientOverlay|stroke (or display name), or blendingOptions (the Layer Styles group with Blending Options only), layers?, merge?}", has_layers, add ), diff --git a/crates/engine/src/tests/styles.rs b/crates/engine/src/tests/styles.rs index e589241fb..1aef9e825 100644 --- a/crates/engine/src/tests/styles.rs +++ b/crates/engine/src/tests/styles.rs @@ -117,3 +117,51 @@ fn styles_render_and_round_trip() { assert_eq!(back, *s.project); assert_eq!(back.comp(cid).unwrap().global_light.angle.value.as_f64(), 33.0); } + +/// #1389: Blending Options on a layer with no styles. `layer.style.add` with `blendingOptions` +/// creates the Layer Styles group with Blending Options only; with the same merge key as the +/// value set right after it, both are one undo step, and undoing it leaves no group behind. +#[test] +fn blending_options_alone_create_the_group_in_one_undo_step() { + let (mut s, a, b) = session(); + let undo = s.history.undo.len(); + let r = s.execute("layer.style.add", json!({"layers": [a], "style": "blendingOptions", "merge": "bo"})).unwrap(); + assert_eq!(r["styles"][0]["style"], "blendingOptions", "{r}"); + let layer = s.active_comp().unwrap().layer(effectcraft_project::LayerId(a)).unwrap().clone(); + let g = layer.layer_styles().expect("the group is created"); + assert!(g.sub("blendingOptions").is_some()); + assert!(styles_of(&s, a).is_empty(), "no style is added with it"); + s.execute("prop.set", json!({"layer": a, "path": "layerStyles/blendingOptions/advancedBlending/fillOpacity", "value": 40, "merge": "bo"})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "layerStyles/blendingOptions/advancedBlending/fillOpacity", "value": 30, "merge": "bo"})).unwrap(); + assert_eq!(s.history.undo.len(), undo + 1, "creating the group and setting the value are one step"); + // The other layer is untouched, and Global Light is the comp's. + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(b)).unwrap().layer_styles().is_none()); + let gl = s.execute("prop.get", json!({"layer": a, "path": "layerStyles/blendingOptions/globalLightAngle"})).unwrap(); + assert!(gl.to_string().contains("120"), "{gl}"); + s.execute("edit.undo", json!({})).unwrap(); + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(a)).unwrap().layer_styles().is_none(), "one undo removes the group"); + // By display name too. + s.execute("layer.style.add", json!({"layers": [a], "style": "Blending Options"})).unwrap(); + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(a)).unwrap().layer_styles().is_some()); +} + +/// Removing the last style keeps a Layer Styles group whose Blending Options were changed (a +/// group with Blending Options only is a state of its own since #1389); at their defaults the +/// group goes, as before. +#[test] +fn removing_the_last_style_keeps_changed_blending_options() { + let (mut s, a, b) = session(); + let fill = "layerStyles/blendingOptions/advancedBlending/fillOpacity"; + s.execute("layer.style.add", json!({"layers": [a], "style": "blendingOptions"})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": fill, "value": 40})).unwrap(); + s.execute("layer.style.dropShadow", json!({"layers": [a]})).unwrap(); + s.execute("layer.style.remove", json!({"layer": a, "style": "dropShadow"})).unwrap(); + let v = s.execute("prop.get", json!({"layer": a, "path": fill})).unwrap(); + assert_eq!(v["value"].as_f64(), Some(40.0), "{v}"); + assert!(styles_of(&s, a).is_empty()); + // At the defaults (here on the other layer, through the same command) the group goes. + s.execute("layer.style.add", json!({"layers": [b], "style": "blendingOptions"})).unwrap(); + s.execute("layer.style.dropShadow", json!({"layers": [b]})).unwrap(); + s.execute("layer.style.remove", json!({"layer": b, "style": "dropShadow"})).unwrap(); + assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(b)).unwrap().layer_styles().is_none()); +} diff --git a/crates/interchange/src/export.rs b/crates/interchange/src/export.rs index 594620f65..375c1bf1b 100644 --- a/crates/interchange/src/export.rs +++ b/crates/interchange/src/export.rs @@ -138,7 +138,7 @@ fn unsupported(project: &Project, comp: &Comp, l: &Layer, format: TimelineFormat if l.is_3d() { return Some("3D layer".into()); } - if l.props.sub("layerStyles").is_some_and(|g| !g.children.is_empty()) { + if l.styles_change_pixels() { return Some("layer styles".into()); } if l.props.get("timeRemap").is_some_and(|p| p.is_animated()) { diff --git a/crates/interchange/src/tests.rs b/crates/interchange/src/tests.rs index 1257ba3fd..97efd76cd 100644 --- a/crates/interchange/src/tests.rs +++ b/crates/interchange/src/tests.rs @@ -433,6 +433,36 @@ fn plan_prerenders_rendered_only_layers() { assert!(plan_prerender(&p, cid, &ExportOptions { prerender: PrerenderMode::None, ..Default::default() }).is_empty()); } +/// A Layer Styles group that doesn't change the picture (Blending Options at their defaults, or +/// every style switched off) keeps a clip a clip; Fill Opacity or a style that is on pre-renders it. +#[test] +fn only_layer_styles_that_change_pixels_prerender() { + use effectcraft_project::styles::{self as st, GROUP}; + let opts = ExportOptions::default(); + let reason = |edit: &dyn Fn(&mut Project, &mut Layer)| { + let (mut p, cid) = ae_project(); + let c = p.comp(cid).unwrap(); + let gl = c.global_light.clone(); + let mut l = c.layers.iter().find(|l| l.name == "clip a").unwrap().clone(); + st::add_blending_options(&mut l, &mut effectcraft_project::build::Ids(&mut p.next_id), &gl).unwrap(); + edit(&mut p, &mut l); + let lid = l.id; + *p.comp_mut(cid).unwrap().layers.iter_mut().find(|x| x.id == lid).unwrap() = l; + prerender_reason(&p, cid, lid, &opts) + }; + let fill = |l: &mut Layer, v: f64| l.props.prop_mut(&format!("{GROUP}/blendingOptions/advancedBlending/fillOpacity")).unwrap().value = Value::Scalar(v); + assert_eq!(reason(&|_, _| {}), None, "Blending Options at their defaults"); + assert_eq!(reason(&|_, l| fill(l, 40.0)).as_deref(), Some("layer styles")); + let style = |on: bool| { + move |p: &mut Project, l: &mut Layer| { + let gl = effectcraft_project::styles::GlobalLight::default(); + st::add_style(l, &mut effectcraft_project::build::Ids(&mut p.next_id), "dropShadow", &gl, on).unwrap(); + } + }; + assert_eq!(reason(&style(false)), None, "a style switched off"); + assert_eq!(reason(&style(true)).as_deref(), Some("layer styles")); +} + #[test] fn export_then_reimport_keeps_the_timeline() { let (p, cid) = ae_project(); diff --git a/crates/project/src/styles.rs b/crates/project/src/styles.rs index 2b2bdf92c..fd59fedda 100644 --- a/crates/project/src/styles.rs +++ b/crates/project/src/styles.rs @@ -312,6 +312,33 @@ impl Layer { pub fn can_have_styles(&self) -> bool { self.source.is_av() && !self.switches.adjustment } + /// Do the layer's styles change its pixels: the group on, and a style on or Advanced + /// Blending away from its defaults (animated counts as changed). + pub fn styles_change_pixels(&self) -> bool { + self.layer_styles() + .filter(|g| g.enabled && self.can_have_styles()) + .is_some_and(|g| g.groups().any(|s| s.enabled && s.match_id != BLENDING) || g.sub(BLENDING).is_some_and(|bo| !blending_is_default(bo))) + } +} + +/// Is Advanced Blending of this Blending Options group at its defaults (no keys, no +/// expression)? Global Light is the comp's and doesn't count. +pub fn blending_is_default(bo: &PropGroup) -> bool { + let mut next = 0; + let fresh = blending_options(&mut Ids(&mut next), &GlobalLight::default()); + let (Some(adv), Some(def)) = (bo.sub("advancedBlending"), fresh.sub("advancedBlending")) else { return true }; + def.props().all(|d| adv.get(&d.match_id).is_none_or(|p| same_anim(p, d))) +} + +/// Give `layer` its Layer Styles group (Blending Options only, after Transform) when it has none. +/// Returns the Blending Options group's uid. +pub fn add_blending_options(layer: &mut Layer, ids: &mut Ids, gl: &GlobalLight) -> Option { + if layer.props.sub(GROUP).is_none() { + let g = layer_styles(ids, gl); + let at = layer.props.children.iter().position(|c| c.match_id() == "transform").map(|i| i + 1).unwrap_or(layer.props.children.len()); + layer.props.children.insert(at, g.into()); + } + Some(layer.props.sub(GROUP)?.sub(BLENDING)?.uid) } /// Add style `id` to `layer` (creating the Layer Styles group after Transform when missing). @@ -319,9 +346,7 @@ impl Layer { pub fn add_style(layer: &mut Layer, ids: &mut Ids, id: &str, gl: &GlobalLight, enabled: bool) -> Option { let order = STYLES.iter().position(|s| s.0 == id)?; if layer.props.sub(GROUP).is_none() { - let g = layer_styles(ids, gl); - let at = layer.props.children.iter().position(|c| c.match_id() == "transform").map(|i| i + 1).unwrap_or(layer.props.children.len()); - layer.props.children.insert(at, g.into()); + add_blending_options(layer, ids, gl); } let ls = layer.props.sub_mut(GROUP)?; if let Some(g) = ls.sub(id) { diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index 713dfe6d4..b646b002e 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/crates/ui-egui/src/panels/layer_styles_dialog.rs @@ -7,7 +7,8 @@ use crate::i18n::tr; use effectcraft_engine::commands::text_edit::active_editor_snapshot; use effectcraft_engine::keyframe::Value as KV; -use effectcraft_engine::project::styles::{BLENDING, STYLES}; +use effectcraft_engine::project::build::Ids; +use effectcraft_engine::project::styles::{BLENDING, GROUP, STYLES, blending_options}; use effectcraft_engine::project::{Layer, LayerId, ParamUi, Property}; use egui::vec2; use serde_json::{Value, json}; @@ -135,6 +136,10 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }; let lid = l.id.0; let styles = l.layer_styles().cloned(); + // Shown only: its uids are never stored in the project. + let mut next = app.session.project.next_id; + let blending_template = + if styles.is_none() && l.can_have_styles() { app.session.active_comp().map(|c| blending_options(&mut Ids(&mut next), &c.global_light)) } else { None }; let sel = app.dialog_state.layer_style.selected.clone(); let time = app.session.time(); let lt = l.layer_time(time); @@ -199,7 +204,10 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let head = ui.label(egui::RichText::new(name).font(Tokens::semibold(13.0))); app.auto.add("dialog.layerStyle.page", head.rect, &sel); ui.add_space(6.0); - let Some(g) = styles.as_ref().and_then(|s| s.sub(&sel)) else { + // A layer with no styles still shows Blending Options, with the values the Layer + // Styles group would start from; the first change creates the group (#1389). + let missing = blending_template.as_ref().filter(|_| sel == BLENDING && styles.is_none()); + let Some(g) = styles.as_ref().and_then(|s| s.sub(&sel)).or(missing) else { ui.label(egui::RichText::new(tr("Not applied: tick it in the list to add it.")).weak()); return; }; @@ -221,13 +229,21 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { let lbl = ui.label(&p.name); let v = p.value_at(lt); let edit = ui.scope(|ui| control(ui, p, &v)); - if let Some(nv) = edit.inner { - actions.push(( - "prop.set".into(), - json!({"layer": lid, "prop": p.uid, "value": nv, "merge": format!("layer-style-dialog-{}", p.uid)}), - )); - } let path = g.match_path_of(p.uid).unwrap_or_default(); + if let Some(nv) = edit.inner { + // Keyed by path, not uid: the group created by the first + // change and the rest of the drag stay one undo step. + let merge = format!("layer-style-dialog-{lid}-{sel}/{path}"); + if missing.is_some() { + actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": BLENDING, "merge": merge}))); + actions.push(( + "prop.set".into(), + json!({"layer": lid, "path": format!("{GROUP}/{BLENDING}/{path}"), "value": nv, "merge": merge}), + )); + } else { + actions.push(("prop.set".into(), json!({"layer": lid, "prop": p.uid, "value": nv, "merge": merge}))); + } + } app.auto.add(&format!("dialog.layerStyle.prop.{sel}/{path}"), lbl.rect, &p.name); app.auto.add(&format!("dialog.layerStyle.value.{sel}/{path}"), edit.response.rect, &p.name); ui.end_row(); diff --git a/crates/ui-egui/tests/it/ui_tail.rs b/crates/ui-egui/tests/it/ui_tail.rs index ca9026623..bd87a01a7 100644 --- a/crates/ui-egui/tests/it/ui_tail.rs +++ b/crates/ui-egui/tests/it/ui_tail.rs @@ -208,6 +208,38 @@ fn layer_style_controls_stay_reachable_in_a_small_window() { } } +/// #1389: on a layer with no styles, Blending Options shows its controls instead of an empty +/// page. Opening the dialog adds nothing; the first change creates the Layer Styles group, and +/// the group and the whole drag are one undo step. +#[test] +fn layer_style_blending_options_edit_a_layer_without_styles() { + let mut h = harness(1400.0); + let lid = h.state().session.active_comp().unwrap().layers[0].id.0; + h.state_mut().session.execute("layer.select", json!({"layers": [lid]})).unwrap(); + let ctx = h.ctx.clone(); + effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "layer.style.options", json!({})).unwrap(); + h.run_steps(5); + let has_group = |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().layers[0].layer_styles().is_some(); + let undo = h.state().session.history.undo.len(); + assert_eq!(h.state().auto.find("dialog.layerStyle.page").unwrap().label, "blendingOptions"); + let fill = "dialog.layerStyle.value.blendingOptions/advancedBlending/fillOpacity"; + assert!(h.state().auto.find("dialog.layerStyle.value.blendingOptions/globalLightAngle").is_some(), "Global Light is shown"); + let at = in_view(&h, fill); + assert!(!has_group(&h), "opening the page adds nothing"); + drag(&mut h, at, at - vec2(40.0, 0.0)); + let v = + h.state_mut().session.execute("prop.get", json!({"layer": lid, "path": "layerStyles/blendingOptions/advancedBlending/fillOpacity"})).unwrap()["value"] + .as_f64() + .unwrap(); + assert!(v < 100.0, "Fill Opacity was dragged: {v}"); + assert!(styles(&mut h).is_empty(), "no style was added"); + assert_eq!(h.state().session.history.undo.len(), undo + 1, "the group and the drag are one undo step"); + assert_eq!(h.state().dialog, Some(effectcraft_ui_egui::Dialog::LayerStyles)); + effectcraft_ui_egui::panels::layer_styles_dialog::finish(h.state_mut(), true); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert!(!has_group(&h), "one undo removes the group"); +} + #[test] fn layer_style_dialog_moves_by_its_title() { let mut h = harness(1400.0); diff --git a/docs/architecture.md b/docs/architecture.md index f52c93e1a..0bfe4023e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -256,7 +256,10 @@ the layer body. Interior styles are clipped to the layer: they blend onto its co shape and keep its alpha, so an anti-aliased edge mixes the style and the background only. Layer opacity fades the whole stack; Knockout and the R/G/B channel switches apply at composite time. Styled pixels are cached separately from the layer content, so editing a style reuses the cached source/masks/effects. Global Light is one setting per comp, mirrored into every -layer's Blending Options and kept in step after each edit. +layer's Blending Options and kept in step after each edit. Blending Options can be edited in the Layer Style dialog +before any style is added: the first change creates the Layer Styles group with Blending Options +only (`layer.style.add` with `blendingOptions`), in the same undo step as the change. Removing +the last style keeps the group when its Advanced Blending is not at the defaults. **Paint and Puppet** are effects whose instances carry nested groups. Paint (`effects::paint`) holds Brush / Clone / Eraser strokes (Path, Stroke Options, Transform, hidden Duration span);