Skip to content

Fix getDimFromCell trailing space for XSSF in-cell picture anchors - #1211

Open
arimu1 wants to merge 2 commits into
apache:trunkfrom
arimu1:fix-imageutils-getdimfromcell-1201
Open

Fix getDimFromCell trailing space for XSSF in-cell picture anchors#1211
arimu1 wants to merge 2 commits into
apache:trunkfrom
arimu1:fix-imageutils-getdimfromcell-1201

Conversation

@arimu1

@arimu1 arimu1 commented Aug 13, 2026

Copy link
Copy Markdown

Summary

  • Fix ImageUtils.getDimFromCell when hssfUnits == 0 (XSSF/OOXML anchors): use endD for trailing space instead of dim - endD, matching the semantics of the HSSF branch above it.
  • Add regression test for a single-cell XSSF anchor where zero offsets previously produced 0-sized dimensions and broke picture.resize().

Fixes #1201

Test plan

  • ./gradlew :poi-ooxml:test --tests org.apache.poi.xssf.usermodel.TestXSSFPicture.testGetDimensionFromAnchorSingleCell
  • ./gradlew :poi:test --tests org.apache.poi.ss.util.TestImageUtils

Made with Cursor

Revert the global XSSF trailSpace change (endD was wrong for multi-cell
anchors). Handle only the degenerate same-cell case where endD - startD
is zero or negative: use full cell size when both offsets are 0, otherwise
remaining cell space (dim - leadSpace).

Fixes apache#1201
@arimu1
arimu1 force-pushed the fix-imageutils-getdimfromcell-1201 branch from cf967aa to 05307f0 Compare August 13, 2026 00:29
@arimu1

arimu1 commented Aug 13, 2026

Copy link
Copy Markdown
Author

Thanks for the review — you're right that the global trailSpace = endD change was incorrect.

What was wrong: For XSSF, endD is the EMU offset from the left of the end cell, so trailing space must stay dim - endD (same fraction semantics as HSSF). Setting trailSpace = endD globally would:

  • add a full extra cell for multi-cell anchors with dx2/dy2 == 0
  • under-report normal in-cell anchors where startD < endD (dim - startD - endD instead of endD - startD)

Actual fix (#1201): The bug is only the degenerate same-cell case (col1==col2, dx1==dy1==dx2==dy2==0) where endD - startD == 0, causing picture.resize() to fail. The updated commit reverts the global flip and handles only that case: when startCell == endCell and computed span <= 0, use full cell size if both offsets are 0, otherwise remaining cell space (dim - leadSpace).

Tests added:

  1. Degenerate in-cell (0,0,0,0) → full cell width/height (issue repro + resize)
  2. Normal in-cell startD < endD → size == endD - startD
  3. Multi-cell dx2==0 → width == first column only (no extra cell)

Tip: 05307f004acc5427f5c1164a37b6e1c299413757

import org.apache.poi.ss.usermodel.ClientAnchor.AnchorType;
import org.apache.poi.ss.usermodel.Shape;
import org.apache.poi.ss.util.ImageUtils;
import org.apache.poi.hssf.HSSFITestDataProvider;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use XSSFITestDataProvider

@arimu1

arimu1 commented Aug 14, 2026

Copy link
Copy Markdown
Author

Thanks @pjfanning — switched testGetDimensionFromAnchorDegenerateInCell to XSSFITestDataProvider.instance.getTestDataFileContent and dropped the unused HSSF import. TestXSSFPicture (13 tests) and TestImageUtils pass locally (JDK 17). Tip: 2b9a8ab760c07e2b19edeedbe5bf4effe9a5868a.

Signed-off-by: arimu1 <19286898+arimu1@users.noreply.github.com>
@arimu1
arimu1 force-pushed the fix-imageutils-getdimfromcell-1201 branch from 2b9a8ab to c3e08c5 Compare August 14, 2026 00:22
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.

Picture Resize fails due to error in getDimFromCell()

2 participants