Skip to content

Optimize histogram generation + fix masked histograms for 2-band images + add test coverage #10005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
radarhere merged 3 commits into from
Sep 17, 2026
+107 −39
Merged

Optimize histogram generation + fix masked histograms for 2-band images + add test coverage #10005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
465ea33
Apply hoist/restrict optimizations to ImagingGetHistogram
akx Sep 16, 2026
6b67f55
Fix masked histograms for 2-band images + add test coverage
akx Sep 16, 2026
349c6ca
Apply suggestions from code review
akx Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions Tests/test_image_histogram.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from __future__ import annotations

import pytest

from PIL import Image

from .helper import hopper


Expand All @@ -20,3 +24,48 @@ def histogram(mode: str) -> tuple[int, int, int]:
assert histogram("RGBA") == (1024, 0, 16384)
assert histogram("CMYK") == (1024, 0, 16384)
assert histogram("YCbCr") == (768, 0, 1908)


def test_histogram_masked() -> None:
mask = Image.new("1", (128, 128), 0)
crop_box = (0, 0, 128, 64)
mask.paste(1, crop_box)

def histogram(mode: str) -> tuple[int, int, int]:
im = hopper(mode)
h = im.histogram(mask)
assert h == im.crop(crop_box).histogram()
return len(h), min(h), max(h)

assert histogram("1") == (256, 0, 5024)
assert histogram("L") == (256, 0, 178)
assert histogram("LA") == (512, 0, 8192)
assert histogram("La") == (512, 0, 8192)
assert histogram("P") == (256, 0, 797)
assert histogram("PA") == (512, 0, 8192)
assert histogram("RGB") == (768, 1, 174)
assert histogram("RGBA") == (1024, 0, 8192)
assert histogram("CMYK") == (1024, 0, 8192)
assert histogram("YCbCr") == (768, 0, 1009)


@pytest.mark.parametrize("mode", ("I", "F"))
def test_histogram_masked_unsupported_mode(mode: str) -> None:
mask = Image.new("1", (128, 128))
with pytest.raises(ValueError, match="image has wrong mode"):
hopper(mode).histogram(mask)


def test_histogram_mask_size_mismatch() -> None:
mask = Image.new("1", (1, 1))
with pytest.raises(ValueError, match="images do not match"):
hopper("L").histogram(mask)


@pytest.mark.parametrize(
"mode", ("LA", "La", "I", "F", "P", "PA", "RGB", "RGBA", "CMYK", "YCbCr")
)
def test_histogram_bad_mask_mode(mode: str) -> None:
mask = Image.new(mode, (128, 128))
with pytest.raises(ValueError, match="bad transparency mask"):
hopper("L").histogram(mask)
7 changes: 7 additions & 0 deletions docs/releasenotes/13.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,3 +159,10 @@ Fixed ImageStat variance rounding

Fixed a floating-point rounding error that may have produced a negative variance in
:py:attr:`~PIL.ImageStat.Stat.var`.

Fixed histograms for masked LA, La and PA images
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

When :py:meth:`~PIL.Image.Image.histogram` was given a mask for a two-band image (LA,
La or PA), the second band was incorrectly derived from the first band of the input
image, rather than the alpha band.
90 changes: 51 additions & 39 deletions src/libImaging/Histo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ ImagingHistogramNew(Imaging im) {
ImagingHistogram
ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
ImagingSectionCookie cookie;
int x, y, i;
Comment thread
radarhere marked this conversation as resolved.
ImagingHistogram h;
INT32 imin, imax;
FLOAT32 fmin, fmax, scale;
Expand All @@ -68,9 +67,10 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
return ImagingError_ModeError();
}

int xsize = im->xsize, ysize = im->ysize;
if (imMask) {
/* Validate mask */
if (im->xsize != imMask->xsize || im->ysize != imMask->ysize) {
if (xsize != imMask->xsize || ysize != imMask->ysize) {
return ImagingError_Mismatch();
}
if (imMask->mode != IMAGING_MODE_1 && imMask->mode != IMAGING_MODE_L) {
Expand All @@ -83,14 +83,22 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
return NULL;
}

// restrict safe: im and imMask are both read-only here
// (they may even be the same image).
// histogram is a fresh allocation from ImagingHistogramNew

long *restrict histogram = h->histogram;

if (imMask) {
/* mask */
if (im->image8) {
ImagingSectionEnter(&cookie);
for (y = 0; y < im->ysize; y++) {
for (x = 0; x < im->xsize; x++) {
if (imMask->image8[y][x] != 0) {
h->histogram[im->image8[y][x]]++;
for (int y = 0; y < ysize; y++) {
UINT8 *restrict in = im->image8[y];
UINT8 *restrict mask = imMask->image8[y];
for (int x = 0; x < xsize; x++) {
if (mask[x] != 0) {
histogram[in[x]]++;
}
}
}
Expand All @@ -101,16 +109,19 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
return ImagingError_ModeError();
}
ImagingSectionEnter(&cookie);
for (y = 0; y < im->ysize; y++) {
UINT8 *in = (UINT8 *)im->image32[y];
for (x = 0; x < im->xsize; x++) {
if (imMask->image8[y][x] != 0) {
h->histogram[(*in++)]++;
h->histogram[(*in++) + 256]++;
h->histogram[(*in++) + 512]++;
h->histogram[(*in++) + 768]++;
} else {
in += 4;
for (int y = 0; y < ysize; y++) {
UINT8 *restrict in = (UINT8 *)im->image32[y];
UINT8 *restrict mask = imMask->image8[y];
for (int x = 0; x < xsize; x++, in += 4) {
if (mask[x] != 0) {
histogram[*in]++;
if (im->bands == 2) {
histogram[*(in + 3) + 256]++;
} else {
histogram[*(in + 1) + 256]++;
histogram[*(in + 2) + 512]++;
histogram[*(in + 3) + 768]++;
}
}
}
}
Expand All @@ -120,26 +131,27 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
/* mask not given; process pixels in image */
if (im->image8) {
ImagingSectionEnter(&cookie);
for (y = 0; y < im->ysize; y++) {
for (x = 0; x < im->xsize; x++) {
h->histogram[im->image8[y][x]]++;
for (int y = 0; y < ysize; y++) {
UINT8 *restrict in = im->image8[y];
for (int x = 0; x < xsize; x++) {
histogram[in[x]]++;
}
}
ImagingSectionLeave(&cookie);
} else {
switch (im->type) {
case IMAGING_TYPE_UINT8:
ImagingSectionEnter(&cookie);
for (y = 0; y < im->ysize; y++) {
UINT8 *in = (UINT8 *)im->image[y];
for (x = 0; x < im->xsize; x++, in += 4) {
h->histogram[*in]++;
for (int y = 0; y < ysize; y++) {
UINT8 *restrict in = (UINT8 *)im->image[y];
for (int x = 0; x < xsize; x++, in += 4) {
histogram[*in]++;
if (im->bands == 2) {
h->histogram[*(in + 3) + 256]++;
histogram[*(in + 3) + 256]++;
} else {
h->histogram[*(in + 1) + 256]++;
h->histogram[*(in + 2) + 512]++;
h->histogram[*(in + 3) + 768]++;
histogram[*(in + 1) + 256]++;
histogram[*(in + 2) + 512]++;
histogram[*(in + 3) + 768]++;
}
}
}
Expand All @@ -150,7 +162,7 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
ImagingHistogramDelete(h);
return ImagingError_ValueError("min/max not given");
}
if (!im->xsize || !im->ysize) {
if (!xsize || !ysize) {
break;
}
memcpy(&imin, minmax, sizeof(imin));
Expand All @@ -160,12 +172,12 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
}
ImagingSectionEnter(&cookie);
scale = 255.0F / (imax - imin);
for (y = 0; y < im->ysize; y++) {
INT32 *in = im->image32[y];
for (x = 0; x < im->xsize; x++) {
i = (int)(((*in++) - imin) * scale);
for (int y = 0; y < ysize; y++) {
INT32 *restrict in = im->image32[y];
for (int x = 0; x < xsize; x++) {
int i = (int)(((*in++) - imin) * scale);
if (i >= 0 && i < 256) {
h->histogram[i]++;
histogram[i]++;
}
}
}
Expand All @@ -176,7 +188,7 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
ImagingHistogramDelete(h);
return ImagingError_ValueError("min/max not given");
}
if (!im->xsize || !im->ysize) {
if (!xsize || !ysize) {
break;
}
memcpy(&fmin, minmax, sizeof(fmin));
Expand All @@ -186,12 +198,12 @@ ImagingGetHistogram(Imaging im, Imaging imMask, void *minmax) {
}
ImagingSectionEnter(&cookie);
scale = 255.0F / (fmax - fmin);
for (y = 0; y < im->ysize; y++) {
FLOAT32 *in = (FLOAT32 *)im->image32[y];
for (x = 0; x < im->xsize; x++) {
i = (int)(((*in++) - fmin) * scale);
for (int y = 0; y < ysize; y++) {
FLOAT32 *restrict in = (FLOAT32 *)im->image32[y];
for (int x = 0; x < xsize; x++) {
int i = (int)(((*in++) - fmin) * scale);
if (i >= 0 && i < 256) {
h->histogram[i]++;
histogram[i]++;
}
}
}
Expand Down
Loading