fix(pdf): align TSR crop margin with Python (fixed 30px, not 3%) (#18387)

This commit is contained in:
Jack
2026-08-18 13:11:41 +08:00
committed by GitHub
parent 344bd4d57a
commit 5ec11f7b07
5 changed files with 124 additions and 26 deletions

View File

@@ -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{

View File

@@ -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 {

View File

@@ -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)
}
}

View File

@@ -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))

View File

@@ -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)
}
}