Skip to content

Commit

Permalink
Overhaul GD test helpers and affected tests (GH-17309)
Browse files Browse the repository at this point in the history
* Use type declarations instead of doc-block annotations

* Inline the terrible get_rgb() function

* Always traverse pixels in Z order

libgd stores the pixel as an array of rows, so we should use row-major-
order traversal to improve caching.

* Add assertions to avoid misuse of the functions

The assertion regarding the image dimensions won't break any tests, and
we had it already as a comment.

However, asserting that the images are truecolor images is important
for `calc_image_dissimilarity()` which otherwise would calculate
nonsense, and not unreasonable for `test_image_equals_image()` which
otherwise is overspecified (for our purposes, it doesn't matter which
palette entry a pixel refers to, but rather whether the actual colors
referred by a palette color match).

Since the truecolor assertions break two tests, we fix these by
converting to truecolor.  That should likely be backported to lower
branches.

* Drop implicit conversion to truecolor

Conversion to truecolor is a relatively expensive operation, and as
such should not be implicit; instead test authors are encouraged to use
truecolor images in the first place where possible, or to even find
better ways to verify expectations than doing a full image comparison.

* Merge similarity.inc into func.inc

There is no particular reason to have a separate file for similarity
comparisons.

* Simplify bug43475.phpt and bug64641.phpt

`calc_image_dissimilarity()` calculates the sum of the euclidean
distance of the RGB channels of all pixels.  The euclidean distance is
either zero or greater than or equal to one (but never in ]0, 1[).  The
sum of these values also has this property, so it doesn't make sense to
check for less than 1e-5.  Thus we just call `test_image_equals_file()`
instead.

* Replace calc_image_dissimilarity() with the well-known mse()

`calc_image_dissimilarity()` has the drawback that it did sum up the
pixel differences, so for large images the result could be way larger
than for small images.  It also has the drawback that it likely is not
as well understood as the mean squared error.  Thus we replace it with
the latter, and calculate the mean squared error of the individual RGB
channels (to be precise).  The result is always in range 0..255**2 what
makes reasoning about thresholds easier.
  • Loading branch information
cmb69 authored Jan 25, 2025
1 parent f698c62 commit 5890761
Show file tree
Hide file tree
Showing 24 changed files with 61 additions and 118 deletions.
8 changes: 4 additions & 4 deletions ext/gd/tests/avif_decode_encode.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ gd
--FILE--
<?php

require_once __DIR__ . '/similarity.inc';
require_once __DIR__ . '/func.inc';

$infile = __DIR__ . '/girl.avif';
$outfile = __DIR__ . '/test.avif';
Expand Down Expand Up @@ -58,8 +58,8 @@ gd

// Note we could also forgive a certain number of pixel differences.
// With the current test image, we just didn't need to.
echo 'How many pixels are different in the two images? ';
print_r(calc_image_dissimilarity($img, $img_from_avif));
echo 'What is the mean squared error of the two images? ',
mse($img, $img_from_avif);

unlink($outfile);

Expand All @@ -79,4 +79,4 @@ Encoding AVIF with illegal quality: imageavif(): Argument #3 ($quality) must be
Encoding AVIF with illegal speed: imageavif(): Argument #4 ($speed) must be between -1 and 10
Encoding AVIF losslessly... ok
Decoding the AVIF we just wrote...
How many pixels are different in the two images? 0
What is the mean squared error of the two images? 0
8 changes: 4 additions & 4 deletions ext/gd/tests/bug43475.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ gd
?>
--FILE--
<?php
require_once __DIR__ . '/similarity.inc';
require_once __DIR__ . '/func.inc';

function setStyleAndThickness($im, $color, $thickness)
{
Expand Down Expand Up @@ -59,8 +59,8 @@ imageline($im, 200, 100, 700, 100, IMG_COLOR_STYLED);
imageline($im, 700, 100, 700, 600, IMG_COLOR_STYLED);
imageline($im, 700, 600, 200, 100, IMG_COLOR_STYLED);

$ex = imagecreatefrompng(__DIR__ . '/bug43475.png');
var_dump(calc_image_dissimilarity($ex, $im) < 1e-5);
imagepalettetotruecolor($im);
test_image_equals_file(__DIR__ . '/bug43475.png', $im);
?>
--EXPECT--
bool(true)
The images are equal.
Binary file modified ext/gd/tests/bug43475.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions ext/gd/tests/bug52070.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ imagedashedline($im, 800, 400, 500, 800, $color);
imagedashedline($im, 800, 400, 600, 800, $color);
imagedashedline($im, 800, 400, 700, 800, $color);
imagedashedline($im, 800, 400, 800, 800, $color);
imagepalettetotruecolor($im);
include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/bug52070.png', $im);
?>
Expand Down
Binary file modified ext/gd/tests/bug52070.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 3 additions & 11 deletions ext/gd/tests/bug64641.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ if (!(imagetypes() & IMG_PNG)) {
?>
--FILE--
<?php
require_once __DIR__ . '/similarity.inc';
require_once __DIR__ . '/func.inc';

$im = imagecreatetruecolor(640, 480);

Expand All @@ -31,15 +31,7 @@ $points = array(
);
imagefilledpolygon($im, $points, 0xFFFF00);

$ex = imagecreatefrompng(__DIR__ . '/bug64641.png');
if (($diss = calc_image_dissimilarity($ex, $im)) < 1e-5) {
echo "IDENTICAL";
} else {
echo "DISSIMILARITY: $diss";
}
imagedestroy($ex);

imagedestroy($im);
test_image_equals_file(__DIR__ . '/bug64641.png', $im);
?>
--EXPECT--
IDENTICAL
The images are equal.
14 changes: 11 additions & 3 deletions ext/gd/tests/bug72913.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,16 @@ imagesavealpha($dst, true);

imagecopy($dst, $src, 0,0, 0,0, 50,50);

include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/bug72913.png', $dst);
var_dump(imagecolorsforindex($dst, imagecolorat($dst, 0, 0)));
?>
--EXPECT--
The images are equal.
array(4) {
["red"]=>
int(255)
["green"]=>
int(255)
["blue"]=>
int(255)
["alpha"]=>
int(127)
}
Binary file removed ext/gd/tests/bug72913.png
Binary file not shown.
47 changes: 22 additions & 25 deletions ext/gd/tests/func.inc
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ function test_image_equals_file(string $filename, GdImage $actual): void
save_actual_image($actual);
return;
}
$actual = test_to_truecolor($actual);
$expected = imagecreatefrompng($filename);
$expected = test_to_truecolor($expected);
test_image_equals_image($expected, $actual, true);
}

Expand All @@ -95,6 +93,7 @@ function test_image_equals_file(string $filename, GdImage $actual): void
*/
function test_image_equals_image(GdImage $expected, GdImage $actual, bool $save_actual = false): void
{
assert(imageistruecolor($expected) && imageistruecolor($actual));
$exp_x = imagesx($expected);
$exp_y = imagesy($expected);
$act_x = imagesx($actual);
Expand Down Expand Up @@ -126,35 +125,13 @@ function test_image_equals_image(GdImage $expected, GdImage $actual, bool $save_
}
}

/**
* Returns the truecolor version of an image.
*
* @param resource $image
* @return resource
*/
function test_to_truecolor($image)
{
if (imageistruecolor($image)) {
return $image;
} else {
$width = imagesx($image);
$height = imagesy($image);
$result = imagecreatetruecolor($width, $height);
imagecopy($result, $image, 0,0, 0,0, $width, $height);
return $result;
}
}

/**
* Saves an actual image to disk.
*
* The image is saved right beside the temporary .php test file with the
* extension .out.png.
*
* @param resource $image
* @return void
*/
function save_actual_image($image)
function save_actual_image(GdImage $image): void
{
$pathinfo = pathinfo($_SERVER['SCRIPT_FILENAME']);
$filename = "{$pathinfo['dirname']}/{$pathinfo['filename']}.out.png";
Expand All @@ -178,3 +155,23 @@ function trycatch_dump(...$tests) {
}
}

/**
* Calculate the mean squared error (in range 0..255**2)
*/
function mse(GdImage $image1, GdImage $image2): float
{
assert(imageistruecolor($image1) && imageistruecolor($image2));
assert(imagesx($image1) === imagesx($image2) && imagesy($image1) === imagesy($image2));
$e = $count = 0;
for ($j = 0, $m = imagesy($image1); $j < $m; $j++) {
for ($i = 0, $n = imagesx($image1); $i < $n; $i++) {
$c1 = imagecolorat($image1, $i, $j);
$c2 = imagecolorat($image2, $i, $j);
$e += ((($c1 >> 16) & 255) - (($c2 >> 16) & 255)) ** 2
+ ((($c1 >> 8) & 255) - (($c2 >> 8) & 255)) ** 2
+ ((($c1 >> 0) & 255) - (($c2 >> 0) & 255)) ** 2;
$count += 3;
}
}
return $e / $count;
}
2 changes: 2 additions & 0 deletions ext/gd/tests/gd276.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ $filename = __DIR__ . "/gd276.bmp";
imagebmp($orig, $filename, true);
$saved = imagecreatefrombmp($filename);
var_dump($saved !== false);
imagepalettetotruecolor($orig);
imagepalettetotruecolor($saved);
test_image_equals_image($orig, $saved);
?>
--EXPECT--
Expand Down
2 changes: 1 addition & 1 deletion ext/gd/tests/imagebmp_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ $im = imagecreate(100, 100);
imagecolorallocate($im, 0, 0, 0);
$white = imagecolorallocate($im, 255, 255, 255);
imageline($im, 10,10, 89,89, $white);

imagepalettetotruecolor($im);
require __DIR__ . "/func.inc";
test_image_equals_file(__DIR__ . "/imagebmp_basic.png", $im);
?>
Expand Down
Binary file modified ext/gd/tests/imagebmp_basic.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions ext/gd/tests/imagecolorset_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ $bg = imagecolorat($im, 0, 0);
// Set the background to be blue
imagecolorset($im, $bg, 0, 0, 255);

imagepalettetotruecolor($im);
include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/imagecolorset_basic.png', $im);
imagedestroy($im);
Expand Down
Binary file modified ext/gd/tests/imagecolorset_basic.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion ext/gd/tests/imagecreatefromstring_bmp.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ $bmp = "\x42\x4D\x3E\x00\x00\x00\x00\x00\x00\x00\x3E\x00\x00\x00\x28\x00"
. "\x01\x00\x05\x00\x00\x00\x02\x00\x01\x01\x01\x00\x06\x00\x00\x00"
. "\x0A\x00\x00\x00\x0A\x00\x00\x00\x00\x01";
$im = imagecreatefromstring($bmp);

imagepalettetotruecolor($im);
include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/imagecreatefromstring_bmp.png', $im);
?>
Expand Down
Binary file modified ext/gd/tests/imagecreatefromstring_bmp.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion ext/gd/tests/imagegammacorrect_variation1.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ $half2 = imagefilledarc ( $image, 75, 75, 70, 70, 0, -180, $gray, IMG_ARC_PIE )

$gamma = imagegammacorrect($image, 1, 5);
var_dump((bool) $gamma);

imagepalettetotruecolor($image);
include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/imagegammacorrect_variation1.png', $image);
?>
Expand Down
Binary file modified ext/gd/tests/imagegammacorrect_variation1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions ext/gd/tests/imagegammacorrect_variation2.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ function test_gamma($in, $out, $constructor)

imagegammacorrect($im, $in, $out);

if ($constructor === "imagecreate") {
imagepalettetotruecolor($im);
}
$filename = __DIR__ . DIRECTORY_SEPARATOR
. "imagegammacorrect_variation2_{$in}_{$out}.png";
$kind = $constructor === 'imagecreate' ? 'palette' : 'truecolor';
Expand Down
2 changes: 1 addition & 1 deletion ext/gd/tests/imagetruecolortopalette_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ $half = imagefilledarc ( $image, 75, 75, 70, 70, 0, 180, $a, IMG_ARC_PIE );
$half2 = imagefilledarc ( $image, 75, 55, 80, 70, 0, -180, $b, IMG_ARC_PIE );

var_dump(imagetruecolortopalette($image, true, 2));

imagepalettetotruecolor($image);
include_once __DIR__ . '/func.inc';
test_image_equals_file(__DIR__ . '/imagetruecolortopalette_basic.png', $image);
?>
Expand Down
Binary file modified ext/gd/tests/imagetruecolortopalette_basic.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
64 changes: 0 additions & 64 deletions ext/gd/tests/similarity.inc

This file was deleted.

3 changes: 3 additions & 0 deletions ext/gd/tests/test_image_equals_file_palette.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ $red = imagecolorallocate($im, 255, 0, 0);
imagefilledrectangle($im, 3,3, 7,7, $red);

$filename = __DIR__ . DIRECTORY_SEPARATOR . 'test_image_equals_file_palette.png';
imagepalettetotruecolor($im);
imagepng($im, $filename);

$im = imagecreate(10, 10);
imagecolorallocate($im, 255, 255, 255);
$blue = imagecolorallocate($im, 0, 0, 255);
imagefilledrectangle($im, 3,3, 7,7, $blue);

imagepalettetotruecolor($im);
test_image_equals_file($filename, $im);

$im = imagecreate(10, 10);
Expand All @@ -33,6 +35,7 @@ imagecolorallocate($im, 0, 0, 0);
$red = imagecolorallocate($im, 255, 0, 0);
imagefilledrectangle($im, 3,3, 7,7, $red);

imagepalettetotruecolor($im);
test_image_equals_file($filename, $im);
?>
--EXPECT--
Expand Down
6 changes: 3 additions & 3 deletions ext/gd/tests/webp_basic.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ if (!function_exists('imagewebp') || !function_exists('imagecreatefromwebp'))
?>
--FILE--
<?php
require_once __DIR__ . '/similarity.inc';
require_once __DIR__ . '/func.inc';

$filename = __DIR__ . '/webp_basic.webp';

Expand All @@ -30,12 +30,12 @@ imagewebp($im1, $filename);
$im2 = imagecreatefromwebp($filename);
imagewebp($im2, $filename);
echo 'Is lossy conversion close enough? ';
var_dump(calc_image_dissimilarity($im1, $im2) < 10e5);
var_dump(mse($im1, $im2) < 500);

imagewebp($im1, $filename, IMG_WEBP_LOSSLESS);
$im_lossless = imagecreatefromwebp($filename);
echo 'Does lossless conversion work? ';
var_dump(calc_image_dissimilarity($im1, $im_lossless) == 0);
var_dump(mse($im1, $im_lossless) == 0);

try {
imagewebp($im1, $filename, -10);
Expand Down

0 comments on commit 5890761

Please sign in to comment.