Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124384
Precedence: bulk
MIME-Version: 1.0
References: <CAML1Pf5mz=10FLKBTduPnw9hbS=0MFSg49cBTDH+jHNCxJomgA@mail.gmail.com>
In-Reply-To: <CAML1Pf5mz=10FLKBTduPnw9hbS=0MFSg49cBTDH+jHNCxJomgA@mail.gmail.com>
Date: Thu, 11 Jul 2024 16:47:58 +0700
Message-ID: <CAEZPtU5B8kcRi=u1-KpQ0rZMZZocX2U643R2gJn16eaVCkFQUQ@mail.gmail.com>
Subject: Re: [PHP-DEV] ext/gd new imagematch function (was: adding imagecompare)
To: giovanni@giacobbi.net
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="00000000000027753d061cf5a900"
From: pierre.php@gmail.com (Pierre Joye)

--00000000000027753d061cf5a900
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

hi,

as I mentioned in the PR, this is an old function which we did not touch
for bc reasons.

I also linked a better image comparison function used for the gd tests
suite. It covers exaxf match which is what it seems you are aiming to get.

Additionally that function also return an image with the difference (the
diff images contains the channels differences).

A key, more tricky, but nevertheless important compared is a perceptual
difference, but that can be done additionally.

best,
Pierre

On Thu, Jul 11, 2024, 4:39=E2=80=AFPM Giovanni Giacobbi <giovanni@giacobbi.=
net>
wrote:

> The recent PR #14877 [1] proposes to add the imagecompare gd function tha=
t
> 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 sl=
ow.
>
> 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 pixe=
l
> 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 implemente=
d
> in userland with existing functions: imageinterlace, imagecolortransparen=
t,
> 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 proposa=
l
> 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 =3D 0, int =
$y1
> =3D 0, int $x2 =3D 0, int $y2 =3D 0, ?int $width =3D null, ?int $height =
=3D 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 jus=
t
> 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/5586d0c7de00acbfb217e1c6321da918b96ef=
960/ext/gd/libgd/gd.c#L2834
> [3] https://github.com/php/php-src/pull/14914
>
>

--00000000000027753d061cf5a900
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<p dir=3D"ltr">hi,</p>
<p dir=3D"ltr">as I mentioned in the PR, this is an old function which we d=
id not touch for bc reasons.</p>
<p dir=3D"ltr">I also linked a better image comparison function used for th=
e gd tests suite. It covers exaxf match which is what it seems you are aimi=
ng to get.</p>
<p dir=3D"ltr">Additionally that function also return an image with the dif=
ference (the diff images contains the channels differences).</p>
<p dir=3D"ltr">A key, more tricky, but nevertheless important compared is a=
 perceptual difference, but that can be done additionally. <br></p>
<p dir=3D"ltr">best,<br>
Pierre</p>
<br><div class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Thu=
, Jul 11, 2024, 4:39=E2=80=AFPM Giovanni Giacobbi &lt;<a href=3D"mailto:gio=
vanni@giacobbi.net">giovanni@giacobbi.net</a>&gt; wrote:<br></div><blockquo=
te class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc so=
lid;padding-left:1ex"><div dir=3D"ltr">The recent PR #14877 [1] proposes to=
 add the imagecompare=C2=A0gd function that mimics the gdImageCompare funct=
ion from libgd. I always thought that a pixel-by-pixel matching function fo=
r two images was a big missing feature in PHP, as the corresponding userlan=
d implementation is really, REALLY slow.<div><br></div><div>The problem wit=
h gdImageCompare is that it checks several aspects of the two images, and f=
ails on its core purpose. The behavior is better seen in its source code [2=
] rather than explained. I see the following problems:</div><div>- The gdIm=
ageCompare 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 curr=
ent implementation reports them as identical.</div><div>- Checking for bitm=
asks 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 t=
o check for this: imagecompare($im1, $im2) &amp; IMG_CMP_IMAGE.</div><div>-=
 The checks in gdImageCompare are also a bit arbitrary, why compare the int=
erlace flag but not the palette order? Why check the number of colors in th=
e 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 userlan=
d with existing functions: imageinterlace, imagecolortransparent, imageistr=
uecolor, imagesx, imagesy, imagecolorstotal.</div><div><div><br></div><div>=
I believe PHP should only offer required building blocks and leave to the l=
ibraries the implementation of more structured functionality. My proposal i=
s 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:</div><div>=
<br></div><div>function imagematch(GdImage $image1, GdImage $image2, int $x=
1 =3D 0, int $y1 =3D 0, int $x2 =3D 0, int $y2 =3D 0, ?int $width =3D null,=
 ?int $height =3D null): bool {}</div><div><br></div><div>I already drafted=
 PR #14914 [3] with the first two parameters. There is not an equivalent li=
bgd implementation, so the implementation is entirely on the php side, but =
as you can see it&#39;s quite trivial. Even with the extended functionality=
 of matching portions of the images it would be just a few extra lines of c=
ode, so not a big maintenance burden.</div><div><br></div><div>If an RFC is=
 required, I&#39;m happy to redact it after the initial feedback round.</di=
v><div><br></div><div>[1] <a href=3D"https://github.com/php/php-src/pull/14=
877" target=3D"_blank" rel=3D"noreferrer">https://github.com/php/php-src/pu=
ll/14877</a><br></div></div><div>[2]=C2=A0<a href=3D"https://github.com/php=
/php-src/blob/5586d0c7de00acbfb217e1c6321da918b96ef960/ext/gd/libgd/gd.c#L2=
834" target=3D"_blank" rel=3D"noreferrer">https://github.com/php/php-src/bl=
ob/5586d0c7de00acbfb217e1c6321da918b96ef960/ext/gd/libgd/gd.c#L2834</a></di=
v><div>[3]=C2=A0<a href=3D"https://github.com/php/php-src/pull/14914" targe=
t=3D"_blank" rel=3D"noreferrer">https://github.com/php/php-src/pull/14914</=
a></div><div><br></div></div>
</blockquote></div>

--00000000000027753d061cf5a900--