fix(writer): emit fill colour for embedded-font text

[norbusan]

Description

PdfWriter::add_element routed embedded-font text through the colour-agnostic add_embedded_text without ever emitting the element's fill colour, so inline_color (and any coloured text) on an embedded font rendered black — no rg operator reached the content stream. The base-14 path (add_text_content) always emits rg from style.color; the embedded path now matches it by emitting fill_color before the glyph run.

Adds a regression test asserting the rg operator is present for embedded-font inline_color.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Performance improvement
• Code refactoring
• Tests
• CI/CD changes

Related Issues

Fixes #

Changes Made

Testing

• I have added tests that prove my fix is effective or that my feature works
• All new and existing tests pass locally
• I have run cargo test (see CONTRIBUTING.md — --all-features fails to build: fips and legacy-crypto are mutually exclusive by design)
• I have run cargo clippy -- -D warnings
• I have run cargo fmt

Python Bindings (if applicable)

• Python bindings updated (if needed)
• Python tests pass
• Python code formatted with ruff format
• Python code linted with ruff check

Documentation

• I have updated the documentation (README, docs/, code comments)
• I have added/updated examples (if applicable)
• I have updated CHANGELOG.md

Checklist

• My code follows the project's coding guidelines (see CONTRIBUTING.md)
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• My changes generate no new warnings
• I have checked my code and corrected any misspellings
• The PR title follows conventional commits format (e.g., feat:, fix:, docs:)

[yfedoseev]

Reviewed + regression-tested — this looks good, thank you @norbusan. Nicely diagnosed: embedded-font text routed through the colour-agnostic add_embedded_text never emitted the element's fill colour, so inline_color painted black. Emitting rg before the glyph run to match the base-14 path is the right fix.

Correctness — confirmed:

• Emitting rg before BT is valid ISO 32000 — nonstroking colour is graphics state (§8.6.8) and carries into the text object (§9.4.1). Your "colour carries across BT, no restore needed" note is accurate.
• No graphics-state leak. I traced every content-emitting path: base-14 text always sets fill_color; path fills only run when fill_color.is_some() (set right before); images are wrapped in q/Q. The only colour-agnostic entry points (add_embedded_text/add_shaped_embedded_text called directly by the HTML painter) set/reset colour around their own calls and don't go through add_element. So the missing q/Q restore is consistent with the pre-existing "always set explicitly" contract on the base-14 path — not a new hazard.
• Byte-change is expected and consistent: since TextStyle.color defaults to black, every embedded-font element now emits 0 0 0 rg (as the base-14 path already does). Base-14-only documents are unchanged.

Verification I ran:

• Full document-builder/writer test suite (7 test files, incl. round-trip build→extract) — 0 failures on main and on your branch.
• Your new test passes and is revert-checked (without the src change, no 0.5 0.5 0.5 rg is emitted → it fails). The /EF1 control assertion is a nice touch.

Two optional nits (non-blocking):

1. The let color = t.style.color; temp is unnecessary — fill_color(t.style.color) inlines fine.
2. Purely defensive note: if add_embedded_text ever early-returned on an unregistered resource, the rg would already be emitted (stray colour op, no glyphs). It can't happen here (the resource was just resolved), just flagging it.

One heads-up for future PRs (we're formalizing this in #901): please open an accepted issue first and include a self-run verification. Not blocking this one — it's small, well-tested, and I've verified it.

Marking this ready to merge.

[yfedoseev]

Follow-up: I reproduced the defect against main to confirm it's a real problem before locking in the approval — took this PR's test but reverted only the pdf_writer.rs fix, and the test fails on main's writer code:

thread 'document_builder_embedded_font_inline_color_emits_rg' panicked at tests/test_document_builder_embedded_font.rs:145:
  fill-colour operator missing for embedded-font inline_color — embedded text would render black
test result: FAILED. 0 passed; 1 failed

So on main, 0.5 0.5 0.5 rg is genuinely absent for embedded inline_color text — it renders black. Confirmed real, and your fix resolves it. Approval stands.

[norbusan]

Thanks for your comments. Yes, I hit the problem in our watermarking text which suddenly turned up black instead of light gray after switching to pdf_oxide.
For now, we hacking around that by manually inserting the necessary pdf commands, but getting this into pdf-oxide would reduce code complexity on our (== arXiv) side.

I will try to remember opening an issue with reproducible code next. There is one more in my pipeline (text extraction).

Thanks

[norbusan]

Is there anything else I need to do at the current state?
I also saw that some other tests are failing (ocr related, due to update to newer packages version), but I didn't investigate more, since it was unrelated code.

Thanks

[yfedoseev]

Thanks — and that's a perfect real-world confirmation (arXiv watermark text going black instead of light-gray is exactly the defect). Good to know it lets you drop the manual-PDF-command workaround on your side.

Nothing more needed from you right now. This PR is approved and in our merge queue; it'll merge automatically once it reaches the front and passes the full CI matrix (we run these one-at-a-time, so it may take a little while). We'll handle the merge.

On the OCR test failures you saw — that's local dependency drift, not a repo issue: our CI pins the OCR package versions and the OCR job is green on main. Newer OCR packages in your local env can shift those results, but nothing in this PR (or on main) needs a fix for it. So you can ignore it.

On the next one (text extraction) — thank you for adopting issue-first. One heads-up: a text-extraction change is a heavier bar than this writer fix, because extraction runs through shared heuristic paths where a change can silently perturb other documents. Per CONTRIBUTING.md (#901) it'll want:

1. an accepted issue first, with a minimal PDF you built yourself (or a description — no third-party/copyrighted PDFs as fixtures) + the ISO 32000 clause + expected before→after;
2. a regression test that fails without the fix (revert-checked);
3. a corpus_sig diff of your branch vs main on a public corpus (Mozilla pdf.js test/pdfs, Apache PDFBox, veraPDF, DARPA SafeDocs), judged with word-Jaccard — confirming both that it does what it claims and that it doesn't regress unrelated docs. The exact commands are in CONTRIBUTING.md → Testing and regression requirements.

Happy to help if anything there is unclear. Thanks again for the clean fix.

[yfedoseev]

Approved — reviewed and regression-passed; in the merge queue. (Formal approval to supersede the earlier review state and record sign-off.)

[norbusan]

(funny, passing through Vancouver on my way home from Calgary in a few hours, beautiful area ❤️ )