Could `glyph_brush_layout::SectionText` be generic over text instead of consuming a `str` ?

[twitchyliquid64]

Context: A multi-line text input or a text editor - the text in question is rarely stored as a &'a str, because such a representation is inefficient for insert operations. Instead something like a Rope would be used.

My immediate thought is that it would be nice for glyph_brush to consume an iterator representing text (like ropey::iter::Chars).

Wdyt is the right approach for layout here without spraying the heap with str ?

[alexheretic]

It's possible layout could be generic over a Char iterator. Generics, of course, add complexity but it seems worth a try to see how such an API would look.

[twitchyliquid64]

The good news is that yous seem to already have a Characters iterator internally, which uses char_indices method on str to yeet out the characters. So this might be straightforward (touch wood).

Open to a PR where i give this a go?

[alexheretic]

Yep have a bash

[twitchyliquid64]

Something like this passes tests:

/// A type which can yield characters for layout.
pub trait CharacterRun {
    fn char_indices(&self) -> std::str::CharIndices<'_>;
}

impl<'a> CharacterRun for &'a str {
    fn char_indices(&self) -> std::str::CharIndices<'_> {
        str::char_indices(self)
    }
}

/// Text to layout together using a font & scale.
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct SectionText<'a, C: CharacterRun = &'a str> {
    /// Text to render
    pub text: C,
    /// Position on screen to render text, in pixels from top-left. Defaults to (0, 0).
    pub scale: PxScale,
    /// Font id to use for this section.
    ///
    /// It must be a valid id in the `FontMap` used for layout calls.
    /// The default `FontId(0)` should always be valid.
    pub font_id: FontId,

    pub marker: std::marker::PhantomData<&'a C>,
}

The downside being you now need to specify marker or use default() (I couldnt figure out how to make this a non-breaking change)

         SectionText {
             text: "hello ",
             scale: PxScale::from(20.0),
             font_id: FontId(0),
             ..SectionText::default()
         },

wdyt? Should I continue with this approach?

[twitchyliquid64]

Friendly ping :) Are you okay with the above approach?

[alexheretic]

Hey, sorry for the delay. I think the best way to test any approach is to raise a pr to try to get it working.

The immediate issues with this approach is that the current line break logic (ie xi_unicode::LineBreakIterator) requires &str to work. So we'd need a way to get unicode line breaking with just a char iterator. That seems the hardest problem.

Getting backward compatibility should be easier. We can keep SectionText as it is, creating a new more general version (SectionText2 or whatever) and replacing ToSectionText with ToSectionText2 in the layout signatures. This would still be a breaking change, but wouldn't break any usages of SectionText so would be quite a small break.