Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126191
Message-ID: <631837de-c2a0-4bde-9400-5e8e5f2a0906@gmx.de>
Date: Tue, 31 Dec 2024 13:42:09 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] ext/gd new imagematch function (was: adding
 imagecompare)
Content-Language: de-DE
To: Giovanni Giacobbi <giovanni@giacobbi.net>,
 PHP Internals List <internals@lists.php.net>
References: <CAML1Pf5mz=10FLKBTduPnw9hbS=0MFSg49cBTDH+jHNCxJomgA@mail.gmail.com>
In-Reply-To: <CAML1Pf5mz=10FLKBTduPnw9hbS=0MFSg49cBTDH+jHNCxJomgA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:urRPys/2iFo=;x3vBUjpx6AOyTZ4lXTIu4lY8+Y+
 8PxrDmdLfuqx4jleAhjCXXBhkAekBdZJzrB4nO6vvDRJQQYLI+pd1OGaUwK1EMoHdxETp483l
 /syWNBlnXyCYJtTkwRSmvQecqU4eKNxQ5bgBbTlM9HZ9EV9u0Qi/T5tyK11sy6RMbgdkmyllY
 hNpEjI9kCkcdQ1UeCB7L3uBSjQJlepi9ZvHhRSxw+NCXXVnkaL9H22zL5ZniOKOzbRQRi0bZl
 7dZccg2JWyaHD4GSCGfkvKB7He3TW83AhiLbmlhTb6m0Fu0f2Ca6YWAfOC26uXlovcJyPxqYY
 VDfrDfREDctW8gFnF+uykcA/bcwuemLyVjwoH77t3fnhawMQFAp6wc/4wx2N2qARsCAt4rfDJ
 H+GNo9IRwy9i0oQFTFR/ElvQ2x10ZK9PI8BDXL1HSmYkTrN2GSyCZq8BONTjv5Oolauvh9o3E
 LJpSu7qqgCL5gqzWbsupaMOb/cQz8EMjKNotEUjzA5EOPxwFDus7lb+Ov7TyUb/QpYTqs+kCi
 qG1YCnAtZOXsxDI7i6nZ+6YmNFVbnOfcQxj/T3owqUPvk/aO7D+UMGRF7V9e0E04S2eB8jPGr
 W4UjjG+j24T8T6c9JbJFwXaTZgPhzoR4e09ug2ocoQPFHwT2kPBgM3YwKX6xpgV1gYryBSVgB
 MGoKAKrYmy/OxRbdvz9UOBZcRFQLsowM/SnWcITC9lo+1o5N8asoMZKGYLfdULH9gUrPYcAxM
 lI+hmJeXbaARV3PaRn+3tpLME2LEWE7wruw+2N+qDkWQn0wUvCc0lnnV1uWutfI5JshC1DXBz
 EmrcsvJMmKxnLjvpua9QtSmvJ2WDCO0i/ZjcyqCvp/wuoDWeeO/zmMtVe5wPgBydxRBJaxZuu
 Tv/z/X+3J5KzQ61TNPXMlj5YbWZYU+K06yDaapbX8i77PVnnK+lirPTE0VWTr+sMVYKmJBeci
 ehX8R1ROjKYkr2Br5ecA25x/vKXlK3tjVzIW3Xrg/FeLU6nSmSH0xDbUBQnh8uK+x0J4ptTE2
 yLEwazk33Knhw4x0PX0pEO1FDZLVMhiU7EBVzYs5QvAU+oH7v2RVdkUwzGJCqFi2+eIIPTREC
 mZmGTOQ3jqmuom8+QttsFN44MHxafiCOOJVZocupFC/8UIOE/StvMtEZgKCXQzycp9RjK0QlU
 =
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 11.07.2024 at 11:35, Giovanni Giacobbi wrote:

> The recent PR #14877 [1] proposes to add the imagecompare gd function th=
at
> mimics the gdImageCompare function from libgd. I always thought that a
> pixel-by-pixel matching function for two images was a big missing featur=
e
> in PHP, as the corresponding userland implementation is really, REALLY s=
low.
>
> 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 i=
n
> 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 pix=
el
> 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 i=
n
> 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 implement=
ed
> in userland with existing functions: imageinterlace, imagecolortranspare=
nt,
> imageistruecolor, imagesx, imagesy, imagecolorstotal.
>
> I believe PHP should only offer required building blocks and leave to th=
e
> libraries the implementation of more structured functionality. My propos=
al
> 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 signatur=
e:
>
> 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 exten=
ded
> 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/5586d0c7de00acbfb217e1c6321da918b96e=
f960/ext/gd/libgd/gd.c#L2834
> [3] https://github.com/php/php-src/pull/14914

Difficult.  It seems to me we first need to consider the use-case.
Likely useful only for testing purposes, but I may miss some other
use-cases.

And now comes the really hard part: when do we consider images to be
equal?  You have mentioned that gdImageCompare() completely ignores the
alpha channel, and that would be a mistake.  I tend to agree, but if the
images are stored via imagejpeg(), for instance, the alpha channel would
be ignored as well, so the resulting images would be equal.

Then there is the question whether two fully transparent pixels (alpha
value 127) are different if their RGB channels have different values.
In theory, sure, but it practice is may not really matter.

Then we have the transparent pixel.  Should equality comparison take
that into account?  Again, in theory, it is not the same as a pixel with
alpha channel 127, but in practice it may very well be.

Now you claim that doing pixel-to-pixel comparisons in PHP would be
*really* slow.  I'd argue that with modern PHP the actual looping over
the pixels isn't that slow.  Even getting the pixel colors
(imagecolorat()) and checking them for equality shouldn't be that slow.
It only gets slow if you want to do more elaborate comparisons; e.g.
getting the actual values a palette entry refers to, but if you move
that out of the loop, performance might not be that bad.  Implementing
that in C is certainly faster, but the difference should be measured,
and I wonder how much could be gained using SIMD instructions (something
libgd still ignores completely).

Christoph