Skip to content

Conversation

@radarhere
Copy link
Member

@radarhere radarhere commented May 1, 2022

First, in quantize2, the variable Pixel furthest is only used for its v property. So the Pixel can be simplified to just that uint32_t.

Second, quantize2 sets furthestDistance to zero,

Pillow/src/libImaging/Quant.c

Lines 1579 to 1583 in 31800a0

data.furthestDistance = 0;
data.secondPixel = (i == 1) ? 1 : 0;
hashtable_foreach_update(h, compute_distances, &data);
p[i].v = data.furthest.v;
data.new.v = data.furthest.v;

and then runs compute_distances, assigning furthest,

Pillow/src/libImaging/Quant.c

Lines 1528 to 1541 in 31800a0

compute_distances(const HashTable *h, const Pixel pixel, uint32_t *dist, void *u) {
DistanceData *data = (DistanceData *)u;
uint32_t oldDist = *dist;
uint32_t newDist;
newDist = _DISTSQR(&(data->new), &pixel);
if (data->secondPixel || newDist < oldDist) {
*dist = newDist;
oldDist = newDist;
}
if (oldDist > data->furthestDistance) {
data->furthestDistance = oldDist;
data->furthest.v = pixel.v;
}
}

before it returns to quantize2 and p[i].v and data.new.v are set to data.furthest.v.

What if oldDist was never greater than zero though? Then furthest.v would never be assigned.
This causes a problem in python-pillow/docker-images#148 - https://github.com/python-pillow/docker-images/runs/6247349541?check_suite_focus=true.
So instead, this PR sets it to the first v of pixelData.

@hugovk hugovk merged commit ef8cec6 into python-pillow:main May 17, 2022
@radarhere radarhere deleted the furthestV branch May 17, 2022 21:57
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.

2 participants