From cc94fccda5fa85f2824554ea0a03c07b61b7d7df Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 15 Oct 2026 22:17:20 +0200 Subject: [PATCH 1/2] Layer Styles: Blending Options can be edited on a layer with no styles (#1389) --- crates/engine/src/cmd/styles.rs | 17 ++++++++-- crates/engine/src/tests/styles.rs | 27 ++++++++++++++++ crates/project/src/styles.rs | 15 +++++++-- .../ui-egui/src/panels/layer_styles_dialog.rs | 32 ++++++++++++++----- crates/ui-egui/tests/it/ui_tail.rs | 32 +++++++++++++++++++ docs/architecture.md | 4 ++- 6 files changed, 113 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index 82bbf5eba..cfe071c38 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); @@ -224,7 +237,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..2251d5982 100644 --- a/crates/engine/src/tests/styles.rs +++ b/crates/engine/src/tests/styles.rs @@ -117,3 +117,30 @@ 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()); +} diff --git a/crates/project/src/styles.rs b/crates/project/src/styles.rs index 83db5e581..d2c76ddf5 100644 --- a/crates/project/src/styles.rs +++ b/crates/project/src/styles.rs @@ -314,14 +314,23 @@ impl Layer { } } +/// 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). /// Returns the style group's uid; an existing style of that kind is kept and returned. 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 14507b868..21e0e7fa2 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}; @@ -134,6 +135,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); @@ -198,7 +203,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; }; @@ -220,13 +228,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 4fad74486..4b7218f2f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -243,7 +243,9 @@ 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. **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); From 09a93b6d6228ea0858e892387677027dd0c74b9a Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 15 Oct 2026 22:34:52 +0200 Subject: [PATCH 2/2] Layer Styles: changed Blending Options outlive the last style and only pixel-changing styles block a clip export (#1389) --- crates/engine/src/cmd/styles.rs | 5 +++-- crates/engine/src/tests/styles.rs | 21 +++++++++++++++++++++ crates/interchange/src/export.rs | 2 +- crates/interchange/src/tests.rs | 30 ++++++++++++++++++++++++++++++ crates/project/src/styles.rs | 16 ++++++++++++++++ docs/architecture.md | 3 ++- 6 files changed, 73 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index cfe071c38..4fcbcc83e 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -152,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); } } diff --git a/crates/engine/src/tests/styles.rs b/crates/engine/src/tests/styles.rs index 2251d5982..1aef9e825 100644 --- a/crates/engine/src/tests/styles.rs +++ b/crates/engine/src/tests/styles.rs @@ -144,3 +144,24 @@ fn blending_options_alone_create_the_group_in_one_undo_step() { 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 7da18913a..56e21d578 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 1e0c7d417..fc15ce579 100644 --- a/crates/interchange/src/tests.rs +++ b/crates/interchange/src/tests.rs @@ -429,6 +429,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 d2c76ddf5..ffedaf7c2 100644 --- a/crates/project/src/styles.rs +++ b/crates/project/src/styles.rs @@ -312,6 +312,22 @@ 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. diff --git a/docs/architecture.md b/docs/architecture.md index 4b7218f2f..d19059b01 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -245,7 +245,8 @@ opacity fades the whole stack; Knockout and the R/G/B channel switches apply at 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. 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. +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);