test coverage and fix pass for compose()
authorTony Cook <tony@develop-help.com>
Fri, 18 Feb 2011 07:09:19 +0000 (18:09 +1100)
committerTony Cook <tony@develop-help.com>
Mon, 16 May 2011 14:02:36 +0000 (00:02 +1000)
went from ~10% coverage to 100% coverage including both 8-bit and
double/sample code paths.

Changes
Imager.pm
MANIFEST
compose.im
image.c
t/t62compose.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index 03fcc4f..5d8a44c 100644 (file)
--- a/Changes
+++ b/Changes
@@ -9,6 +9,16 @@ Bug fixes:
    Win32.  Thanks to kmx for the report and fix.
    https://rt.cpan.org/Ticket/Display.html?id=67963
 
+ - compose() with the target, source or mask position off the top or
+   left of the target image didn't clip the source image correctly.
+   https://rt.cpan.org/Ticket/Display.html?id=67220
+
+ - compose() now returns a useful error message on a non-positive
+   opacity.
+
+ - compose.im now at 100% test coverage. (As opposed to, umm, much,
+   much less.)
+
 Imager 0.82 - 14 Mar 2011
 ===========
 
index b2575c1..ed2b098 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -2501,16 +2501,20 @@ sub compose {
     defined $mask_top or $mask_top = $opts{mask_miny};
     defined $mask_top or $mask_top = 0;
 
-    i_compose_mask($self->{IMG}, $src->{IMG}, $opts{mask}{IMG}, 
+    unless (i_compose_mask($self->{IMG}, $src->{IMG}, $opts{mask}{IMG}, 
                   $left, $top, $src_left, $src_top,
                   $mask_left, $mask_top, $width, $height, 
-                  $combine, $opts{opacity})
-      or return;
+                          $combine, $opts{opacity})) {
+      $self->_set_error(Imager->_error_as_msg);
+      return;
+    }
   }
   else {
-    i_compose($self->{IMG}, $src->{IMG}, $left, $top, $src_left, $src_top,
-             $width, $height, $combine, $opts{opacity})
-      or return;
+    unless (i_compose($self->{IMG}, $src->{IMG}, $left, $top, $src_left, $src_top,
+                     $width, $height, $combine, $opts{opacity})) {
+      $self->_set_error(Imager->_error_as_msg);
+      return;
+    }
   }
 
   return $self;
index c26ccd7..c159baa 100644 (file)
--- a/MANIFEST
+++ b/MANIFEST
@@ -308,6 +308,7 @@ t/t57infix.t
 t/t58trans2.t
 t/t59assem.t
 t/t61filters.t
+t/t62compose.t
 t/t63combine.t                 Test combine() method
 t/t64copyflip.t                        Test copy, flip, rotate, matrix_transform
 t/t65crop.t
index 5e0c236..d7e0e81 100644 (file)
@@ -16,6 +16,11 @@ i_compose_mask(i_img *out, i_img *src, i_img *mask,
   i_fill_combinef_f combinef_double;
   int channel_zero = 0;
 
+  mm_log((1, "i_compose_mask(out %p, src %p, mask %p, out(%d, %d), src(%d, %d),"
+         " mask(%d,%d), size(%d,%d), combine %d opacity %f\n", out, src, 
+         mask, out_left, out_top, src_left, src_top, mask_left, mask_top, width,
+         height, combine, opacity));
+
   i_clear_error();
   if (out_left >= out->xsize
       || out_top >= out->ysize
@@ -35,6 +40,8 @@ i_compose_mask(i_img *out, i_img *src, i_img *mask,
 
   if (out_left < 0) {
     width = out_left + width;
+    src_left -= out_left;
+    mask_left -= out_left;
     out_left = 0;
   }
   if (out_left + width > out->xsize)
@@ -42,6 +49,8 @@ i_compose_mask(i_img *out, i_img *src, i_img *mask,
 
   if (out_top < 0) {
     height = out_top + height;
+    mask_top -= out_top;
+    src_top -= out_top;
     out_top = 0;
   }
   if (out_top + height > out->ysize)
@@ -49,6 +58,8 @@ i_compose_mask(i_img *out, i_img *src, i_img *mask,
 
   if (src_left < 0) {
     width = src_left + width;
+    out_left -= src_left;
+    mask_left -= src_left;
     src_left = 0;
   }
   if (src_left + width > src->xsize)
@@ -56,29 +67,42 @@ i_compose_mask(i_img *out, i_img *src, i_img *mask,
 
   if (src_top < 0) {
     height = src_top + height;
+    out_top -= src_top;
+    mask_top -= src_top;
     src_top = 0;
   }
   if (src_top + height > src->ysize)
-    height = src->ysize - src_left;
+    height = src->ysize - src_top;
 
   if (mask_left < 0) {
     width = mask_left + width;
+    out_left -= mask_left;
+    src_left -= mask_left;
     mask_left = 0;
   }
   if (mask_left + width > mask->xsize)
     width = mask->xsize - mask_left;
   
   if (mask_top < 0) {
-    height = mask->ysize + height;
+    height = mask_top + height;
+    src_top -= mask_top;
+    out_top -= mask_top;
     mask_top = 0;
   }
   if (mask_top + height > mask->ysize)
-    height = mask->xsize - mask_top;
+    height = mask->ysize - mask_top;
 
   if (opacity > 1.0)
     opacity = 1.0;
-  else if (opacity <= 0)
+  else if (opacity <= 0) {
+    i_push_error(0, "opacity must be positive");
     return 0;
+  }
+
+  mm_log((1, "after adjustments: (out(%d, %d), src(%d, %d),"
+         " mask(%d,%d), size(%d,%d)\n", 
+         out_left, out_top, src_left, src_top, mask_left, mask_top, width,
+         height));
 
   i_get_combine(combine, &combinef_8, &combinef_double);
 
@@ -128,6 +152,10 @@ i_compose(i_img *out, i_img *src,
   i_fill_combine_f combinef_8;
   i_fill_combinef_f combinef_double;
 
+  mm_log((1, "i_compose(out %p, src %p, out(%d, %d), src(%d, %d), size(%d,%d),"
+         " combine %d opacity %f\n", out, src, out_left, out_top,
+         src_left, src_top, width, height, combine, opacity));
+
   i_clear_error();
   if (out_left >= out->xsize
       || out_top >= out->ysize
@@ -143,6 +171,7 @@ i_compose(i_img *out, i_img *src,
 
   if (out_left < 0) {
     width = out_left + width;
+    src_left -= out_left;
     out_left = 0;
   }
   if (out_left + width > out->xsize)
@@ -150,6 +179,7 @@ i_compose(i_img *out, i_img *src,
 
   if (out_top < 0) {
     height = out_top + height;
+    src_top -= out_top;
     out_top = 0;
   }
   if (out_top + height > out->ysize)
@@ -157,6 +187,7 @@ i_compose(i_img *out, i_img *src,
 
   if (src_left < 0) {
     width = src_left + width;
+    out_left -= src_left;
     src_left = 0;
   }
   if (src_left + width > src->xsize)
@@ -164,15 +195,18 @@ i_compose(i_img *out, i_img *src,
 
   if (src_top < 0) {
     height = src_top + height;
+    out_top -= src_top;
     src_top = 0;
   }
   if (src_top + height > src->ysize)
-    height = src->ysize - src_left;
+    height = src->ysize - src_top;
 
   if (opacity > 1.0)
     opacity = 1.0;
-  else if (opacity <= 0)
+  else if (opacity <= 0) {
+    i_push_error(0, "opacity must be positive");
     return 0;
+  }
 
   i_get_combine(combine, &combinef_8, &combinef_double);
 
diff --git a/image.c b/image.c
index 85a305e..229bccb 100644 (file)
--- a/image.c
+++ b/image.c
@@ -1165,7 +1165,7 @@ i_img_diffd(i_img *im1,i_img *im2) {
   yb=(im1->ysize<im2->ysize)?im1->ysize:im2->ysize;
   chb=(im1->channels<im2->channels)?im1->channels:im2->channels;
 
-  mm_log((1,"i_img_diff: xb=%d xy=%d chb=%d\n",xb,yb,chb));
+  mm_log((1,"i_img_diffd: xb=%d xy=%d chb=%d\n",xb,yb,chb));
 
   tdiff=0;
   for(y=0;y<yb;y++) for(x=0;x<xb;x++) {
diff --git a/t/t62compose.t b/t/t62compose.t
new file mode 100644 (file)
index 0000000..d7e415b
--- /dev/null
@@ -0,0 +1,254 @@
+#!perl -w
+use strict;
+use Imager qw(:handy);
+use Test::More tests => 114;
+use Imager::Test qw(is_image is_imaged);
+
+-d "testout" or mkdir "testout";
+
+Imager::init_log("testout/t62compose.log", 1);
+
+my @files;
+
+my %types =
+  (
+   double =>
+   {
+    blue => NCF(0, 0, 1),
+    red =>  NCF(1, 0, 0),
+    green2 => NCF(0, 1, 0, 0.5),
+    green2_on_blue => NCF(0, 0.5, 0.5),
+    red3_on_blue => NCF(1/3, 0, 2/3),
+    green6_on_blue => NCF(0, 1/6, 5/6),
+    red2_on_blue => NCF(0.5, 0, 0.5),
+    green4_on_blue => NCF(0, 0.25, 0.75),
+    gray100 => NCF(1.0, 0, 0),
+    gray50 => NCF(0.5, 0, 0),
+    is_image => \&is_imaged,
+   },
+   8 =>
+   {
+    blue => NC(0, 0, 255),
+    red =>  NC(255, 0, 0),
+    green2 => NC(0, 255, 0, 128),
+    green2_on_blue => NC(0, 128, 127),
+    red3_on_blue => NC(85, 0, 170),
+    green6_on_blue => NC(0, 42, 213),
+    red2_on_blue => NC(128, 0, 127),
+    green4_on_blue => NC(0, 64, 191),
+    gray100 => NC(255, 0, 0),
+    gray50 => NC(128, 0, 0),
+    is_image => \&is_image,
+   },
+  );
+
+for my $type_id (sort keys %types) {
+  my $type = $types{$type_id};
+  my $blue = $type->{blue};
+  my $red = $type->{red};
+  my $green2 = $type->{green2};
+  my $green2_on_blue = $type->{green2_on_blue};
+  my $red3_on_blue = $type->{red3_on_blue};
+  my $green6_on_blue = $type->{green6_on_blue};
+  my $red2_on_blue = $type->{red2_on_blue};
+  my $green4_on_blue = $type->{green4_on_blue};
+  my $gray100 = $type->{gray100};
+  my $gray50 = $type->{gray50};
+  my $is_image = $type->{is_image};
+
+  print "# type $type_id\n";
+  my $targ = Imager->new(xsize => 100, ysize => 100, bits => $type_id);
+  $targ->box(color => $blue, filled => 1);
+  is($targ->type, "direct", "check target image type");
+  is($targ->bits, $type_id, "check target bits");
+
+  my $src = Imager->new(xsize => 40, ysize => 40, channels => 4, bits => $type_id);
+  $src->box(filled => 1, color => $red, xmax => 19, ymax => 19);
+  $src->box(filled => 1, xmin => 20, color => $green2);
+  save_to($src, "${type_id}_src");
+
+  my $mask_ones = Imager->new(channels => 1, xsize => 40, ysize => 40, bits => $type_id);
+  $mask_ones->box(filled => 1, color => NC(255, 255, 255));
+
+
+  # mask or full mask, should be the same
+  for my $mask_info ([ "nomask" ], [ "fullmask", mask => $mask_ones ]) {
+    my ($mask_type, @mask_extras) = @$mask_info;
+    print "# $mask_type\n";
+    {
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red,
+               xmin=> 5, ymin => 10, xmax => 24, ymax => 29);
+      $cmp->box(filled => 1, color => $green2_on_blue,
+               xmin => 25, ymin => 10, xmax => 44, ymax => 49);
+      {
+       my $work = $targ->copy;
+       ok($work->compose(src => $src, tx => 5, ty => 10, @mask_extras),
+          "$mask_type - simple compose");
+       $is_image->($work, $cmp, "check match");
+       save_to($work, "${type_id}_${mask_type}_simple");
+      }
+      { # >1 opacity
+       my $work = $targ->copy;
+       ok($work->compose(src => $src, tx => 5, ty => 10, opacity => 2.0, @mask_extras),
+          "$mask_type - compose with opacity > 1.0 acts like opacity=1.0");
+       $is_image->($work, $cmp, "check match");
+      }
+      { # 0 opacity is a failure
+       my $work = $targ->copy;
+       ok(!$work->compose(src => $src, tx => 5, ty => 10, opacity => 0.0, @mask_extras),
+          "$mask_type - compose with opacity = 0 is an error");
+       is($work->errstr, "opacity must be positive", "check message");
+      }
+    }
+    { # compose at 1/3
+      my $work = $targ->copy;
+      ok($work->compose(src => $src, tx => 7, ty => 33, opacity => 1/3, @mask_extras),
+        "$mask_type - simple compose at 1/3");
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red3_on_blue,
+               xmin => 7, ymin => 33, xmax => 26, ymax => 52);
+      $cmp->box(filled => 1, color => $green6_on_blue,
+               xmin => 27, ymin => 33, xmax => 46, ymax => 72);
+      $is_image->($work, $cmp, "check match");
+    }
+    { # targ off top left
+      my $work = $targ->copy;
+      ok($work->compose(src => $src, tx => -5, ty => -3, @mask_extras),
+        "$mask_type - compose off top left");
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red,
+               xmin=> 0, ymin => 0, xmax => 14, ymax => 16);
+      $cmp->box(filled => 1, color => $green2_on_blue,
+               xmin => 15, ymin => 0, xmax => 34, ymax => 36);
+      $is_image->($work, $cmp, "check match");
+    }
+    { # targ off bottom right
+      my $work = $targ->copy;
+      ok($work->compose(src => $src, tx => 65, ty => 67, @mask_extras),
+        "$mask_type - targ off bottom right");
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red,
+               xmin=> 65, ymin => 67, xmax => 84, ymax => 86);
+      $cmp->box(filled => 1, color => $green2_on_blue,
+               xmin => 85, ymin => 67, xmax => 99, ymax => 99);
+      $is_image->($work, $cmp, "check match");
+    }
+    { # src off top left
+      my $work = $targ->copy;
+      my @more_mask_extras;
+      if (@mask_extras) {
+       push @more_mask_extras,
+         (
+          mask_left => -5,
+          mask_top => -15,
+         );
+      }
+      ok($work->compose(src => $src, tx => 10, ty => 20,
+                       src_left => -5, src_top => -15,
+                       @mask_extras, @more_mask_extras),
+        "$mask_type - source off top left");
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red,
+               xmin=> 15, ymin => 35, xmax => 34, ymax => 54);
+      $cmp->box(filled => 1, color => $green2_on_blue,
+             xmin => 35, ymin => 35, xmax => 54, ymax => 74);
+      $is_image->($work, $cmp, "check match");
+    }
+    {
+      # src off bottom right
+      my $work = $targ->copy;
+      ok($work->compose(src => $src, tx => 10, ty => 20,
+                       src_left => 10, src_top => 15,
+                       width => 40, height => 40, @mask_extras),
+        "$mask_type - source off bottom right");
+      my $cmp = $targ->copy;
+      $cmp->box(filled => 1, color => $red,
+               xmin=> 10, ymin => 20, xmax => 19, ymax => 24);
+      $cmp->box(filled => 1, color => $green2_on_blue,
+               xmin => 20, ymin => 20, xmax => 39, ymax => 44);
+      $is_image->($work, $cmp, "check match");
+    }
+    {
+      # simply out of bounds
+      my $work = $targ->copy;
+      ok(!$work->compose(src => $src, tx => 100, @mask_extras),
+        "$mask_type - off the right of the target");
+      $is_image->($work, $targ, "no changes");
+      ok(!$work->compose(src => $src, ty => 100, @mask_extras),
+        "$mask_type - off the bottom of the target");
+      $is_image->($work, $targ, "no changes");
+      ok(!$work->compose(src => $src, tx => -40, @mask_extras),
+        "$mask_type - off the left of the target");
+      $is_image->($work, $targ, "no changes");
+      ok(!$work->compose(src => $src, ty => -40, @mask_extras),
+        "$mask_type - off the top of the target");
+      $is_image->($work, $targ, "no changes");
+    }
+  }
+
+  # masked tests
+  my $mask = Imager->new(xsize => 40, ysize => 40, channels => 1, bits => $type_id);
+  $mask->box(filled => 1, xmax => 19, color => $gray100);
+  $mask->box(filled => 1, xmin => 20, ymax => 14, xmax => 34,
+            color => $gray50);
+  is($mask->bits, $type_id, "check mask bits");
+  {
+    my $work = $targ->copy;
+    ok($work->compose(src => $src, tx => 5, ty => 7,
+                     mask => $mask),
+       "simple draw masked");
+    my $cmp = $targ->copy;
+    $cmp->box(filled => 1, color => $red,
+             xmin => 5, ymin => 7, xmax => 24, ymax => 26);
+    $cmp->box(filled => 1, color => $green4_on_blue,
+             xmin => 25, ymin => 7, xmax => 39, ymax => 21);
+    $is_image->($work, $cmp, "check match");
+    save_to($work, "${type_id}_simp_masked");
+    save_to($work, "${type_id}_simp_masked_cmp");
+  }
+  {
+    my $work = $targ->copy;
+    ok($work->compose(src => $src, tx => 5, ty => 7,
+                     mask_left => 5, mask_top => 2, 
+                     mask => $mask),
+       "draw with mask offset");
+    my $cmp = $targ->copy;
+    $cmp->box(filled => 1, color => $red,
+             xmin => 5, ymin => 7, xmax => 19, ymax => 26);
+    $cmp->box(filled => 1, color => $red2_on_blue,
+             xmin => 20, ymin => 7, xmax => 24, ymax => 19);
+    $cmp->box(filled => 1, color => $green4_on_blue,
+             xmin => 25, ymin => 7, xmax => 34, ymax => 19);
+    $is_image->($work, $cmp, "check match");
+  }
+  {
+    my $work = $targ->copy;
+    ok($work->compose(src => $src, tx => 5, ty => 7,
+                     mask_left => -3, mask_top => -2, 
+                     mask => $mask),
+       "draw with negative mask offsets");
+    my $cmp = $targ->copy;
+    $cmp->box(filled => 1, color => $red,
+             xmin => 8, ymin => 9, xmax => 24, ymax => 26);
+    $cmp->box(filled => 1, color => $green2_on_blue,
+             xmin => 25, ymin => 9, xmax => 27, ymax => 46);
+    $cmp->box(filled => 1, color => $green4_on_blue,
+             xmin => 28, ymin => 9, xmax => 42, ymax => 23);
+    $is_image->($work, $cmp, "check match");
+  }
+}
+
+unless ($ENV{IMAGER_KEEP_FILES}) {
+  unlink @files;
+}
+
+sub save_to {
+  my ($im, $name) = @_;
+
+  my $type = $ENV{IMAGER_SAVE_TYPE} || "ppm";
+  $name = "testout/t62_$name.$type";
+  $im->write(file => $name,
+            pnm_write_wide_data => 1);
+  push @files, $name;
+}