Hi,
I plan to add imagecompare to the gd extension
https://github.com/php/php-src/pull/14877. 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
https://www.php.net/manual/en/function.imagetypes.php.
Let me know your thoughts.
Cheers.
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
Hi Saki.
Hi David,
Hi,
I plan to add imagecompare to the gd extension. No problem so far
however a little discussion of the returned valuehttps://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?
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
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:
- Users who simply want to know if there is a match (i.e. a bool return value is sufficient)
- 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
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:
- 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.
- 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
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
Hi
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 theGdnamespace - 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..
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?
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.
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