Skip to content

fix(table): ignore opaque matching fill+stroke rects under lines_strict - #5210

Open
ManojDevarajulu wants to merge 1 commit into
pymupdf:mainfrom
ManojDevarajulu:fix-lines-strict-invisible-fs-rect
Open

ManojDevarajulu wants to merge 1 commit into
pymupdf:mainfrom
ManojDevarajulu:fix-lines-strict-invisible-fs-rect

Conversation

@ManojDevarajulu

Copy link
Copy Markdown

Fixes #5175.
Credit to @ToukoUrsin for identifying the root cause and providing the reproduction script.

Problem

When using strategy="lines_strict" with use_layout=False, Page.find_tables() ignored fill-only rectangles (type="f"), but treated opaque rectangles drawn with both fill and stroke (type="fs") of the same color as table ruling lines.

For example, a white redaction or white masking box drawn inside an empty table cell draws no visible border line, but its bounding edges were treated as structural table lines, creating phantom rows or columns (e.g. splitting a 6-column table into 7 columns).

Solution

In src/table.py (clean_graphics), treat type="fs" paths whose fill color matches stroke color (and both opacities are 1) like fill-only paths under lines_strict.

Boxes with contrasting borders (e.g. black stroke on white fill) or semi-transparent fills are unaffected and continue contributing lines as intended.

Tests

  • Added regression test test_5175 in tests/test_tables.py:
    • Invisible border (white on white) -> 3 rows (phantom row prevented).
    • Contrasting border (black on white) -> 4 rows (visible border preserved).
  • All 47 tests in tests/test_tables.py pass.

@JorjMcKie
JorjMcKie requested a review from veget-able October 25, 2026 12:51

@veget-able veget-able left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked this over and ran it against our table benchmark set, and I don’t see any regression. Looks good from my side, so I’ve approved.

@JorjMcKie one quick process question. When you ask me to review a PR like this, would you prefer that I merge it myself after approving, or that I just approve and leave the merge to you? Either is fine with me, I just want to follow whatever you find easiest.

@JorjMcKie

Copy link
Copy Markdown
Collaborator

I looked this over and ran it against our table benchmark set, and I don’t see any regression. Looks good from my side, so I’ve approved.

@JorjMcKie one quick process question. When you ask me to review a PR like this, would you prefer that I merge it myself after approving, or that I just approve and leave the merge to you? Either is fine with me, I just want to follow whatever you find easiest.

In cases like these (an external contributor), after all concerned people have agreed, you could do the merge.
We have to wait anyway until those final checks are done - which must be approved by a maintainer to even run ...

…_strict

Treat opaque 'fs' paths whose stroke matches fill like fill-only paths to prevent phantom table rows, and add regression test for pymupdf#5175.

Fixes pymupdf#5175
@ManojDevarajulu
ManojDevarajulu force-pushed the fix-lines-strict-invisible-fs-rect branch from c8f14c6 to 3a75282 Compare October 26, 2026 13:17
@ManojDevarajulu

Copy link
Copy Markdown
Author

Thanks @ToukoUrsin and @JorjMcKie!

I noticed test_quick failed earlier on test_pylint.py due to R0916: Too many boolean expressions in if statement (8/5) in src/table.py:clean_graphics().

I have updated the branch to extract the fs check into an intermediate variable so the boolean count complies with pylint, and consolidated the branch into a single clean commit. All tests pass cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

find_tables(lines_strict): opaque white fill+stroke rectangle creates a phantom row

3 participants