# ext/gd adding imagecompare - Thread: https://externals.io/message/124306 - Original archive: https://news-web.php.net/php.internals/124306 - Started by David CARLIER on 2024-07-09T16:27:35+00:00 - Messages: 10 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #124306 — David CARLIER — 2024-07-09T16:27:35+00:00 URL: https://externals.io/message/124306#124306 Hi, I plan to add imagecompare to the gd extension . No problem so far however a little discussion of the returned value https://github.com/php/php-src/pull/14877#issuecomment-2217686123 I personally see a value of a bitmask rather than just boolean which is not harder to handle than imagetypes . Let me know your thoughts. Cheers. --- ## #124307 — Saki Takamachi — 2024-07-09T16:49:03+00:00 URL: https://externals.io/message/124306#124307 · In reply to: #124306 Hi David, > Hi, > > I plan to add imagecompare to the gd extension. No problem so far however a little discussion of the returned value > > https://github.com/php/php-src/pull/14877#issuecomment-2217686123 > > I personally see a value of a bitmask rather than just boolean which is not harder to handle than imagetypes. > > Let me know your thoughts. > > Cheers. If the return value is bool, does this mean that it will be true if the comparison targets are exactly the same, and false if there is even one difference? In other words, will we no longer be able to see where the differences were? Regards, Saki --- ## #124309 — Kamil Tekiela — 2024-07-09T16:59:09+00:00 URL: https://externals.io/message/124306#124309 · In reply to: #124306 Boolean is a much more confusing value here. A bit mask is ok, but I would prefer a better solution. Maybe a simple value object? --- ## #124310 — David CARLIER — 2024-07-09T17:09:26+00:00 URL: https://externals.io/message/124306#124310 · In reply to: #124307 Hi Saki. > Hi David, > > > […] > however a little discussion of the returned value > > […] > not harder to handle than imagetypes. > > […] > > If the return value is bool, does this mean that it will be true if the > comparison targets are exactly the same, and false if there is even one > difference? In other words, will we no longer be able to see where the > differences were? > > It seems to be the opposite regarding his proposition gdImageCompare(im1, im2) & GD_CMP_IMAGE. Then yes you are right, we are losing the specifics. If you want to figure out how the image 1 and image 2 differ, you need to use the rest of the api e.g. imagecolortransparent and all that jazz ... A bit too complex I feel. > Regards, > > Saki --- ## #124311 — David CARLIER — 2024-07-09T17:10:01+00:00 URL: https://externals.io/message/124306#124311 · In reply to: #124309 Hi, > Boolean is a much more confusing value here. A bit mask is ok, but I would > prefer a better solution. Maybe a simple value object? > That could be an option too sure. --- ## #124313 — Saki Takamachi — 2024-07-09T17:31:30+00:00 URL: https://externals.io/message/124306#124313 · In reply to: #124310 Hi David, > It seems to be the opposite regarding his proposition gdImageCompare(im1, im2) & GD_CMP_IMAGE. Then yes you are right, we are losing the specifics. If you want to figure out how the image 1 and image 2 differ, you need to use the rest of the api e.g. imagecolortransparent and all that jazz ... A bit too complex I feel. Thanks for the explanation. What I think is important to note here is that there are two types of users who will use this feature: 1. Users who simply want to know if there is a match (i.e. a bool return value is sufficient) 2. Users who want to know the details of the differences The object return type that Kamil mentioned was something I was considering as well, but I was concerned that it would be a bit verbose for type 1 users and would necessarily incur the overhead of object creation. (This is probably an unnecessary concern, as image analysis takes much longer.) Regards, Saki --- ## #124317 — David CARLIER — 2024-07-09T17:53:38+00:00 URL: https://externals.io/message/124306#124317 · In reply to: #124313 > Hi David, > > It seems to be the opposite regarding his proposition gdImageCompare(im1, > im2) & GD_CMP_IMAGE. Then yes you are right, we are losing the specifics. > If you want to figure out how the image 1 and image 2 differ, you need to > use the rest of the api e.g. imagecolortransparent and all that jazz ... A > bit too complex I feel. > > > Thanks for the explanation. > > What I think is important to note here is that there are two types of > users who will use this feature: > > 1. Users who simply want to know if there is a match (i.e. a bool return > value is sufficient) > True. Anyhow, such users can always do imagecompare($im1, $im2) & IMG_CMP_IMAGE themselves eventually. > 2. Users who want to know the details of the differences > > The object return type that Kamil mentioned was something I was > considering as well, but I was concerned that it would be a bit verbose for > type 1 users and would necessarily incur the overhead of object creation. > (This is probably an unnecessary concern, as image analysis takes much > longer.) > Agreed, I ll likely just commit as is sometime this week. Cheers. > > Regards, > > Saki --- ## #124318 — tim@bastelstu.be — 2024-07-09T18:05:58+00:00 URL: https://externals.io/message/124306#124318 · In reply to: #124309 Hi Yes, please no more global constants with names that are shortened beyond all recognition. > Boolean is a much more confusing value here. A bit mask is ok, but I would > prefer a better solution. Maybe a simple value object? > If it's "simple" then it would also be unwieldy to use. To me the obvious modern alternative would be a list of enum cases that represent the differences, with an empty array meaning no differences. The function should probably be renamed 'imagediff()' or similar then. namespace Gd; enum ImageDifference { case Height; case Width; case AlphaChannel; // or: case Transparency case Interlacing; // ... } Best regards Tim Düsterhus --- ## #124319 — tim@bastelstu.be — 2024-07-09T18:16:23+00:00 URL: https://externals.io/message/124306#124319 · In reply to: #124317 Hi > Agreed, I ll likely just commit as is sometime this week. I hereby formally claim that this is not a simple self-contained feature. It does require an RFC. The addition of global `IMAGE_CMP_*` constants effectively introduces an entirely new "namespace" within the existing list of constants and thus I do not believe that consistency with the existing GD functionality is a valid reason. As such I believe they should follow the https://wiki.php.net/rfc/namespaces_in_bundled_extensions RFC and be placed inside the `Gd` namespace - or ideally be replaced by a proper enum or so. Best regards Tim Düsterhus --- ## #124352 — David CARLIER — 2024-07-10T15:24:04+00:00 URL: https://externals.io/message/124306#124352 · In reply to: #124319 Hi > Hi > > > > […] > > I hereby formally claim that this is not a simple self-contained > feature. It does require an RFC. > > The addition of global `IMAGE_CMP_*` constants effectively introduces an > entirely new "namespace" within the existing list of constants and thus > I do not believe that consistency with the existing GD functionality is > a valid reason. As such I believe they should follow the > https://wiki.php.net/rfc/namespaces_in_bundled_extensions RFC and be > placed inside the `Gd` namespace - or ideally be replaced by a proper > enum or so. > > Best regards > Tim Düsterhus > Alright, I hear you. Anyhow, there is a debate for having a full internal version of a somewhat similar feature. We will see..