From 5ec11f7b07c523528ea343b91b940c0cc39c64de Mon Sep 17 00:00:00 2001 From: Jack Date: Tue, 18 Aug 2026 13:11:41 +0800 Subject: [PATCH] fix(pdf): align TSR crop margin with Python (fixed 30px, not 3%) (#18387) --- .../parser/pdf/table/table_construct_test.go | 9 ++- internal/deepdoc/parser/pdf/table_extract.go | 6 +- .../deepdoc/parser/pdf/table_extract_test.go | 74 +++++++++++++++++-- internal/deepdoc/parser/pdf/util/crop.go | 18 +++-- internal/deepdoc/parser/pdf/util/crop_test.go | 43 +++++++++-- 5 files changed, 124 insertions(+), 26 deletions(-) diff --git a/internal/deepdoc/parser/pdf/table/table_construct_test.go b/internal/deepdoc/parser/pdf/table/table_construct_test.go index 719a3d837f..7fde522aa8 100644 --- a/internal/deepdoc/parser/pdf/table/table_construct_test.go +++ b/internal/deepdoc/parser/pdf/table/table_construct_test.go @@ -143,13 +143,14 @@ func TestExtractTableAndReplace_CellTextFilled(t *testing.T) { // Simulate 公司差旅费 page 0 table coordinates. // DLA region: X0=217, X1=1584, Y0=985, Y1=1599 at 216 DPI → PDF: 72-528 x 328-533 // Scale = 216/72 = 3.0 - // cropOff ≈ region.X - region.W*0.03 + // cropOff = region.X - 10pt*ZM = region.X - 30px (fixed margin, matches + // Python's MARGIN=10; NOT the old Go 3% proportional margin). const scale = 3.0 - const cropOffX = 176.0 - const cropOffY = 967.0 + const cropOffX = 187.0 + const cropOffY = 955.0 // Post-merge boxes in PDF point space (inside the table region). - // PDF Y=470 → crop Top = 470*3-967 = 443 → overlaps crop cell at Y0=441. + // PDF Y=470 → crop Top = 470*3-955 = 455 → overlaps crop cell at Y0=441. // Boxes must have R (row) and C (col) annotations matching cells, // matching Python's construct_table which assigns boxes to cells by R/C. boxes := []pdf.TextBox{ diff --git a/internal/deepdoc/parser/pdf/table_extract.go b/internal/deepdoc/parser/pdf/table_extract.go index 3f9030914c..465d4c9c09 100644 --- a/internal/deepdoc/parser/pdf/table_extract.go +++ b/internal/deepdoc/parser/pdf/table_extract.go @@ -97,10 +97,8 @@ func (p *Parser) processOneTable(ctx context.Context, pageImg image.Image, boxes if tsrErr != nil { slog.Warn("TSR failed", "page", pageNum, "err", tsrErr) } - w := tm.Region.X1 - tm.Region.X0 - h := tm.Region.Y1 - tm.Region.Y0 - cropOffX := math.Max(0, tm.Region.X0-w*0.03) - cropOffY := math.Max(0, tm.Region.Y0-h*0.03) + cropOffX := math.Max(0, tm.Region.X0-util.TSRRegionMarginPx) + cropOffY := math.Max(0, tm.Region.Y0-util.TSRRegionMarginPx) var boxInCrop []pdf.TextBox if tsrErr == nil && len(cells) > 0 { if bestAngle != 0 { diff --git a/internal/deepdoc/parser/pdf/table_extract_test.go b/internal/deepdoc/parser/pdf/table_extract_test.go index 5c8faf46c7..eb7b551e2c 100644 --- a/internal/deepdoc/parser/pdf/table_extract_test.go +++ b/internal/deepdoc/parser/pdf/table_extract_test.go @@ -3,10 +3,12 @@ package pdf import ( "context" "image" + "math" "testing" tbl "ragflow/internal/deepdoc/parser/pdf/table" pdf "ragflow/internal/deepdoc/parser/pdf/type" + util "ragflow/internal/deepdoc/parser/pdf/util" ) type orientationScoringDoc struct{} @@ -87,11 +89,73 @@ func TestProcessOneTable_AutoRotateNormalizesCellBounds(t *testing.T) { } got := item.Cells[0] - if got.X0 != 20 || got.Y0 != 45 || got.X1 != 80 || got.Y1 != 95 { - t.Errorf("cell bounds = (%.0f,%.0f,%.0f,%.0f), want (20,45,80,95)", - got.X0, got.Y0, got.X1, got.Y1) - } - if got.X0 > got.X1 || got.Y0 > got.Y1 { + + // Auto-rotate must return axis-aligned (non-inverted) bounds — the core + // "normalize" invariant. Asserting this instead of absolute pixels makes + // the test immune to TSR crop margin / crop-size changes. + if got.X0 >= got.X1 || got.Y0 >= got.Y1 { t.Fatalf("cell bounds are inverted: (%.0f,%.0f,%.0f,%.0f)", got.X0, got.Y0, got.X1, got.Y1) } + + // Rotation is area-preserving: the processed cell keeps the input cell's + // area (50*60 = 3000) regardless of crop size or rotation angle. + const inW, inH = 50.0, 60.0 + gotW, gotH := got.X1-got.X0, got.Y1-got.Y0 + if math.Abs(gotW*gotH-inW*inH) > 1e-6 { + t.Errorf("cell area = %.0f, want %.0f (rotation preserves area)", gotW*gotH, inW*inH) + } + + // The cell must stay inside the cropped image. Crop bounds are derived + // from the shared TSRRegionMarginPx constant, so they track margin changes. + cropX0 := math.Max(0, match.Region.X0-util.TSRRegionMarginPx) + cropY0 := math.Max(0, match.Region.Y0-util.TSRRegionMarginPx) + cropX1 := math.Min(float64(pageImg.Bounds().Dx()), match.Region.X1+util.TSRRegionMarginPx) + cropY1 := math.Min(float64(pageImg.Bounds().Dy()), match.Region.Y1+util.TSRRegionMarginPx) + if got.X0 < cropX0-1 || got.Y0 < cropY0-1 || got.X1 > cropX1+1 || got.Y1 > cropY1+1 { + t.Errorf("cell (%.0f,%.0f,%.0f,%.0f) outside crop (%.0f,%.0f,%.0f,%.0f)", + got.X0, got.Y0, got.X1, got.Y1, cropX0, cropY0, cropX1, cropY1) + } +} + +// TestProcessOneTable_CropOffUsesFixedMargin locks the parity contract that +// the TSR crop offset is a fixed margin (TSRRegionMarginPx = 10pt * DlaScale = +// 30px), not a proportional percentage of the region. processOneTable computes +// cropOffX = max(0, region.X0 - TSRRegionMarginPx); for a region whose X0/Y0 +// lie beyond the margin the offsets must equal region.X - 30 (nonzero), which +// is exactly the inverse of CropImageRegion's forward 30px expansion. The +// pre-fix code used w*0.03/h*0.03 here, diverging from Python. +func TestProcessOneTable_CropOffUsesFixedMargin(t *testing.T) { + cfg := pdf.DefaultParserConfig() + cfg.SkipOCR = true + p := NewParser(cfg) + + pageImg := image.NewRGBA(image.Rect(0, 0, 320, 220)) + boxes := []pdf.TextBox{ + {X0: 10, X1: 60, Top: 10, Bottom: 30, Text: "cell", LayoutType: pdf.LayoutTypeTable}, + } + // Region beyond TSRRegionMarginPx (30px), with distinct X/Y origins so a + // regression that uses the wrong origin for CropOffY is caught. Offset + // must be region.X - 30 (nonzero), not the old proportional w*0.03 and + // not clamped to 0. + const regionX0, regionY0 = 100.0, 140.0 + match := tbl.TableMatch{ + Region: pdf.DLARegion{X0: regionX0, Y0: regionY0, X1: 210, Y1: 200, Label: pdf.LayoutTypeTable}, + BoxIdx: []int{0}, + } + builder := &staticTableBuilder{ + cells: []pdf.TSRCell{ + {X0: 10, Y0: 20, X1: 60, Y1: 80, Label: "table row"}, + }, + } + + item := p.processOneTable(context.Background(), pageImg, boxes, 0, &orientationScoringDoc{}, builder, match, pdf.DlaScale) + + const wantOffX = regionX0 - util.TSRRegionMarginPx // 100 - 30 = 70 + const wantOffY = regionY0 - util.TSRRegionMarginPx // 140 - 30 = 110 + if item.CropOffX != wantOffX { + t.Errorf("cropOffX = %v, want %v (region.X0 - fixed 30px margin)", item.CropOffX, wantOffX) + } + if item.CropOffY != wantOffY { + t.Errorf("cropOffY = %v, want %v (region.Y0 - fixed 30px margin)", item.CropOffY, wantOffY) + } } diff --git a/internal/deepdoc/parser/pdf/util/crop.go b/internal/deepdoc/parser/pdf/util/crop.go index 214679b0f3..9f86e3e372 100644 --- a/internal/deepdoc/parser/pdf/util/crop.go +++ b/internal/deepdoc/parser/pdf/util/crop.go @@ -558,13 +558,19 @@ func MapRotatedRectToOriginal(x0, y0, x1, y1 float64, angle int, origW, origH in return minX, minY, maxX, maxY } -// CropImageRegion crops a pdf.DLARegion from an image with a 3% margin -// (matching Python's _table_transformer_job: w*0.03, h*0.03). +// TSRRegionMarginPx is the fixed pixel margin added around a DLA table bbox +// before it is sent to TSR. It matches Python's _table_transformer_job, which +// expands the box by MARGIN = 10 PDF points and then scales by ZM +// (10 * DlaScale = 30px at the 216-DPI render). The margin is FIXED, not +// proportional to table size — an earlier Go port mistakenly used w*0.03 / +// h*0.03, which diverged from Python. +const TSRRegionMarginPx = 10.0 * pdf.DlaScale + +// CropImageRegion crops a pdf.DLARegion from an image with a fixed margin +// (matching Python's _table_transformer_job: MARGIN=10 points scaled by ZM). func CropImageRegion(img image.Image, r pdf.DLARegion) (image.Image, error) { - w := r.X1 - r.X0 - h := r.Y1 - r.Y0 - marginX := w * 0.03 - marginY := h * 0.03 + marginX := TSRRegionMarginPx + marginY := TSRRegionMarginPx maxX := float64(img.Bounds().Dx()) maxY := float64(img.Bounds().Dy()) x0 := int(math.Max(0, r.X0-marginX)) diff --git a/internal/deepdoc/parser/pdf/util/crop_test.go b/internal/deepdoc/parser/pdf/util/crop_test.go index f9a8b1b061..f8d6425dc3 100644 --- a/internal/deepdoc/parser/pdf/util/crop_test.go +++ b/internal/deepdoc/parser/pdf/util/crop_test.go @@ -369,9 +369,10 @@ func TestCropSectionByDLA(t *testing.T) { // Decode and verify. decoded, _ := base64.StdEncoding.DecodeString(result) img := decodePNG(t, decoded) - // The DLA figure region is (30,60)-(270,420) with 3% margin. - // Expected: ~(30-7.2, 60-10.8)-(270+7.2, 420+10.8) ≈ (22.8, 49.2)-(277.2, 430.8) - // width ≈ 254px, height ≈ 381px + // The DLA figure region is (30,60)-(270,420). CropImageRegion now adds a + // fixed 30px margin (TSRRegionMarginPx = 10pt * ZM), so the crop is + // (0,30)-(300,450) → 300x420. The assertions below only check the crop is + // reasonably large, not exact pixels. w, h := img.Bounds().Dx(), img.Bounds().Dy() t.Logf("cropSectionByDLA result: %dx%d", w, h) if w < 200 || h < 300 { @@ -464,15 +465,31 @@ func TestCropSectionByDLA_EmptyInputs(t *testing.T) { func TestCropImageRegion(t *testing.T) { img := image.NewRGBA(image.Rect(0, 0, 200, 300)) - t.Run("normal crop", func(t *testing.T) { + t.Run("normal crop uses fixed 30px margin", func(t *testing.T) { r := pdf.DLARegion{X0: 10, Y0: 20, X1: 100, Y1: 150} cropped, err := CropImageRegion(img, r) if err != nil { t.Fatalf("unexpected error: %v", err) } - // 3% proportional margin: 90×3%≈3px, 130×3%≈4px → 95×137 - if cropped.Bounds().Dx() != 95 || cropped.Bounds().Dy() != 137 { - t.Errorf("size %v, want 95x137", cropped.Bounds()) + // Fixed margin = 10 PDF points * ZM(3) = 30px on each side (matches + // Python's MARGIN=10, NOT a proportional 3%). Region is 90x130px; + // expanded by 30px/side and clamped to the 200x300 image → 130x180. + if cropped.Bounds().Dx() != 130 || cropped.Bounds().Dy() != 180 { + t.Errorf("size %v, want 130x180 (fixed 30px margin)", cropped.Bounds()) + } + }) + + t.Run("margin is fixed, not proportional", func(t *testing.T) { + // A small 40x40 region: a 3% margin adds only ~1.2px/side (→ 42x42), + // but the fixed 30px margin adds 30px/side (→ 100x100). This locks + // parity with Python, which uses a fixed MARGIN=10 points. + r := pdf.DLARegion{X0: 50, Y0: 50, X1: 90, Y1: 90} + cropped, err := CropImageRegion(img, r) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cropped.Bounds().Dx() != 100 || cropped.Bounds().Dy() != 100 { + t.Errorf("size %v, want 100x100 (fixed 30px margin, not 3%%)", cropped.Bounds()) } }) @@ -502,3 +519,15 @@ func TestCropImageRegion(t *testing.T) { } }) } + +// TestTSRRegionMarginPx locks the parity contract with Python: the TSR crop +// margin is a fixed 10 PDF points scaled by ZM (DlaScale=3) = 30px, NOT a +// proportional percentage. Both CropImageRegion and the cropOff inverse map in +// table_extract.go read this single constant, so the crop and the coordinate +// mapping can never drift apart. +func TestTSRRegionMarginPx(t *testing.T) { + const want = 10.0 * pdf.DlaScale // 10 points * ZM(3) = 30px + if TSRRegionMarginPx != want { + t.Errorf("TSRRegionMarginPx = %v, want %v (10pt * ZM)", TSRRegionMarginPx, want) + } +}