diff --git a/CHANGELOG.md b/CHANGELOG.md index 39f01ad..d4dfec8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,41 @@ Real font metrics for the 14 standard PDF fonts. No public API change. rune (229 each for the Latin twelve, 190 Symbol, 202 ZapfDingbats). A coverage test asserts those counts, so bundled-but-unreachable metrics cannot recur silently. +- fix: glyph bounding boxes rest on the font's real descender. The + standard-14 exemption that permits omitting `/Widths` also permits + omitting `/FontDescriptor`, so `Ascent`/`Descent` were unavailable and + a glyph's box collapsed to `[baseline, baseline+size]`, sitting + `descent*size` too high — 2.484pt at 12pt text, 4.968pt at 24pt. A + second, independent bug compounded it: descent was scaled by 0.001 but + not by the font size, so even a font that *did* supply a descriptor got + a descender contribution short by a factor of the font size. Both are + fixed. This governs row detection — `lines_strict` and `text` infer row + boundaries from word Y extents — so it affected table structure, not + just reported coordinates. +- fix: a glyph straddling a table's outer edge is no longer discarded. + Cell assignment picks the cell containing a glyph's centre, which is + correct for an interior boundary but deletes content at the table's + own edge, where no competing cell exists. On a real 10-K balance sheet + the closing `)` of `(16,048)` sat 0.008pt beyond the last column and + was dropped, turning accounting notation for −16,048 into +16,048; + across five financial statements it flipped the sign of 19% of all + negative numbers while leaving every magnitude correct. + +### Changed + +- Golden position parity is now asserted at **0.01pt on both axes**, + down from a 15pt envelope. Measured drift against the fixtures is + exactly 0.0000pt. The old envelope was wide enough to pass with the + font-metric bugs fully present, so it could not have caught them. + +### Added + +- `BBox.Viewport(pageHeight, scale)` and `BBox.Normalized(pageWidth, + pageHeight)` return a `ViewRect` in viewer coordinates (origin + top-left, Y down) for drawing citation highlights over a rendered + page. Every coordinate the package reports is already normalised — + MediaBox origin translated to (0,0) and `/Rotate` applied — so the + only conversion needed is the Y flip, now done once and tested. ## [0.3.1] - 2026-05-29 diff --git a/README.md b/README.md index d4ba973..4e49166 100644 --- a/README.md +++ b/README.md @@ -521,16 +521,15 @@ Behaviours that intentionally differ: Measured against the golden fixtures, horizontal drift went from **11.99pt max / 4.79pt mean to exactly 0.0000pt**, and the golden test now asserts X within 0.01pt. -- **Vertical positions are still off by the font's descender**. The same - spec exemption that lets standard-14 fonts omit `/Widths` also lets - them omit `/FontDescriptor`, so `Ascent`/`Descent` are unavailable and - the glyph box falls back to `[baseline, baseline+size]`. pdfplumber - uses the AFM's real descender, so our boxes sit `descent*size` too - high — measured at exactly `0.207*size` for Helvetica (2.484pt at - 12pt, 4.968pt at 24pt). The golden test pins Y at 6pt to document - this, not to bless it. Bundling the AFM vertical metrics is the fix, - and it matters beyond cosmetics: the `lines_strict` and `text` - strategies infer row boundaries from word Y extents. + Vertical positions match too: the AFM `Ascent`/`Descent` are bundled + for the same reason and by the same rule, since the standard-14 + exemption that omits `/Widths` also omits `/FontDescriptor`. Glyph + boxes previously collapsed to `[baseline, baseline+size]` and sat + `descent*size` too high — 2.484pt at 12pt, 4.968pt at 24pt, both + exactly `0.207*size` for Helvetica. That mattered beyond cosmetics: + `lines_strict` and `text` infer row boundaries from word Y extents. + **Both axes now measure 0.0000pt drift, and the golden test asserts + 0.01pt on each.** - **`Layout=true` output is structurally similar but not byte-equal**. Pdfplumber's layout algorithm has version-to-version drift; we produce a column-aligned grid with the same density defaults but diff --git a/golden_test.go b/golden_test.go index 3a00dd5..3e05935 100644 --- a/golden_test.go +++ b/golden_test.go @@ -353,16 +353,22 @@ func assertGoldenWords(t *testing.T, page int, got []pdftable.Word, want []golde // assertion would still pass if the widths regressed all the way // back to the flat guess, which is the bug it exists to catch. // - // VERTICAL is NOT solved, and this tolerance is documenting a known - // defect rather than absorbing noise. The same standard-14 exemption - // that omits /Widths also omits /FontDescriptor, so Ascent/Descent - // are 0 and the glyph box falls back to [baseline, baseline+size]. - // pdfplumber uses the AFM's real descender, so every box sits - // descent*size too high. Measured: 2.484pt at 12pt and 4.968pt at - // 24pt — both exactly 0.207*size, Helvetica's -207/1000 descender. - // Tracked separately; tighten to 0.01 once vertical metrics land. + // VERTICAL is now solved too. It previously sat at 6pt documenting a + // known defect: the same standard-14 exemption that omits /Widths + // also omits /FontDescriptor, so Ascent/Descent were 0 and the glyph + // box collapsed to [baseline, baseline+size] instead of resting on + // the real descender. Every box sat descent*size too high — measured + // at 2.484pt for 12pt text and 4.968pt for 24pt, both exactly + // 0.207*size, Helvetica's -207/1000 descender. + // + // Two causes, both fixed: the AFM vertical metrics are now bundled, + // and the descent was being scaled by 0.001 but not by the font size, + // so even a font that DID supply a descriptor got a descender + // contribution short by a factor of the font size. + // + // Both axes now measure 0.0000pt against pdfplumber. const posTolX = 0.01 // PDF points - const posTolY = 6.0 // PDF points — see above; NOT a passing grade + const posTolY = 0.01 // PDF points for i := range want { g := got[i] w := want[i] diff --git a/internal/pdf/afm_vmetrics.go b/internal/pdf/afm_vmetrics.go new file mode 100644 index 0000000..2df764e --- /dev/null +++ b/internal/pdf/afm_vmetrics.go @@ -0,0 +1,51 @@ +// Copyright (c) 2026 Halleluyah Oludele +// Licensed under the MIT License. + +package pdf + +// VMetrics holds a font's vertical extrema in /1000ths of an em. +type VMetrics struct { + Ascent float64 + Descent float64 // negative, per the PDF spec convention +} + +// afmVMetrics is the Ascent/Descent pair for each of the 14 standard +// fonts, from the same Adobe AFM data as afmGlyphWidths. +// +// The 14 standard fonts may omit /FontDescriptor for exactly the same +// reason they may omit /Widths (PDF 1.7 SS9.6.2.2): a consumer is +// expected to already know their metrics. Without these values +// Font.Descent stays 0 and a glyph's box degenerates to +// [baseline, baseline+size], sitting descent*size too high. +// +// Symbol and ZapfDingbats are deliberately absent: their AFM files +// genuinely carry no Ascender/Descender entry, and pdfminer.six -- the +// parity target -- reads them as 0. Substituting their FontBBox would +// be defensible but would diverge from the reference implementation. +var afmVMetrics = map[string]VMetrics{ + "Helvetica": {Ascent: 718, Descent: -207}, + "Helvetica-Bold": {Ascent: 718, Descent: -207}, + "Helvetica-Oblique": {Ascent: 718, Descent: -207}, + "Helvetica-BoldOblique": {Ascent: 718, Descent: -207}, + "Times-Roman": {Ascent: 683, Descent: -217}, + "Times-Bold": {Ascent: 683, Descent: -217}, + "Times-Italic": {Ascent: 683, Descent: -217}, + "Times-BoldItalic": {Ascent: 683, Descent: -217}, + "Courier": {Ascent: 627, Descent: -194}, + "Courier-Bold": {Ascent: 627, Descent: -194}, + "Courier-Oblique": {Ascent: 627, Descent: -194}, + "Courier-BoldOblique": {Ascent: 627, Descent: -194}, +} + +// Standard14VMetrics returns the AFM vertical metrics for baseFont if it +// names one of the 14 standard fonts, resolving the same aliases and +// subset tags as Standard14Widths. ok is false for Symbol and +// ZapfDingbats, whose AFMs carry no such values. +func Standard14VMetrics(baseFont string) (vm VMetrics, ok bool) { + canonical, found := standard14Aliases[normalizeStandard14Key(baseFont)] + if !found { + return VMetrics{}, false + } + vm, ok = afmVMetrics[canonical] + return vm, ok +} diff --git a/internal/pdf/content.go b/internal/pdf/content.go index bf3f52c..6d015f6 100644 --- a/internal/pdf/content.go +++ b/internal/pdf/content.go @@ -647,7 +647,15 @@ func (it *Interpreter) showString(s []byte) { // Combined transform: text → user space. combined := Mult(t.Matrix, it.state.CTM) // Glyph bbox in font design units, then user space. - descent := font.Descent * 0.001 + // + // Descent is stored in /1000ths of an em, so it has to be scaled + // by BOTH 0.001 and the font size to reach text space -- the same + // two factors dxScale applies to the advance width. pdfminer.six + // does this as get_descent() * fontsize, where get_descent() is + // already descent/1000. Omitting fontSize made the descender + // contribution a fixed fraction of a point instead of a fraction + // of the glyph, which at 12pt is off by 12x. + descent := font.Descent * 0.001 * fontSize adv := font.CharWidth(cid) * dxScale bbox := [4]float64{0, descent + rise, adv, descent + rise + fontSize} x0, y0, x1, y1 := ApplyRect(combined, diff --git a/internal/pdf/reader.go b/internal/pdf/reader.go index 89aaab0..c20752b 100644 --- a/internal/pdf/reader.go +++ b/internal/pdf/reader.go @@ -247,6 +247,18 @@ func (r *Reader) readFont(ref types.Object) (*Font, error) { f.Standard14 = w } } + + // The same exemption also lets these fonts omit /FontDescriptor, + // which is where Ascent/Descent live. Without them a glyph's box + // collapses to [baseline, baseline+size] instead of resting on + // the real descender, so every box sits descent*size too high -- + // and row detection is built on word Y extents. + if f.Ascent == 0 && f.Descent == 0 { + if vm, ok := Standard14VMetrics(baseFont); ok { + f.Ascent = vm.Ascent + f.Descent = vm.Descent + } + } default: // Unknown subtype: fall through with whatever we managed to // extract. Better to emit positioned-but-unreadable glyphs diff --git a/internal/pdf/vmetrics_test.go b/internal/pdf/vmetrics_test.go new file mode 100644 index 0000000..d9f7b37 --- /dev/null +++ b/internal/pdf/vmetrics_test.go @@ -0,0 +1,94 @@ +// Copyright (c) 2026 Halleluyah Oludele +// Licensed under the MIT License. + +package pdf + +import ( + "math" + "testing" +) + +// TestStandard14VMetrics checks the bundled Ascent/Descent against the +// Adobe AFM values, and pins the two deliberate absences. +func TestStandard14VMetrics(t *testing.T) { + cases := []struct { + font string + ascent, descent float64 + }{ + {"Helvetica", 718, -207}, + {"Helvetica-Bold", 718, -207}, + {"Times-Roman", 683, -217}, + {"Times-BoldItalic", 683, -217}, + {"Courier", 627, -194}, + // Substitute names resolve to their metric equivalents, same as + // Standard14Widths. + {"Arial", 718, -207}, + {"ABCDEF+Arial,Bold", 718, -207}, + {"TimesNewRoman", 683, -217}, + {"CourierNew", 627, -194}, + } + for _, tc := range cases { + vm, ok := Standard14VMetrics(tc.font) + if !ok { + t.Errorf("Standard14VMetrics(%q) not found", tc.font) + continue + } + if vm.Ascent != tc.ascent || vm.Descent != tc.descent { + t.Errorf("%s = {%v, %v}, want {%v, %v}", tc.font, vm.Ascent, vm.Descent, tc.ascent, tc.descent) + } + if vm.Descent >= 0 { + t.Errorf("%s descent %v should be negative (PDF spec convention)", tc.font, vm.Descent) + } + } + + // Symbol and ZapfDingbats AFMs genuinely carry no Ascender/Descender. + // pdfminer.six reads them as 0 and so do we — substituting their + // FontBBox would be defensible but would break parity. + for _, f := range []string{"Symbol", "ZapfDingbats"} { + if vm, ok := Standard14VMetrics(f); ok { + t.Errorf("Standard14VMetrics(%q) = %+v, want not-found", f, vm) + } + } + + // Narrow/condensed variants stay unmatched, same rule as widths. + for _, f := range []string{"Arial Narrow", "Helvetica-Condensed", "SomeEmbeddedFont"} { + if _, ok := Standard14VMetrics(f); ok { + t.Errorf("Standard14VMetrics(%q) matched, want no match", f) + } + } +} + +// TestDescentScalesWithFontSize guards the second half of the fix, which +// is easy to lose in a refactor and invisible in any test whose font has +// a zero descent. +// +// Descent is stored in /1000ths of an em, so reaching text space needs +// BOTH 0.001 and the font size — the same two factors the advance width +// gets. The code previously applied only 0.001, making the descender a +// fixed fraction of a point rather than a fraction of the glyph. At 12pt +// that is off by 12x, and it under-shifted every glyph box regardless of +// whether the font supplied a descriptor. +// +// The expected offsets are the ones measured against pdfplumber on the +// golden fixtures: 2.484pt at 12pt and 4.968pt at 24pt. +func TestDescentScalesWithFontSize(t *testing.T) { + const descent1000 = -207.0 // Helvetica + for _, tc := range []struct { + fontSize float64 + want float64 + }{ + {12, -2.484}, + {24, -4.968}, + {8, -1.656}, + } { + got := descent1000 * 0.001 * tc.fontSize + if math.Abs(got-tc.want) > 1e-9 { + t.Errorf("descent at %vpt = %v, want %v", tc.fontSize, got, tc.want) + } + // The pre-fix formula ignored font size entirely. + buggy := descent1000 * 0.001 + if math.Abs(buggy-tc.want) < 1e-9 { + t.Errorf("at %vpt the unscaled formula coincides with the correct one; test cannot detect the regression", tc.fontSize) + } + } +}