Skip to content

Improve DOCX layout for floating images, drop caps, and footnotes - #2

Closed
peterwilli wants to merge 2 commits into
tensorbee:mainfrom
ISAWarden:fix/document-rendering
Closed

Improve DOCX layout for floating images, drop caps, and footnotes#2
peterwilli wants to merge 2 commits into
tensorbee:mainfrom
ISAWarden:fix/document-rendering

Conversation

@peterwilli

Copy link
Copy Markdown

The current library lacks proper render capabilities for images and footnotes. Also added some missing features from the format.

To demonstrate, I used this sample docx: https://filesamples.com/samples/document/docx/sample1.docx

Here is page 5 before the PR:

image

Here is the same page, but after this PR:

page-5

Here is page 7 before this PR:

image

Here is page 7 after:

page-7

Add richer paragraph reflow so anchored images render on the correct page, wrap text across nearby paragraphs, and use visible PNG bounds for tighter square wrapping; also add drop-cap/footnote-aware pagination and supporting OOXML parsing updates.
@peterwilli

Copy link
Copy Markdown
Author
page-5

Fixed the headers now as well

@mantissaman

Copy link
Copy Markdown
Contributor

Sorry for the long silence on this one. The before and after renders are convincing and I do want the feature.

The branch has drifted from main since March, and there are three things that need your hand before it can land.

  1. The image crate. crates/rdocx-layout/Cargo.toml asks for image = { workspace = true }, but I removed image from the workspace dependencies in 9b01beb while clearing dependency CVEs. That was my call, so to be clear, I am happy to take it back for this feature. Please add it to [workspace.dependencies] in the root Cargo.toml as part of the rebase.

  2. quick-xml went from 0.37 to 0.41 on main. crates/rdocx-oxml/src/drawing.rs around lines 340 and 377 uses BytesText::unwrap_or_default and .trim(), and neither of those exists in 0.41 any more.

  3. cargo fmt --all --check fails on the branch. engine.rs around 822 and 968, and paginator.rs around 499.

There is also a conflict on Cargo.lock, but that one just needs regenerating.

One design note while you are in there. anchored_image_wrap_extents decodes the whole PNG to RGBA and walks every pixel with get_pixel to find the alpha bounds, and it does that on every call with no caching. On a large photo that is a big buffer and a lot of bounds checked reads, all inside layout. Caching by embed_id and iterating with enumerate_pixels would help. You can also stop early once the bounds already cover the full width.

No rush. Give me a shout when it is rebased and I will get to it quickly this time.

@mantissaman

Copy link
Copy Markdown
Contributor

Thank you for this, and sorry it sat open so long.

Your before and after renders were right, and they stayed right. Everything this PR set out to fix was genuinely broken, and it is fixed now. What I could not do was merge the branch itself, so I want to be clear about what happened to the work.

Why it could not be merged directly

The branch was cut in March, and main moved 706 commits since. The overlap that mattered was the anchored-drawing half: PRs #18 and #20 rewrote that code in the meantime, introducing content-addressed MediaId, an AnchoredDrawing carried on the paragraph block, and per-paragraph offset resolution against the relativeFrom frame. Rebasing your version of that on top would have been a regression rather than an improvement, so that part is superseded rather than rejected.

The rest of the PR was still missing from main, and that is what shipped.

What shipped, and where

Released today in rdocx v0.7.0 and rpptx-v0.3.0.

  • Footnote segments no longer overprint. This was the one-line advance you spotted. Fixing it exposed a second defect it had been hiding: notes were line-broken at the full content width but drawn one marker indent in, so every line overran the right margin. Both fixed.
  • Notes reserve their space during pagination. Body text and the footnote area no longer collide, and a note too tall for the space left now splits and continues on the next page with the continuation separator.
  • Endnotes moved to the end of the document, and the two note streams are keyed apart. Your sample has a footnote 2 and an endnote 2, and they were previously indistinguishable to the layout, so one silently shadowed the other.
  • Wrap modes, text distances and anchor alignments are parsed and used. wrapSquare, wrapTopAndBottom, wrapTight and wrapThrough, plus distT/B/L/R and the wp:align children.
  • Text flows around floating drawings. Page 7 of your sample now reads with text between both arrows and the green dot on its own line, which is what your screenshots were showing.
  • Kashida justification values are accepted. Worth calling out because it turned out worse than a layout bug: lowKashida and friends made CT_PPr parsing fail outright, so a document using them would not open at all.

Your sample1.docx was used as the working reference throughout, and it earned its keep. Three defects were found only by rendering it rather than by unit tests, including the endnote collision above and a wrapping case where a drawing anchored to a later paragraph still has to push earlier text aside. The tests all passed while that was broken, because tests anchor the drawing to the paragraph it affects and real Word documents do not.

What I did not take, and why

In fairness, three changes were left out on purpose rather than overlooked.

  • w:pPr/w:rPr applied to every run. Per ECMA-376 that element is the formatting of the paragraph mark, and it does not cascade to runs. It may have improved your sample by coincidence.
  • w:webHidden mapped to vanish. webHidden hides text in web view, not in print, so this would have dropped text from PDF output that Word renders.
  • normalize_paragraph_frames mutating the document model at open. Document::save serialises that same model, so open followed by save would have silently merged and deleted a paragraph. Drop-cap joining belongs in the layout input rather than the model, and drop caps are still open as a result.

I also did not take the alpha-channel scan for wrap extents. It decoded every image and walked every pixel inside layout with no cache, and it applied outline extents to wrapSquare, which reserves the frame rather than the outline. The frame is used instead.

Still open

Drop caps are not implemented. That part of your PR is the one piece with no equivalent on main, and it needs a design that does not mutate the parsed document. If you would like to take another run at it, it would be very welcome, and this time it will not sit for months.

Closing this as superseded. The work is not: thank you for finding all of it, and for the renders that made it obvious.

@mantissaman

Copy link
Copy Markdown
Contributor

Superseded by S41 and S42, released in rdocx v0.7.0 and rpptx-v0.3.0. Details in the comment above.

@peterwilli

Copy link
Copy Markdown
Author

@mantissaman thanks for the wonderful explanation!

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.

2 participants