Skip to content

fix(images): refit the frame on replace so a differently-shaped image isn't distorted - #54

Open
kklimuk wants to merge 1 commit into
mainfrom
claude/triage-issue-46-replace-refit
Open

kklimuk wants to merge 1 commit into
mainfrom
claude/triage-issue-46-replace-refit

Conversation

@kklimuk

@kklimuk kklimuk commented Nov 6, 2026

Copy link
Copy Markdown
Owner

🤖 docx-cli triage

What was wrong

images replace swapped the bytes but kept the old <wp:extent>. A differently-shaped picture got stretched into the old box, and the command still exited 0. With a 400×100 image at 4in replaced by a 100×400 one, read still said size="4x1in", so a 1:4 picture was drawn 4:1.

What changed

  • Default: keeps the displayed width and recomputes the height from the new image's pixel aspect ratio. If the result doesn't fit the text area (width or height), it's scaled down with the aspect kept.
  • New flags: --width / --height use the same length grammar and math as images add. One flag keeps the aspect ratio; both are honored as given. --keep-size keeps the old box and is rejected when combined with --width/--height.
  • Frame updates: <wp:extent> and pic:spPr/a:xfrm/a:ext are both updated. An <a:srcRect> crop is dropped. A tight/through <wp:wrapPolygon> is reset to the full frame.
  • Shared pictures: every drawing that shows the same picture (same rId) is refitted, each from its own width. All fits are planned before anything is written.
  • Unknown pixel size (emf/wmf/webp/bmp/tiff): the old box is kept and the confirmation says so (↳ kept 2x1.33in: size unknown for .emf — …). A single --width/--height on such a file is a usage error that writes nothing.
  • Ack names the size: e.g. images.replace img0 (2x6in). --dry-run reports the size too.
  • Files:
    • new src/core/image/refit.ts
    • getPageContentHeightEmu in core/sections.tsx
    • parseImageSize shared with images add
    • formatImageSize shared with the docx:image read hint
    • docs: REFERENCE.md, src/core/image/CLAUDE.md, the invoice criteria.md ("logo not distorted")
  • Fixture: images.ts now replaces a wide PNG with a new 1:3 tests/fixtures/assets/tall.png.

Verification

  • Before (main @ e830c0c): images replace a.docx --at img0 --with tall.png printed images.replace img0 and read still showed size="4x1in".
  • After, same commands: images.replace img0 (2.25x9in). At 4in wide the new height would be 16in, so it's scaled down to fit the 9in text height; read shows the same size. With a 2in-wide original: (2x6in).
  • Invoice scenario logo (floating, wrapThrough + polygon): the replace refits it, the polygon is reset, and docx validate passes.
  • Unit tests: wide→tall, clamping to the text width and height, --width / --height / both, --keep-size and its conflict error, emf (box kept with a note, one-axis refused), a cropped floating image from large-mixed.docx, a shared rId refitted per drawing, and core tests for both extents, the crop, and the wrap polygon.
  • bun run check ✅ bun run test:unit ✅ (2134 tests, run by the pre-push hook)
  • /simplify (single pass, no fan-out): extracted formatImageSize and reused computeExtentEmu for the native-size fallback. /code-review xhigh: the forked run reviewed the wrong checkout, not this branch, so its results don't apply here. I re-read this diff by hand instead.
  • Not run here: the LibreOffice round-trip and the Word oracle (neither is available in this sandbox).

Decisions for you

  • Confirm Word's Change Picture behavior matches. Does Word keep the width, fit inside the old box, or keep the box? Does it reset the crop? I couldn't run scripts/word-redlines.sh without Word. The default here keeps the width and never distorts.
  • Height clamp. Keeping the width can make a tall picture swapped into a wide banner huge, so I also fit it to the text height (the issue only asked for the width clamp). It still fits the page, not the old box. Would you rather fit inside the old box?
  • Invoice scenario asset. assets/logo.svg is square, like the original logo, so the scenario doesn't exercise a shape change yet. I added a criteria check but left the asset alone. Swap in a non-square logo?
  • Possible overlap with feat(images): images set — resize, wrap/float, alt text and captions on an existing image #47. --width/--height on replace overlap feat(images): images set — resize, wrap/float, alt text and captions on an existing image #47's planned images set resizing (both touch respond.ts and the image docs).

Fixes #46


Generated by Claude Code

… isn't distorted

`images replace` kept the old <wp:extent>, stretching the new picture into
the old picture's box. It now keeps the displayed width, recomputes the
height from the new image's aspect ratio, and scales down (aspect kept) to
fit the text area. --width/--height size it like `images add`; --keep-size
keeps the old box. Both extents (wp:extent + a:xfrm/a:ext) are updated, an
a:srcRect crop is dropped, a tight/through wrap polygon is reset, and every
drawing sharing the picture is refitted. A format with no parsed pixel size
keeps the box and the ack says so. The ack names the new size.

Fixes #46

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LkctEPC6PsxKcGTNCZPmN4
Triage-Routine: true
@kklimuk

kklimuk commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

🤖 docx-cli triage

This PR is ready for review and fixes #46. images replace now refits the frame instead of stretching the new picture into the old box. bun run check and the unit tests (2134) pass. The open questions, including whether this matches Word's Change Picture, are in the description's Decisions for you checklist.

Action needed: approve/merge — review PR #54 and answer the Decisions checklist (whether this matches Word's Change Picture, the height clamp, the invoice scenario asset).


Generated by Claude Code

This branch has not been deployed

No deployments
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.

fix(images): replace keeps the old frame, so a differently-shaped image is distorted

2 participants