[marcoarnulfo]

Fixes #2894.

Since #1044 a field is drawn in the /DA font from /DR when it can encode the value, but FontText::width still measured that font with Helvetica's table, so wrapping, quadding and 0 Tf auto size were laid out for a different face. In Courier (0.6 em) mixed-case text overflows: the issue's field gets a 128-character first line in a box that holds 94. This measures a reused simple font with its own Metrics, i.e. its /Widths, or the standard-14 table when it has none (Courier, Times, Helvetica and their styles), which is what @MONUABST suggested in the issue, thanks for the clear write-up and the repro. For a standard-14 font without /Widths, Metrics::width (pdfcraft-fonts) now looks a code up through the font's encoding, so a /Differences remap is measured as the glyph it draws rather than the one WinAnsi has at that code; this also applies to the other Metrics::width callers (edit, redact, a11y). Helvetica fallback and the CJK path are unchanged; Symbol and ZapfDingbats never get here, since dr_font already sends them to Helvetica. The module comment said widths were Helvetica's, so it's updated too.

#2466 adds a Composite arm next to the same line with the same measurement, so whichever lands second has a one-line conflict in that match, happy to rebase.

Validation:

• reused_standard_fonts_wrap_and_align_with_their_own_widths (crates/forms): the issue's Courier field plus right-aligned Courier and Times-Roman fields. Fails on main (every line fits the box in Courier: [128, 71]), passes here; measuring every simple font as Courier makes it fail on the Times field (Times: 174)
• The issue's PDF and script through pdfcraft-cli run and render: the text now wraps at "going" and stays in the box
• cargo test -p pdfcraft-forms --lib (appearance tests), cargo clippy -p pdfcraft-forms --all-targets -- -D warnings, cargo fmt --all -- --check
• remapped_standard_fonts_are_measured_by_the_glyph_each_code_draws (crates/forms) and standard_fonts_without_widths_measure_the_glyph_their_differences_name (crates/fonts): Helvetica with /Differences [65 /W 87 /A] and no /Widths. The forms test fails on b2150f3 (no /Widths: 160.24) and with the encoding lookup removed; explicit /Widths still go by code
• cargo test -p pdfcraft-a11y --lib, cargo test -p pdfcraft-edit --lib, cargo test -p pdfcraft-redact --lib, cargo clippy -p pdfcraft-forms -p pdfcraft-fonts --all-targets -- -D warnings, cargo xtask layers
• (macOS, Apple silicon)

Summary by CodeRabbit

• Bug Fixes
  • Text-field layouts now use the correct character widths for standard fonts, improving multiline wrapping and right alignment.
  • Text measurement now reflects glyphs selected by font encoding differences, including when explicit width data is present. When text cannot be encoded, Helvetica metrics are used as a fallback.

[mtlcl410]

Test report for b2150f3 on Windows 11 (x64), compared with a main build (dc41044). Fixtures: the issue's 728-byte file (multiline field, 570 pt wide, /Cour 10 Tf reused from /DR), plus the same file with Times-Roman (/TiRo) instead. Each was filled with the issue's sentence via form_fill and saved. I then took the Tj lines of the widget's /AP /N and measured each with the real font's widths at 10 pt:

font	main	this PR
Courier	2 lines, widest 768 pt (cut off at the field's edge)	3 lines, widest 546 pt ✅
Times-Roman	2 lines, widest 507.7 pt (wrapped early, since Helvetica is wider)	2 lines, widest 551.6 pt ✅

Both now wrap against the field's own font: Courier no longer runs past the box, and Times uses the width it has.

[ahmadmuj]

Found a standard-14 regression with Helvetica, /Differences [65 /W 87 /A], and no /Widths. AAAA encodes as WWWW, but the new measurement uses W's width instead of the mapped A glyph.

At 10 Tf, a 200 pt right-aligned field with a one-point border starts at 159.24 pt instead of 170.32 pt. The saved-appearance test fails on b2150f3 and passes when only this width change is removed. Could you cover this case and preserve mapped glyph widths? The complete-width and auto-size save/reopen probes pass.

[FredApps]

Requesting changes at b2150f3. This confirms the regression ahmadmuj reported in the earlier review.

Defect: FontText::width (crates/forms/src/appearance.rs, the new Self::Simple(metrics) arm) sums metrics.width(code) over the encoded bytes. For a standard-14 font with no /Widths, that table is looked up by code, so a /Differences remap is ignored: the width used is that of the glyph the code has in the base encoding, not of the glyph the code now draws.

Reproduction: a Helvetica font with /Encoding << /BaseEncoding /WinAnsiEncoding /Differences [65 /W 87 /A] >>, no /Widths, reused from /DR. A 200 pt wide field with /Q 2 (right-aligned) and /DA (/Dif 10 Tf 0 g) holds "AAAA". The text encodes to code 87 four times (glyph A, 0.667 em), so it should start at 198 - 26.68 = 171.32. With this PR it starts at 160.24, i.e. it is laid out with the width of W (0.944 em). On the merge base the same field starts at 171.32. So this is a regression against the current behaviour for any remapped standard-14 font, in alignment, wrapping and auto size.

Probe I ran (not part of the PR): batchfa6_probe_differences_widths asserts the start is within 0.05 of 171.32. It passes on the merge base and fails on this head with "starts at 160.24". Please measure by the glyph the code draws (the /Differences name) when there is no /Widths, and add a test for it.

Everything else checked out, so this is the only blocker:

• reused_standard_fonts_wrap_and_align_with_their_own_widths passes on this head (1 passed) and fails with crates/forms/src/appearance.rs restored to the merge base, at tests.rs:393 ("every line fits the box in Courier: [128, 71]"), so it does prove the Courier and Times behaviour.
• cargo test --locked -p pdfcraft-forms: 79 passed, 0 failed
• cargo fmt --all -- --check, cargo clippy --locked -p pdfcraft-forms --all-targets -- -D warnings, cargo check --locked -p pdfcraft-forms --target wasm32-unknown-unknown, cargo xtask layers: all pass

Run on a Linux validation host in a detached worktree. Not verified: native Windows or macOS, and fonts with an explicit /Widths plus /Differences.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 889445f8-6c27-4ee2-a1f4-b386e59668d6

📥 Commits

Reviewing files that changed from the base of the PR and between f0c9901 and a61bd92.

📒 Files selected for processing: 3

• crates/fonts/src/pdf.rs
• crates/forms/src/appearance.rs
• crates/forms/src/tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Standard-14 width lookup now accounts for glyph names from font encoding differences. Form text measurement now uses the selected simple font’s encoded character widths. Added tests cover width lookup, wrapping, and text alignment.

Changes: Font Width Measurement

Layer / File(s)	Summary
Standard-14 glyph width lookup crates/fonts/src/pdf.rs	Standard-14 fonts without /Widths use WinAnsi-indexed metrics for mapped glyphs. Symbol and ZapfDingbats retain their code values, unmappable names fall back to the original code, and /Widths remains code-indexed. Tests cover swapped glyph widths and fallbacks.
Form text measurement and layout crates/forms/src/appearance.rs, crates/forms/src/tests.rs	Simple-font text width uses encoded character codes and the selected font’s widths, with Helvetica width as the encoding-failure fallback. Tests cover wrapping and alignment with Courier and Times-Roman, including encoding differences and explicit /Widths.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: tanish2k09

Merge Risk: ⚪ Minimal · up to a61bd

The change is mergeable after normal checks; no actionable risk remains in the supplied evidence.

Pre-merge checks: 5

Check name	Status	Explanation
Linked Issues check		Issue #2894 requires field-font widths for wrapping, alignment, and auto sizing. FontText::width now measures Simple fonts through their encoding and Metrics, including /Widths and standard-14…
Out of Scope Changes check		The changes stay within issue #2894. The crates/forms changes fix field-font measurement and add focused layout tests. The crates/fonts change supports correct standard-14 encoding and `/Differenc…
Docstring Coverage		Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files.
Title check		The title clearly identifies the main behavior change: measuring a field with its own simple font instead of Helvetica.
Description check		The description clearly explains the issue, implementation, affected behavior, test coverage, and validation commands. It does not reproduce every template heading, but the omitted sections appear non…

Full details: Linked Issues check

Explanation

Issue #2894 requires field-font widths for wrapping, alignment, and auto sizing. FontText::width now measures Simple fonts through their encoding and Metrics, including /Widths and standard-14 metrics. Metrics::width also resolves standard-14 /Differences through the encoded glyph while preserving code-based /Widths. The added forms tests cover Courier and Times wrapping/alignment, and remapped standard fonts. The implementation supports the existing 0 Tf auto-size path because that path uses the same width function.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #2894. The crates/forms changes fix field-font measurement and add focused layout tests. The crates/fonts change supports correct standard-14 encoding and /Differences measurement, including regression tests. No unrelated behavior or files are changed.

Full details: Description check

Explanation

The description clearly explains the issue, implementation, affected behavior, test coverage, and validation commands. It does not reproduce every template heading, but the omitted sections appear non-applicable or non-critical for this code-only change.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[marcoarnulfo]

@ahmadmuj @FredApps you're right, and thanks for measuring it on both sides of the merge base, that made it quick to pin down.

The lookup was wrong one level down: Metrics::width (crates/fonts/src/pdf.rs:769) indexed the WinAnsi-ordered standard-14 table by the raw code, so a /Differences remap got the width of whatever WinAnsi has at that code. It now goes through the font's encoding first (std14_code, pdf.rs:781): the glyph name the code draws is turned into its character, and that character's WinAnsi slot is what gets measured. /Widths, when present, still win by code. Symbol and ZapfDingbats keep their own code order, and a name that WinAnsi has no slot for (or that we can't read) falls back to the code as before.

Tests:

• remapped_standard_fonts_are_measured_by_the_glyph_each_code_draws (crates/forms/src/tests.rs:412) is your case: Helvetica with [65 /W 87 /A], no /Widths, right-aligned "AAAA" in a 200 pt field, expects the start at 171.32. On b2150f3 it fails with no /Widths: 160.24. The same field with /Widths [500] at code 87 checks that explicit widths still go by code (178).
• standard_fonts_without_widths_measure_the_glyph_their_differences_name (pdf.rs:1342) pins the unit: A/W swapped, plus uni03A9 and an unreadable name falling back to the code's width.

Since the fix is in pdfcraft-fonts, it also changes measurement for the other Metrics::width callers (edit, redact, a11y) on standard-14 fonts without /Widths whose encoding isn't WinAnsi, which I think is the same bug there. Their lib tests pass unchanged. One side effect worth knowing: a standard-14 font with no /Encoding uses StandardEncoding, so codes like 0x27 (quoteright) and 0x60 (quoteleft) are now measured as those glyphs rather than WinAnsi's quotesingle and grave.

I couldn't run the wasm check locally (target not installed here); the change is plain Rust with no platform code.

[mtlcl410]

Re-tested a61bd92 on Windows 11 x64 (CLI built from this head, compared with main @ d8b2705).

Regression fixed. This is the case @ahmadmuj and @FredApps reported: Helvetica, /Differences [65 /W 87 /A], no /Widths, /Q 2, a 200 pt field, "AAAA". The appearance is now 1 0 0 1 171.32 7.445 Tm (WWWW) Tj, the expected start (b2150f3 gave 160.24).

The original fix still holds. On the issue's Courier fixture and my Times-Roman variant, the widget appearance is identical to what I validated on b2150f3: Courier wraps to lines of 91/86/21 characters (widest 546 pt in the 570 pt box), Times to 139/60.

The StandardEncoding side effect you mentioned is right. A Times-Roman /DR font with no /Encoding, right-aligned, filled with it''s ``ok'':

• the encoder writes StandardEncoding codes (0xA9 quotesingle, 0xC1 grave);
• this head starts the line at 162.19, exactly what the Times-Roman AFM widths of those glyphs give (198 − 35.81);
• main starts it at 160.36, measuring those codes as WinAnsi's © and Á.

Tests fail without the fix. Calling f.width(code) instead of f.width(self.std14_code(f, code)) fails standard_fonts_without_widths_measure_the_glyph_their_differences_name. The --lib suites of forms (80), fonts (71), edit (156), redact (43) and a11y (49) pass on this head.

A small leftover, not a blocker. The standard-14 tables are WinAnsi-ordered, so a glyph the encoding names that WinAnsi has no slot for still falls back to the raw code. Examples are StandardEncoding's fi/fl (0xAE/0xAF), Lslash/lslash and dotlessi, or a /Differences entry naming Lslash. They are measured as whatever WinAnsi has at that code (fi as ®: 760 vs 556 in Times). That's the documented fallback and rare in form text. #3076 adds Core14 widths for the Latin Extended-A letters, which could serve those names later.

[FredApps]

Re-reviewed at a61bd92. The /Differences width blocker is fixed.

• Fix: Metrics::width (crates/fonts/src/pdf.rs:769) now looks up the standard-14 table by std14_code (pdf.rs:781-786). That takes the glyph the font's encoding (base plus /Differences) gives the code, then maps it to its WinAnsi slot, which is how the table is indexed. Symbol and ZapfDingbats keep their own code, and so does any glyph WinAnsi has no slot for. Fonts that have /Widths still go by code. Because the fix is in the shared fonts crate, it also covers the forms appearance path I reported, not only forms.
• My reproduction: the right-aligned "AAAA" field with /Differences [65 /W 87 /A] and no /Widths now starts at 171.32, the same as on the merge base. The new forms test checks exactly that case, plus the /Widths case I hadn't verified before (178.0). The new fonts unit test covers the A/W swap, a glyph with no WinAnsi slot, an unreadable name, and /Widths overriding by code.
• Tests guard the fix: I put back b2150f3's crates/fonts/src/pdf.rs and kept the new tests. Both fail: the fonts test at pdf.rs:1343 and the forms test at tests.rs:439 (the 171.32 assertion). Both pass with the fix.

Local checks on this head, Linux: cargo fmt --all -- --check OK; cargo clippy --locked --workspace --all-targets -- -D warnings clean; cargo test --locked --workspace all passing (167 test binaries); cargo xtask layers OK; cargo xtask wasm (31 crates) OK. Hosted ubuntu, wasm32 and licence jobs are green. Windows is still pending, and I didn't test it or macOS locally.

[ahmadmuj]

Rechecked a61bd92 with the retained bordered fixture. The saved/reopened AAAA appearance now starts at 170.32 pt; restoring the old lookup gives 159.24 pt. This resolves my earlier finding. The one-point difference from your fixture comes from the border padding.

The five explicit-width/save-reopen cases and StandardEncoding, Symbol and Dingbats controls pass. Fonts/forms tests (154), formatting and scoped Clippy pass too. I did not run a viewer check.