Properly track the Text node's text frame via the attribute system to fix misalignment (#4097)

* Compensate for upstream row-0 transform absorption in viewport-space 'TransformSet'

* Add 'editor:text_frame' row attribute so the Text tool's drag cage tracks multi-row text

* "Separate Glyph Elements" -> "Separate Glyphs"

* Improve artboard migration robustness from older documents

* Code review

* Make the tools visualize the text frame based on attribute not upstream node
This commit is contained in:
Keavon Chambers
2026-05-02 21:04:30 -07:00
committed by GitHub
parent de2ae29edd
commit ebdb835890
17 changed files with 169 additions and 59 deletions

View File

@@ -34,8 +34,8 @@ use std::any::TypeId;
use std::future::Future;
use std::pin::Pin;
pub use table::{
ATTR_BACKGROUND, ATTR_BLEND_MODE, ATTR_CLIP, ATTR_CLIPPING_MASK, ATTR_DIMENSIONS, ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_END, ATTR_GRADIENT_TYPE,
ATTR_LOCATION, ATTR_NAME, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_START, ATTR_TRANSFORM, ATTR_TYPE,
ATTR_BACKGROUND, ATTR_BLEND_MODE, ATTR_CLIP, ATTR_CLIPPING_MASK, ATTR_DIMENSIONS, ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_EDITOR_TEXT_FRAME, ATTR_END,
ATTR_GRADIENT_TYPE, ATTR_LOCATION, ATTR_NAME, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_START, ATTR_TRANSFORM, ATTR_TYPE,
};
#[cfg(feature = "wasm")]
pub use tsify;

View File

@@ -41,6 +41,12 @@ pub const ATTR_EDITOR_MERGED_LAYERS: &str = "editor:merged_layers";
/// by clicking anywhere within their bounds, not just the filled letterform.
pub const ATTR_EDITOR_CLICK_TARGET: &str = "editor:click_target";
/// `DAffine2` mapping the unit square `[(0, 0), (1, 1)]` (top-left convention) onto the 'Text'
/// node's text frame in this row's local space. Each row carries the frame relative to its own
/// glyph origin so it survives `Index Elements` filtering. The Text tool reads this to position
/// its drag cage. Stored as an affine to allow non-axis-aligned frames in the future.
pub const ATTR_EDITOR_TEXT_FRAME: &str = "editor:text_frame";
/// Byte offset where a regex match begins ('Regex Find All', 'Regex Capture' text nodes).
pub const ATTR_START: &str = "start";

View File

@@ -11,8 +11,8 @@ use core_types::table::{Table, TableRow};
use core_types::transform::Footprint;
use core_types::uuid::{NodeId, generate_uuid};
use core_types::{
ATTR_BACKGROUND, ATTR_BLEND_MODE, ATTR_CLIP, ATTR_CLIPPING_MASK, ATTR_DIMENSIONS, ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_GRADIENT_TYPE, ATTR_LOCATION,
ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_TRANSFORM,
ATTR_BACKGROUND, ATTR_BLEND_MODE, ATTR_CLIP, ATTR_CLIPPING_MASK, ATTR_DIMENSIONS, ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_EDITOR_TEXT_FRAME,
ATTR_GRADIENT_TYPE, ATTR_LOCATION, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_TRANSFORM,
};
use dyn_any::DynAny;
use glam::{DAffine2, DVec2};
@@ -327,6 +327,9 @@ pub struct RenderMetadata {
/// Source-geometry outlines for hover/selection overlays, separate from `click_targets` so
/// nodes with an `editor:click_target` override still outline the precise geometry.
pub outlines: HashMap<NodeId, Vec<Arc<ClickTarget>>>,
/// Per-layer text frame from row 0's `editor:text_frame` attribute.
/// The Text tool composes this with `transform_to_viewport(layer)` to position its drag cage.
pub text_frames: HashMap<NodeId, DAffine2>,
pub clip_targets: HashSet<NodeId>,
pub vector_data: HashMap<NodeId, Arc<Vector>>,
pub backgrounds: Vec<Background>,
@@ -349,6 +352,7 @@ impl RenderMetadata {
first_element_source_id,
click_targets,
outlines,
text_frames,
clip_targets,
vector_data,
backgrounds,
@@ -358,6 +362,7 @@ impl RenderMetadata {
first_element_source_id.extend(other.first_element_source_id.iter());
click_targets.extend(other.click_targets.iter().map(|(k, v)| (*k, v.clone())));
outlines.extend(other.outlines.iter().map(|(k, v)| (*k, v.clone())));
text_frames.extend(other.text_frames.iter());
clip_targets.extend(other.clip_targets.iter());
vector_data.extend(other.vector_data.iter().map(|(id, data)| (*id, data.clone())));
@@ -1366,7 +1371,7 @@ impl Render for Table<Vector> {
}
fn collect_metadata(&self, metadata: &mut RenderMetadata, footprint: Footprint, caller_element_id: Option<NodeId>) {
// Aggregate all items' targets per element_id so multi-item tables (e.g. 'Text' node with "Separate Glyph Elements" active) produce hit areas for every glyph.
// Aggregate all items' targets per element_id so multi-item tables (e.g. 'Text' node with "Separate Glyphs" active) produce hit areas for every glyph.
// Targets are baked relative to item 0's transform since `Graphic::collect_metadata` records that as `local_transforms[element_id]`.
let item_zero_transform: DAffine2 = if !self.is_empty() {
self.attribute_cloned_or_default(ATTR_TRANSFORM, 0)
@@ -1414,6 +1419,11 @@ impl Render for Table<Vector> {
// Source geometry (not the click-target override) so editing tools work on letterforms.
// Only item 0 is recorded since editing tools can only target a single item currently.
metadata.vector_data.entry(element_id).or_insert_with(|| Arc::new(source.clone()));
// Surface `editor:text_frame` for the Text tool's drag cage
if let Some(&frame) = self.attribute::<DAffine2>(ATTR_EDITOR_TEXT_FRAME, index) {
metadata.text_frames.entry(element_id).or_insert(frame);
}
}
// If this item carries a snapshot of upstream graphic content (e.g. it was produced by Boolean Operation,

View File

@@ -59,8 +59,8 @@ fn text<'i: 'n>(
/// To have an effect on a single line of text, *Max Width* must be set.
#[widget(ParsedWidgetOverride::Custom = "text_align")]
align: TextAlign,
/// Whether to split every letterform into its own vector path element. Otherwise, a single compound path is produced.
separate_glyph_elements: bool,
/// Whether to split every letterform into its own vector item. Otherwise, a single vector compound path is produced.
separate_glyphs: bool,
) -> Table<Vector> {
let typesetting = TypesettingConfig {
font_size: size,
@@ -72,5 +72,5 @@ fn text<'i: 'n>(
align,
};
to_path(&text, &font, &editor_resources.font_cache, typesetting, separate_glyph_elements)
to_path(&text, &font, &editor_resources.font_cache, typesetting, separate_glyphs)
}

View File

@@ -1,5 +1,5 @@
use core_types::table::{Table, TableRow};
use core_types::{ATTR_EDITOR_CLICK_TARGET, ATTR_TRANSFORM};
use core_types::{ATTR_EDITOR_CLICK_TARGET, ATTR_EDITOR_TEXT_FRAME, ATTR_TRANSFORM};
use glam::{DAffine2, DVec2};
use parley::GlyphRun;
use skrifa::GlyphId;
@@ -15,19 +15,26 @@ pub struct PathBuilder {
origin: DVec2,
glyph_subpaths: Vec<Subpath<PointId>>,
pub vector_table: Table<Vector>,
/// Per-glyph bbox rectangles collected in single-row mode, published as `ATTR_EDITOR_CLICK_TARGET` in `finalize()`.
/// Per-glyph bbox rectangles collected in single-item mode, published as `ATTR_EDITOR_CLICK_TARGET` in `finalize()`.
merged_click_target_subpaths: Vec<Subpath<PointId>>,
/// Text frame size, stamped per item as `ATTR_EDITOR_TEXT_FRAME` relative to each item's origin.
text_frame_size: DVec2,
/// First glyph's baseline offset (pre-height-filter). Used for the empty placeholder item so
/// `local_transforms` stays stable when all glyphs are clipped during a resize drag.
first_glyph_offset: DVec2,
scale: f64,
id: PointId,
}
impl PathBuilder {
pub fn new(per_glyph_items: bool, scale: f64) -> Self {
pub fn new(per_glyph_items: bool, scale: f64, text_frame_size: DVec2, first_glyph_offset: DVec2) -> Self {
Self {
current_subpath: Subpath::new(Vec::new(), false),
glyph_subpaths: Vec::new(),
vector_table: if per_glyph_items { Table::new() } else { Table::new_from_element(Vector::default()) },
merged_click_target_subpaths: Vec::new(),
text_frame_size,
first_glyph_offset,
scale,
id: PointId::ZERO,
origin: DVec2::default(),
@@ -58,12 +65,18 @@ impl PathBuilder {
let glyph_bbox_rectangle = subpaths_bounding_box(&self.glyph_subpaths).map(|[min, max]| Subpath::new_rectangle(min, max));
if per_glyph_items {
let row = TableRow::new_from_element(Vector::from_subpaths(core::mem::take(&mut self.glyph_subpaths), false)).with_attribute(ATTR_TRANSFORM, DAffine2::from_translation(glyph_offset));
let row = match glyph_bbox_rectangle {
Some(rect) => row.with_attribute(ATTR_EDITOR_CLICK_TARGET, Vector::from_subpaths([rect], false)),
None => row,
// Frame in item-local space: top-left at `-glyph_offset` so the item transform cancels it
// back to the layer-local frame origin, regardless of which glyph survived
let frame_in_item_local = DAffine2::from_scale_angle_translation(self.text_frame_size, 0., -glyph_offset);
let item = TableRow::new_from_element(Vector::from_subpaths(core::mem::take(&mut self.glyph_subpaths), false))
.with_attribute(ATTR_TRANSFORM, DAffine2::from_translation(glyph_offset))
.with_attribute(ATTR_EDITOR_TEXT_FRAME, frame_in_item_local);
let item = match glyph_bbox_rectangle {
Some(rect) => item.with_attribute(ATTR_EDITOR_CLICK_TARGET, Vector::from_subpaths([rect], false)),
None => item,
};
self.vector_table.push(row);
self.vector_table.push(item);
} else {
for subpath in self.glyph_subpaths.drain(..) {
// Unwrapping here is ok because `self.vector_table` is initialized with a single `Table<Vector>` item
@@ -130,16 +143,31 @@ impl PathBuilder {
}
pub fn finalize(mut self) -> Table<Vector> {
// Empty table = all glyphs clipped by height. Create a placeholder with the same item-0
// transform a populated table would have so `local_transforms` stays stable mid-drag.
// TODO: Remove this hack and move the attribute up to the parent return value when <https://github.com/GraphiteEditor/Graphite/issues/3779> is done.
if self.vector_table.is_empty() {
self.vector_table = Table::new_from_element(Vector::default());
let frame_in_item_local = DAffine2::from_scale_angle_translation(self.text_frame_size, 0., -self.first_glyph_offset);
let item = TableRow::new_from_element(Vector::default())
.with_attribute(ATTR_TRANSFORM, DAffine2::from_translation(self.first_glyph_offset))
.with_attribute(ATTR_EDITOR_TEXT_FRAME, frame_in_item_local);
self.vector_table.push(item);
}
// With "Separate Glyph Elements" inactive, combine the accumulated per-glyph AABBs as one override `Vector`
// With "Separate Glyphs" inactive, combine the accumulated per-glyph AABBs as one override `Vector`
if !self.merged_click_target_subpaths.is_empty() {
self.vector_table
.set_attribute(ATTR_EDITOR_CLICK_TARGET, 0, Vector::from_subpaths(self.merged_click_target_subpaths, false));
}
// Fill in text frame for items that don't have one yet (single-item mode, where item 0 = identity)
let frame = DAffine2::from_scale(self.text_frame_size);
for index in 0..self.vector_table.len() {
if self.vector_table.attribute::<DAffine2>(ATTR_EDITOR_TEXT_FRAME, index).is_none() {
self.vector_table.set_attribute(ATTR_EDITOR_TEXT_FRAME, index, frame);
}
}
self.vector_table
}
}

View File

@@ -92,7 +92,23 @@ impl TextContext {
return Table::new_from_element(Vector::default());
};
let mut path_builder = PathBuilder::new(per_glyph_items, layout.scale() as f64);
let text_frame_size = DVec2::new(
typesetting.max_width.unwrap_or_else(|| layout.full_width() as f64),
typesetting.max_height.unwrap_or_else(|| layout.height() as f64),
);
// First glyph offset (pre-height-filter) so the empty placeholder item in `per_glyph_items`
// mode keeps the same item 0's transform, preventing `local_transforms` from jumping mid-drag
let first_glyph_offset = layout
.lines()
.flat_map(|line| line.items())
.find_map(|item| match item {
PositionedLayoutItem::GlyphRun(run) => run.glyphs().next().map(|glyph| DVec2::new((run.offset() + glyph.x) as f64, (run.baseline() - glyph.y) as f64)),
_ => None,
})
.unwrap_or_default();
let mut path_builder = PathBuilder::new(per_glyph_items, layout.scale() as f64, text_frame_size, first_glyph_offset);
for line in layout.lines() {
for item in line.items() {