From e8ab6846c0b69bbf3805eef91c7cf484165ac9f4 Mon Sep 17 00:00:00 2001 From: David Mann Date: Sun, 18 Oct 2026 09:07:40 +0200 Subject: [PATCH] Place Scale Marker: lay out the label in points, not pixels (#5754) (#5755) fontSize is in points and type.create renders it at resolution/72 px per point, but the bar height, label height and baseline treated it as pixels. At 300 ppi a 12 pt label (50 px) overlapped the bar. Lay out in font_size * dpi / 72 px; type.create still gets points. 72 ppi is unchanged. Co-authored-by: Claude Opus 5.5 (1M context) --- crates/engine/src/analysis_cmds.rs | 10 +++++++--- crates/engine/src/analysis_cmds/tests.rs | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/analysis_cmds.rs b/crates/engine/src/analysis_cmds.rs index 85a6be4513..ae16b7e4e4 100644 --- a/crates/engine/src/analysis_cmds.rs +++ b/crates/engine/src/analysis_cmds.rs @@ -1121,8 +1121,12 @@ fn place_scale_marker(s: &mut Session, p: &Value) -> Result { }; // Bottom-left with a margin; the text sits under (or over) the bar. let margin = (w.min(h) * 0.03).round().max(2.0); - let bar_h = (font_size / 4.0).round().clamp(2.0, h / 4.0); - let text_h = if show_text { font_size * 1.25 } else { 0.0 }; + // `fontSize` is in points; `type.create` renders it at resolution/72 px per point, so lay out + // the bar and label in those pixels (at 300 ppi, 12 pt is 50 px) (#5754). + let dpi = f64::from(d.resolution_dpi); + let font_px = font_size * if dpi.is_finite() && dpi > 0.0 { dpi / 72.0 } else { 1.0 }; + let bar_h = (font_px / 4.0).round().clamp(2.0, h / 4.0); + let text_h = if show_text { font_px * 1.25 } else { 0.0 }; // Keep the bar on the canvas when the text under it doesn't fit; the label is clipped instead (#932). let bar_y = if text_top { h - margin - bar_h } else { h - margin - bar_h - text_h }.max(0.0); // Likewise pull a bar nearly as wide as the image in from the right edge (#932). @@ -1143,7 +1147,7 @@ fn place_scale_marker(s: &mut Session, p: &Value) -> Result { })?; let mut text_layer = Value::Null; if show_text { - let baseline = if text_top { f64::from(bar.y0) - font_size * 0.3 } else { f64::from(bar.y1) + font_size }; + let baseline = if text_top { f64::from(bar.y0) - font_px * 0.3 } else { f64::from(bar.y1) + font_px }; let mut tp = json!({"x": bar.x0, "y": baseline, "text": label, "size": font_size, "color": [rgb[0], rgb[1], rgb[2], 1.0], "name": label}); if let Some(f) = &font { tp["font"] = json!(f); diff --git a/crates/engine/src/analysis_cmds/tests.rs b/crates/engine/src/analysis_cmds/tests.rs index 124f9abf1e..388d5dac97 100644 --- a/crates/engine/src/analysis_cmds/tests.rs +++ b/crates/engine/src/analysis_cmds/tests.rs @@ -1,5 +1,5 @@ use super::*; -use photocraft_doc::ColorMode; +use photocraft_doc::{ColorMode, LayerContent, LayerId}; fn session(w: u32, h: u32, depth: u32) -> Session { let mut s = Session::new(); @@ -355,6 +355,27 @@ fn place_scale_marker_builds_one_group_in_one_step() { assert!(d(&s).layers.iter().all(|l| l.name != "Measurement Scale Marker")); } +#[test] +fn place_scale_marker_lays_out_its_label_in_points_at_high_resolution() { + // `fontSize` is in points and `type.create` renders it at resolution/72 px per point, but the + // layout treated it as pixels: at 300 ppi a 12 pt label (50 px) overlapped the bar. + for res in [72.0, 300.0] { + let mut s = Session::new(); + s.execute("file.new", json!({"width": 1200, "height": 900, "resolution": res, "background": "white"})).unwrap(); + let r = s.execute("image.analysis.placeScaleMarker", json!({"fontSize": 12, "textPosition": "bottom"})).unwrap(); + let bar_bottom = r["barRect"][3].as_f64().unwrap(); + let doc = d(&s); + let Some(LayerContent::Text(t)) = doc.layer(LayerId(r["text"].as_u64().unwrap())).map(|l| &l.content) else { panic!("a type layer: {r}") }; + assert_eq!(t.size_pt, 12.0, "{res} ppi: type.create still gets points"); + let layout = photocraft_text::shared().lock().unwrap().layout(t, doc.resolution_dpi); + let b = layout.bounds().expect("the label has a line"); + let ty = t.transform.m[5]; + let (top, bottom) = (f64::from(b[1]) + ty, f64::from(b[3]) + ty); + assert!(top >= bar_bottom - 0.5, "{res} ppi: label top {top} overlaps the bar ending at {bar_bottom}: {r}"); + assert!(bottom <= 900.5, "{res} ppi: label bottom {bottom} stays on the canvas: {r}"); + } +} + #[test] fn place_scale_marker_on_a_short_document_errors_without_panicking() { // A document under 8 px tall inverted the bar-height clamp and panicked (#932). Call `run`