Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124383
Precedence: bulk
MIME-Version: 1.0
Date: Thu, 11 Jul 2024 11:35:11 +0200
Message-ID: <CAML1Pf5mz=10FLKBTduPnw9hbS=0MFSg49cBTDH+jHNCxJomgA@mail.gmail.com>
Subject: [PHP-DEV] ext/gd new imagematch function (was: adding imagecompare)
To: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000606bed061cf57bef"
From: giovanni@giacobbi.net (Giovanni Giacobbi)

--000000000000606bed061cf57bef
Content-Type: text/plain; charset="UTF-8"

The recent PR #14877 [1] proposes to add the imagecompare gd function that
mimics the gdImageCompare function from libgd. I always thought that a
pixel-by-pixel matching function for two images was a big missing feature
in PHP, as the corresponding userland implementation is really, REALLY slow.

The problem with gdImageCompare is that it checks several aspects of the
two images, and fails on its core purpose. The behavior is better seen in
its source code [2] rather than explained. I see the following problems:
- The gdImageCompare is kind of buggy, as it completely disregards the
alpha channel. As a user, I would expect that two images that have a pixel
rgba(255, 0, 0, 100%) and one rgba(255, 0, 0, 50%) are considered
different, but the current implementation reports them as identical.
- Checking for bitmasks is cumbersome, and it requires 9 new constants in
the global namespace (the IMG_CMP_*), which are nearly useless. Most use
cases will just want to check for this: imagecompare($im1, $im2) &
IMG_CMP_IMAGE.
- The checks in gdImageCompare are also a bit arbitrary, why compare the
interlace flag but not the palette order? Why check the number of colors in
the palette if they are not even used in the image?
- If a user is interested in the other checks, they can all be implemented
in userland with existing functions: imageinterlace, imagecolortransparent,
imageistruecolor, imagesx, imagesy, imagecolorstotal.

I believe PHP should only offer required building blocks and leave to the
libraries the implementation of more structured functionality. My proposal
is a completely different function called imagematch() that solves the
exact problem of the pixel-by-pixel matching, with the added value of
optionally matching portions of the images, using the following signature:

function imagematch(GdImage $image1, GdImage $image2, int $x1 = 0, int $y1
= 0, int $x2 = 0, int $y2 = 0, ?int $width = null, ?int $height = null):
bool {}

I already drafted PR #14914 [3] with the first two parameters. There is not
an equivalent libgd implementation, so the implementation is entirely on
the php side, but as you can see it's quite trivial. Even with the extended
functionality of matching portions of the images it would be just a few
extra lines of code, so not a big maintenance burden.

If an RFC is required, I'm happy to redact it after the initial feedback
round.

[1] https://github.com/php/php-src/pull/14877
[2]
https://github.com/php/php-src/blob/5586d0c7de00acbfb217e1c6321da918b96ef960/ext/gd/libgd/gd.c#L2834
[3] https://github.com/php/php-src/pull/14914

--000000000000606bed061cf57bef
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">The recent PR #14877 [1] proposes to add the imagecompare=
=C2=A0gd function that mimics the gdImageCompare function from libgd. I alw=
ays thought that a pixel-by-pixel matching function for two images was a bi=
g missing feature in PHP, as the corresponding userland implementation is r=
eally, REALLY slow.<div><br></div><div>The problem with gdImageCompare is t=
hat it checks several aspects of the two images, and fails on its core purp=
ose. The behavior is better seen in its source code [2] rather than explain=
ed. I see the following problems:</div><div>- The gdImageCompare is kind of=
 buggy, as it completely disregards the alpha channel. As a user, I would e=
xpect that two images that have a pixel rgba(255, 0, 0, 100%) and one rgba(=
255, 0, 0, 50%) are considered different, but the current implementation re=
ports them as identical.</div><div>- Checking for bitmasks is cumbersome, a=
nd it requires 9 new constants in the global namespace (the IMG_CMP_*), whi=
ch are nearly useless. Most use cases will just want to check for this: ima=
gecompare($im1, $im2) &amp; IMG_CMP_IMAGE.</div><div>- The checks in gdImag=
eCompare are also a bit arbitrary, why compare the interlace flag but not t=
he palette order? Why check the number of colors in the palette if they are=
 not even used in the image?</div><div>- If a user is interested in the=C2=
=A0other checks, they can all be implemented in userland with existing func=
tions: imageinterlace, imagecolortransparent, imageistruecolor, imagesx, im=
agesy, imagecolorstotal.</div><div><div><br></div><div>I believe PHP should=
 only offer required building blocks and leave to the libraries the impleme=
ntation of more structured functionality. My proposal is a completely diffe=
rent function called imagematch() that solves the exact problem of the pixe=
l-by-pixel matching, with the added value of optionally matching portions o=
f the images, using the following signature:</div><div><br></div><div>funct=
ion imagematch(GdImage $image1, GdImage $image2, int $x1 =3D 0, int $y1 =3D=
 0, int $x2 =3D 0, int $y2 =3D 0, ?int $width =3D null, ?int $height =3D nu=
ll): bool {}</div><div><br></div><div>I already drafted PR #14914 [3] with =
the first two parameters. There is not an equivalent libgd implementation, =
so the implementation is entirely on the php side, but as you can see it=
9;s quite trivial. Even with the extended functionality of matching portion=
s of the images it would be just a few extra lines of code, so not a big ma=
intenance burden.</div><div><br></div><div>If an RFC is required, I&#39;m h=
appy to redact it after the initial feedback round.</div><div><br></div><di=
v>[1] <a href=3D"https://github.com/php/php-src/pull/14877">https://github.=
com/php/php-src/pull/14877</a><br></div></div><div>[2]=C2=A0<a href=3D"http=
s://github.com/php/php-src/blob/5586d0c7de00acbfb217e1c6321da918b96ef960/ex=
t/gd/libgd/gd.c#L2834">https://github.com/php/php-src/blob/5586d0c7de00acbf=
b217e1c6321da918b96ef960/ext/gd/libgd/gd.c#L2834</a></div><div>[3]=C2=A0<a =
href=3D"https://github.com/php/php-src/pull/14914">https://github.com/php/p=
hp-src/pull/14914</a></div><div><br></div></div>

--000000000000606bed061cf57bef--