collective
Browse / github.com/storytold/photocraft / patch
fix(text): bring named type styles along when copying layers to another document
Copied text binds to unrelated named styles in the destination document `layer.copyToDocument` carries a text run's document-local named-style ID without importing or remapping its definition. Source ID 1 (`Source black 24`) consequently binds to unrelated destination ID 1 (`Destination red 48`). The copy initially looks correct, but Clear Override changes it from black 24 pt to red 48 pt. **Actual Photoshop 27.11.0's Layer > Duplicate Layer imports the source style beside the destination style and retains the correct association.** ### Reproduce 1. Create two white 1024×512 sRGB RGB8 documents at **72 ppi**. 2. In source, create Arial Regular `HHOOnn 123`, blac
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- bercedev (imported from pull request #9655, open)
- Base
07bd794686ea4db0b557c978d58173a62760e4ca- Size
- +132 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
a32fdaec9b2e48f83408a6ed
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/layer_copy_cmds.rs crates/engine/src/cmd/layer_copy_cmds/tests.rs
@@ -11,7 +11,8 @@ use std::collections::{HashMap, HashSet}; use photocraft_color::ColorMode;-use photocraft_doc::{Document, Layer, LayerContent, LayerId, SmartContentsId};+use photocraft_doc::text_styles::{CharacterStyleDef, ParagraphStyleDef};+use photocraft_doc::{Document, Layer, LayerContent, LayerId, SmartContentsId, TextStyles}; use photocraft_geom::Rect; use serde_json::{Value, json}; @@ -139,6 +140,52 @@ pub(crate) fn translate_copies(copies: &mut Document, destination: &Document, dx } } +/// Named type styles belong to their document, so a style id means nothing in another one+/// (#9620): rebinds the copied text to the destination's styles, importing each source style+/// it lacks under a fresh id. A reference with no source definition, or no free id, is dropped;+/// the runs keep their resolved attributes either way.+fn import_text_styles(copies: &mut Document, from: &TextStyles, into: &mut TextStyles) {+ let (mut chars, mut paras) = (HashMap::new(), HashMap::new());+ // An earlier copy may have imported the style under a numbered name ("Body 1"); that one counts too.+ let same_name =+ |name: &str, base: &str| name == base || name.strip_prefix(base).and_then(|n| n.strip_prefix(' ')).is_some_and(|n| n.parse::<u32>().is_ok());+ let ids: Vec<LayerId> = copies.walk().into_iter().map(|(_, _, l)| l.id).collect();+ for id in ids {+ let Some(LayerContent::Text(t)) = copies.layer_mut(id).map(|l| &mut l.content) else { continue };+ for r in &mut t.runs {+ if let Some(old) = r.style.style_sheet {+ r.style.style_sheet = *chars.entry(old).or_insert_with(|| {+ let def = from.char_style(old)?;+ if let Some(same) = into.character.iter().find(|d| same_name(&d.name, &def.name) && d.attrs == def.attrs) {+ return Some(same.id);+ }+ let id = into.next_char_id()?;+ let name = if into.character.iter().any(|d| d.name == def.name) { into.unique_name(false, &def.name) } else { def.name.clone() };+ into.character.push(CharacterStyleDef { id, name, attrs: def.attrs.clone() });+ Some(id)+ });+ }+ }+ for r in &mut t.paragraphs {+ // 0 is Basic Paragraph, which every document has.+ if let Some(old) = r.style.style_sheet.filter(|id| *id != 0) {+ r.style.style_sheet = *paras.entry(old).or_insert_with(|| {+ let def = from.para_style(old)?;+ if let Some(same) =+ into.paragraph.iter().find(|d| same_name(&d.name, &def.name) && d.para_attrs == def.para_attrs && d.char_attrs == def.char_attrs)+ {+ return Some(same.id);+ }+ let id = into.next_para_id()?;+ let name = if into.paragraph.iter().any(|d| d.name == def.name) { into.unique_name(true, &def.name) } else { def.name.clone() };+ into.paragraph.push(ParagraphStyleDef { id, name, para_attrs: def.para_attrs.clone(), char_attrs: def.char_attrs.clone() });+ Some(id)+ });+ }+ }+ }+}+ /// `layer.copyToDocument`. fn copy_to_document(s: &mut Session, p: &Value) -> Result<Value> { let dest = doc_index(s, p, "document")?.ok_or_else(|| bad("missing `document` (the destination's index)"))?;@@ -210,10 +257,13 @@ fn copy_to_document(s: &mut Session, p: &Value) -> Result<Value> { let (dx, dy) = (dx.round() as i32, dy.round() as i32); translate_copies(&mut copies, &ddoc, dx, dy); let patterns: Vec<_> = sdoc.patterns.iter().filter(|pat| !ddoc.patterns.iter().any(|d| d.id == pat.id)).cloned().collect();+ let mut text_styles = ddoc.text_styles.clone();+ import_text_styles(&mut copies, &sdoc.text_styles, &mut text_styles); s.set_active(dest); let label = if ids.len() == 1 { "Duplicate Layer" } else { "Duplicate Layers" }; let edited = s.edit(label, |doc, active| { doc.patterns.extend(patterns);+ doc.text_styles = text_styles; let mut above = *active; let mut new = Vec::with_capacity(copies.layers.len()); for l in copies.layers {@@ -263,6 +313,7 @@ pub(crate) fn duplicate_to_new_document(s: &mut Session, p: &Value) -> Result<Va nd.resolution_dpi = sdoc.resolution_dpi; nd.icc_profile.clone_from(&sdoc.icc_profile); nd.patterns.clone_from(&sdoc.patterns);+ nd.text_styles.clone_from(&sdoc.text_styles); nd.global_light = sdoc.global_light; for id in &ids { let mut l = sdoc.layer(*id).ok_or(EngineError::NoLayer(*id))?.duplicate();@@ -369,3 +369,83 @@ fn a_copied_group_cannot_nest_the_destination_past_the_group_depth_limit() { assert!(e.to_string().contains("deeper than"), "{e}"); assert_eq!(*doc(&s, 1), before, "nothing was copied"); }++/// #9620: style ids are per document, so the source's style 1 must not bind to the destination's.+#[test]+fn copied_text_brings_its_named_styles_instead_of_binding_the_destinations_ids() {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 200, "height": 100, "background": "transparent"})).unwrap();+ let text = s.execute("type.create", json!({"x": 10, "y": 50, "text": "Hello", "size": 24})).unwrap()["layer"].as_u64().unwrap();+ let cs = s.execute("type.characterStyle.new", json!({"name": "Source black 24", "fromSelection": false, "attrs": {"size": 24}})).unwrap()["id"]+ .as_u64()+ .unwrap();+ s.execute("type.characterStyle.apply", json!({"id": cs, "layer": text, "range": [0, 5]})).unwrap();+ let ps = s.execute("type.paragraphStyle.new", json!({"name": "Source body", "fromSelection": false, "attrs": {"align": "center"}})).unwrap()["id"]+ .as_u64()+ .unwrap();+ s.execute("type.paragraphStyle.apply", json!({"id": ps, "layer": text, "range": [0, 5]})).unwrap();+ s.execute("file.new", json!({"width": 200, "height": 100})).unwrap();+ let dcs = s.execute("type.characterStyle.new", json!({"name": "Destination red 48", "fromSelection": false, "attrs": {"size": 48}})).unwrap()["id"]+ .as_u64()+ .unwrap();+ let dps = s.execute("type.paragraphStyle.new", json!({"name": "Destination head", "fromSelection": false, "attrs": {"align": "right"}})).unwrap()["id"]+ .as_u64()+ .unwrap();+ assert_eq!((cs, ps), (dcs, dps), "both documents number their first styles 1");+ let copy = |s: &mut Session| {+ s.set_active(0);+ let r = s.execute("layer.copyToDocument", json!({"document": 1, "layers": [text]})).unwrap();+ let LayerContent::Text(t) = &doc(s, 1).layer(copied(&r)).unwrap().content else { panic!("not text") };+ (t.runs[0].style.style_sheet.unwrap(), t.paragraphs[0].style.style_sheet.unwrap())+ };+ let (c, p) = copy(&mut s);+ let st = &doc(&s, 1).text_styles;+ assert_ne!((c, p), (dcs as u32, dps as u32));+ assert_eq!(st.char_style(c).unwrap().name, "Source black 24");+ assert_eq!(st.para_style(p).unwrap().name, "Source body");+ assert_eq!(st.char_style(dcs as u32).unwrap().name, "Destination red 48", "the destination's own styles are untouched");+ assert_eq!(st.para_style(dps as u32).unwrap().name, "Destination head");+ // A second copy reuses the imported styles, and undo takes an import back with its copy.+ assert_eq!(copy(&mut s), (c, p));+ assert_eq!((doc(&s, 1).text_styles.character.len(), doc(&s, 1).text_styles.paragraph.len()), (2, 2));+ assert!(s.undo());+ assert!(s.undo());+ assert_eq!((doc(&s, 1).text_styles.character.len(), doc(&s, 1).text_styles.paragraph.len()), (1, 1));+}++/// A name the destination already uses for another style is imported once as "Body 1", and later+/// copies reuse it instead of adding "Body 2".+#[test]+fn a_clashing_style_name_is_imported_once_across_copies() {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 200, "height": 100, "background": "transparent"})).unwrap();+ let text = s.execute("type.create", json!({"x": 10, "y": 50, "text": "Hello", "size": 24})).unwrap()["layer"].as_u64().unwrap();+ let cs = s.execute("type.characterStyle.new", json!({"name": "Body", "fromSelection": false, "attrs": {"size": 24}})).unwrap()["id"].as_u64().unwrap();+ s.execute("type.characterStyle.apply", json!({"id": cs, "layer": text, "range": [0, 5]})).unwrap();+ s.execute("file.new", json!({"width": 200, "height": 100})).unwrap();+ s.execute("type.characterStyle.new", json!({"name": "Body", "fromSelection": false, "attrs": {"size": 48}})).unwrap();+ let copy = |s: &mut Session| {+ s.set_active(0);+ let r = s.execute("layer.copyToDocument", json!({"document": 1, "layers": [text]})).unwrap();+ let LayerContent::Text(t) = &doc(s, 1).layer(copied(&r)).unwrap().content else { panic!("not text") };+ t.runs[0].style.style_sheet.unwrap()+ };+ let first = copy(&mut s);+ assert_eq!(doc(&s, 1).text_styles.char_style(first).unwrap().name, "Body 1");+ assert_eq!(copy(&mut s), first, "the second copy reuses Body 1");+ assert_eq!(doc(&s, 1).text_styles.character.len(), 2);+}++#[test]+fn a_layer_duplicated_into_a_new_document_keeps_its_named_styles() {+ let mut s = Session::new();+ s.execute("file.new", json!({"width": 200, "height": 100, "background": "transparent"})).unwrap();+ let text = s.execute("type.create", json!({"x": 10, "y": 50, "text": "Hello", "size": 24})).unwrap()["layer"].as_u64().unwrap();+ let cs = s.execute("type.characterStyle.new", json!({"name": "Source black 24", "fromSelection": false, "attrs": {"size": 24}})).unwrap()["id"]+ .as_u64()+ .unwrap();+ s.execute("type.characterStyle.apply", json!({"id": cs, "layer": text, "range": [0, 5]})).unwrap();+ let r = s.execute("layer.duplicate", json!({"layer": text, "document": "new"})).unwrap();+ let d = doc(&s, r["document"].as_u64().unwrap() as usize);+ assert_eq!(d.text_styles.char_style(cs as u32).unwrap().name, "Source black 24");+}