From 996e61a72cba510d522202716bc022f235ec1836 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 16:40:31 +0200 Subject: [PATCH 1/2] fix(pdf): keep a matrix-placed word from spacing out its glyphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A producer that fits a line to an exact width writes a `Tm` scaling x and y differently. `is_matrix` then holds for every run on that line, and the selection layer opened a fresh block — and closed the previous one with a space — for each of them, because page-space `ox`/`baseline` cannot say whether two runs share a line once a general transform is in play. A page laid out one `Tj` per glyph came out as `L a g e`. The extractor's pen already infers the break along each run's own writing line, so its leading space is the evidence to go by; the layer no longer fabricates a second one on the matrix path alone. --- CHANGELOG.md | 5 +++++ src/odr/internal/html/pdf_file.cpp | 15 ++++++++++--- test/data.cmake | 6 ++--- test/src/internal/pdf/pdf_file.cpp | 35 ++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc792f39..db2dff2e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ The release run heads these entries with the version and opens a fresh `` LibreOffice writes for a repeating header row. They were dropped from the output entirely, in text documents and spreadsheets alike. +- Text selected or copied out of a rendered pdf reads as words again where the + producer scales a line's x and y differently to fit an exact width. Such a + line goes down the CSS matrix path, which used to start a fresh selection + block — and a space — for every run, so a page laid out glyph by glyph came + out as `L a g e`. ## 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 e4b27123a..a0c815b15 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1546,19 +1546,28 @@ class HtmlServiceImpl final : public HtmlService { // covered by the spacer span, not by the run text. std::string core = starts_space ? text.text.substr(1) : text.text; - bool new_sel_line = - !sel_have_prev || is_matrix || sel_prev_was_matrix; + const bool matrix_break = is_matrix || sel_prev_was_matrix; + bool new_sel_line = !sel_have_prev || matrix_break; 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; } + // A block opened only because the two transforms are not comparable + // says nothing about a break: `ox`/`baseline` live in page space, so + // a matrix run gets its own block even mid-word. The extractor's pen + // already inferred the break along the run's own writing line, so the + // leading space it left is the evidence — closing the line on the + // matrix path alone puts a space between every glyph of a word laid + // out glyph by glyph (a `Tm` that scales x and y differently). + const bool break_space = !matrix_break || starts_space; if (new_sel_line) { // 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) { + if (sel_cur_line >= 0 && sel_have_prev && !sel_prev_ends_space && + break_space) { std::string space_cls = "sg"; add_class(space_cls, "f", pt_decl("font-size", sel_prev_font_size_pt)); diff --git a/test/data.cmake b/test/data.cmake index 05d914f32..30fdc2dd0 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -12,14 +12,14 @@ odr_test_data( odr_test_data( PATH "input/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.git" - REVISION "b1deaf20eb08054cf88fcc4cae33d0e90e185da3") + REVISION "112cf18f0ef246dfa31ac21f95f5167d0aaa2bcc") odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "c7258ff998e22aaedf514ef2c1475dc3c22d0a96") + REVISION "e352a4626b8e7cffa7f7a077d07af4edc8d46809") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "21f9c3f49de727acee20768d6177980577423764") + REVISION "460544f350ea0f7c4e009721a184e4f496ff2733") diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index b9ae1721c..5a31b4abb 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -86,6 +86,20 @@ std::string info_mini_pdf() { return builder.trailer("/Root 1 0 R /Info 5 0 R").build_classic(); } +/// A one-page mini-PDF drawing `content` with a non-embedded Helvetica as +/// `/F1`, whose standard-14 metrics give the glyph advances without a font +/// program. +std::string text_mini_pdf(const std::string &content) { + PdfFileBuilder builder; + builder.object("<< /Type /Catalog /Pages 2 0 R >>") + .object("<< /Type /Pages /Kids [3 0 R] /Count 1 >>") + .object("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] " + "/Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >>") + .stream_object("", content) + .object("<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>"); + return builder.trailer("/Root 1 0 R").build_classic(); +} + } // namespace // `/Info` document-information strings and the page count surface through @@ -154,6 +168,27 @@ TEST(PdfFile, link_annotations_render_as_anchors) { } } +// A `Tm` that scales x and y differently — what a producer writes to fit a line +// to an exact width — puts every run on the CSS matrix path, where the +// page-space origin cannot say whether two runs share a line. The selection +// layer must not read that as a break: the extractor's pen already inferred the +// spaces along each run's own writing line, so glyphs shown one `Tj` at a time +// still read as one word, while a real gap keeps its space. +TEST(PdfFile, anisotropic_placement_does_not_space_out_glyphs) { + const std::string tight = render_html( + text_mini_pdf("BT /F1 12 Tf 0.9 0 0 1 72 700 Tm (H) Tj (i) Tj ET"), + PdfTextMode::dual_layer); + EXPECT_TRUE(contains(tight, ">H")); + EXPECT_TRUE(contains(tight, ">i")); + EXPECT_FALSE(contains(tight, R"( Date: Thu, 20 Aug 2026 20:27:15 +0200 Subject: [PATCH 2/2] fix(pdf): hold a break a segment with no text cannot carry The layer above reads the extractor's leading space as the evidence that two runs are separated, so a segment that has nothing extractable must not consume the break it sits in. A `no_unicode` run or an `/ActualText` tail between two words did exactly that: the pen advanced past the gap, the empty text could carry no space, and the words either side read as one. The pen now holds the inferred break until a segment can take it. --- CHANGELOG.md | 7 ++----- src/odr/internal/html/pdf_file.cpp | 9 ++------- src/odr/internal/pdf/pdf_page_extractor.cpp | 16 ++++++++++++--- test/src/internal/pdf/pdf_file.cpp | 8 ++------ test/src/internal/pdf/pdf_page_extractor.cpp | 21 ++++++++++++++++++++ 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db2dff2e2..23d7ea928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,11 +36,8 @@ The release run heads these entries with the version and opens a fresh `` LibreOffice writes for a repeating header row. They were dropped from the output entirely, in text documents and spreadsheets alike. -- Text selected or copied out of a rendered pdf reads as words again where the - producer scales a line's x and y differently to fit an exact width. Such a - line goes down the CSS matrix path, which used to start a fresh selection - block — and a space — for every run, so a page laid out glyph by glyph came - out as `L a g e`. +- 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. ## 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 a0c815b15..4e9738584 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -1554,13 +1554,8 @@ class HtmlServiceImpl final : public HtmlService { sel_prev_end, sel_prev_font_pt); sel_gap = ox - sel_prev_end > 0.25 * sel_prev_font_pt; } - // A block opened only because the two transforms are not comparable - // says nothing about a break: `ox`/`baseline` live in page space, so - // a matrix run gets its own block even mid-word. The extractor's pen - // already inferred the break along the run's own writing line, so the - // leading space it left is the evidence — closing the line on the - // matrix path alone puts a space between every glyph of a word laid - // out glyph by glyph (a `Tm` that scales x and y differently). + // The extractor's leading space is the break; a block the matrix + // path opens is not. const bool break_space = !matrix_break || starts_space; if (new_sel_line) { diff --git a/src/odr/internal/pdf/pdf_page_extractor.cpp b/src/odr/internal/pdf/pdf_page_extractor.cpp index b35437b3c..9cda42196 100644 --- a/src/odr/internal/pdf/pdf_page_extractor.cpp +++ b/src/odr/internal/pdf/pdf_page_extractor.cpp @@ -114,11 +114,14 @@ SegmentAdvances segment_advances(const GraphicsState::Text &text, /// inter-word spaces PDFs routinely omit. `position` is the segment /// origin *after* its advance; `direction`/`em` give the writing line and scale /// the gap threshold; `trailing_space` suppresses a doubled space. +/// `pending_space` carries a break a segment with nothing extractable could +/// not. struct Pen { std::array position{0, 0}; std::array direction{1, 0}; double em{0}; bool trailing_space{false}; + bool pending_space{false}; }; /// Whether a space should be inferred before a segment starting at `start` @@ -183,8 +186,10 @@ void show(std::vector &out, GraphicsState &state, const std::array direction = basis > 0 ? std::array{m.a / basis, m.b / basis} : std::array{1, 0}; - if (!element.text.empty() && element.text.front() != ' ' && pen.has_value() && - infer_space(*pen, start)) { + // Nothing extractable can carry no break, so hold it on the pen. + const bool break_here = + pen.has_value() && (infer_space(*pen, start) || pen->pending_space); + if (!element.text.empty() && element.text.front() != ' ' && break_here) { element.text.insert(element.text.begin(), ' '); element.leading_space_inferred = true; } @@ -197,13 +202,18 @@ void show(std::vector &out, GraphicsState &state, : element.text.back() == ' '; const double advance = element.width; + const bool element_text_empty = element.text.empty(); out.push_back(std::move(element)); state.advance_text(advance, 0); // Record the pen at the true post-advance origin (so `TJ` adjustments and any // explicit repositioning before the next segment fold into the next gap). const util::math::Transform2D after = state.text_placement_transform(); - pen = Pen{{after.e, after.f}, direction, text.size * basis, trailing_space}; + pen = Pen{{after.e, after.f}, + direction, + text.size * basis, + trailing_space, + element_text_empty && break_here}; } /// Resolve a colour-space operator (`cs`/`CS`): set the active colour space on diff --git a/test/src/internal/pdf/pdf_file.cpp b/test/src/internal/pdf/pdf_file.cpp index 5a31b4abb..ae1ea1500 100644 --- a/test/src/internal/pdf/pdf_file.cpp +++ b/test/src/internal/pdf/pdf_file.cpp @@ -168,12 +168,8 @@ TEST(PdfFile, link_annotations_render_as_anchors) { } } -// A `Tm` that scales x and y differently — what a producer writes to fit a line -// to an exact width — puts every run on the CSS matrix path, where the -// page-space origin cannot say whether two runs share a line. The selection -// layer must not read that as a break: the extractor's pen already inferred the -// spaces along each run's own writing line, so glyphs shown one `Tj` at a time -// still read as one word, while a real gap keeps its space. +// A `Tm` scaling x and y differently takes the CSS matrix path: glyphs shown +// one `Tj` at a time read as one word, and a real gap keeps its space. TEST(PdfFile, anisotropic_placement_does_not_space_out_glyphs) { const std::string tight = render_html( text_mini_pdf("BT /F1 12 Tf 0.9 0 0 1 72 700 Tm (H) Tj (i) Tj ET"), diff --git a/test/src/internal/pdf/pdf_page_extractor.cpp b/test/src/internal/pdf/pdf_page_extractor.cpp index 745dce866..ea63cf1b2 100644 --- a/test/src/internal/pdf/pdf_page_extractor.cpp +++ b/test/src/internal/pdf/pdf_page_extractor.cpp @@ -463,6 +463,27 @@ TEST(PdfPageExtractor, no_unicode_marks_composite_without_tounicode) { EXPECT_EQ(texts[0].codes, std::string("\x00\x01", 2)); } +// The pen holds a break a `no_unicode` run cannot carry. +TEST(PdfPageExtractor, break_survives_a_segment_with_no_text) { + Font simple = simple_font(0x41, {500, 500}); // A, B = 0.5 em + Font opaque; + opaque.composite = true; // no cmap, no encoding: no recoverable Unicode + Resources res; + res.font["F1"] = &simple; + res.font["F2"] = &opaque; + + // `A` at 0, the opaque glyph a wide gap later, then `B` right behind it. + const auto texts = run("BT /F1 10 Tf 1 0 0 1 0 0 Tm (A) Tj " + "1 0 0 1 40 0 Tm /F2 10 Tf <0001> Tj " + "/F1 10 Tf (B) Tj ET", + res); + ASSERT_EQ(texts.size(), 3); + EXPECT_EQ(texts[0].text, "A"); + EXPECT_TRUE(texts[1].text.empty()); + EXPECT_EQ(texts[2].text, " B"); + EXPECT_TRUE(texts[2].leading_space_inferred); +} + // `/ActualText` on a marked-content sequence overrides the per-glyph text for // extraction (ligatures, reordered glyphs); a literal string is taken as-is. TEST(PdfPageExtractor, actual_text_overrides_segment) {