linebender / resvg

An SVG rendering library.
Apache License 2.0
2.84k stars 229 forks source link

`<text>` with `font-size="0px"` containing empty `<tspan>` followed by whitespaces is rendered in the wrong location #739

Open paxbun opened 7 months ago

paxbun commented 7 months ago

resvg version: 0.41.0

When a <text> with font-size="0px" starts with an empty <tspan> followed by whitespace, the <text> is rendered in the wrong place.

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="300px" height="300px" version="1.1" xmlns="http://www.w3.org/2000/svg"
    xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:mei="http://www.music-encoding.org/ns/mei"
    overflow="visible">
    <svg viewBox="-100 -100 400 400">
        <!-- Rendered in a wrong location -->
        <text x="40" y="80" font-size="0px">
            <tspan></tspan>
            <tspan font-size="48px">Text</tspan>
        </text>
        <!-- Rendered correctly -->
        <text x="40" y="120" font-size="0px">
            <tspan font-size="48px">Text</tspan>
            <tspan></tspan>
        </text>
        <!-- Rendered correctly -->
        <text x="40" y="160" font-size="1px">
            <tspan></tspan>
            <tspan font-size="48px">Text</tspan>
        </text>
        <!-- Rendered correctly -->
        <text x="40" y="200" font-size="0px"><tspan></tspan><tspan font-size="48px">Text</tspan></text>
        <!-- Rendered correctly -->
        <text x="40" y="240" font-size="0px">
            <tspan font-size="48px">Text</tspan>
        </text>
    </svg>
</svg>

Render result: out

paxbun commented 7 months ago

Changing usvg::parser::text::collect_text_chunks_impl as follows fixes this problem, with all unit tests passed. Parameters pos_list of collect_text_chunks and collect_text_chunks_impl is also changed from &[CharacterPosition] to &mut [CharacterPosition].

        // TODO: what to do when <= 0? UB?
        let font_size = super::units::resolve_font_size(parent, state);
        let font_size = match NonZeroPositiveF32::new(font_size) {
            Some(n) => n,
            None => {
                // Skip this span.
+++             let pos = pos_list.get(iter_state.chars_count).copied();
                iter_state.chars_count += child.text().chars().count();
+++             if iter_state.chars_count < pos_list.len() {
+++                 if let Some(pos) = pos {
+++                     pos_list[iter_state.chars_count] = pos;
+++                 }
+++             }
                // iter_state
                continue;
            }
        };
RazrFalcon commented 7 months ago

Thanks, will take a look.

LaurenzV commented 7 months ago

Input SVG:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg viewBox="0 0 200 200" version="1.1" xmlns="http://www.w3.org/2000/svg">
       <text x="40" y="140" font-size="0px">
              <tspan></tspan>
              <tspan font-size="48px">Text</tspan>
       </text>
</svg>

Out:

<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg">
    <defs/>
    <text xml:space="preserve"><tspan><tspan font-family="Times New Roman" font-size="48" fill="#000000" stroke="none">Text</tspan></tspan></text>
</svg>

If you remove the first tspan:

<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg">
    <defs/>
    <text xml:space="preserve"><tspan x="40" y="140"><tspan font-family="Times New Roman" font-size="48" fill="#000000" stroke="none">Text</tspan></tspan></text>
</svg>

So the underlying issue seems to be that x and y are not propagated properly.

RazrFalcon commented 7 months ago

@LaurenzV I don't think so. tspan in usvg cannot be nested, but it is. Either usvg SVG writing is wrong or the issue is somewhere else.

LaurenzV commented 7 months ago

Yeah, the nesting of the tspan was a necessity in some cases when I wrote the writer. It's not an exact representation of how it looks in the tree. But regardless of that, the x and y still disappeared in the first svg, even though they shouldn't.

RazrFalcon commented 7 months ago

Got it, will take a look.

Zero-sized fonts are still UB, therefore we can do whatever we want, but it would be better if we could match Chrome.