Skip to content

Conversation

@waywardmonkeys
Copy link
Contributor

@waywardmonkeys waywardmonkeys commented Dec 29, 2025

  • Add text_style: a no_std + alloc style vocabulary for text (inline + paragraph declarations with Specified semantics).
  • Add text_style_resolve: engine layer for specified→computed resolution, including OpenType settings parsing and structured errors.
  • Add styled_text: span application and a lightweight block/document model built on attributed_text + text_style, yielding resolved run iterators.
  • Add styled_text_parley: Parley backend that lowers resolved runs into parley::StyleProperty calls to build parley::Layout.

@waywardmonkeys
Copy link
Contributor Author

There's at least one big problem with this ... text-style is already a taken name, so text_style can't be the name we use.

This is a great bike shedding opportunity for people (the name).

@waywardmonkeys
Copy link
Contributor Author

This also doesn't yet lower to Parley and have an example ... coming soon. Other things too.

@waywardmonkeys
Copy link
Contributor Author

I will also peel off the attributed_text change to a separate PR and land it separately ...

@waywardmonkeys waywardmonkeys force-pushed the stylamizing_yer_text branch 2 times, most recently from 5eac6a3 to ea54de2 Compare December 29, 2025 16:05
@dfrg
Copy link
Collaborator

dfrg commented Dec 29, 2025

I'm still reading through this but my general feeling is that we'll want a simpler vocabulary crate that only provides the fundamental resolved property types. Things like Tag, FontWeight, Language*, FontVariation, FontFeature, GenericFamily, WordBreak, etc. And maybe some parsing for these. I'm thinking of the color crate as an example.

* A zero alloc language type the only captures language, script and region subtags would be nice. We don't really care about the rest.

@nicoburns
Copy link
Collaborator

I'm still reading through this but my general feeling is that we'll want a simpler vocabulary crate that only provides the fundamental resolved property types. Things like Tag, FontWeight, Language*, FontVariation, FontFeature, GenericFamily, WordBreak, etc. And maybe some parsing for these. I'm thinking of the color crate as an example.

  • A zero alloc language type the only captures language, script and region subtags would be nice. We don't really care about the rest.

I agree with this. And would say that I think it would be useful to have the "computed" styles (and likely a few other complementary low-level types like Tag) in a low-level type-only vocabulary crate that can be shared by everything which consumes parley (and parley/fontique themselves). Whereas I would expect the "specified" styles to be coupled with the "style systems" (of whatever variety) that sit on top.

@waywardmonkeys
Copy link
Contributor Author

I'll be pushing a proposal to address some of this shortly... and then I'll be passing out. :)

@waywardmonkeys
Copy link
Contributor Author

@dfrg I think the last set of changes adding text_primitives (yay for more bike shedding opportunities!) may help ...

If so, I could separate that out from this PR and submit a new PR that contains just that and the changes to have Parley / Fontique use it. (After I sleep...)

@waywardmonkeys
Copy link
Contributor Author

(It would still need some of the code that we have for some of these types in the existing stuff ... like parse functions, display impls, and bytemuck stuff. But that's easy to bring over.)

@dfrg
Copy link
Collaborator

dfrg commented Dec 30, 2025

If so, I could separate that out from this PR

Yes, this looks much better to me and I think it makes sense to split it out into a separate PR so we can bikeshed a bit on name and get it merged.

Thanks!

@waywardmonkeys
Copy link
Contributor Author

Splitting a very small font_stack from parley would be useful to me as well.

@dfrg
Copy link
Collaborator

dfrg commented Jan 5, 2026

Splitting a very small font_stack from parley would be useful to me as well.

Adding this to text_primitives (with the name FontFamily to match CSS) makes sense to me. We just need to figure out how to handle names in a way that makes allocation optional.

@nicoburns
Copy link
Collaborator

Splitting a very small font_stack from parley would be useful to me as well.

+1 for this. I think basically everything that's a computed style in Parley makes sense here (I'm assuming that where this is headed is that Parley will end up with less style resolution than it currently has (basically only things that can't be computed ahead of time becuase they depend on the font metrics or font fallback).

Adding this to text_primitives (with the name FontFamily to match CSS) makes sense to me. We just need to figure out how to handle names in a way that makes allocation optional.

Some kind Arc-based solution might also make sense. I think a lot of consumers will be converting into this type from some other similar type, and being able to cache that conversion would make a lot of sense. Especially because even without a single layout having a lot of spans with identical font-family specification is a common case.

In my case my source will be https://docs.rs/stylo/latest/style/values/computed/font/struct.FontFamilyList.html which is itself an Arc so I could probably have an Arc-> Arc lookup which I'd imagine would be quite cheap. Blitz currently is allocating a new Vec of Strings every time a font-family style is specified which isn't ideal. I haven't benchmarked how much of an impact that has.

@waywardmonkeys
Copy link
Contributor Author

Another part of this has been pulled out and turned into #501.

@waywardmonkeys
Copy link
Contributor Author

And the extraction of FontFamily (with improvements to parsing along the same lines as #501) is now in #502.

@waywardmonkeys waywardmonkeys force-pushed the stylamizing_yer_text branch 2 times, most recently from e49e03d to 46972b7 Compare January 12, 2026 04:24
@waywardmonkeys waywardmonkeys marked this pull request as ready for review January 12, 2026 04:45
@waywardmonkeys waywardmonkeys force-pushed the stylamizing_yer_text branch 4 times, most recently from c65f198 to f9ebc0a Compare January 16, 2026 08:06
Copy link
Contributor

@taj-p taj-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent my allotted review time this morning getting back up to speed with attributed_text (that crate is very nice! 🏆 ) and understanding the high level structure of the code.

I haven't yet delved into low or mid level details entirely and will follow that up either tomorrow or Monday or later today if we're lucky!


styled
.apply_span(
find_range(&text, "Parley"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't how a consumer would use this - it's also incorrect if the needle can be found twice in the search string. Would it be possible to explicitly specify the range instead of performing the O(n) search for each span?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You a specify the range explicitly ... just that it makes for fragile code for tests.

brush: B,
) {
builder.push_default(StyleProperty::Brush(brush));
builder.push_default(StyleProperty::FontFamily(style.font_family().clone()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a consumer has an owned String for a font family name (e.g., from user input, config files, or runtime construction), the current API forces multiple allocations even when reusing the same FontFamily across runs.

Consider a consumer with:

let my_font: String = "User's Custom Font".to_string();

When they need to pass the font to InlineStyle, they need to allocate:

        let font_family = FontFamily::Single(FontFamilyName::Named(Cow::Owned(
            dynamic_font_name.clone(), //  ALLOCATION
       )));

Then, they pass it to InlineStyle:

        let style = InlineStyle::new().font_family(Specified::Value(font_family));

Then, it's cloned again in engine.rs during resolution:

https://github.com/waywardmonkeys/parley/blob/9f033f9d0a0d7c8868a1dd672bcb7db6dbbe7e38/styled_text/src/resolve/engine.rs#L68

Then, it's cloned again when lowering to Parley:

https://github.com/waywardmonkeys/parley/blob/9f033f9d0a0d7c8868a1dd672bcb7db6dbbe7e38/styled_text_parley/src/lib.rs#L112


Although to fully remove the allocations, we may require a change in Parley, I think we should opt for an API here that does not allocate by design (and pay any allocation costs at the styled_text <> parley interface until such time that we can address this in Parley).


We could accepting Arc. I note how ComputedInlineStyle uses Arc for FontVariation and FontSetting but not FontFamily.

AI assisted reproduction
// Copyright 2025 the Parley Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

#![allow(unsafe_code)]

//! Minimal reproduction demonstrating allocation overhead for dynamic font families.
//!
//! This module shows that consumers with dynamically-created font family strings
//! cannot reuse their allocation across frames — the current API forces multiple
//! String clones per frame.
//!
//! Run with: `cargo test -p vello_cpu_render_styled_text --lib`

use std::alloc::{GlobalAlloc, Layout, System};
use std::borrow::Cow;
use std::sync::atomic::{AtomicUsize, Ordering};

use parley::{FontContext, LayoutContext};
use styled_text::{
    ComputedInlineStyle, ComputedParagraphStyle, FontFamily, FontFamilyName, InlineStyle,
    Specified, StyledText,
};
use styled_text_parley::build_layout_from_styled_text;

// --------------------------------------------------------------------------
// Allocation counting allocator
// --------------------------------------------------------------------------

#[global_allocator]
static ALLOCATOR: CountingAllocator = CountingAllocator;

static ALLOCATION_COUNT: AtomicUsize = AtomicUsize::new(0);
static ALLOCATION_BYTES: AtomicUsize = AtomicUsize::new(0);

struct CountingAllocator;

unsafe impl GlobalAlloc for CountingAllocator {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        ALLOCATION_COUNT.fetch_add(1, Ordering::SeqCst);
        ALLOCATION_BYTES.fetch_add(layout.size(), Ordering::SeqCst);
        // SAFETY: Delegating to the system allocator.
        unsafe { System.alloc(layout) }
    }

    unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
        // SAFETY: Delegating to the system allocator.
        unsafe { System.dealloc(ptr, layout) }
    }

    unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
        ALLOCATION_COUNT.fetch_add(1, Ordering::SeqCst);
        ALLOCATION_BYTES.fetch_add(new_size, Ordering::SeqCst);
        // SAFETY: Delegating to the system allocator.
        unsafe { System.realloc(ptr, layout, new_size) }
    }
}

fn reset_counts() {
    ALLOCATION_COUNT.store(0, Ordering::SeqCst);
    ALLOCATION_BYTES.store(0, Ordering::SeqCst);
}

fn get_counts() -> (usize, usize) {
    (
        ALLOCATION_COUNT.load(Ordering::SeqCst),
        ALLOCATION_BYTES.load(Ordering::SeqCst),
    )
}

// --------------------------------------------------------------------------
// Reproduction
// --------------------------------------------------------------------------

/// Simulates a consumer who has a dynamically-created font family string
/// (e.g., from user input, config file, or runtime construction) and wants
/// to reuse it across multiple frames without allocating.
#[test]
fn dynamic_font_family_allocates_every_frame() {
    // Setup (done once at app startup)
    let mut font_cx = FontContext::new();
    let mut layout_cx = LayoutContext::new();
    let base_inline = ComputedInlineStyle::default();
    let base_paragraph = ComputedParagraphStyle::default();

    // Consumer's dynamically-created font family string.
    // In a real app, this might come from user preferences, a config file, etc.
    let dynamic_font_name: String = "My Custom Dynamic Font".to_string();

    // Simulate multiple frames
    for frame in 0..3 {
        reset_counts();

        // -----------------------------------------------------------------
        // PROBLEM 1: No From<String> impl, must clone to keep original
        // -----------------------------------------------------------------
        // The consumer wants to reuse `dynamic_font_name` across frames,
        // but FontFamily takes ownership via Cow::Owned. To keep the original,
        // they must clone.
        let font_family = FontFamily::Single(FontFamilyName::Named(Cow::Owned(
            dynamic_font_name.clone(), // 🔴 ALLOCATION: clone to preserve original
        )));

        let (alloc_after_fontfamily, _) = get_counts();
        println!("Frame {frame}: After FontFamily creation: {alloc_after_fontfamily} allocations");

        // Create styled text with the font family
        let style = InlineStyle::new().font_family(Specified::Value(font_family));
        let mut styled =
            StyledText::new("Hello, world!", base_inline.clone(), base_paragraph.clone());
        styled.apply_span(styled.range(0..5).unwrap(), style);

        let (alloc_after_style, _) = get_counts();
        println!("Frame {frame}: After apply_span: {alloc_after_style} allocations");

        // -----------------------------------------------------------------
        // PROBLEM 2: Resolution clones the FontFamily
        // -----------------------------------------------------------------
        // Inside build_layout_from_styled_text -> resolved_inline_runs_coalesced
        // -> resolve_inline_declarations, the font_family is cloned:
        //
        //   out.font_family = resolve_specified(...).clone();
        //
        // This clones Cow::Owned(String), causing another allocation.

        // -----------------------------------------------------------------
        // PROBLEM 3: Parley lowering clones again
        // -----------------------------------------------------------------
        // In push_run_diffs:
        //
        //   StyleProperty::FontFamily(run.font_family().clone())
        //
        // Another clone of the FontFamily -> another String allocation.

        let _layout =
            build_layout_from_styled_text(&mut layout_cx, &mut font_cx, &styled, 1.0, true, ());

        let (alloc_after_layout, bytes) = get_counts();
        println!(
            "Frame {frame}: After layout build: {alloc_after_layout} allocations ({bytes} bytes)"
        );
        println!();

        // We'd expect 0 allocations if we could reuse our String,
        // but instead we see multiple allocations per frame.
        assert!(
            alloc_after_layout >= 2,
            "Expected at least 2 allocations for dynamic font family, got {alloc_after_layout}"
        );
    }
}

/// Contrast: static font families don't allocate on clone because
/// Cow::Borrowed(&'static str) just copies the pointer.
#[test]
fn static_font_family_does_not_allocate() {
    let mut font_cx = FontContext::new();
    let mut layout_cx = LayoutContext::new();
    let base_inline = ComputedInlineStyle::default();
    let base_paragraph = ComputedParagraphStyle::default();

    // Static string - Cow::Borrowed, cloning is free
    let font_family = FontFamily::Single(FontFamilyName::Named(Cow::Borrowed("Arial")));

    reset_counts();

    let style = InlineStyle::new().font_family(Specified::Value(font_family));
    let mut styled = StyledText::new("Hello!", base_inline.clone(), base_paragraph.clone());
    styled.apply_span(styled.range(0..5).unwrap(), style);

    let _layout =
        build_layout_from_styled_text(&mut layout_cx, &mut font_cx, &styled, 1.0, true, ());

    let (alloc_count, _) = get_counts();
    println!("Static font family: {alloc_count} allocations");

    // With static strings, cloning Cow::Borrowed is just a pointer copy.
    // There will still be some allocations (Vec for runs, etc.) but not
    // for the font family string itself.
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — with an owned String font name, the current “owned-in-declarations → owned-in-computed → owned-in-lowering” path can cause repeated allocations/clones.

A couple of clarifications + what I think we should do:

  • In the first step, the example doesn’t have to allocate: you can avoid dynamic_font_name.clone() by moving the String into Cow::Owned(dynamic_font_name) if you only need to build it once. But that doesn’t address the later clones you pointed out.
  • The later clones are the real issue: we currently treat FontFamily as an owned value that gets cloned as styles are resolved and then lowered. That’s fine for static/borrowed font lists, but it’s not great for runtime-created family names.

I agree with your suggestion on direction: we should aim for an API that is “non-allocating by design” for this case, and push any unavoidable allocation costs to the styled_text ↔ Parley boundary until Parley can participate.

Concretely, the shape I’d like to explore next is: store dynamic family names behind shared storage (e.g. Arc<str> or an interned string) in the styled_text model so cloning is cheap and allocation is paid once per distinct family name. Then lowering can borrow from that shared storage. Whether Parley needs changes depends on how far we want this to propagate, but we can improve things substantially on the styled_text side even before that.

I’ll follow up with a separate PR proposal focused specifically on dynamic font-family storage/cloning (so it doesn’t complicate this one).


styled.apply_span(
styled.range(find_range(&text, "StyledText")).unwrap(),
styled_text_style,
Copy link
Contributor

@taj-p taj-p Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your thoughts about the allocation patterns of this API?

It's currently difficult to reuse allocations across evolutions of rich text.

InlineStyle allocates a Vec<InlineDeclaration>.

  • It has .clear() for reuse, but apply_span takes ownership, preventing scratch-storage patterns.

StyledText::new allocates vectors for:

  • AttributedText::attributes (span storage)
  • ParagraphStyle::declarations

There's no StyledText::clear() or reset() to reuse these allocations across different text. For example, ParagraphStyle lacks a .clear().

Is StyledText where consumers will store their text model? Or do you see StyledText as a proxy that sits between user land models and various downstream text crates? I.e., does a user store their text model in StyledText or do they lower their own model into StyledText?

Going by the current API, given the "write once, read for layout" approach and inability to "read back" styles that were subsequently input, I believe this is a transient intermediary - is that true?

If so, then I think we will want to support a "clear and refill" pattern to allow for allocation reuse across builds and have a stated goal of cheap construction and rebuild.


That said, perhaps I've totally missed the mark here, which I am wont to do 😅 !!! So, I would appreciate more context if that's the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StyledText is intended to be a durable attributed-text model (think “attributed string”): it can be retained as an app’s in-memory representation (often parsed from markup), but it can also be used transiently as a layout input if you already have your own model. We should document that intent more clearly.

On allocation reuse: agreed. Even without a full editing/mutation story yet, it should support a “clear and refill” rebuild pattern so consumers can reuse capacity across evolutions of the same document. I’ve added StyledText::set_text/clear_*-style APIs (and lower-level AttributedText::set_text + ParagraphStyle::clear) that clear spans/declarations while retaining storage. Longer-term, we’ll need richer mutation APIs (insert/delete with span adjustment), but this is a good first step toward cheap rebuilds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that clean change to support a better clear and refill pattern via set_text and clear_* API. Very nice!!

One question I am wondering about is how we would tackle the allocations lost to InlineStyle::declarations?

I think if we aim to be both a durable model and a transient stepping stone to rich text layout, we might want to address those allocations.

Currently, apply_span takes ownership of the attribute (InlineStyle), which means each span's Vec<InlineDeclaration> gets moved into the attributed text storage. When clear_attributes() is called, those InlineStyle values are dropped along with their internal allocations (i.e. InlineStyles::declarations).

With the current API, I imagine we could use a pooling object from which consumers get the InlineStyle and StyledText returns InlineStyle allocations in "clear"-like operations. But that may require API change (which perhaps could be additive).

let mut pool = InlineStylePool::new();

let style = pool.get();
style.push_declaration(...);
styled.apply_span(range, style);

styled.clear_spans_into(&mut pool);

Separately, I've noted that there are other cases where we may want to control allocations (e.g. the ResolvedInlineRuns struct). So, another option would be for the entry point into any StyledText API to be through some context struct where we can manage allocations (durable pools and transient allocations) if we want to hide that from consumers:

let ctx = StyledTextContext::new();
let inline_style = ctx.create_inline_style();
let styled_text = ctx.create_styled_text();

...

ctx.resolve_inline_runs(styled_text); // ctx manages re-useable allocations for `resolve_inline_runs`

This approach may better balance the goal of minimal allocations and API ergonomics - otherwise, to achieve minimal allocations, consumers may need to manage cache objects and pass them in on an as-needs basis (but that approach may also work!).

In any case, I highlight this case for your consideration - perhaps it's something that may help direct strategy.

@waywardmonkeys waywardmonkeys force-pushed the stylamizing_yer_text branch 3 times, most recently from 430eb28 to b47bab2 Compare January 17, 2026 03:58
Comment on lines +59 to +60
font_variations: Arc::from([]),
font_features: Arc::from([]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These allocations trigger even when these settings are overwritten with new settings or if they're never used. What do you think of making them Option to avoid these allocations?

diff --git a/styled_text/src/resolve/computed.rs b/styled_text/src/resolve/computed.rs
index 2356a79..4ce4034 100644
--- a/styled_text/src/resolve/computed.rs
+++ b/styled_text/src/resolve/computed.rs
@@ -37,8 +37,8 @@ pub struct ComputedInlineStyle {
     pub(crate) font_width: FontWidth,
     pub(crate) font_style: FontStyle,
     pub(crate) font_weight: FontWeight,
-    pub(crate) font_variations: Arc<[FontVariation]>,
-    pub(crate) font_features: Arc<[FontFeature]>,
+    pub(crate) font_variations: Option<Arc<[FontVariation]>>,
+    pub(crate) font_features: Option<Arc<[FontFeature]>>,
     pub(crate) locale: Option<Language>,
     pub(crate) underline: bool,
     pub(crate) strikethrough: bool,
@@ -56,8 +56,8 @@ impl Default for ComputedInlineStyle {
             font_width: FontWidth::NORMAL,
             font_style: FontStyle::default(),
             font_weight: FontWeight::NORMAL,
-            font_variations: Arc::from([]),
-            font_features: Arc::from([]),
+            font_variations: None,
+            font_features: None,
             locale: None,
             underline: false,
             strikethrough: false,
@@ -110,13 +110,13 @@ impl ComputedInlineStyle {
     /// Returns computed font variation settings (OpenType axis values).
     #[inline]
     pub fn font_variations(&self) -> &[FontVariation] {
-        &self.font_variations
+        self.font_variations.as_deref().unwrap_or(&[])
     }
 
     /// Returns computed font feature settings (OpenType feature values).
     #[inline]
     pub fn font_features(&self) -> &[FontFeature] {
-        &self.font_features
+        self.font_features.as_deref().unwrap_or(&[])
     }
 
     /// Returns the locale/language tag, if any.
diff --git a/styled_text/src/resolve/engine.rs b/styled_text/src/resolve/engine.rs
index a7ec2c4..0eb64df 100644
--- a/styled_text/src/resolve/engine.rs
+++ b/styled_text/src/resolve/engine.rs
@@ -232,25 +232,25 @@ fn resolve_line_height(
 #[inline]
 fn resolve_variations(
     specified: &Specified<FontVariations>,
-    parent: &Arc<[FontVariation]>,
-    initial: &Arc<[FontVariation]>,
-) -> Arc<[FontVariation]> {
+    parent: &Option<Arc<[FontVariation]>>,
+    initial: &Option<Arc<[FontVariation]>>,
+) -> Option<Arc<[FontVariation]>> {
     match specified {
-        Specified::Inherit => Arc::clone(parent),
-        Specified::Initial => Arc::clone(initial),
-        Specified::Value(value) => Arc::clone(value.as_arc_slice()),
+        Specified::Inherit => parent.clone(),
+        Specified::Initial => initial.clone(),
+        Specified::Value(value) => Some(Arc::clone(value.as_arc_slice())),
     }
 }
 
 #[inline]
 fn resolve_features(
     specified: &Specified<FontFeatures>,
-    parent: &Arc<[FontFeature]>,
-    initial: &Arc<[FontFeature]>,
-) -> Arc<[FontFeature]> {
+    parent: &Option<Arc<[FontFeature]>>,
+    initial: &Option<Arc<[FontFeature]>>,
+) -> Option<Arc<[FontFeature]>> {
     match specified {
-        Specified::Inherit => Arc::clone(parent),
-        Specified::Initial => Arc::clone(initial),
-        Specified::Value(value) => Arc::clone(value.as_arc_slice()),
+        Specified::Inherit => parent.clone(),
+        Specified::Initial => initial.clone(),
+        Specified::Value(value) => Some(Arc::clone(value.as_arc_slice())),
     }
 }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that these and the font family are maybe not a great design from top to bottom (through the entire stack) ... and maybe we need a more fundamental re-think and address it from text_primitives up through parley and into here.

impl FontFeatures {
/// Creates settings from a parsed list.
#[inline]
pub fn list(list: impl Into<Arc<[FontFeature]>>) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like your consultation about this signature. On one hand, taking impl Into<Arc<...>> makes the API ergonomic, but on the other hand, it can hide allocation cost.

In a hot loop, someone might use a vec! directly because that's the path of least resistance:

for span in my_spans {
    InlineStyle::new().with_font_features(FontFeatures::list(vec![
        FontFeature::new(Tag::new(b"liga"), 1),
    ]))
}

Instead of the more performant:

let enabled_liga = Arc::from([FontFeature::new(Tag::new(b"liga"), 1)]);
for span in my_spans {
    InlineStyle::new().with_font_features(FontFeatures::list(Arc::clone(&enabled_liga)))
}

If we're focused on allocation efficiency and performance, it might make more sense to make the allocation more visible - aligning with our goal of minimising allocations.

Users who care about performance will appreciate the honesty, whereas users who don't can simply wrap with Arc::from().

An alternative would be to change list to take Arc<T> directly.


That said, I'm curious what your thoughts are here. Certainly, this might be better overcome with documentation - I'm curious on your stance and philosophy when it comes to signatures like these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is more evidence that the design of this part of things needs consideration from top to bottom.


styled.apply_span(
styled.range(find_range(&text, "StyledText")).unwrap(),
styled_text_style,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that clean change to support a better clear and refill pattern via set_text and clear_* API. Very nice!!

One question I am wondering about is how we would tackle the allocations lost to InlineStyle::declarations?

I think if we aim to be both a durable model and a transient stepping stone to rich text layout, we might want to address those allocations.

Currently, apply_span takes ownership of the attribute (InlineStyle), which means each span's Vec<InlineDeclaration> gets moved into the attributed text storage. When clear_attributes() is called, those InlineStyle values are dropped along with their internal allocations (i.e. InlineStyles::declarations).

With the current API, I imagine we could use a pooling object from which consumers get the InlineStyle and StyledText returns InlineStyle allocations in "clear"-like operations. But that may require API change (which perhaps could be additive).

let mut pool = InlineStylePool::new();

let style = pool.get();
style.push_declaration(...);
styled.apply_span(range, style);

styled.clear_spans_into(&mut pool);

Separately, I've noted that there are other cases where we may want to control allocations (e.g. the ResolvedInlineRuns struct). So, another option would be for the entry point into any StyledText API to be through some context struct where we can manage allocations (durable pools and transient allocations) if we want to hide that from consumers:

let ctx = StyledTextContext::new();
let inline_style = ctx.create_inline_style();
let styled_text = ctx.create_styled_text();

...

ctx.resolve_inline_runs(styled_text); // ctx manages re-useable allocations for `resolve_inline_runs`

This approach may better balance the goal of minimal allocations and API ergonomics - otherwise, to achieve minimal allocations, consumers may need to manage cache objects and pass them in on an as-needs basis (but that approach may also work!).

In any case, I highlight this case for your consideration - perhaps it's something that may help direct strategy.

/// This is intended to carry semantic structure (headings, lists, etc.) suitable for accessibility,
/// even before layout provides geometry.
#[derive(Debug)]
pub struct StyledDocument<T: Debug + TextStorage, A: Debug> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Do we want to extract this into a styled_document crate (along with Block and BlockKind)?

I'm curious about the strategy here - does StyledDocument's purpose align with StyledText's dual role as both durable storage and transient intermediary?

I am assuming that some consumers may never use StyledDocument (e.g., those translating their own document model into StyledText transiently).

By extracting StyledDocument to its own crate, we'd get:

  • API discipline: Forces StyledText functionality needed by document abstractions to be pub, which means any consumer building their own document model gets the same capabilities.
  • Clear layering: styled_document becomes a reference implementation of "document built on StyledText" rather than privileged internal code.

That said, StyledDocument is currently quite small, so this may be premature. Curious whether you see it growing significantly or staying minimal.

Comment on lines +56 to +62
pub(crate) boundaries: Vec<usize>,
start_offsets: Vec<usize>,
start_events: Vec<usize>,
end_offsets: Vec<usize>,
end_events: Vec<usize>,
spans: Vec<Span<'a>>,
active: Vec<usize>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect any of these to grow above u32::MAX?

We can halve the cost of these structures on 64 bit systems by using u32. It means that we have to as usize for access, but since these are internal implementation details, that's preferable for performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this locally and ... it makes the code a bit grosser.

Parley already imposes u32 requirements on some stuff, so it isn't like we can actually address more than u32::MAX.

I don't think this is a clear win, but I'll try it out more.

}

impl<'a, T: Debug + TextStorage, A: Debug + HasInlineStyle> ResolvedInlineRuns<'a, T, A> {
pub(crate) fn new(styled: &'a StyledText<T, A>) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allocates a lot. From the perspective of a transient consumer, it would be beneficial if we could pass in a context or allocations structure from which new could re-use allocations. Would it be possible to enable passing in a context or similar?

…n, Parley lowering)

  - Add `styled_text`: attributed text + span styles with a CSS-inspired vocabulary (`styled_text::style`), specified→computed resolution (`styled_text::resolve`), and a `StyledText`/`StyledDocument` model for block + span styling.
  - Provide deterministic inline-run resolution (including overlap handling and run coalescing) plus unit tests and doctests for the core semantics.
  - Add `styled_text_parley`: lowers `styled_text` computed runs into Parley `StyleProperty` spans to build a `parley::Layout`.
  - Add `vello_cpu_render_styled_text` example exercising the end-to-end pipeline and update workspace/CI wiring for the new crates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants