Skip unsupported bitmap fonts in Rust PDF output - #171
Conversation
|
Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
🟡 Changes recommended
OTTO CFF handling and an end-to-end regression test remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR filters bitmap-only SFNT fonts before PDF embedding to avoid subsetter failures while retaining supported outline fonts.
Changes:
- Reject fonts without
glyforCFFoutline tables. - Add synthetic SFNT classification coverage.
File summaries
| File | Summary |
|---|---|
minipdf-rs/crates/minipdf/src/pdf.rs |
Adds outline validation and focused tests. |
Review details
Suppressed comments (1)
minipdf-rs/crates/minipdf/src/pdf.rs:1249
- This test only exercises the table-presence helper with a synthetic directory; it never calls
font_supports,split_font_runs, orPdfDocument::to_bytes. It would still pass if the new guard were removed or disconnected, so it cannot regress-test the reportedUnknownKindpanic. Please add a unit or end-to-end regression that runs a bitmap-only registered font through selection/serialization and asserts conversion completes, with an outline control case.
fn accepts_only_font_outlines_supported_by_subsetter() {
for tag in [b"glyf", b"CFF "] {
let data = sfnt_with_table(tag);
let face = ttf_parser::RawFace::parse(&data, 0).expect("sfnt directory is valid");
assert!(font_has_subsettable_outlines(&face));
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if !is_embeddable_truetype(&font.data) { | ||
| return false; | ||
| } | ||
| let Ok(face) = ttf_parser::Face::parse(&font.data, 0) else { | ||
| return false; | ||
| }; | ||
| font_has_subsettable_outlines(face.raw_face()) && face.glyph_index(ch).is_some() | ||
| } | ||
|
|
||
| fn is_embeddable_truetype(data: &[u8]) -> bool { | ||
| data.starts_with(b"\0\x01\0\0") || data.starts_with(b"true") || data.starts_with(b"ttcf") | ||
| } |
Summary
glyfand CFF outline fonts supported by the subsettersubsetter::Error::UnknownKindValidation
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace— 110 passedThis also addresses the Ubuntu Node CI failure observed on PR #169; Windows and macOS were already passing.