From fd21788959a274f292b1c383469b89649bf19db8 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 19:56:24 +0200 Subject: [PATCH 1/3] fix(pdf): flow matrix runs into one selection block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The selection layer measured every run in page space, so two runs a general transform placed could not be compared and each got a line block of its own — one per glyph on a page laid out glyph by glyph. Those blocks carry no PDF-derived width either, so each shrink-wrapped the half-size fallback font and the marking sat beside the glyphs rather than over them. Runs one CSS matrix can place now flow inside a single block, measured in that block's own frame (origins resolved along its axes, the unit its font size), so the existing gap and line tests apply there unchanged and the widths follow. The advance of a whitespace-only run went missing along the way: it emits no `.sr` to carry it, and the spacer span was sized by the gap before it alone. One `Tj` per line hid that; word by word it cost a space per word. Only the selection layer and the width-class table move — no glyph, path or `@font-face` line differs in the reference output. --- CHANGELOG.md | 6 +- src/odr/internal/html/pdf_file.cpp | 121 ++++++++++++++++++++++------- test/src/internal/pdf/pdf_file.cpp | 32 ++++++++ 3 files changed, 128 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 288d44bc..97692822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,10 +37,14 @@ The release run heads these entries with the version and opens a fresh They were dropped from the output entirely, in text documents and spreadsheets alike. - Text copied out of a pdf laid out glyph by glyph reads as words, not as - `L a g e`. A word break also survives a run with nothing extractable in it. + `L o r e m`. A word break also survives a run with nothing extractable in it. - An encrypted `.doc`, `.ppt` or `.xls` reports itself encrypted and raises `FileEncrypted` instead of a parse error, so a reader can prompt for the password. Decrypting them is still out of reach. +- Marking text in a rendered pdf highlights the words rather than a trail of + narrow boxes beside them. Runs one CSS transform can place share a selection + block and carry the PDF's advances, and a whitespace-only run hands its + advance on instead of dropping it — a space per word short on every line. ## v6.9.0 - 2026-08-18 diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 4e973858..d08f5f9a 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1411,7 +1411,8 @@ class HtmlServiceImpl final : public HtmlService { std::string vis_cur_flow_key; const auto vis_close_line = [&] { vis_cur_line = -1; }; - // Selection layer state: content-stream (reading) order grouping. + // Selection layer state: content-stream (reading) order grouping, in the + // open block's flow frame (see the layer below). bool sel_have_prev = false; double sel_prev_baseline = 0; double sel_prev_end = 0; @@ -1420,10 +1421,13 @@ class HtmlServiceImpl final : public HtmlService { bool sel_prev_ends_space = false; bool sel_prev_was_matrix = false; std::int32_t sel_cur_line = -1; - /// `ox` where the open `.sr` run starts, to recompute its width on merge + /// origin the open `.sr` run starts at, to recompute its width on merge double sel_cur_run_start_ox = 0; /// font-size of the previous element, for its line's trailing space double sel_prev_font_size_pt = 0; + /// the open block's transform: its linear part identifies runs one CSS + /// matrix can place, its origin anchors the frame they flow in + util::math::Transform2D sel_block; for (const pdf::PageElement &element : page_elements(*page, stream, m_logger)) { @@ -1537,28 +1541,59 @@ class HtmlServiceImpl final : public HtmlService { } // --- Selection layer ----------------------------------------------- - // Matrix runs get their own single-run line block. + // Runs flow inside a line block, so they are measured in the frame that + // block lays out in: page space for an axis-aligned block, the block's + // own space for a matrix one — where the CSS matrix, not the page, + // relates the runs to each other. Both frames put x along the writing + // line and carry the run's font size as their unit of height, so one + // set of gap and line tests serves both. if (!text.text.empty()) { - const double width_pt = round2(extent); - const double gap_pt = std::max(0.0, ox - sel_prev_end); const bool starts_space = text.text.front() == ' '; // A leading inferred space is dropped: the gap between runs is // covered by the spacer span, not by the run text. std::string core = starts_space ? text.text.substr(1) : text.text; - const bool matrix_break = is_matrix || sel_prev_was_matrix; - bool new_sel_line = !sel_have_prev || matrix_break; + const double tz = text.horizontal_scaling / 100.0; + // In the block's frame the unit is text space: `text.width` carries + // the horizontal scaling the CSS matrix applies again. + const double local_extent = tz != 0 ? text.width / tz : 0; + double sel_ox = is_matrix ? 0 : ox; + double sel_baseline = is_matrix ? 0 : baseline; + const double sel_extent = is_matrix ? local_extent : extent; + const double sel_font_pt = is_matrix ? text.size : font_pt; + + bool sel_frame_kept = + sel_have_prev && !is_matrix && !sel_prev_was_matrix; + if (is_matrix && sel_cur_line >= 0 && sel_prev_was_matrix && + same_linear(sel_block, m)) { + if (const std::optional> local = + local_origin(sel_block, m)) { + sel_ox = (*local)[0]; + sel_baseline = (*local)[1]; + sel_frame_kept = true; + } + } + + const double width_pt = round2(sel_extent); + const double gap_pt = std::max(0.0, sel_ox - sel_prev_end); + + bool new_sel_line = !sel_frame_kept; bool sel_gap = false; - if (sel_have_prev && sel_prev_font_pt > 0 && !new_sel_line) { - new_sel_line = starts_new_line(baseline, sel_prev_baseline, ox, - sel_prev_end, sel_prev_font_pt); - sel_gap = ox - sel_prev_end > 0.25 * sel_prev_font_pt; + if (sel_frame_kept && sel_prev_font_pt > 0) { + new_sel_line = + starts_new_line(sel_baseline, sel_prev_baseline, sel_ox, + sel_prev_end, sel_prev_font_pt); + sel_gap = sel_ox - sel_prev_end > 0.25 * sel_prev_font_pt; } - // The extractor's leading space is the break; a block the matrix - // path opens is not. - const bool break_space = !matrix_break || starts_space; + // The extractor's leading space is the break; a block opened because + // the frames are not comparable is not. + const bool break_space = sel_frame_kept || starts_space; if (new_sel_line) { + // The block starts here, so this run sits at its origin. + sel_ox = is_matrix ? 0 : ox; + sel_baseline = is_matrix ? 0 : baseline; + sel_block = m; // Close the previous line with a trailing space. `sg`, not `sr`: // it carries no PDF-derived width, just the space. if (sel_cur_line >= 0 && sel_have_prev && !sel_prev_ends_space && @@ -1578,12 +1613,12 @@ class HtmlServiceImpl final : public HtmlService { if (!core.empty()) { std::string cls = "sr"; add_class(cls, "f", pt_decl("font-size", font_size_pt)); - if (width_pt > 0 && !is_matrix) { + if (width_pt > 0) { add_class(cls, "w", pt_decl("width", width_pt)); } page_out.sel_lines[sel_cur_line].runs.push_back( SelRunOut{std::move(cls), escape_markup(std::move(core))}); - sel_cur_run_start_ox = ox; + sel_cur_run_start_ox = sel_ox; } } else if (sel_gap || sel_prev_ends_space || starts_space) { std::vector &runs = @@ -1591,7 +1626,12 @@ class HtmlServiceImpl final : public HtmlService { if (!sel_prev_ends_space && !runs.empty()) { std::string gap_cls = "sg"; add_class(gap_cls, "f", pt_decl("font-size", font_size_pt)); - const double rounded_gap = round2(gap_pt); + // A run that is only whitespace emits no `.sr`, so its advance + // has to ride the spacer or the line comes up short — invisible + // while a whole line arrives as one `Tj`, plain once it arrives + // word by word. + const double rounded_gap = + round2(gap_pt + (core.empty() ? sel_extent : 0)); if (rounded_gap > 0) { add_class(gap_cls, "w", pt_decl("width", rounded_gap)); // Only a gap that still reads as a word space: a column of @@ -1605,12 +1645,12 @@ class HtmlServiceImpl final : public HtmlService { if (!core.empty()) { std::string cls = "sr"; add_class(cls, "f", pt_decl("font-size", font_size_pt)); - if (width_pt > 0 && !is_matrix) { + if (width_pt > 0) { add_class(cls, "w", pt_decl("width", width_pt)); } runs.push_back( SelRunOut{std::move(cls), escape_markup(std::move(core))}); - sel_cur_run_start_ox = ox; + sel_cur_run_start_ox = sel_ox; } } else { // Tight continuation on the same baseline: merge into the previous @@ -1621,21 +1661,19 @@ class HtmlServiceImpl final : public HtmlService { page_out.sel_lines[sel_cur_line].runs; if (!runs.empty()) { runs.back().text += escape_markup(text.text); - if (!is_matrix) { - strip_width_class(runs.back().classes); - const double merged_width_pt = - round2(ox + extent - sel_cur_run_start_ox); - if (merged_width_pt > 0) { - add_class(runs.back().classes, "w", - pt_decl("width", merged_width_pt)); - } + strip_width_class(runs.back().classes); + const double merged_width_pt = + round2(sel_ox + sel_extent - sel_cur_run_start_ox); + if (merged_width_pt > 0) { + add_class(runs.back().classes, "w", + pt_decl("width", merged_width_pt)); } } } - sel_prev_baseline = baseline; - sel_prev_end = ox + extent; - sel_prev_font_pt = font_pt; + sel_prev_baseline = sel_baseline; + sel_prev_end = sel_ox + sel_extent; + sel_prev_font_pt = sel_font_pt; sel_prev_ends_space = !text.text.empty() && text.text.back() == ' '; sel_prev_was_matrix = is_matrix; sel_prev_font_size_pt = font_size_pt; @@ -2265,6 +2303,29 @@ class HtmlServiceImpl final : public HtmlService { ox < prev_end - 0.5 * prev_font_pt; } + /// Whether two runs share a linear part, so one line block's CSS matrix + /// places both. + static bool same_linear(const util::math::Transform2D &l, + const util::math::Transform2D &r) { + return l.a == r.a && l.b == r.b && l.c == r.c && l.d == r.d; + } + + /// `m`'s origin in `block`'s own frame: the offset between the two origins + /// resolved along `block`'s axes, x along the writing line. Empty for a + /// singular linear part, which spans no frame to measure in. + static std::optional> + local_origin(const util::math::Transform2D &block, + const util::math::Transform2D &m) { + const double det = block.a * block.d - block.b * block.c; + if (det == 0) { + return std::nullopt; + } + const double dx = m.e - block.e; + const double dy = m.f - block.f; + return std::array{(dx * block.d - dy * block.c) / det, + (-dx * block.b + dy * block.a) / det}; + } + /// Per-run geometry from a `TextElement` and the page's `to_box`. Identical /// in every text mode, and kept in one place so no call site can drift. struct RunGeometry { diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index ae1ea150..de9c0ada 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -48,6 +48,15 @@ bool contains(const std::string &haystack, const std::string &needle) { return haystack.find(needle) != std::string::npos; } +std::size_t count(const std::string &haystack, const std::string &needle) { + std::size_t result = 0; + for (std::size_t i = haystack.find(needle); i != std::string::npos; + i = haystack.find(needle, i + needle.size())) { + ++result; + } + return result; +} + /// A three-page mini-PDF whose first page carries four `/Link` annotations: a /// `/URI` action, a direct `/Dest` array to page 2, a `/GoTo` action to a named /// destination (`chap3` → page 3, via the catalog `/Dests`), and a `/URI` @@ -185,6 +194,29 @@ TEST(PdfFile, anisotropic_placement_does_not_space_out_glyphs) { EXPECT_TRUE(contains(spaced, R"()"), 1u); + EXPECT_TRUE(contains(html, ">Hi")); + EXPECT_TRUE(contains(html, ">to")); + // Both words and the space between them carry a PDF-derived width. + EXPECT_EQ(count(html, R"( Date: Thu, 20 Aug 2026 20:41:11 +0200 Subject: [PATCH 2/3] fix(pdf): carry every whitespace advance, and keep a raised run off the block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes in the selection flow. A whitespace-only run emits no `.sr`, and the spacer that should carry its advance was skipped wherever a space was already there — a line opening with one, or two in a row. The advance is now owed to whatever comes next, reaching it as the spacer's width or as a `margin-left` on the run itself. A block anchors its runs to one baseline, so a run a rise lifts off it — a superscript — took the block's baseline instead of its own once the frame let it flow. It gets a block of its own again, without the separator a real line break would bring. --- CHANGELOG.md | 4 +-- src/odr/internal/html/pdf_file.cpp | 52 +++++++++++++++++++----------- test/data.cmake | 4 +-- test/src/internal/pdf/pdf_file.cpp | 11 +++---- 4 files changed, 41 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97692822..dbd3cd0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,9 +42,7 @@ The release run heads these entries with the version and opens a fresh `FileEncrypted` instead of a parse error, so a reader can prompt for the password. Decrypting them is still out of reach. - Marking text in a rendered pdf highlights the words rather than a trail of - narrow boxes beside them. Runs one CSS transform can place share a selection - block and carry the PDF's advances, and a whitespace-only run hands its - advance on instead of dropping it — a space per word short on every line. + narrow boxes beside them. ## v6.9.0 - 2026-08-18 diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index d08f5f9a..db2f9acc 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1411,8 +1411,8 @@ class HtmlServiceImpl final : public HtmlService { std::string vis_cur_flow_key; const auto vis_close_line = [&] { vis_cur_line = -1; }; - // Selection layer state: content-stream (reading) order grouping, in the - // open block's flow frame (see the layer below). + // Selection layer state: reading-order grouping, in the open block's + // frame. bool sel_have_prev = false; double sel_prev_baseline = 0; double sel_prev_end = 0; @@ -1428,6 +1428,9 @@ class HtmlServiceImpl final : public HtmlService { /// the open block's transform: its linear part identifies runs one CSS /// matrix can place, its origin anchors the frame they flow in util::math::Transform2D sel_block; + /// advance owed to the next run: whitespace no span could carry, plus a + /// gap no spacer took + double sel_pending_space = 0; for (const pdf::PageElement &element : page_elements(*page, stream, m_logger)) { @@ -1541,12 +1544,9 @@ class HtmlServiceImpl final : public HtmlService { } // --- Selection layer ----------------------------------------------- - // Runs flow inside a line block, so they are measured in the frame that - // block lays out in: page space for an axis-aligned block, the block's - // own space for a matrix one — where the CSS matrix, not the page, - // relates the runs to each other. Both frames put x along the writing - // line and carry the run's font size as their unit of height, so one - // set of gap and line tests serves both. + // A run is measured in the frame its block lays out in: page space for + // an axis-aligned block, the block's own space for a matrix one. Both + // put x along the writing line, so one set of tests serves both. if (!text.text.empty()) { const bool starts_space = text.text.front() == ' '; // A leading inferred space is dropped: the gap between runs is @@ -1585,9 +1585,18 @@ class HtmlServiceImpl final : public HtmlService { sel_prev_end, sel_prev_font_pt); sel_gap = sel_ox - sel_prev_end > 0.25 * sel_prev_font_pt; } + // A block anchors its runs to one baseline, so a run a rise lifts + // off it — a superscript — needs its own, though it breaks nothing. + const bool baseline_shift = + is_matrix && sel_frame_kept && !new_sel_line && + sel_prev_font_pt > 0 && + std::abs(sel_baseline - sel_prev_baseline) > + 0.02 * sel_prev_font_pt; + new_sel_line = new_sel_line || baseline_shift; // The extractor's leading space is the break; a block opened because // the frames are not comparable is not. - const bool break_space = sel_frame_kept || starts_space; + const bool break_space = + (sel_frame_kept && !baseline_shift) || starts_space; if (new_sel_line) { // The block starts here, so this run sits at its origin. @@ -1610,6 +1619,9 @@ class HtmlServiceImpl final : public HtmlService { sel_base += " i"; // transparent page_out.sel_lines.push_back(SelLineOut{std::move(sel_base), {}}); sel_cur_line = static_cast(page_out.sel_lines.size()) - 1; + // Nothing is owed at the origin — unless this run is whitespace, + // which emits no span to carry its advance. + sel_pending_space = core.empty() ? sel_extent : 0; if (!core.empty()) { std::string cls = "sr"; add_class(cls, "f", pt_decl("font-size", font_size_pt)); @@ -1623,15 +1635,15 @@ class HtmlServiceImpl final : public HtmlService { } else if (sel_gap || sel_prev_ends_space || starts_space) { std::vector &runs = page_out.sel_lines[sel_cur_line].runs; + // The gap before this run, plus its own advance when it is only + // whitespace and emits no `.sr`. Dropping either shortens the line. + sel_pending_space += gap_pt + (core.empty() ? sel_extent : 0); + // One space character per run of whitespace; a second would be + // copied as one. The advance waits for the next `.sr` instead. if (!sel_prev_ends_space && !runs.empty()) { std::string gap_cls = "sg"; add_class(gap_cls, "f", pt_decl("font-size", font_size_pt)); - // A run that is only whitespace emits no `.sr`, so its advance - // has to ride the spacer or the line comes up short — invisible - // while a whole line arrives as one `Tj`, plain once it arrives - // word by word. - const double rounded_gap = - round2(gap_pt + (core.empty() ? sel_extent : 0)); + const double rounded_gap = round2(sel_pending_space); if (rounded_gap > 0) { add_class(gap_cls, "w", pt_decl("width", rounded_gap)); // Only a gap that still reads as a word space: a column of @@ -1641,10 +1653,15 @@ class HtmlServiceImpl final : public HtmlService { } } runs.push_back(SelRunOut{std::move(gap_cls), " "}); + sel_pending_space = 0; } if (!core.empty()) { std::string cls = "sr"; add_class(cls, "f", pt_decl("font-size", font_size_pt)); + if (const double owed = round2(sel_pending_space); owed > 0) { + add_class(cls, "ml", pt_decl("margin-left", owed)); + } + sel_pending_space = 0; if (width_pt > 0) { add_class(cls, "w", pt_decl("width", width_pt)); } @@ -2310,9 +2327,8 @@ class HtmlServiceImpl final : public HtmlService { return l.a == r.a && l.b == r.b && l.c == r.c && l.d == r.d; } - /// `m`'s origin in `block`'s own frame: the offset between the two origins - /// resolved along `block`'s axes, x along the writing line. Empty for a - /// singular linear part, which spans no frame to measure in. + /// `m`'s origin along `block`'s own axes, x along the writing line. Empty + /// for a singular linear part, which spans no frame. static std::optional> local_origin(const util::math::Transform2D &block, const util::math::Transform2D &m) { diff --git a/test/data.cmake b/test/data.cmake index 856cacb3..30873504 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "1c2342a880046484425b2a5ffcdac85c33401f20") + REVISION "e400d463fdb45eab82d51e030849ff42c405e4eb") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "115b2f1cb29e5ca4366c97b4891fba9cc8b6b9f0") + REVISION "12dc988aea7ff6a8fa43df8db98e1dbc22f55356") diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index de9c0ada..39f681b6 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -194,13 +194,10 @@ TEST(PdfFile, anisotropic_placement_does_not_space_out_glyphs) { EXPECT_TRUE(contains(spaced, R"( Date: Thu, 20 Aug 2026 20:17:51 +0200 Subject: [PATCH 3/3] fix(pdf): end an operator at every delimiter, and say when a font is dropped `read_operator_name` stopped at four of the nine delimiters of 7.2.2. `(` was not among them, so `Tm(text)Tj` let the name run on and swallow the string, and the show was dropped as an unknown operator. The whole set now ends a bareword; a stray closing one, which opens no token for any reader above to consume, is eaten so the caller's loop still makes progress. `font_is_usable` swallowed the re-encode failure with a bare `catch (...)`, though failing there swaps in a substitute and the page shows it. It now warns through the `Logger`. No corpus font takes that path, so no reference output moves. --- CHANGELOG.md | 4 ++ src/odr/internal/html/pdf_file.cpp | 47 ++++++++++++------- .../pdf/pdf_graphics_operator_parser.cpp | 25 ++++++++-- src/odr/internal/pdf/pdf_object_parser.cpp | 5 ++ src/odr/internal/pdf/pdf_object_parser.hpp | 2 + test/src/internal/pdf/pdf_page_extractor.cpp | 19 ++++++++ 6 files changed, 81 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbd3cd0b..46ad93a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ The release run heads these entries with the version and opens a fresh password. Decrypting them is still out of reach. - Marking text in a rendered pdf highlights the words rather than a trail of narrow boxes beside them. +- A pdf that writes `Tm(text)Tj`, with nothing between the operator and the + string, renders that text instead of dropping it. +- An embedded font that will not re-encode says so in the log rather than being + swapped for a substitute in silence. ## v6.9.0 - 2026-08-18 diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index db2f9acc..7bfcc83e 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1348,10 +1348,11 @@ class HtmlServiceImpl final : public HtmlService { std::unordered_map family_index; const auto font_family = [&](const pdf::Font *font) { - return intern_font(family_index, family_count, font, [&](std::uint32_t) { - accepted_fonts.push_back(font); - font_class_used.push_back({false, false}); - }); + return intern_font(family_index, family_count, font, m_logger, + [&](std::uint32_t) { + accepted_fonts.push_back(font); + font_class_used.push_back({false, false}); + }); }; const auto add_class = [&styles](std::string &classes, @@ -1912,12 +1913,13 @@ class HtmlServiceImpl final : public HtmlService { std::unordered_map family_index; const auto font_family = [&](pdf::Font *font) { - return intern_font(family_index, family_count, font, [&](std::uint32_t) { - accepted_fonts.push_back(font); - glyph_freq.emplace_back(); - used_unicode.emplace_back(); - font_class_used.push_back({false, false}); - }); + return intern_font(family_index, family_count, font, m_logger, + [&](std::uint32_t) { + accepted_fonts.push_back(font); + glyph_freq.emplace_back(); + used_unicode.emplace_back(); + font_class_used.push_back({false, false}); + }); }; AtomicStyles styles; @@ -2490,13 +2492,13 @@ class HtmlServiceImpl final : public HtmlService { template static std::uint32_t intern_font( std::unordered_map &family_index, - std::uint32_t &family_count, const pdf::Font *font, + std::uint32_t &family_count, const pdf::Font *font, const Logger &logger, OnAccept &&on_accept) { const auto [it, inserted] = family_index.try_emplace(font, 0); if (!inserted) { return it->second; } - if (!font_is_usable(*font)) { + if (!font_is_usable(*font, logger)) { return 0; } const std::uint32_t index = ++family_count; @@ -2639,15 +2641,26 @@ class HtmlServiceImpl final : public HtmlService { } /// Whether `font`'s embedded program re-encodes without throwing. Probes the - /// real encode path so failures surface here, not in the post-pass. - static bool font_is_usable(const pdf::Font &font) { + /// real encode path so failures surface here, not in the post-pass. Failing + /// swaps in a substitute, which the page shows, so say so. + static bool font_is_usable(const pdf::Font &font, const Logger &logger) { + const auto dropped = [&](const std::string &why) { + ODR_WARNING(logger, "pdf: rendering '" + << font.embedded_font->name() + << "' with a substitute, its embedded program " + "does not re-encode: " + << why); + return false; + }; if (const auto sfnt = std::dynamic_pointer_cast( font.embedded_font)) { try { (void)write_sfnt_pua(*sfnt, {}); return true; + } catch (const std::exception &e) { + return dropped(e.what()); } catch (...) { - return false; + return dropped("sfnt re-encode failed"); } } if (const auto cff = @@ -2655,8 +2668,10 @@ class HtmlServiceImpl final : public HtmlService { try { (void)font::cff::wrap_to_otf(*cff); return true; + } catch (const std::exception &e) { + return dropped(e.what()); } catch (...) { - return false; + return dropped("cff wrap failed"); } } return false; diff --git a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp index 1fab7f07..998520b0 100644 --- a/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp +++ b/src/odr/internal/pdf/pdf_graphics_operator_parser.cpp @@ -209,11 +209,10 @@ std::string GraphicsOperatorParser::read_operator_name() { if (c == eof) { return result; } - // Any white-space (7.2.2, incl. `\r` in CRLF streams) or the start of a - // following token ends the bareword. `%` is a delimiter too (7.2.2), so a - // comment may follow an operator with nothing in between. - if (ObjectParser::is_whitespace(static_cast(c)) || c == '/' || - c == '<' || c == '[' || c == '%') { + // White space or a delimiter ends the bareword (7.2.2): producers write + // `Tm(text)Tj` with nothing in between. + if (ObjectParser::is_whitespace(static_cast(c)) || + ObjectParser::is_delimiter(static_cast(c))) { return result; } @@ -252,6 +251,22 @@ GraphicsOperator GraphicsOperatorParser::read_operator() { } else if (operator_name == "false") { result.arguments.emplace_back(Boolean(false)); } else { + // A closing delimiter opens nothing, so no reader above consumes it and + // an unmatched one would stall the caller's loop. Eat it and go on. + if (operator_name.empty()) { + if (const int_type c = m_parser.geti(); + c != eof && + ObjectParser::is_delimiter(static_cast(c)) && + !m_parser.peek_name() && !m_parser.peek_string() && + !m_parser.peek_array() && !m_parser.peek_dictionary()) { + ODR_DEBUG(m_logger, "pdf: skipping stray delimiter '" + + std::string(1, static_cast(c)) + + "' in a content stream"); + m_parser.bumpc(); + m_parser.skip_whitespace_and_comments(); + continue; + } + } break; } } diff --git a/src/odr/internal/pdf/pdf_object_parser.cpp b/src/odr/internal/pdf/pdf_object_parser.cpp index 9859d1f1..0e56a4f7 100644 --- a/src/odr/internal/pdf/pdf_object_parser.cpp +++ b/src/odr/internal/pdf/pdf_object_parser.cpp @@ -115,6 +115,11 @@ bool ObjectParser::is_whitespace(const char c) { c == ' '; } +bool ObjectParser::is_delimiter(const char c) { + return c == '(' || c == ')' || c == '<' || c == '>' || c == '[' || c == ']' || + c == '{' || c == '}' || c == '/' || c == '%'; +} + bool ObjectParser::peek_whitespace() { const int_type c = geti(); return c != eof && is_whitespace(static_cast(c)); diff --git a/src/odr/internal/pdf/pdf_object_parser.hpp b/src/odr/internal/pdf/pdf_object_parser.hpp index a3126413..9ca15428 100644 --- a/src/odr/internal/pdf/pdf_object_parser.hpp +++ b/src/odr/internal/pdf/pdf_object_parser.hpp @@ -42,6 +42,8 @@ class ObjectParser { char_type third); static bool is_whitespace(char c); + /// The delimiters of 7.2.2, each of which opens a token of its own. + static bool is_delimiter(char c); [[nodiscard]] bool peek_whitespace(); void skip_whitespace(); /// White space plus the comments (`%` to the end of the line, 7.2.4) it may diff --git a/test/src/internal/pdf/pdf_page_extractor.cpp b/test/src/internal/pdf/pdf_page_extractor.cpp index ea63cf1b..b63bbacf 100644 --- a/test/src/internal/pdf/pdf_page_extractor.cpp +++ b/test/src/internal/pdf/pdf_page_extractor.cpp @@ -144,6 +144,25 @@ TEST(PdfPageExtractor, comment_directly_after_operator) { EXPECT_EQ(texts[0].codes, "Hi"); } +// A closing delimiter opens no token, so an unmatched one has to be eaten or +// the operator loop makes no progress. +TEST(PdfPageExtractor, stray_closing_delimiter_does_not_stall) { + const auto texts = run("BT /F1 12 Tf 1 0 0 1 5 5 Tm ) ] > } (Hi) Tj ET"); + ASSERT_EQ(texts.size(), 1); + EXPECT_EQ(texts[0].codes, "Hi"); +} + +// Nothing need separate an operator from the token after it (7.2.2). Without +// `(` ending the name, it ran on and swallowed the string behind it. +TEST(PdfPageExtractor, operator_directly_followed_by_a_string) { + const auto texts = run("BT /F1 12 Tf 1 0 0 1 100 700 Tm(Hi)Tj(there)Tj ET"); + ASSERT_EQ(texts.size(), 2); + EXPECT_DOUBLE_EQ(texts[0].transform.e, 100); + EXPECT_DOUBLE_EQ(texts[0].transform.f, 700); + EXPECT_EQ(texts[0].codes, "Hi"); + EXPECT_EQ(texts[1].codes, "there"); +} + // `Tm` sets the text matrix outright, scaling and all. TEST(PdfPageExtractor, tm_scaling) { const auto texts = run("BT /F1 10 Tf 2 0 0 2 50 60 Tm (X) Tj ET");