Possible bug in IsPixelWandSimilar

[Danack]

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.

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);
  }
}

cheers
Dan

[snibgo]

I suppose the distance is scaled so the maximum distance is 255, and this represents the diagonal of the cube.

So the test should be whether:

sqrt ((deltaR ^2 + deltaG^2 + deltaB ^ 2) / 3 ) < fuzz

Rearranged:

deltaR ^2 + deltaG^2 + deltaB ^ 2 < (fuzz^2) * 3

So it doesn't sound like a bug.

[Danack]

Thanks for the reply.

Just to be clear then, if I want to know if two RGB colours are within a distance of 5 units (with the axes being 255 units long), then I should do:

Code: Select all

#define ROOT_3 1.732050807568877

fuzz = (5 * QuantumRange / 255.0) / ROOT_3;

That's correct isn't it?

cheers
Dan

*edit* clarified the question.

[snibgo]

I haven't look at the code. If we assume 8 bits, just to keep things simple, then two colours rgb(255,255,255) and rgb(250,250,250) are 5 units distant in all three dimensions so are clearly 5 units away from each other. Remember that 255 represents the diagonal of the colour cube.

More explicitly, the 3-D distance is:

d = sqrt ((deltaR ^2 + deltaG^2 + deltaB ^ 2) / 3 )
= sqrt ((5^2 + 5^2 + 5^2) / 3)
= sqrt (5^2)
= 5


For two colours rgb(255,0,0) and rgb(250,0,0), which are 5 units distant in only one dimension and zero distant in the other dimensions:

d = sqrt ((5^2 + 0^2 + 0^2) / 3)
= sqrt (25/3)
= sqrt (8.333)
= 2.8868

This is, as you say, 5/sqrt(3).