#3674: PDF export writes a glyph id above 65535 as .notdef instead of failing (#4554)

* #3674: PDF export writes a glyph id above 65535 as .notdef instead of failing

Shaping gives 32-bit glyph ids (a format-12 cmap can map a character to
any of them), and the PDF exporter passed them to krilla unchecked.
krilla's CID font takes 16-bit ids and unwraps the conversion
(vendor/krilla/src/text/cid.rs:161 at craft-libs b926b5c0), so one such
glyph failed the whole export natively and aborted the tab on wasm.

The three places that build a KrillaGlyph (body text, metafile text and
the text watermark) now go through glyph_id(), which writes an id that
doesn't fit as glyph 0. The glyph keeps its place and its text range, so
the rest of the run is unchanged.

Regression test: glyph_id_above_u16_exports lays out a paragraph, sets
one glyph's id to 70000 and exports the layout. It failed before with
"PDF export failed internally: ... TryFromIntError(PosOverflow)".

* #3674: the replaced glyph keeps its place, not its text

krilla writes no ToUnicode entry for glyph 0, so a glyph written as
.notdef doesn't extract: "Wide glyph" with the W's id set to 70000
extracts as "\u{FFFD}ide glyph". The rest of the run extracts as
before. The comment on glyph_id() said the glyph kept its text.

* Adapt glyph_id imports to main's exporter text module

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
vikoabastian
2026-10-29 23:52:04 -04:00
committed by GitHub
co-authored by Brandon Thomas
parent 48f5a905b1
commit f2debdecff
4 changed files with 43 additions and 9 deletions
+5 -5
View File
@@ -1,14 +1,14 @@
use super::export::Fit;
use super::glyphs::{baseline, cluster_ends, glyph_outlines, left_to_right, split_clusters, visual_order};
use super::glyphs::{baseline, cluster_ends, glyph_id, glyph_outlines, left_to_right, split_clusters, visual_order};
use super::options::{PdfError, PdfOptions};
use super::paint::{append_path, fill, grown, norm, ok, paint_fill, pdf_line_cap, rect_path, shape_effects, shape_path};
use super::picture::{Vector, build_vector, load_image, paint_of, vector_picture};
use super::tags::{Role, TagEntry, figure_alt, graphic_alt, holds_picture};
use super::{
Annotation, Arc, Artifact, ArtifactType, ClKind, ContentTag, DisplayOptions, DocLayout, DocPath, Document, Draw, FaceRef, Fill, FillRule, Font,
GlyphId, HashMap, HashSet, Identifier, Image, InlineObject, ItemDisplay, KrillaGlyph, LineCap, LineEnd, LineJoin, LineStyle, LinkAnnotation,
Node, NonZeroU16, Outline, OutlineNode, Page, PathBuilder, Placed, Point, Rect, Rgb, ShapeKind, Size, SpanTag, StoryRef, Stroke, StrokeDash,
Surface, Tag, TagGroup, TagKind, TagTree, Target, Transform, WriteError, XyzDestination, krilla, rgb, write,
HashMap, HashSet, Identifier, Image, InlineObject, ItemDisplay, KrillaGlyph, LineCap, LineEnd, LineJoin, LineStyle, LinkAnnotation, Node,
NonZeroU16, Outline, OutlineNode, Page, PathBuilder, Placed, Point, Rect, Rgb, ShapeKind, Size, SpanTag, StoryRef, Stroke, StrokeDash, Surface,
Tag, TagGroup, TagKind, TagTree, Target, Transform, WriteError, XyzDestination, krilla, rgb, write,
};
mod draw;
@@ -346,7 +346,7 @@ impl Exporter<'_> {
let end = ends.get(i).copied().unwrap_or(text.len());
let start = g.cluster.min(end);
KrillaGlyph::new(
GlyphId::new(g.gid),
glyph_id(g.gid),
g.x_advance as f32 / upem,
g.x_offset as f32 / upem,
g.y_offset as f32 / upem,
+3 -3
View File
@@ -1,5 +1,5 @@
use super::{
Exporter, FaceRef, Fill, FillRule, GlyphId, KrillaGlyph, Point, Rgb, Stroke, Surface, Transform, baseline, cluster_ends, fill, glyph_outlines,
Exporter, FaceRef, Fill, FillRule, KrillaGlyph, Point, Rgb, Stroke, Surface, Transform, baseline, cluster_ends, fill, glyph_id, glyph_outlines,
left_to_right, norm, ok, paint_of, rgb, split_clusters, visual_order,
};
@@ -126,7 +126,7 @@ impl Exporter<'_> {
Some((_, nx, _, _)) => (nx - x) / size,
None => face.advance(*g) as f32 / upem,
};
KrillaGlyph::new(GlyphId::new(*g), adv, 0.0, (y0 - y) / size, 0.0, r.clone(), None)
KrillaGlyph::new(glyph_id(*g), adv, 0.0, (y0 - y) / size, 0.0, r.clone(), None)
})
.collect();
let pushed = italic;
@@ -216,7 +216,7 @@ impl Exporter<'_> {
None => g.x_advance as f32 / upem,
};
let dy = lay.glyphs.get(i).map_or(0.0, |p| ((lay.baseline - p.1) / lay.em) as f32);
KrillaGlyph::new(GlyphId::new(g.gid), adv, 0.0, dy, 0.0, start..end, None)
KrillaGlyph::new(glyph_id(g.gid), adv, 0.0, dy, 0.0, start..end, None)
})
.collect();
let x0 = lay.glyphs.first().map_or(0.0, |p| p.0) as f32;
+9 -1
View File
@@ -1,5 +1,13 @@
use super::paint::append_path;
use super::{FaceRef, HashMap, Path, PathBuilder};
use super::{FaceRef, GlyphId, HashMap, Path, PathBuilder};
/// A shaped glyph id as krilla takes it. Shaping gives 32-bit ids (a format-12 cmap can map to
/// any), but krilla's font subsetting takes 16-bit ones and panics on a larger id (#3674): one
/// that doesn't fit is written as .notdef (glyph 0), keeping its place. Its own character doesn't
/// extract, since krilla maps no text to .notdef.
pub(crate) fn glyph_id(g: u32) -> GlyphId {
GlyphId::new(u16::try_from(g).map_or(0, u32::from))
}
/// The outlines of `glyphs` (glyph id, pen position) at `size`, as one path in page coordinates.
pub(crate) fn glyph_outlines(face: &FaceRef, size: f32, glyphs: &[(u32, f32, f32)]) -> Option<Path> {
+26
View File
@@ -773,6 +773,32 @@ fn unsubsettable_font_is_drawn_as_outlines() {
assert!(paths > 0, "its glyphs are drawn as outlines");
}
/// #3674: shaping gives 32-bit glyph ids (a format-12 cmap can map a character to any of them),
/// but krilla takes 16-bit ones and panics on a larger id. That failed the export, and on wasm
/// (`panic = "abort"`) lost the tab. A glyph id above 65535 is now written as .notdef.
#[test]
fn glyph_id_above_u16_exports() {
let mut d = Document::new();
d.body = vec![para_block(Paragraph::with_text("Wide glyph", CharProps { font: Some("Source Sans 3".into()), ..Default::default() }))];
let opts = PdfOptions { tagged: false, ..Default::default() };
let mut lay = layout(&d, &mut LayoutCache::new(), &LayoutOptions::default());
let (want, _) = count_glyphs_and_paths(&export_layout(&d, &lay, &opts).unwrap());
assert!(want > 0);
let mut wide = 0;
for item in lay.pages.iter_mut().flat_map(|p| p.items.iter_mut()) {
if let Placed::Lines { para, .. } = item
&& let Some(g) = Arc::make_mut(para).glyphs.first_mut()
{
g.gid = 70_000;
wide += 1;
}
}
assert_eq!(wide, 1, "one glyph id is out of range");
let bytes = export_layout(&d, &lay, &opts).expect("a glyph id above 65535 must not fail the export");
let (glyphs, _) = count_glyphs_and_paths(&bytes);
assert_eq!(glyphs, want, "every glyph is still drawn, the wide one as .notdef");
}
#[test]
fn flat_lines_in_a_group_are_drawn() {
// A horizontal divider inside a group has no height (`a:ext cy="0"`): it used to be left out