Skip to content

Commit 4e47b5d

Browse files
authored
Restructure gcore/text module and fix memory leak (#3221)
* Restructure gcore/text module and fix memory leak * Remove unused import * Fix default font fallback causing wrong caching and rename to TextContext * Upgrade demo art
1 parent d595089 commit 4e47b5d

File tree

14 files changed

+381
-342
lines changed

14 files changed

+381
-342
lines changed

demo-artwork/parametric-dunescape.graphite

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

demo-artwork/procedural-string-lights.graphite

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

demo-artwork/red-dress.graphite

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

demo-artwork/valley-of-spires.graphite

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

editor/src/messages/portfolio/document/overlays/overlays_message_handler.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ pub struct OverlaysMessageHandler {
1515
canvas: Option<web_sys::HtmlCanvasElement>,
1616
#[cfg(target_family = "wasm")]
1717
context: Option<web_sys::CanvasRenderingContext2d>,
18+
#[cfg(all(not(target_family = "wasm"), not(test)))]
19+
context: Option<super::utility_types::OverlayContext>,
1820
}
1921

2022
#[message_handler_data]
@@ -80,7 +82,11 @@ impl MessageHandler<OverlaysMessage, OverlaysMessageContext<'_>> for OverlaysMes
8082

8183
let size = ipp.viewport_bounds.size();
8284

83-
let overlay_context = OverlayContext::new(size, device_pixel_ratio, visibility_settings);
85+
if self.context.is_none() {
86+
self.context = Some(OverlayContext::new(size, device_pixel_ratio, visibility_settings));
87+
}
88+
89+
let overlay_context = self.context.as_mut().unwrap();
8490

8591
if visibility_settings.all() {
8692
responses.add(DocumentMessage::GridOverlays { context: overlay_context.clone() });
@@ -89,7 +95,7 @@ impl MessageHandler<OverlaysMessage, OverlaysMessageContext<'_>> for OverlaysMes
8995
responses.add(provider(overlay_context.clone()));
9096
}
9197
}
92-
responses.add(FrontendMessage::RenderOverlays { context: overlay_context });
98+
responses.add(FrontendMessage::RenderOverlays { context: overlay_context.clone() });
9399
}
94100
#[cfg(all(not(target_family = "wasm"), test))]
95101
OverlaysMessage::Draw => {

editor/src/messages/portfolio/document/overlays/utility_types_vello.rs

Lines changed: 28 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ use graphene_std::Color;
1212
use graphene_std::math::quad::Quad;
1313
use graphene_std::subpath::{self, Subpath};
1414
use graphene_std::table::Table;
15-
use graphene_std::text::{TextAlign, TypesettingConfig, load_font, to_path};
15+
use graphene_std::text::TextContext;
16+
use graphene_std::text::{Font, FontCache, TextAlign, TypesettingConfig};
1617
use graphene_std::vector::click_target::ClickTargetType;
1718
use graphene_std::vector::misc::point_to_dvec2;
1819
use graphene_std::vector::{PointId, SegmentId, Vector};
@@ -215,7 +216,7 @@ impl OverlayContext {
215216

216217
pub fn take_scene(self) -> Scene {
217218
let mut internal = self.internal.lock().expect("Failed to lock internal overlay context");
218-
std::mem::take(&mut *internal).scene
219+
std::mem::take(&mut internal.scene)
219220
}
220221

221222
fn internal(&'_ self) -> MutexGuard<'_, OverlayContextInternal> {
@@ -411,26 +412,31 @@ pub(super) struct OverlayContextInternal {
411412
size: DVec2,
412413
device_pixel_ratio: f64,
413414
visibility_settings: OverlaysVisibilitySettings,
415+
font_cache: FontCache,
416+
thread_text: TextContext,
414417
}
415418

416419
impl Default for OverlayContextInternal {
417420
fn default() -> Self {
418-
Self {
419-
scene: Scene::new(),
420-
size: DVec2::ZERO,
421-
device_pixel_ratio: 1.0,
422-
visibility_settings: OverlaysVisibilitySettings::default(),
423-
}
421+
Self::new(DVec2::new(100., 100.), 1., OverlaysVisibilitySettings::default())
424422
}
425423
}
426424

427425
impl OverlayContextInternal {
428426
pub(super) fn new(size: DVec2, device_pixel_ratio: f64, visibility_settings: OverlaysVisibilitySettings) -> Self {
427+
let mut font_cache = FontCache::default();
428+
// Initialize with the hardcoded font used by overlay text
429+
const FONT_DATA: &[u8] = include_bytes!("source-sans-pro-regular.ttf");
430+
let font = Font::new("Source Sans Pro".to_string(), "Regular".to_string());
431+
font_cache.insert(font, String::new(), FONT_DATA.to_vec());
432+
429433
Self {
430434
scene: Scene::new(),
431435
size,
432436
device_pixel_ratio,
433437
visibility_settings,
438+
font_cache,
439+
thread_text: TextContext::default(),
434440
}
435441
}
436442

@@ -1007,7 +1013,7 @@ impl OverlayContextInternal {
10071013
self.scene.fill(peniko::Fill::NonZero, self.get_transform(), &brush, None, &path);
10081014
}
10091015

1010-
fn get_width(&self, text: &str) -> f64 {
1016+
fn get_width(&mut self, text: &str) -> f64 {
10111017
// Use the actual text-to-path system to get precise text width
10121018
const FONT_SIZE: f64 = 12.0;
10131019

@@ -1024,13 +1030,9 @@ impl OverlayContextInternal {
10241030
// Load Source Sans Pro font data
10251031
// TODO: Grab this from the node_modules folder (either with `include_bytes!` or ideally at runtime) instead of checking the font file into the repo.
10261032
// TODO: And maybe use the WOFF2 version (if it's supported) for its smaller, compressed file size.
1027-
const FONT_DATA: &[u8] = include_bytes!("source-sans-pro-regular.ttf");
1028-
let font_blob = Some(load_font(FONT_DATA));
1029-
1030-
// Convert text to paths and calculate actual bounds
1031-
let text_table = to_path(text, font_blob, typesetting, false);
1032-
let text_bounds = self.calculate_text_bounds(&text_table);
1033-
text_bounds.width()
1033+
let font = Font::new("Source Sans Pro".to_string(), "Regular".to_string());
1034+
let bounds = self.thread_text.bounding_box(text, &font, &self.font_cache, typesetting, false);
1035+
bounds.x
10341036
}
10351037

10361038
fn text(&mut self, text: &str, font_color: &str, background_color: Option<&str>, transform: DAffine2, padding: f64, pivot: [Pivot; 2]) {
@@ -1051,15 +1053,17 @@ impl OverlayContextInternal {
10511053
// Load Source Sans Pro font data
10521054
// TODO: Grab this from the node_modules folder (either with `include_bytes!` or ideally at runtime) instead of checking the font file into the repo.
10531055
// TODO: And maybe use the WOFF2 version (if it's supported) for its smaller, compressed file size.
1054-
const FONT_DATA: &[u8] = include_bytes!("source-sans-pro-regular.ttf");
1055-
let font_blob = Some(load_font(FONT_DATA));
1056+
let font = Font::new("Source Sans Pro".to_string(), "Regular".to_string());
1057+
1058+
// Get text dimensions directly from layout
1059+
let text_size = self.thread_text.bounding_box(text, &font, &self.font_cache, typesetting, false);
1060+
let text_width = text_size.x;
1061+
let text_height = text_size.y;
1062+
// Create a rect from the size (assuming text starts at origin)
1063+
let text_bounds = kurbo::Rect::new(0.0, 0.0, text_width, text_height);
10561064

1057-
// Convert text to vector paths using the existing text system
1058-
let text_table = to_path(text, font_blob, typesetting, false);
1059-
// Calculate text bounds from the generated paths
1060-
let text_bounds = self.calculate_text_bounds(&text_table);
1061-
let text_width = text_bounds.width();
1062-
let text_height = text_bounds.height();
1065+
// Convert text to vector paths for rendering
1066+
let text_table = self.thread_text.to_path(text, &font, &self.font_cache, typesetting, false);
10631067

10641068
// Calculate position based on pivot
10651069
let mut position = DVec2::ZERO;
@@ -1094,56 +1098,6 @@ impl OverlayContextInternal {
10941098
self.render_text_paths(&text_table, font_color, vello_transform);
10951099
}
10961100

1097-
// Calculate bounds of text from vector table
1098-
fn calculate_text_bounds(&self, text_table: &Table<Vector>) -> kurbo::Rect {
1099-
let mut min_x = f64::INFINITY;
1100-
let mut min_y = f64::INFINITY;
1101-
let mut max_x = f64::NEG_INFINITY;
1102-
let mut max_y = f64::NEG_INFINITY;
1103-
1104-
for row in text_table.iter() {
1105-
// Use the existing segment_bezier_iter to get all bezier curves
1106-
for (_, bezier, _, _) in row.element.segment_bezier_iter() {
1107-
let transformed_bezier = bezier.apply_transformation(|point| row.transform.transform_point2(point));
1108-
1109-
// Add start and end points to bounds
1110-
let points = [transformed_bezier.start, transformed_bezier.end];
1111-
for point in points {
1112-
min_x = min_x.min(point.x);
1113-
min_y = min_y.min(point.y);
1114-
max_x = max_x.max(point.x);
1115-
max_y = max_y.max(point.y);
1116-
}
1117-
1118-
// Add handle points if they exist
1119-
match transformed_bezier.handles {
1120-
subpath::BezierHandles::Quadratic { handle } => {
1121-
min_x = min_x.min(handle.x);
1122-
min_y = min_y.min(handle.y);
1123-
max_x = max_x.max(handle.x);
1124-
max_y = max_y.max(handle.y);
1125-
}
1126-
subpath::BezierHandles::Cubic { handle_start, handle_end } => {
1127-
for handle in [handle_start, handle_end] {
1128-
min_x = min_x.min(handle.x);
1129-
min_y = min_y.min(handle.y);
1130-
max_x = max_x.max(handle.x);
1131-
max_y = max_y.max(handle.y);
1132-
}
1133-
}
1134-
_ => {}
1135-
}
1136-
}
1137-
}
1138-
1139-
if min_x.is_finite() && min_y.is_finite() && max_x.is_finite() && max_y.is_finite() {
1140-
kurbo::Rect::new(min_x, min_y, max_x, max_y)
1141-
} else {
1142-
// Fallback for empty text
1143-
kurbo::Rect::new(0.0, 0.0, 0.0, 12.0)
1144-
}
1145-
}
1146-
11471101
// Render text paths to the vello scene using existing infrastructure
11481102
fn render_text_paths(&mut self, text_table: &Table<Vector>, font_color: &str, base_transform: kurbo::Affine) {
11491103
let color = Self::parse_color(font_color);

editor/src/messages/tool/common_functionality/utility_functions.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use graph_craft::document::value::TaggedValue;
1515
use graphene_std::renderer::Quad;
1616
use graphene_std::subpath::{Bezier, BezierHandles};
1717
use graphene_std::table::Table;
18-
use graphene_std::text::{FontCache, load_font};
18+
use graphene_std::text::FontCache;
1919
use graphene_std::vector::algorithms::bezpath_algorithms::pathseg_compute_lookup_table;
2020
use graphene_std::vector::misc::{HandleId, ManipulatorPointId, dvec2_to_point};
2121
use graphene_std::vector::{HandleExt, PointId, SegmentId, Vector, VectorModification, VectorModificationType};
@@ -74,8 +74,7 @@ pub fn text_bounding_box(layer: LayerNodeIdentifier, document: &DocumentMessageH
7474
return Quad::from_box([DVec2::ZERO, DVec2::ZERO]);
7575
};
7676

77-
let font_data = font_cache.get(font).map(|data| load_font(data));
78-
let far = graphene_std::text::bounding_box(text, font_data, typesetting, false);
77+
let far = graphene_std::text::bounding_box(text, font, font_cache, typesetting, false);
7978

8079
// TODO: Once the instance tables refactor is complete and per_glyph_instances can be removed (since it'll be the default),
8180
// TODO: remove this because the top of the dashed bounding overlay should no longer be based on the first line's baseline.

editor/src/messages/tool/tool_messages/text_tool.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use graph_craft::document::value::TaggedValue;
1717
use graph_craft::document::{NodeId, NodeInput};
1818
use graphene_std::Color;
1919
use graphene_std::renderer::Quad;
20-
use graphene_std::text::{Font, FontCache, TextAlign, TypesettingConfig, lines_clipping, load_font};
20+
use graphene_std::text::{Font, FontCache, TextAlign, TypesettingConfig, lines_clipping};
2121
use graphene_std::vector::style::Fill;
2222

2323
#[derive(Default, ExtractField)]
@@ -513,8 +513,7 @@ impl Fsm for TextToolFsmState {
513513
transform: document.metadata().transform_to_viewport(tool_data.layer).to_cols_array(),
514514
});
515515
if let Some(editing_text) = tool_data.editing_text.as_mut() {
516-
let font_data = font_cache.get(&editing_text.font).map(|data| load_font(data));
517-
let far = graphene_std::text::bounding_box(&tool_data.new_text, font_data, editing_text.typesetting, false);
516+
let far = graphene_std::text::bounding_box(&tool_data.new_text, &editing_text.font, font_cache, editing_text.typesetting, false);
518517
if far.x != 0. && far.y != 0. {
519518
let quad = Quad::from_box([DVec2::ZERO, far]);
520519
let transformed_quad = document.metadata().transform_to_viewport(tool_data.layer) * quad;
@@ -562,8 +561,7 @@ impl Fsm for TextToolFsmState {
562561
// Draw red overlay if text is clipped
563562
let transformed_quad = layer_transform * bounds;
564563
if let Some((text, font, typesetting, _)) = graph_modification_utils::get_text(layer.unwrap(), &document.network_interface) {
565-
let font_data = font_cache.get(font).map(|data| load_font(data));
566-
if lines_clipping(text.as_str(), font_data, typesetting) {
564+
if lines_clipping(text.as_str(), font, font_cache, typesetting) {
567565
overlay_context.line(transformed_quad.0[2], transformed_quad.0[3], Some(COLOR_OVERLAY_RED), Some(3.));
568566
}
569567
}

node-graph/gcore/src/text.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
mod font_cache;
2+
mod path_builder;
3+
mod text_context;
24
mod to_path;
35

46
use dyn_any::DynAny;
57
pub use font_cache::*;
8+
pub use text_context::TextContext;
69
pub use to_path::*;
710

811
/// Alignment of lines of type within a text block.
@@ -29,3 +32,28 @@ impl From<TextAlign> for parley::Alignment {
2932
}
3033
}
3134
}
35+
36+
#[derive(PartialEq, Clone, Copy, Debug, serde::Serialize, serde::Deserialize)]
37+
pub struct TypesettingConfig {
38+
pub font_size: f64,
39+
pub line_height_ratio: f64,
40+
pub character_spacing: f64,
41+
pub max_width: Option<f64>,
42+
pub max_height: Option<f64>,
43+
pub tilt: f64,
44+
pub align: TextAlign,
45+
}
46+
47+
impl Default for TypesettingConfig {
48+
fn default() -> Self {
49+
Self {
50+
font_size: 24.,
51+
line_height_ratio: 1.2,
52+
character_spacing: 0.,
53+
max_width: None,
54+
max_height: None,
55+
tilt: 0.,
56+
align: TextAlign::default(),
57+
}
58+
}
59+
}

node-graph/gcore/src/text/font_cache.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use dyn_any::DynAny;
2+
use parley::fontique::Blob;
23
use std::collections::HashMap;
4+
use std::sync::Arc;
35

46
/// A font type (storing font family and font style and an optional preview URL)
57
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, Hash, PartialEq, Eq, DynAny, specta::Type)]
@@ -50,8 +52,13 @@ impl FontCache {
5052
}
5153

5254
/// Try to get the bytes for a font
53-
pub fn get(&self, font: &Font) -> Option<&Vec<u8>> {
54-
self.resolve_font(font).and_then(|font| self.font_file_data.get(font))
55+
pub fn get<'a>(&'a self, font: &'a Font) -> Option<(&'a Vec<u8>, &'a Font)> {
56+
self.resolve_font(font).and_then(|font| self.font_file_data.get(font).map(|data| (data, font)))
57+
}
58+
59+
/// Get font data as a Blob for use with parley/skrifa
60+
pub fn get_blob<'a>(&'a self, font: &'a Font) -> Option<(Blob<u8>, &'a Font)> {
61+
self.get(font).map(|(data, font)| (Blob::new(Arc::new(data.clone())), font))
5562
}
5663

5764
/// Check if the font is already loaded

0 commit comments

Comments
 (0)