php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64736 Incorrect calculation in gdColorMatch
Submitted: 2013-04-29 23:05 UTC Modified: 2015-03-26 05:30 UTC
From: sixd@php.net Assigned: pajoye (profile)
Status: Wont fix Package: GD related
PHP Version: 5.5Git-2013-04-29 (Git) OS: Linux
Private report: No CVE-ID: None
 [2013-04-29 23:05 UTC] sixd@php.net
Description:
------------
Compilations warnings in gd_crop.c flag a potential calculation issue in gdColorMatch:

/home/cjones/php-5.5/ext/gd/libgd/gd_crop.c:340: warning: suggest parentheses around arithmetic in operand of ^
/home/cjones/php-5.5/ext/gd/libgd/gd_crop.c:340: warning: suggest parentheses around arithmetic in operand of ^

The code at line 340 is:

	const double dist_perc = sqrt(dist / (255^2 + 255^2 + 255^2));

The denominator is currently evaluated as 253. Was it instead intended be (3 * (255 * 255)) == 195075 ?


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-04-30 05:21 UTC] pajoye@php.net
That's the classic distance calculation in the RGB cube, does ^ not have 
precedence with the compiler you use? It should (or has in all platforms I use) 
:).
 [2013-04-30 05:21 UTC] pajoye@php.net
-Status: Open +Status: Feedback
 [2013-04-30 14:48 UTC] sixd@php.net
In C, "^" is the bitwise exclusive OR operator (and has lower precedence than 
"+"). Pow() is the power function.
 [2013-04-30 14:48 UTC] sixd@php.net
-Status: Feedback +Status: Open
 [2015-03-25 01:24 UTC] cmb@php.net
-Assigned To: +Assigned To: pajoye
 [2015-03-25 01:24 UTC] cmb@php.net
It also seems to me, that the calculation is wrong.

Assuming that dr, dg, db and da are in range [0, 255], dist
would be in the range [0, 510]. Assuming 255^2 is supposed to mean
squaring, the argument to sqrt() (in the assignment to dist_perc)
is in the range [0, 0.00196], so dist_perc is in the range [0,
0.04428] -- apparently not, what is supposed to represent a
percentage.

Assuming 255^2 is actually supposed to mean XOR, the respective
argument to sqrt() would be in the range [0, 0.6719], so dist_perc
is in the range [0, 0.8197] -- looks rather fine, but seems
somewhat arbitrarily.

With the following, dist_perc would be in the range [0, 1].

  const double dist_perc = dist / sqrt(4 * 255 * 255);
  
However, I have some doubts, that the Pythagorean theorem applies
to four dimensions in this manner.

Anyhow, changing the calculation of dist_perc would constitute a BC
break...
 [2015-03-26 05:30 UTC] pajoye@php.net
-Status: Assigned +Status: Wont fix
 [2015-03-26 05:30 UTC] pajoye@php.net
@cmb
it is a good enough approximation. I may change it at some time in gd 2.2, using actual color comparison methods in sRGB space, if not too slow :)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Mon Dec 30 14:01:28 2024 UTC