diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index ebc7c49640..b5878fa3db 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -2061,7 +2061,7 @@ pub fn specs() -> Vec { "New Guide Layout…", &["View"], None, - r##"{"columns":n=0,"width":px?,"gutter":px=0,"rows":n=0,"height":px?,"rowGutter":px=gutter,"margin":px|[top,left,bottom,right]=0,"centerColumns":bool=false,"clearExisting":bool=false,"target":"document|selectedArtboards"="document"}"##, + r##"{"columns":n=0,"width":len?,"gutter":len=0,"rows":n=0,"height":len?,"rowGutter":len=gutter,"margin":len|[top,left,bottom,right]=0,"centerColumns":bool=false,"clearExisting":bool=false,"target":"document|selectedArtboards"="document"} (len: pixels, or text with px, in, cm, mm, pt, pc or % as typed in Photoshop, e.g. "1 cm"; % is of the target's width for columns, gutters and left/right margins, its height for rows and top/bottom margins)"##, has_doc, new_guide_layout ), diff --git a/crates/engine/src/file_cmds/tests.rs b/crates/engine/src/file_cmds/tests.rs index 9132af44d9..f034a62656 100644 --- a/crates/engine/src/file_cmds/tests.rs +++ b/crates/engine/src/file_cmds/tests.rs @@ -1242,6 +1242,50 @@ fn exported_selected_cube_round_trips_through_lookup_importer() { assert_eq!(s.active().unwrap().doc.layers.len(), 3); } +/// #4984: New Guide Layout takes lengths with units as typed in Photoshop's dialog, at the +/// document's resolution; percent is of the target's width or height, by axis. +#[test] +fn guide_layouts_take_units() { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 1200, "height": 600, "resolution": 300})).unwrap(); + let close = |a: &[f64], b: &[f64]| a.len() == b.len() && a.iter().zip(b).all(|(x, y)| (x - y).abs() < 0.01); + // 1 cm at 300 ppi is 118.11 px; 0.25 in is 75 px. + s.execute("view.newGuideLayout", json!({"columns": 2, "gutter": "1 cm", "margin": "0.25in"})).unwrap(); + let g = &doc(&s).guides; + let col = (1050.0 - 118.110_236) / 2.0; + assert!(close(&g.vertical, &[75.0, 1125.0, 75.0 + col, 75.0 + col + 118.110_236]), "{:?}", g.vertical); + assert!(close(&g.horizontal, &[75.0, 525.0]), "{:?}", g.horizontal); + // Percent of the width for columns, of the height for rows and top/bottom margins; points + // follow Units & Rulers (72 per inch: 36 pt is 150 px, centred like the columns). + s.execute( + "view.newGuideLayout", + json!({"columns": 1, "width": "25%", "centerColumns": true, "rows": 1, "height": "36 pt", "margin": ["10%", 0, "10%", 0], "clearExisting": true}), + ) + .unwrap(); + let g = &doc(&s).guides; + assert!(close(&g.vertical, &[0.0, 1200.0, 450.0, 750.0]), "{:?}", g.vertical); + assert!(close(&g.horizontal, &[60.0, 540.0, 225.0, 375.0]), "{:?}", g.horizontal); + // With selected artboards, percent is of each board, not of the document. + s.execute("layer.new.artboard", json!({"rect": [200, 100, 140, 40]})).unwrap(); + s.execute( + "view.newGuideLayout", + json!({"columns": 1, "width": "50%", "centerColumns": true, "rows": 1, "height": "50%", "target": "selectedArtboards", "clearExisting": true}), + ) + .unwrap(); + let g = &doc(&s).guides; + // Clear Existing removes guides inside the board only; those outside it stay. + assert!(close(&g.vertical, &[0.0, 1200.0, 450.0, 750.0, 235.0, 305.0]), "{:?}", g.vertical); + assert!(close(&g.horizontal, &[60.0, 540.0, 225.0, 375.0, 110.0, 130.0]), "{:?}", g.horizontal); + for bad in [ + json!({"columns": 2, "gutter": "1 furlong"}), + json!({"columns": 2, "gutter": "-1 cm"}), + json!({"columns": 2, "margin": [0, "x", 0, 0]}), + json!({"columns": 2, "gutter": true}), + ] { + assert!(s.execute("view.newGuideLayout", bad.clone()).is_err(), "{bad}"); + } +} + #[test] fn guide_layouts() { let mut s = session(100, 50, 8); diff --git a/crates/engine/src/guide_layout.rs b/crates/engine/src/guide_layout.rs index 4a4261faff..6c4eb89c00 100644 --- a/crates/engine/src/guide_layout.rs +++ b/crates/engine/src/guide_layout.rs @@ -4,6 +4,7 @@ use photocraft_doc::Guides; use photocraft_geom::Rect; use serde_json::Value; +use crate::prefs::Unit; use crate::{EngineError, Result, Session}; const COMMAND: &str = "view.newGuideLayout"; @@ -14,10 +15,46 @@ fn bad(message: impl Into) -> EngineError { EngineError::BadParams { cmd: COMMAND.into(), msg: message.into() } } -fn number(p: &Value, key: &str, default: f64) -> Result { +/// A length as given: a number of pixels, or text with a unit as typed in Photoshop's New Guide +/// Layout fields ("2 cm", "0.25in", "12 pt", "10%"), resolved against each target (#4984). +#[derive(Clone, Copy, Debug, PartialEq)] +enum Length { + Px(f64), + In(f64, Unit), +} + +impl Length { + /// Pixels at the document's `dpi`; percent is of `extent` (the target's width for column + /// sizes and left/right margins, its height for row sizes and top/bottom margins). + fn px(self, key: &str, dpi: f64, extent: f64, ppi: f64) -> Result { + let v = match self { + Length::Px(v) => v, + Length::In(v, unit) => unit.to_px(v, dpi, extent, ppi), + }; + if v.is_finite() && v >= 0.0 { Ok(v) } else { Err(bad(format!("{key} must be a finite non-negative length"))) } + } +} + +fn length_of(key: &str, v: &Value) -> Result { + let invalid = || bad(format!("{key} must be a non-negative number of pixels, or text with px, in, cm, mm, pt, pc or %")); + let length = match (v.as_f64(), v.as_str()) { + (Some(n), _) => Length::Px(n), + (None, Some(text)) => match Unit::parse_length(text).ok_or_else(invalid)? { + (n, None | Some(Unit::Pixels)) => Length::Px(n), + (n, Some(unit)) => Length::In(n, unit), + }, + _ => return Err(invalid()), + }; + match length { + Length::Px(n) | Length::In(n, _) if n.is_finite() && n >= 0.0 => Ok(length), + _ => Err(invalid()), + } +} + +fn number(p: &Value, key: &str, default: Length) -> Result { match p.get(key) { None | Some(Value::Null) => Ok(default), - Some(v) => v.as_f64().filter(|n| n.is_finite() && *n >= 0.0).ok_or_else(|| bad(format!("{key} must be a finite non-negative number"))), + Some(v) => length_of(key, v), } } @@ -110,9 +147,10 @@ pub fn calculate(s: &Session, p: &Value) -> Result { let st = s.active().ok_or(EngineError::NoDocument)?; let cols = count(p, "columns")?; let rows = count(p, "rows")?; - let width = number(p, "width", 0.0)?; - let height = number(p, "height", 0.0)?; - let gap = number(p, "gutter", 0.0)?; + let zero = Length::Px(0.0); + let width = number(p, "width", zero)?; + let height = number(p, "height", zero)?; + let gap = number(p, "gutter", zero)?; let row_gap = number(p, "rowGutter", gap)?; let center = flag(p, "centerColumns")?; let clear = flag(p, "clearExisting")?; @@ -121,15 +159,16 @@ pub fn calculate(s: &Session, p: &Value) -> Result { if a.len() != 4 { return Err(bad("margin must contain top, left, bottom and right")); } - let mut m = [0.0; 4]; + let mut m = [zero; 4]; for (out, value) in m.iter_mut().zip(a) { - *out = value.as_f64().filter(|v| v.is_finite() && *v >= 0.0).ok_or_else(|| bad("margins must be finite non-negative numbers"))?; + *out = length_of("margin", value)?; } m } - _ => [number(p, "margin", 0.0)?; 4], + _ => [number(p, "margin", zero)?; 4], }; - let [top, left, bottom, right] = margins; + let dpi = f64::from(st.doc.resolution_dpi); + let ppi = s.prefs().units_and_rulers.point_size.per_inch(); let bounds = targets(s, p)?; // Bound the aggregate work as well as each count, including multi-artboard requests. if (cols + rows + 4).saturating_mul(2).saturating_mul(bounds.len() as u64) > MAX_EDGES { @@ -137,16 +176,22 @@ pub fn calculate(s: &Session, p: &Value) -> Result { } let mut added = Guides::default(); for rect in &bounds { + let (across, down) = (f64::from(rect.width()), f64::from(rect.height())); + let [top, left, bottom, right] = + [(margins[0], down), (margins[1], across), (margins[2], down), (margins[3], across)].map(|(m, extent)| m.px("margin", dpi, extent, ppi)); + let (top, left, bottom, right) = (top?, left?, bottom?, right?); let (x0, y0, x1, y1) = (f64::from(rect.x0) + left, f64::from(rect.y0) + top, f64::from(rect.x1) - right, f64::from(rect.y1) - bottom); if x0 >= x1 || y0 >= y1 { return Err(bad("margins must leave space inside the target")); } - if margins.iter().any(|x| *x != 0.0) { + if [top, left, bottom, right].iter().any(|x| *x != 0.0) { push_unique(&mut added.vertical, x0)?; push_unique(&mut added.vertical, x1)?; push_unique(&mut added.horizontal, y0)?; push_unique(&mut added.horizontal, y1)?; } + let (width, gap) = (width.px("width", dpi, across, ppi)?, gap.px("gutter", dpi, across, ppi)?); + let (height, row_gap) = (height.px("height", dpi, down, ppi)?, row_gap.px("rowGutter", dpi, down, ppi)?); axis(x0, x1, cols, width, gap, center, &mut added.vertical)?; axis(y0, y1, rows, height, row_gap, center, &mut added.horizontal)?; }