Skip to content

Commit

Permalink
Fixed missing text when a text element uses multiple fonts and one …
Browse files Browse the repository at this point in the history
…of them produces ligatures.

Closes #614
  • Loading branch information
RazrFalcon committed Apr 3, 2024
1 parent bf476d7 commit b2cb92b
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This changelog also contains important changes in dependencies.

## [Unreleased]
### Fixed
- Missing text when a `text` element uses multiple fonts and one of them produces ligatures.
- Absolute transform propagation during `use` resolving.
- Absolute transform propagation during nested `svg` resolving.
- `Node::abs_transform` documentation. The current element's transform _is_ included.
Expand Down
3 changes: 3 additions & 0 deletions crates/resvg/tests/integration/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ use crate::render;
#[test] fn painting_context_in_nested_use_and_marker() { assert_eq!(render("tests/painting/context/in-nested-use-and-marker"), 0); }
#[test] fn painting_context_in_nested_use() { assert_eq!(render("tests/painting/context/in-nested-use"), 0); }
#[test] fn painting_context_in_use() { assert_eq!(render("tests/painting/context/in-use"), 0); }
#[test] fn painting_context_on_shape_with_zero_size_bbox() { assert_eq!(render("tests/painting/context/on-shape-with-zero-size-bbox"), 0); }
#[test] fn painting_context_with_gradient_and_gradient_transform() { assert_eq!(render("tests/painting/context/with-gradient-and-gradient-transform"), 0); }
#[test] fn painting_context_with_gradient_in_use() { assert_eq!(render("tests/painting/context/with-gradient-in-use"), 0); }
#[test] fn painting_context_with_gradient_on_marker() { assert_eq!(render("tests/painting/context/with-gradient-on-marker"), 0); }
Expand Down Expand Up @@ -1478,6 +1479,8 @@ use crate::render;
#[test] fn text_text_escaped_text_4() { assert_eq!(render("tests/text/text/escaped-text-4"), 0); }
#[test] fn text_text_fill_rule_eq_evenodd() { assert_eq!(render("tests/text/text/fill-rule=evenodd"), 0); }
#[test] fn text_text_filter_bbox() { assert_eq!(render("tests/text/text/filter-bbox"), 0); }
#[test] fn text_text_ligatures_handling_in_mixed_fonts_1() { assert_eq!(render("tests/text/text/ligatures-handling-in-mixed-fonts-1"), 0); }
#[test] fn text_text_ligatures_handling_in_mixed_fonts_2() { assert_eq!(render("tests/text/text/ligatures-handling-in-mixed-fonts-2"), 0); }
#[test] fn text_text_mm_coordinates() { assert_eq!(render("tests/text/text/mm-coordinates"), 0); }
#[test] fn text_text_nested() { assert_eq!(render("tests/text/text/nested"), 0); }
#[test] fn text_text_no_coordinates() { assert_eq!(render("tests/text/text/no-coordinates"), 0); }
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
73 changes: 62 additions & 11 deletions crates/usvg/src/text/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,8 +752,38 @@ fn process_chunk(
fonts_cache: &FontsCache,
fontdb: &fontdb::Database,
) -> Vec<GlyphCluster> {
let mut glyphs = Vec::new();
// The way this function works is a bit tricky.
//
// The first problem is BIDI reordering.
// We cannot shape text span-by-span, because glyph clusters are not guarantee to be continuous.
//
// For example:
// <text>Hel<tspan fill="url(#lg1)">lo של</tspan>ום.</text>
//
// Would be shaped as:
// H e l l o ש ל ו ם . (characters)
// 0 1 2 3 4 5 12 10 8 6 14 (cluster indices in UTF-8)
// --- --- (green span)
//
// As you can see, our continuous `lo של` span was split into two separated one.
// So our 3 spans: black - green - black, become 5 spans: black - green - black - green - black.
// If we shape `Hel`, then `lo של` an then `ום` separately - we would get an incorrect output.
// To properly handle this we simply shape the whole chunk.
//
// But this introduces another issue - what to do when we have multiple fonts?
// The easy solution would be to simply shape text with each font,
// where the first font output is used as a base one and all others overwrite it.
// This way in case of:
// <text font-family="Arial">Hello <tspan font-family="Helvetica">world</tspan></text>
// we would replace Arial glyphs for `world` with Helvetica one. Pretty simple.
//
// Well, it would work most of the time, but not always.
// This is because different fonts can produce different amount of glyphs for the same text.
// The most common example are ligatures. Some fonts can shape `fi` as two glyphs `f` and `i`,
// but some can use `fi` (U+FB01) instead.
// Meaning that during merging we have to overwrite not individual glyphs, but clusters.

let mut glyphs = Vec::new();
for span in &chunk.spans {
let font = match fonts_cache.get(&span.font) {
Some(v) => v.clone(),
Expand All @@ -774,18 +804,35 @@ fn process_chunk(
continue;
}

// We assume, that shaping with an any font will produce the same amount of glyphs.
// Otherwise an error.
if glyphs.len() != tmp_glyphs.len() {
log::warn!("Text layouting failed.");
return Vec::new();
}
// Overwrite span's glyphs.
let mut iter = tmp_glyphs.into_iter();
while let Some(new_glyph) = iter.next() {
if !span_contains(span, new_glyph.byte_idx) {
continue;
}

let Some(idx) = glyphs.iter().position(|g| g.byte_idx == new_glyph.byte_idx) else {
continue;
};

// Copy span's glyphs.
for (i, glyph) in tmp_glyphs.iter().enumerate() {
if span_contains(span, glyph.byte_idx) {
glyphs[i] = glyph.clone();
let prev_cluster_len = glyphs[idx].cluster_len;
if prev_cluster_len < new_glyph.cluster_len {
// If the new font represents the same cluster with fewer glyphs
// then remove remaining glyphs.
for _ in 1..new_glyph.cluster_len {
glyphs.remove(idx + 1);
}
} else if prev_cluster_len > new_glyph.cluster_len {
// If the new font represents the same cluster with more glyphs
// then insert them after the current one.
for j in 1..prev_cluster_len {
if let Some(g) = iter.next() {
glyphs.insert(idx + j, g);
}
}
}

glyphs[idx] = new_glyph;
}
}

Expand Down Expand Up @@ -1326,6 +1373,7 @@ fn shape_text_with_font(

glyphs.push(Glyph {
byte_idx: ByteIndex::new(idx),
cluster_len: end.checked_sub(start).unwrap_or(0), // TODO: can fail?
text: sub_text[start..end].to_string(),
id: GlyphId(info.glyph_id as u16),
dx: pos.x_offset,
Expand Down Expand Up @@ -1468,6 +1516,9 @@ pub(crate) struct Glyph {
/// We use it to match a glyph with a character in the text chunk and therefore with the style.
pub(crate) byte_idx: ByteIndex,

// The length of the cluster in bytes.
pub(crate) cluster_len: usize,

/// The text from the original string that corresponds to that glyph.
pub(crate) text: String,

Expand Down

0 comments on commit b2cb92b

Please sign in to comment.