Skip to content

Don't let _resize_and_fill/_resize_and_crop scale a side down to zero - #14323

Open
ErenAta16 wants to merge 1 commit into
huggingface:mainfrom
ErenAta16:fix-resize-zero-dimension
Open

Don't let _resize_and_fill/_resize_and_crop scale a side down to zero#14323
ErenAta16 wants to merge 1 commit into
huggingface:mainfrom
ErenAta16:fix-resize-zero-dimension

Conversation

@ErenAta16

Copy link
Copy Markdown

What does this PR do?

_resize_and_fill and _resize_and_crop compute the scaled size with floor division:

src_w = width if ratio < src_ratio else image.width * height // image.height
src_h = height if ratio >= src_ratio else image.height * width // image.width

For a very thin image that rounds to zero. Fitting a 1x2000 image into 512x512 gives 1 * 512 // 2000 == 0, and Image.resize((0, 512)) fails:

1x2000 -> 512x512   fill=ValueError  crop=ok
2000x1 -> 512x512   fill=ValueError  crop=ok
3x900  -> 512x512   fill=ok          crop=ok
900x3  -> 512x512   fill=ok          crop=ok
64x64  -> 512x512   fill=ok          crop=ok
ValueError: height and width must be > 0

which names neither the input image nor the side that collapsed.

_resize_and_crop shows ok for those two rows only because its comparisons (ratio > src_ratio / ratio <= src_ratio) send those particular aspect ratios down the branch that returns width/height directly. The same floor-divided expression is present and collapses the same way for the mirrored inputs, so clamping both keeps the pair consistent rather than fixing one and leaving a latent copy next to it.

Reachable from the public API:

VaeImageProcessor().resize(image, height=512, width=512, resize_mode="fill")

The change

max(1, ...) on the four floor-divided expressions. A resize target of zero pixels is never a valid answer, and the clamp only fires where the previous value was 0, so any size that was computed correctly before is untouched.

Tests

Added to tests/others/test_image_processor.py:

  • test_resize_fill_and_crop_handle_extreme_aspect_ratios(1, 2000), (2000, 1) and (1, 1) across both fill and crop, asserting a (512, 512) result
  • test_resize_fill_and_crop_unchanged_for_ordinary_images(64, 64), (900, 3), (3, 900), (1024, 768), so the clamp cannot be widened into perturbing sizes that already worked

Reverting only image_processor.py:

FAILED tests/others/test_image_processor.py::ImageProcessorTest::test_resize_fill_and_crop_handle_extreme_aspect_ratios
1 failed, 1 passed

and with the change:

$ pytest tests/others/test_image_processor.py
12 passed

make quality clean on both files.

Note

I have a second, unrelated PR open against the same file (#14322, empty mask in get_crop_region). The hunks do not overlap; both append their tests at the end of tests/others/test_image_processor.py, so whichever lands second may want a trivial rebase there.

Who can review?

Anyone in the community is free to review the PR once the tests have passed.

Both helpers compute the scaled size with floor division:

    src_w = width if ratio < src_ratio else image.width * height // image.height
    src_h = height if ratio >= src_ratio else image.height * width // image.width

For a very thin image that rounds to 0. Fitting a 1x2000 image into 512x512
gives `1 * 512 // 2000 == 0`, and `Image.resize((0, 512))` fails with

    ValueError: height and width must be > 0

which names neither the input nor the side that vanished.

    1x2000 -> 512x512   fill=ValueError  crop=ok
    2000x1 -> 512x512   fill=ValueError  crop=ok
    3x900  -> 512x512   fill=ok          crop=ok

`_resize_and_crop` survives these two only because the branch that would round
to zero is the one it does not take for those aspect ratios; the same
expression is there and the same collapse happens for the mirrored inputs.
Clamping both to a minimum of one pixel in both helpers keeps them consistent.

Reachable from the public API as
`VaeImageProcessor.resize(image, height, width, resize_mode="fill")`.

Sizes that already computed correctly are untouched: the clamp only fires where
the old value was 0.
@github-actions github-actions Bot added tests size/S PR with diff < 50 LOC labels Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi @ErenAta16, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant