Skip to content
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.

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

Ensure that furthest v is set in quantize2 #6256

Merged
merged 2 commits into from May 17, 2022

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.

None yet

2 participants