Possible bug in IsPixelWandSimilar
Posted: 2013-10-18T13:19:12-07:00
Hi,
I think I may have found a bug in the IsPixelWandSimilar() function.
When comparing the two colours 'rgb(255, 0, 0)' and 'rgb(250, 0, 0)' with a fuzz factor of 4 (before scaling the fuzz factor into the appropriate scale) IsPixelWandSimilar() returns true, when it should return false, as those colours are at least '5' apart.
The error seems to occur because the fuzz distance that is passed into the function is multiplied by 3 before being compared to the distance.
e.g. I've compiled ImageMagick with Q16, so fuzz distance should be (65535 * 4 / 255) ^ 2 = 1028 ^ 2 = 1,056,784
but it is multiplied by 3 on color.c line 1863 so it is now 3,170,352
and the test fails as
Distance = 1,651,225 < fuzz = 3,170,352 and so the distance between the two colours is less than the fuzz value, as the fuzz value is larger than it should be
Below is a small set of tests for the IsPixelWandSimilar() function, that show the incorrect behaviour.
cheers
Dan
I think I may have found a bug in the IsPixelWandSimilar() function.
When comparing the two colours 'rgb(255, 0, 0)' and 'rgb(250, 0, 0)' with a fuzz factor of 4 (before scaling the fuzz factor into the appropriate scale) IsPixelWandSimilar() returns true, when it should return false, as those colours are at least '5' apart.
The error seems to occur because the fuzz distance that is passed into the function is multiplied by 3 before being compared to the distance.
e.g. I've compiled ImageMagick with Q16, so fuzz distance should be (65535 * 4 / 255) ^ 2 = 1028 ^ 2 = 1,056,784
but it is multiplied by 3 on color.c line 1863 so it is now 3,170,352
and the test fails as
Distance = 1,651,225 < fuzz = 3,170,352 and so the distance between the two colours is less than the fuzz value, as the fuzz value is larger than it should be
Below is a small set of tests for the IsPixelWandSimilar() function, that show the incorrect behaviour.
Code: Select all
typedef struct _IsPixelWandSimilarTest {
char *color1;
char *color2;
double fuzz;
MagickBooleanType expectedResult;
} IsPixelWandSimilarTest;
void IsPixelWandSimilarTests() {
IsPixelWandSimilarTest *test;
IsPixelWandSimilarTest wandSimilarTests[] = {
{"rgb(255, 0, 0)", "rgb(250, 0, 0)", 10.0, MagickTrue},
{"rgb(255, 0, 0)", "rgb(250, 0, 0)", 5.0, MagickTrue},
// Fuzz distance should be (65535 * 4 / 255) ^ 2 = 1028 ^ 2 = 1,056,784
// but it is multiplied by 3 on color.c line 1863 so it is now 3,170,352
// and the test fails
// Distance = 1651225.000000, fuzz = 3170352.000000
{"rgb(255, 0, 0)", "rgb(250, 0, 0)", 4.0, MagickFalse},
{"rgba(255, 0, 0, 1.0)", "rgba(250, 0, 0, 1.0)", 10.0, MagickTrue},
{"rgba(255, 0, 0, 1.0)", "rgba(250, 0, 0, 1.0)", 5.0, MagickTrue},
//This test fails for the same reason as test 3
{"rgba(255, 0, 0, 1.0)", "rgba(250, 0, 0, 1.0)", 4.0, MagickFalse},
{"black", "rgba(10, 0, 0, 1.0)", 10.0, MagickTrue},
};
int numberTests = sizeof(wandSimilarTests) / sizeof(IsPixelWandSimilarTest);
for (int i = 0; i<numberTests ; i++) {
test = &wandSimilarTests[i];
PixelWand *color1;
PixelWand *color2;
MagickBooleanType actualResult;
color1=NewPixelWand();
(void) PixelSetColor(color1,test->color1);
color2=NewPixelWand();
(void) PixelSetColor(color2,test->color2);
double fuzz = test->fuzz * QuantumRange / 255.0;
actualResult = IsPixelWandSimilar(color1, color2, fuzz);
if (actualResult != test->expectedResult) {
fprintf(stderr, "IsPixelWandSimilar test failed for colors %s \
& %s, distance %f, expected result %s not met.\n", test->color1, \
test->color2, test->fuzz, (test->expectedResult ? "true" : "false"));
exit(0);
}
color2=DestroyPixelWand(color2);
color1=DestroyPixelWand(color1);
}
}
Dan