From 2bc8e5cee68cdac98d22902746f0737382c63a41 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 2 Jun 2026 11:44:13 +0300 Subject: [PATCH 1/4] Fill: hoist static sizes out of loop; restrict row pointer --- src/libImaging/Fill.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/libImaging/Fill.c b/src/libImaging/Fill.c index cbd303204d5..8896fec7498 100644 --- a/src/libImaging/Fill.c +++ b/src/libImaging/Fill.c @@ -21,7 +21,6 @@ Imaging ImagingFill(Imaging im, const void *colour) { - int x, y; ImagingSectionCookie cookie; /* 0-width or 0-height image. No need to do anything */ @@ -29,19 +28,23 @@ ImagingFill(Imaging im, const void *colour) { return im; } + // xsize and ysize are invariant during the loops below. + int xsize = im->xsize; + int ysize = im->ysize; + if (im->type == IMAGING_TYPE_SPECIAL) { /* use generic API */ ImagingAccess access = ImagingAccessNew(im); if (access) { - for (y = 0; y < im->ysize; y++) { - for (x = 0; x < im->xsize; x++) { + for (int y = 0; y < ysize; y++) { + for (int x = 0; x < xsize; x++) { access->put_pixel(im, x, y, colour); } } ImagingAccessDelete(im, access); } else { /* wipe the image */ - for (y = 0; y < im->ysize; y++) { + for (int y = 0; y < ysize; y++) { memset(im->image[y], 0, im->linesize); } } @@ -50,14 +53,16 @@ ImagingFill(Imaging im, const void *colour) { ImagingSectionEnter(&cookie); memcpy(&c, colour, im->pixelsize); if (im->image32 && c != 0L) { - for (y = 0; y < im->ysize; y++) { - for (x = 0; x < im->xsize; x++) { - im->image32[y][x] = c; + for (int y = 0; y < ysize; y++) { + // Restrict safe: sole owner of image data here. + INT32 *restrict row = im->image32[y]; + for (int x = 0; x < xsize; x++) { + row[x] = c; } } } else { unsigned char cc = (unsigned char)*(UINT8 *)colour; - for (y = 0; y < im->ysize; y++) { + for (int y = 0; y < ysize; y++) { memset(im->image[y], cc, im->linesize); } } From 93a26495f21ff5d5acec5e16ff62bd39d22834c5 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 30 Jun 2026 09:33:54 +0300 Subject: [PATCH 2/4] Add benchmarks for linear_gradient and radial_gradient --- Tests/benchmarks.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Tests/benchmarks.py b/Tests/benchmarks.py index 9d7439d321b..ac863977f1b 100644 --- a/Tests/benchmarks.py +++ b/Tests/benchmarks.py @@ -354,6 +354,22 @@ def test_fill(bench: BenchmarkFixture, mode: str, size: tuple[int, int]) -> None bench(Image.new, mode, size, color) +@pytest.mark.benchmark(group="allocate") +@pytest.mark.parametrize("mode", ["1", "F", "I", "L", "P"]) +def test_linear_gradient(bench: BenchmarkFixture, mode: str) -> None: + result = bench(Image.linear_gradient, mode) + assert result.size == (256, 256) + assert result.mode == mode + + +@pytest.mark.benchmark(group="allocate") +@pytest.mark.parametrize("mode", ["1", "F", "I", "L", "P"]) +def test_radial_gradient(bench: BenchmarkFixture, mode: str) -> None: + result = bench(Image.radial_gradient, mode) + assert result.size == (256, 256) + assert result.mode == mode + + CHOPS_OPS = [ ImageChops.add, ImageChops.subtract, From 6368935856ddf37e1f11da33cafd01303118a938 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 30 Jun 2026 10:01:15 +0300 Subject: [PATCH 3/4] Apply hoists and restricts in `Image.*_gradient()`` --- src/libImaging/Fill.c | 70 ++++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/src/libImaging/Fill.c b/src/libImaging/Fill.c index 8896fec7498..85aeae63388 100644 --- a/src/libImaging/Fill.c +++ b/src/libImaging/Fill.c @@ -75,7 +75,6 @@ ImagingFill(Imaging im, const void *colour) { Imaging ImagingFillLinearGradient(const ModeID mode) { Imaging im; - int y; if (mode != IMAGING_MODE_1 && mode != IMAGING_MODE_F && mode != IMAGING_MODE_I && mode != IMAGING_MODE_L && mode != IMAGING_MODE_P) { @@ -87,19 +86,24 @@ ImagingFillLinearGradient(const ModeID mode) { return NULL; } + // Branch on pixel type outside the loops so the compiler can tighten them. + // Restrict safe: sole owner of the freshly-allocated image data here. if (im->image8) { - for (y = 0; y < 256; y++) { + for (int y = 0; y < 256; y++) { memset(im->image8[y], (unsigned char)y, 256); } + } else if (im->type == IMAGING_TYPE_FLOAT32) { + for (int y = 0; y < 256; y++) { + FLOAT32 *restrict row = (FLOAT32 *)im->image32[y]; + for (int x = 0; x < 256; x++) { + row[x] = y; + } + } } else { - int x; - for (y = 0; y < 256; y++) { - for (x = 0; x < 256; x++) { - if (im->type == IMAGING_TYPE_FLOAT32) { - IMAGING_PIXEL_FLOAT32(im, x, y) = y; - } else { - IMAGING_PIXEL_INT32(im, x, y) = y; - } + for (int y = 0; y < 256; y++) { + INT32 *restrict row = im->image32[y]; + for (int x = 0; x < 256; x++) { + row[x] = y; } } } @@ -110,8 +114,6 @@ ImagingFillLinearGradient(const ModeID mode) { Imaging ImagingFillRadialGradient(const ModeID mode) { Imaging im; - int x, y; - int d; if (mode != IMAGING_MODE_1 && mode != IMAGING_MODE_F && mode != IMAGING_MODE_I && mode != IMAGING_MODE_L && mode != IMAGING_MODE_P) { @@ -123,22 +125,36 @@ ImagingFillRadialGradient(const ModeID mode) { return NULL; } - for (y = 0; y < 256; y++) { - for (x = 0; x < 256; x++) { - d = (int)sqrt( - (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 - ); - if (d >= 255) { - d = 255; + // Branch on pixel type outside the loops so the compiler can tighten them. + // Restrict safe: sole owner of the freshly-allocated image data here. + if (im->image8) { + for (int y = 0; y < 256; y++) { + UINT8 *restrict row = im->image8[y]; + for (int x = 0; x < 256; x++) { + int d = (int)sqrt( + (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 + ); + row[x] = d >= 255 ? 255 : d; } - if (im->image8) { - im->image8[y][x] = d; - } else { - if (im->type == IMAGING_TYPE_FLOAT32) { - IMAGING_PIXEL_FLOAT32(im, x, y) = d; - } else { - IMAGING_PIXEL_INT32(im, x, y) = d; - } + } + } else if (im->type == IMAGING_TYPE_FLOAT32) { + for (int y = 0; y < 256; y++) { + FLOAT32 *restrict row = (FLOAT32 *)im->image32[y]; + for (int x = 0; x < 256; x++) { + int d = (int)sqrt( + (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 + ); + row[x] = d >= 255 ? 255 : d; + } + } + } else { + for (int y = 0; y < 256; y++) { + INT32 *restrict row = im->image32[y]; + for (int x = 0; x < 256; x++) { + int d = (int)sqrt( + (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 + ); + row[x] = d >= 255 ? 255 : d; } } } From f7f350a007a1d72bdec9b1b2f45a5e7d7e645ee5 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Tue, 30 Jun 2026 16:45:15 +0300 Subject: [PATCH 4/4] De-triplicate `Image.*_gradient()`'s loops with a macro --- src/libImaging/Fill.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/libImaging/Fill.c b/src/libImaging/Fill.c index 85aeae63388..25344cf3a7e 100644 --- a/src/libImaging/Fill.c +++ b/src/libImaging/Fill.c @@ -125,39 +125,32 @@ ImagingFillRadialGradient(const ModeID mode) { return NULL; } +#define ASSIGN_ROW(row, y) \ + for (int x = 0; x < 256; x++) { \ + int d = \ + (int)sqrt((double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0); \ + row[x] = d >= 255 ? 255 : d; \ + } + // Branch on pixel type outside the loops so the compiler can tighten them. // Restrict safe: sole owner of the freshly-allocated image data here. if (im->image8) { for (int y = 0; y < 256; y++) { UINT8 *restrict row = im->image8[y]; - for (int x = 0; x < 256; x++) { - int d = (int)sqrt( - (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 - ); - row[x] = d >= 255 ? 255 : d; - } + ASSIGN_ROW(row, y); } } else if (im->type == IMAGING_TYPE_FLOAT32) { for (int y = 0; y < 256; y++) { FLOAT32 *restrict row = (FLOAT32 *)im->image32[y]; - for (int x = 0; x < 256; x++) { - int d = (int)sqrt( - (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 - ); - row[x] = d >= 255 ? 255 : d; - } + ASSIGN_ROW(row, y); } } else { for (int y = 0; y < 256; y++) { INT32 *restrict row = im->image32[y]; - for (int x = 0; x < 256; x++) { - int d = (int)sqrt( - (double)((x - 128) * (x - 128) + (y - 128) * (y - 128)) * 2.0 - ); - row[x] = d >= 255 ? 255 : d; - } + ASSIGN_ROW(row, y); } } +#undef ASSIGN_ROW return im; }