Consolidating the open rendering, text and font PRs: scope and evidence requirements

Open
  • Is something wrong?
#428 6 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 5 days

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
1/100
Issue type
Documentation
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
backend

Research direction

Read the consolidation guidance in this issue and review the linked open PRs and regression dataset discussions. For each resubmitted PR, check that it covers one topic, names real regression documents and pages, and includes the required before/after evidence; rendering changes also need pypdfium2 comparisons, corpus metrics, and an enforced image regression test. Done means the checklist is satisfied and superseded PRs and ground-truth dataset PRs are closed.

Written by the indexing model from the issue text.

Description

Summary

First of all, thank you @wittjeff for your continued contributions across the board: rendering, fonts, text segmentation, structure, forms. The amount of work and the depth of the analysis behind these PRs are much appreciated, and we want to get them merged.

There are currently 21 open PRs from @wittjeff against docling-parse, plus several draft PRs against the regression dataset. Many of them overlap: some are stacked copies of each other, several change the same functions, and three pin HF_DATASET_REVISION to different, mutually exclusive dataset revisions. They can't be reviewed in this form.

This issue sets out how they should be consolidated and what evidence each change needs. All open PRs listed below are on hold until they have been resubmitted on this basis.

1. Consolidation by topic

Resubmit as one PR per topic, rebased on current main, without merge commits. One commit per logical fix within a PR is fine. Close the superseded PRs.

Topic Current PRs Proposal PR Notes
Blend2D library patches #387 (closed),
#396 (closed),
#400 (closed),
#401 (closed)		 #433 (merged) Already stacked: each one contains the previous one. Keep one.
Renderer: stroke clipping, tiling patterns, soft masks #397 (closed),
#386 (closed),
#390 (closed),
#413 (closed)		 #431 (merged) Change the same files: blend2d_renderer.h, render_instructions.h, stream.h, grph.h, shape.h. #390 contains #386. #413 conflicts with main.
Word and cell segmentation #356,
#358,
#409		 #437 All change the contraction and word-gap logic in cells.h.
Font and ToUnicode decoding #322,
#327,
#388,
#389,
#417,
#419		 TBD All change glyph-to-Unicode resolution (page_font.h, cmap.h, sanitizer constants) and share test files.
Standalone #337 (merged),
#410 (merged),
#411 (merged),
#427 (closed)		 TBD Unrelated to the topics above; can stay separate.

Please open a new PR in a topic only once the previous one in that topic has been merged or closed.

2. Evidence from real PDFs

Every change must be shown on a real document. Synthetic test PDFs (tests/pdf_builder) are welcome as unit tests, but they don't demonstrate that a change matters in practice.

  • Use the regression dataset first. If a document in the regression dataset already shows the problem, name it in the PR description, with the document name and page numbers. Don't open a dataset PR in that case.
  • Dataset PRs only for documents that are actually new. Open a PR against the regression dataset only when no existing document reproduces the problem. It should add the new document, with a license that allows redistribution, and say which problem it reproduces.
  • Text, font and segmentation fixes: for each cited page, show a before/after diff of the affected text cells or words.

3. Rendering changes: image regression evidence

Rendering changes must show that the output improves, which in practice means it moves closer to pypdfium2.

  • For each cited page, include a three-panel comparison: pypdfium2, before, after.
  • Include a before/after table of mean_abs_error and changed_pixels_ratio against pypdfium2 over the whole regression corpus. The cited pages must improve, and no page may get worse by more than the tolerance in tests/test_regression_pypdfium_render.py.
  • Add an image regression test that fails on main and passes with the change, comparing against pypdfium2 on the cited pages. test_regression_pypdfium_render.py only reports differences and never fails, so on its own it doesn't count as this test.
  • Blend2D patches also need a link to the issue or PR filed upstream with Blend2D, and a real document that crashes, hangs or renders incorrectly without the patch.

4. Ground truth

Don't pin HF_DATASET_REVISION to revisions from open dataset PRs. If a change alters the ground truth of existing regression pages, list those pages in the PR description and say why each one changes; the maintainers update the ground truth when merging. The existing ground-truth dataset PRs (3, 4, 7, 8, 9) should be closed. PRs that only add a new document under section 2 are fine.

Checklist for each resubmitted PR

  • One topic per PR, rebased on main, no merge commits
  • Superseded PRs closed and linked
  • Regression dataset documents and pages named, or a dataset PR that adds a new document
  • Before/after evidence for each cited page
  • Rendering only: pypdfium2 comparison images, corpus-wide metrics table, and an enforced image regression test
  • Blend2D only: link to the upstream report
  • Changes to existing ground truth listed with the reason for each page
Dominant language
C++
Stars
346
Forks
92
Avg merge
4d 15h
Merged PRs (30d)
27

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from docling-project/docling-parse

All issues in docling-project/docling-parse

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.