reorganize convert.c to convert.im
authorTony Cook <tony@develop=help.com>
Sat, 23 Oct 2010 22:31:37 +0000 (22:31 +0000)
committerTony Cook <tony@develop=help.com>
Sat, 23 Oct 2010 22:31:37 +0000 (22:31 +0000)
test conversion in a 16-bit image a bit more precisely

Changes
Imager.xs
MANIFEST
convert.c [deleted file]
convert.im [new file with mode: 0644]
imager.h
lib/Imager/Test.pm
t/t67convert.t

diff --git a/Changes b/Changes
index 98d4520..8ded547 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,15 @@
 Imager release history.  Older releases can be found in Changes.old
 
+Imager 0.79 - unreleased
+===========
+
+ - add Imager::Test to the POD coverage tests and document the missing
+   functions.
+
+Bug fixes:
+
+ - treat the co-efficients for convert() as doubles instead of floats.
+
 Imager 0.78 - 4 Oct 2010
 ===========
 
index 970d7df..3a9ee1b 100644 (file)
--- a/Imager.xs
+++ b/Imager.xs
@@ -1874,7 +1874,7 @@ i_convert(src, avmain)
     Imager::ImgRaw     src
     AV *avmain
        PREINIT:
-         float *coeff;
+         double *coeff;
          int outchan;
          int inchan;
           SV **temp;
@@ -1894,7 +1894,7 @@ i_convert(src, avmain)
                inchan = len;
            }
           }
-          coeff = mymalloc(sizeof(float) * outchan * inchan);
+          coeff = mymalloc(sizeof(double) * outchan * inchan);
          for (j = 0; j < outchan; ++j) {
            avsub = (AV*)SvRV(*av_fetch(avmain, j, 0));
            len = av_len(avsub)+1;
index 2b37b4d..ce6501d 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -172,7 +172,7 @@ bmp.c           Reading and writing Windows BMP files
 color.c         Color translation and handling
 compose.im
 conv.im
-convert.c
+convert.im
 doco.perl
 datatypes.c
 draw.c
diff --git a/convert.c b/convert.c
deleted file mode 100644 (file)
index db8ac1d..0000000
--- a/convert.c
+++ /dev/null
@@ -1,197 +0,0 @@
-/*
-=head1 NAME
-
-  convert.c - image conversions
-
-=head1 SYNOPSIS
-
-  i_convert(outimage, srcimage, coeff, outchans, inchans)
-
-=head1 DESCRIPTION
-
-Converts images from one format to another, typically in this case for
-converting from RGBA to greyscale and back.
-
-=over
-
-=cut
-*/
-
-#include "imager.h"
-
-
-/*
-=item i_convert(src, coeff, outchan, inchan)
-
-Converts the image src into another image.
-
-coeff contains the co-efficients of an outchan x inchan matrix, for
-each output pixel:
-
-              coeff[0], coeff[1] ...
-  im[x,y] = [ coeff[inchan], coeff[inchan+1]...        ] * [ src[x,y], 1]
-              ...              coeff[inchan*outchan-1]
-
-If im has the wrong number of channels or is the wrong size then
-i_convert() will re-create it.
-
-Now handles images with more than 8-bits/sample.
-
-=cut
-*/
-
-i_img *
-i_convert(i_img *src, const float *coeff, int outchan, int inchan) {
-  int x, y;
-  int i, j;
-  int ilimit;
-  double work[MAXCHANNELS];
-  i_img *im = NULL;
-
-  mm_log((1,"i_convert(src %p, coeff %p,outchan %d, inchan %d)\n",im,src, coeff,outchan, inchan));
-  i_clear_error();
-
-  ilimit = inchan;
-  if (ilimit > src->channels)
-    ilimit = src->channels;
-  if (outchan > MAXCHANNELS) {
-    i_push_error(0, "cannot have outchan > MAXCHANNELS");
-    return 0;
-  }
-
-  if (src->type == i_direct_type) {
-    im = i_sametype_chans(src, src->xsize, src->ysize, outchan);
-    if (src->bits == i_8_bits) {
-      i_color *vals;
-
-      /* we can always allocate a single scanline of i_color */
-      vals = mymalloc(sizeof(i_color) * src->xsize); /* checked 04Jul05 tonyc */
-      for (y = 0; y < src->ysize; ++y) {
-        i_glin(src, 0, src->xsize, y, vals);
-        for (x = 0; x < src->xsize; ++x) {
-          for (j = 0; j < outchan; ++j) {
-            work[j] = 0;
-            for (i = 0; i < ilimit; ++i) {
-              work[j] += coeff[i+inchan*j] * vals[x].channel[i];
-            }
-            if (i < inchan) {
-              work[j] += coeff[i+inchan*j] * 255.9;
-            }
-          }
-          for (j = 0; j < outchan; ++j) {
-            if (work[j] < 0)
-              vals[x].channel[j] = 0;
-            else if (work[j] >= 256)
-              vals[x].channel[j] = 255;
-            else
-              vals[x].channel[j] = work[j];
-          }
-        }
-        i_plin(im, 0, src->xsize, y, vals);
-      }
-      myfree(vals);
-    }
-    else {
-      i_fcolor *vals;
-
-      /* we can always allocate a single scanline of i_fcolor 
-         for a >8 image */
-      vals = mymalloc(sizeof(i_fcolor) * src->xsize); /* checked 4Jul05 tonyc */
-      for (y = 0; y < src->ysize; ++y) {
-        i_glinf(src, 0, src->xsize, y, vals);
-        for (x = 0; x < src->xsize; ++x) {
-          for (j = 0; j < outchan; ++j) {
-            work[j] = 0;
-            for (i = 0; i < ilimit; ++i) {
-              work[j] += coeff[i+inchan*j] * vals[x].channel[i];
-            }
-            if (i < inchan) {
-              work[j] += coeff[i+inchan*j];
-            }
-          }
-          for (j = 0; j < outchan; ++j) {
-            if (work[j] < 0)
-              vals[x].channel[j] = 0;
-            else if (work[j] >= 1)
-              vals[x].channel[j] = 1;
-            else
-              vals[x].channel[j] = work[j];
-          }
-        }
-        i_plinf(im, 0, src->xsize, y, vals);
-      }
-      myfree(vals);
-    }
-  }
-  else {
-    int count;
-    int outcount;
-    int index;
-    i_color *colors;
-    i_palidx *vals;
-
-    im = i_img_pal_new(src->xsize, src->ysize, outchan, 
-                      i_maxcolors(src));
-
-    /* just translate the color table */
-    count = i_colorcount(src);
-    outcount = i_colorcount(im);
-    /* color table allocated for image, so it must fit */
-    colors = mymalloc(count * sizeof(i_color)); /* check 04Jul05 tonyc */
-    i_getcolors(src, 0, colors, count);
-    for (index = 0; index < count; ++index) {
-      for (j = 0; j < outchan; ++j) {
-        work[j] = 0;
-        for (i = 0; i < ilimit; ++i) {
-          work[j] += coeff[i+inchan*j] * colors[index].channel[i];
-        }
-        if (i < inchan) {
-          work[j] += coeff[i+inchan*j] * 255.9;
-        }
-      }
-      for (j = 0; j < outchan; ++j) {
-        if (work[j] < 0)
-          colors[index].channel[j] = 0;
-        else if (work[j] >= 255)
-          colors[index].channel[j] = 255;
-        else
-          colors[index].channel[j] = work[j];
-      }
-    }
-    if (count < outcount) {
-      i_setcolors(im, 0, colors, count);
-    }
-    else {
-      i_setcolors(im, 0, colors, outcount);
-      i_addcolors(im, colors, count-outcount);
-    }
-    /* and copy the indicies */
-    /* i_palidx is always unsigned char and will never be bigger than short
-       and since a line of 4-byte i_colors can fit then a line of i_palidx
-       will fit */
-    vals = mymalloc(sizeof(i_palidx) * im->xsize); /* checked 4jul05 tonyc */
-    for (y = 0; y < im->ysize; ++y) {
-      i_gpal(src, 0, im->xsize, y, vals);
-      i_ppal(im, 0, im->xsize, y, vals);
-    }
-    myfree(vals);
-    myfree(colors);
-  }
-
-  return im;
-}
-
-/*
-=back
-
-=head1 SEE ALSO
-
-Imager(3)
-
-=head1 AUTHOR
-
-Tony Cook <tony@develop-help.com>
-
-=cut
-*/
diff --git a/convert.im b/convert.im
new file mode 100644 (file)
index 0000000..ada2f26
--- /dev/null
@@ -0,0 +1,166 @@
+/*
+=head1 NAME
+
+  convert.im - image conversions
+
+=head1 SYNOPSIS
+
+  i_convert(outimage, srcimage, coeff, outchans, inchans)
+
+=head1 DESCRIPTION
+
+Converts images from one format to another, typically in this case for
+converting from RGBA to greyscale and back.
+
+=over
+
+=cut
+*/
+
+#include "imager.h"
+
+
+/*
+=item i_convert(src, coeff, outchan, inchan)
+
+Converts the image src into another image.
+
+coeff contains the co-efficients of an outchan x inchan matrix, for
+each output pixel:
+
+              coeff[0], coeff[1] ...
+  im[x,y] = [ coeff[inchan], coeff[inchan+1]...        ] * [ src[x,y], 1]
+              ...              coeff[inchan*outchan-1]
+
+If im has the wrong number of channels or is the wrong size then
+i_convert() will re-create it.
+
+Now handles images with more than 8-bits/sample.
+
+=cut
+*/
+
+i_img *
+i_convert(i_img *src, const double *coeff, int outchan, int inchan) {
+  int x, y;
+  int i, j;
+  int ilimit;
+  double work[MAXCHANNELS];
+  i_img *im = NULL;
+
+  mm_log((1,"i_convert(src %p, coeff %p,outchan %d, inchan %d)\n",im,src, coeff,outchan, inchan));
+  i_clear_error();
+
+  ilimit = inchan;
+  if (ilimit > src->channels)
+    ilimit = src->channels;
+  if (outchan > MAXCHANNELS) {
+    i_push_error(0, "cannot have outchan > MAXCHANNELS");
+    return 0;
+  }
+
+  if (src->type == i_direct_type) {
+    im = i_sametype_chans(src, src->xsize, src->ysize, outchan);
+#code src->bits <= i_8_bits
+    IM_COLOR *vals;
+    
+    /* we can always allocate a single scanline of i_color */
+    vals = mymalloc(sizeof(IM_COLOR) * src->xsize); /* checked 04Jul05 tonyc */
+    for (y = 0; y < src->ysize; ++y) {
+      IM_GLIN(src, 0, src->xsize, y, vals);
+      for (x = 0; x < src->xsize; ++x) {
+       for (j = 0; j < outchan; ++j) {
+         work[j] = 0;
+         for (i = 0; i < ilimit; ++i) {
+           work[j] += coeff[i+inchan*j] * vals[x].channel[i];
+         }
+         if (i < inchan) {
+           work[j] += coeff[i+inchan*j] * IM_SAMPLE_MAX;
+         }
+       }
+       for (j = 0; j < outchan; ++j) {
+         if (work[j] < 0)
+           vals[x].channel[j] = 0;
+         else if (work[j] >= IM_SAMPLE_MAX)
+           vals[x].channel[j] = IM_SAMPLE_MAX;
+         else
+           vals[x].channel[j] = work[j];
+       }
+      }
+      IM_PLIN(im, 0, src->xsize, y, vals);
+    }
+    myfree(vals);
+#/code
+  }
+  else {
+    int count;
+    int outcount;
+    int index;
+    i_color *colors;
+    i_palidx *vals;
+
+    im = i_img_pal_new(src->xsize, src->ysize, outchan, 
+                      i_maxcolors(src));
+
+    /* just translate the color table */
+    count = i_colorcount(src);
+    outcount = i_colorcount(im);
+    /* color table allocated for image, so it must fit */
+    colors = mymalloc(count * sizeof(i_color)); /* check 04Jul05 tonyc */
+    i_getcolors(src, 0, colors, count);
+    for (index = 0; index < count; ++index) {
+      for (j = 0; j < outchan; ++j) {
+        work[j] = 0;
+        for (i = 0; i < ilimit; ++i) {
+          work[j] += coeff[i+inchan*j] * colors[index].channel[i];
+        }
+        if (i < inchan) {
+          work[j] += coeff[i+inchan*j] * 255.9;
+        }
+      }
+      for (j = 0; j < outchan; ++j) {
+        if (work[j] < 0)
+          colors[index].channel[j] = 0;
+        else if (work[j] >= 255)
+          colors[index].channel[j] = 255;
+        else
+          colors[index].channel[j] = work[j];
+      }
+    }
+    if (count < outcount) {
+      i_setcolors(im, 0, colors, count);
+    }
+    else {
+      i_setcolors(im, 0, colors, outcount);
+      i_addcolors(im, colors, count-outcount);
+    }
+    /* and copy the indicies */
+    /* i_palidx is always unsigned char and will never be bigger than short
+       and since a line of 4-byte i_colors can fit then a line of i_palidx
+       will fit */
+    vals = mymalloc(sizeof(i_palidx) * im->xsize); /* checked 4jul05 tonyc */
+    for (y = 0; y < im->ysize; ++y) {
+      i_gpal(src, 0, im->xsize, y, vals);
+      i_ppal(im, 0, im->xsize, y, vals);
+    }
+    myfree(vals);
+    myfree(colors);
+  }
+
+  return im;
+}
+
+/*
+=back
+
+=head1 SEE ALSO
+
+Imager(3)
+
+=head1 AUTHOR
+
+Tony Cook <tony@develop-help.com>
+
+=cut
+*/
index 89adcf4..085fea7 100644 (file)
--- a/imager.h
+++ b/imager.h
@@ -214,7 +214,7 @@ int i_conv        (i_img *im,const double *coeff,int len);
 void i_unsharp_mask(i_img *im, double stddev, double scale);
 
 /* colour manipulation */
-extern i_img *i_convert(i_img *src, const float *coeff, int outchan, int inchan);
+extern i_img *i_convert(i_img *src, const double *coeff, int outchan, int inchan);
 extern void i_map(i_img *im, unsigned char (*maps)[256], unsigned int mask);
 
 float i_img_diff   (i_img *im1,i_img *im2);
index 0b4e776..c9d7f50 100644 (file)
@@ -18,6 +18,7 @@ $VERSION = "1.000";
      is_color3
      is_color4
      is_color_close3
+     is_fcolor1
      is_fcolor3
      is_fcolor4
      color_cmp
@@ -68,7 +69,7 @@ sub is_color3($$$$$) {
 
   my ($cr, $cg, $cb) = $color->rgba;
   unless ($builder->ok($cr == $red && $cg == $green && $cb == $blue, $comment)) {
-    $builder->diag(<<END_DIAG);
+    print <<END_DIAG;
 Color mismatch:
   Red: $red vs $cr
 Green: $green vs $cg
@@ -185,6 +186,41 @@ END_DIAG
   return 1;
 }
 
+sub is_fcolor1($$$;$) {
+  my ($color, $grey, $comment_or_diff, $comment_or_undef) = @_;
+  my ($comment, $mindiff);
+  if (defined $comment_or_undef) {
+    ( $mindiff, $comment ) = ( $comment_or_diff, $comment_or_undef )
+  }
+  else {
+    ( $mindiff, $comment ) = ( 0.001, $comment_or_diff )
+  }
+
+  my $builder = Test::Builder->new;
+
+  unless (defined $color) {
+    $builder->ok(0, $comment);
+    $builder->diag("color is undef");
+    return;
+  }
+  unless ($color->can('rgba')) {
+    $builder->ok(0, $comment);
+    $builder->diag("color is not a color object");
+    return;
+  }
+
+  my ($cgrey) = $color->rgba;
+  unless ($builder->ok(abs($cgrey - $grey) <= $mindiff, $comment)) {
+    print <<END_DIAG;
+Color mismatch:
+  Gray: $cgrey vs $grey
+END_DIAG
+    return;
+  }
+
+  return 1;
+}
+
 sub is_fcolor3($$$$$;$) {
   my ($color, $red, $green, $blue, $comment_or_diff, $comment_or_undef) = @_;
   my ($comment, $mindiff);
@@ -665,6 +701,13 @@ Tests if $color matches the given ($red, $green, $blue)
 
 Tests if $color matches the given ($red, $green, $blue, $alpha)
 
+=item is_fcolor1($fcolor, $grey, $comment)
+
+=item is_fcolor1($fcolor, $grey, $epsilon, $comment)
+
+Tests if $fcolor's first channel is within $epsilon of ($grey).  For
+the first form $epsilon is taken as 0.001.
+
 =item is_fcolor3($fcolor, $red, $green, $blue, $comment)
 
 =item is_fcolor3($fcolor, $red, $green, $blue, $epsilon, $comment)
index dadc9dc..fe89401 100644 (file)
@@ -1,7 +1,8 @@
 #!perl -w
 use strict;
 use Imager qw(:all :handy);
-use Test::More tests=>21;
+use Test::More tests => 27;
+use Imager::Test qw(test_colorf_gpix is_fcolor1 is_fcolor3);
 
 Imager::init("log"=>'testout/t67convert.log');
 
@@ -49,23 +50,41 @@ SKIP:
 }
 
 # test against 16-bit/sample images
-SKIP:
 {
-  my $imbase16 = Imager::i_img_16_new(200, 200, 3);
-  
-  my $im16targ = i_convert($imbase16, [ [ 0, 0, 0, 1 ],
-                                       [ 0, 0, 0, 0 ],
-                                       [ 0, 0, 0, 0 ] ]);
-  skip("could not convert 16-bit image", 2)
-    unless ok($im16targ, "convert 16/bit sample image");
-  # image should still be 16-bit
-  is(Imager::i_img_bits($im16targ), 16, "Image still 16-bit/sample");
-  # make sure that it's roughly red
-  my $c = Imager::i_gpixf($im16targ, 0, 0);
-  my @ch = $c->rgba;
-  ok(abs($ch[0] - 1) <= 0.0001 && abs($ch[1]) <= 0.0001 && abs($ch[2]) <= 0.0001,
-     "image roughly red")
-    or print "# @ch\n";
+ SKIP:
+  {
+    my $imbase16 = Imager::i_img_16_new(200, 200, 3);
+
+    my $im16targ = i_convert($imbase16, [ [ 0, 0, 0, 1 ],
+                                         [ 0, 0, 0, 0 ],
+                                         [ 0, 0, 0, 0 ] ]);
+    ok($im16targ, "convert 16/bit sample image")
+      or skip("could not convert 16-bit image", 2);
+
+    # image should still be 16-bit
+    is(Imager::i_img_bits($im16targ), 16, "Image still 16-bit/sample");
+
+    # make sure that it's roughly red
+    test_colorf_gpix($im16targ, 0, 0, NCF(1, 0, 0), 0.001, "image roughly red");
+  }
+ SKIP:
+  {
+    my $imbase16 = Imager->new(xsize => 10, ysize => 10, bits => 16);
+    ok($imbase16->setpixel
+       (x => 5, y => 2, color => Imager::Color::Float->new(0.1, 0.2, 0.3)),
+       "set a sample pixel");
+    my $c1 = $imbase16->getpixel(x => 5, y => 2, type => "float");
+    is_fcolor3($c1, 0.1, 0.2, 0.3, "check it was set")
+      or print "#", join(",", $c1->rgba), "\n";
+    
+    my $targ16 = $imbase16->convert(matrix => [ [ 0.05, 0.15, 0.01, 0.5 ] ]);
+    ok($targ16, "convert another 16/bit sample image")
+      or skip("could not convert", 3);
+    is($targ16->getchannels, 1, "convert should be 1 channel");
+    is($targ16->bits, 16, "and 16-bits");
+    my $c = $targ16->getpixel(x => 5, y => 2, type => "float");
+    is_fcolor1($c, 0.538, 1/32768, "check grey value");
+  }
 }
 
 # test against palette based images
@@ -95,6 +114,7 @@ SKIP:
 }
 
 { # http://rt.cpan.org/NoAuth/Bug.html?id=9672
+  # methods that return a new image should warn in void context
   my $warning;
   local $SIG{__WARN__} = 
     sub { 
@@ -111,6 +131,7 @@ SKIP:
 }
 
 { # http://rt.cpan.org/NoAuth/Bug.html?id=28492
+  # convert() doesn't preserve image sample size
   my $im = Imager->new(xsize => 20, ysize => 20, channels => 3, 
                       bits => 'double');
   is($im->bits, 'double', 'check source bits');