Skip to content

If height is scaled down significantly more than width, resize vertically first - #9549

Open
radarhere wants to merge 6 commits into
python-pillow:mainfrom
radarhere:resize
Open

If height is scaled down significantly more than width, resize vertically first#9549
radarhere wants to merge 6 commits into
python-pillow:mainfrom
radarhere:resize

Conversation

@radarhere

Copy link
Copy Markdown
Member

Refining #9524, applying a solution in C, rather than Python

@akx

This comment was marked as outdated.

@radarhere

Copy link
Copy Markdown
Member Author

I merged main in, and Benchmark passed, but I don't know how if/how much the improvement is.

@akx

akx commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

According to CodSpeed, ~4% faster, so could be measurement noise and as such unreported.

@radarhere

Copy link
Copy Markdown
Member Author

Ok. The benchmark tests don't cover reducing height but not width, so it at least confirms that this doesn't make things worse for other scenarios.

@hugovk hugovk left a comment

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.

There's a performance regression for reducing very tall images also with a width reduction:

python -c "
  import timeit
  from PIL import Image
  im = Image.new('RGB', (50, 500000))
  print(min(timeit.repeat(lambda: im.resize((40, 1000)), number=5, repeat=3)) / 5 *
  1000, 'ms')
  "
155.0545250007417 ms
python -c "
  import timeit
  from PIL import Image
  im = Image.new('RGB', (50, 500000))
  print(min(timeit.repeat(lambda: im.resize((40, 1000)), number=5, repeat=3)) / 5 *
  1000, 'ms')
  "
212.88325839996105 ms

Comment thread src/libImaging/Resample.c
bounds_vert[i * 2] -= ybox_first;
#define PASS(function, w, h, offset, ksize, bounds, kk) \
second_pass = imTemp != NULL; \
imTemp = ImagingNewDirty(imIn->mode, w, h); \

@hugovk hugovk Sep 7, 2026

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.

Possible memory leak here: if the second pass's ImagingNewDirty fails, imTemp is overwritten with NULL before we goto end, so the intermediate image from the first pass (now only referenced by imIn) is never deleted. We had an ImagingDelete(imTemp) call before.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. I've pushed a commit.

@radarhere radarhere changed the title If height is being scaled down, but not width, resize vertically first If height is scaled down more than width, resize vertically first Sep 7, 2026
@codspeed-hq

codspeed-hq Bot commented Sep 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 603 untouched benchmarks
⏩ 335 skipped benchmarks1


Comparing radarhere:resize (eb69fd6) with main (6b5a7db)

Open in CodSpeed

Footnotes

  1. 335 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@radarhere
radarhere force-pushed the resize branch 2 times, most recently from 8fe0002 to 68c7846 Compare September 7, 2026 22:31
@radarhere radarhere changed the title If height is scaled down more than width, resize vertically first If height is scaled down twice more than width, resize vertically first Sep 7, 2026
@radarhere radarhere changed the title If height is scaled down twice more than width, resize vertically first If height is scaled down significantly more than width, resize vertically first Sep 7, 2026
@radarhere
radarhere force-pushed the resize branch 2 times, most recently from 79498e6 to 4194a81 Compare September 8, 2026 12:22
@radarhere

Copy link
Copy Markdown
Member Author

There's a performance regression for reducing very tall images also with a width reduction

Ok, that should be fixed now.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants