Split text layouting and text outlining into two separate steps

[LaurenzV]

I had to update one reference image, but it was only a one pixel difference. Probably due to some float rounding issue.

[LaurenzV]

The feature thing is probably not solved very nicely... I'm open for suggestions on how else to do it.

[RazrFalcon]

What is Path here? The Path node? Why?

[LaurenzV]

Yes, it's a usvg path. Right now it's basically just a decoration span. I guess I should rename the enum variant.

[RazrFalcon]

Oh, so those are for underlines and stuff? Hm... good question about we suppose to store them.
I guess we need just a list of rectangles. Maybe something like:

struct Layout {
    fragments: Vec<TextFragment>,
    underlines: Vec<NonZeroRect>,
    overlines: Vec<TextFragment>,
    strikethrougs: Vec<TextFragment>,
}

[LaurenzV]

I'm not sure I understand, are you suggesting to store text fragments that have different decoration spans in different vectors? But that would mess up the order, no? What problem do you see with the current approach? By storing decorations as paths and text spans in the same vector, we can simply iterate over them in the order they are defined in the vector and render them separately so we don't need to worry about that.

[LaurenzV]

And it's also nice if I as a consumer of the API don't need to worry about resolving the text decorations myself but can simply render the resulting paths.

[LaurenzV]

Actually... What I could do is that we store the paths of underlines, line-throughs and overlines in the span itself. And then we basically only have a list of spans. This might be a bit cleaner. I'll go ahead and try to implement this.

[RazrFalcon]

My problem is that text decoration should not be part of TextFragment. Because it's kinda isn't.
If we have a underline over the whole text - then we have a single line/path. Why should it be part of TextFragment ? It doesn't make any sense to me.

In case of text-on-path decorations can be rotated, meaning just a Rect wouldn't be enough. So Path is fine for now.

And no, no one would be resolving text decoration manually, mainly because it would be impossible.

[LaurenzV]

As mentioned below, I've removed TextFragment and store text decorations with the span now, I hope that's better!

[RazrFalcon]

Looks good to me. I'm still confused why we need Path in TextFragment, but other than that it's fine.

The current limitation is that there is no way to get the original text/character position from the layout, which will be useful when generation a PDF with ToUnicode object.

[LaurenzV]

Looks good to me. I'm still confused why we need Path in TextFragment, but other than that it's fine.

I removed text fragment now. What do you think?

The current limitation is that there is no way to get the original text/character position from the layout, which will be useful when generation a PDF with ToUnicode object.

I think it should be okay because I will still load the requested font and then save the cmap into the PDF, which should be enough (at least that's what Typst does and it seems to work fine). But I guess I will find out soon.

[RazrFalcon]

You cannot really get a Unicode character from TrueType cmap. You're suppose to use ToUnicode.
PS: I had a "pleasure" of working with PDF as well...

[LaurenzV]

Oh damn, yeah, you are right, I need to somehow store the corresponding string for the glyphs... Will see if I can figure out how.

[LaurenzV]

Actually, sorry, there is one more small thing I would like to implement, so it would be good if we could hold off merging for now. I'll write here once I think it's done.

[LaurenzV]

Okay well, I've managed to produce the same visual output for all test cases but embedded text instead, so from my side I have everything I need in that regard. I still want to get copy-pasting to work, so still want to try to figure out a way to map the glyphs to their respective character, but I guess this can happen in a separate PR in the next few days, if I find the time. I'm already pretty happy with what I have for now. :D

[RazrFalcon]

Sure, sounds good.

I'm still trying to understand how do you handle decorations. I'm not sure they should be part of layout::Span.
Let's say we have "Hello world" where the second word has a different color, but the whole text has the same style otherwise and we have underline for the whole text. In this case we would have two spans but one underline. How do you handle it right now?
Because the old implementation would join continuous decorations.

[LaurenzV]

So, I just checked with the following SVG:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg"
     font-family="Noto Sans" font-size="64">
    <title>Simple case</title>

    <path id="crosshair" d="M 20 100 L 180 100 M 100 20 L 100 180"
          stroke="gray" stroke-width="0.5"/>

    <text id="text1" x="32" y="100" text-decoration="underline">Te<tspan fill="green">x</tspan>t</text>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

In this case, there will be 3 spans and each of them has a separate underline (but each underline only covers the range of the glyphs they contain, so they are not overlapping)... But I think it should be fine like that, right? I mean it can't be noticed that they are three different underlines, anyway. Or is that a problem?

[RazrFalcon]

Great, the current implementation also breaks underlines. I don't know how my own code works anymore... I wrote it like 4 years ago.

Either way, let's leave decoration as a part of a span. Not ideal, but not breaking either.

[LaurenzV]

Thanks!