Re: How do I get the offset result from "-trim"?

Adam Richter <[email protected]> Wed, 9 Sep 2020 00:28:35 -0700
Newsgroups gmane.comp.video.graphicsmagick.help
Message-ID <CAGn-TgjPS-8HAjY1DuHxp5zW+xiEZXcfcG4EpWzsx+WsjF2pzA@mail.gmail.com>
On Tue, Sep 8, 2020 at 7:03 AM Bob Friesenhahn
<[email protected]> wrote:

>Interesting.  I have not updated this function [FuzzyColorMatch] significantly since
>2008 when an optimization was added.  The function is used in many
>(22!) places.

I have attached a patch, but the only test I have run with that patch
so far is just trying
to trim one file and comparing results with ImageMagick.  With the
attached patch,
the resulting geometry is the same.  I am guessing that the other call
sites should be
OK, but I have not taken much of a look at them yet.

>Looking closer, it seems that there is ample opportunity for possible
>speed-up as well.

You also apparently opened bug #345 in 2016 indicating that -trim could produce
negative width results, and it is currently marked as open.  Looking
at the code, I
was wondering if there were cases where that sort of result was
intended.  If not,
if the region should never have negative width or negative height,
then that might open
the door to optimizations that would not need to preserve that presumed bug.

> Unless the user knows what they are doing (unlikely), the results of
> the distance vector are not likely to be very useful because they are
> performed in an inadequate color-space.  In many cases the user can
> not "fix" FuzzyColorMatch() without breaking something else because it
> is called deep in the bowels of other algorithms.

I don't know about theories of color spaces, but I will pass along
what I noticed
from looking at ImageMagick's IsFuzzyEquivalcnePixel function (its version of
FuzzyColorMatch), in case that is helpful.  That function has some "if" branches
for doing other color matching criteria and a comment that mentions
"arc distance
for hue."  My original guess was that the branch with that comment was being
taken, as I thought that the problems matching the colors of the platens on
my scanner (to find the edge of a piece of paper) were getting more
mismatches because perhaps IM's method was more sensitive to differences
in color and less in brightness, and that would be less sensitive to changes
as my scanner got warmer while doing long batches.  After all, if radial
distances (brightness) remained the same, but differences in hue and
saturation or UV in YUV had to follow some sort of curve, that would
gove differences in brightness a little less weight, which seemed like
a plausible explanation.  However, some debug messages revealed that
ImageMagick was not doing those fancier comparisons in my case.

[...]
> A simple emailed text patch would be great.  If there is a SourceForge
> problem report, then a text patch attached to the problem report will
> work as well.

Searching the bug tracker on sourceforge for "trim" only turned up three
closed bug reports and the one that you opened.  I did not open a bug report
for this issue.  I have attached a proposed patch, and I hereby
release any copyright
interest I may have in it into the public domain.  Besides tacking on a "* 3.0",
it also renames the "fuzz_squared" local variable to "bound", because it no
longer is fuzz * fuzz.  Those are the only changes.

Anyhow, thanks in advance for considering this patch for possible integration.

Adam

_______________________________________________
Graphicsmagick-help mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/graphicsmagick-help
FuzzyColorMatch.diff (text/x-patch, 863 B)
diff -r c202f5d505cb magick/color.c
--- a/magick/color.c	Fri Aug 21 11:39:34 2020 -0500
+++ b/magick/color.c	Tue Sep 08 23:30:43 2020 -0700
@@ -375,22 +375,22 @@
   double
     difference,
     distance,
-    fuzz_squared;
+    bound;
 
   if (fuzz <= MagickEpsilon)
     return (ColorMatch(q,p));
-  fuzz_squared=fuzz*fuzz;
+  bound = fuzz * fuzz * 3.0;
   difference=p->red-(double) q->red;
   distance=difference*difference;
-  if (distance > (fuzz_squared))
+  if (distance > bound)
     return(MagickFalse);
   difference=p->green-(double) q->green;
   distance+=difference*difference;
-  if (distance > (fuzz_squared))
+  if (distance > bound)
     return(MagickFalse);
   difference=p->blue-(double) q->blue;
   distance+=difference*difference;
-  if (distance > (fuzz_squared))
+  if (distance > bound)
     return(MagickFalse);
   return(MagickTrue);
 }