make getpixel/setpixel report failure consistently and document it

fix 16 and double/sample bounds checking
test bounds checks on each image type

diff --git a/Imager.pm b/Imager.pm
index 007c303a8bb2709f6c33c3b42e35b9c25ba193a3..1ad448d1f9c3b4feebe5dfbe399cd77d794b1259 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -897,6 +897,26 @@ sub to_rgb8 {
   return $result;
 }
 
+# convert a paletted (or any image) to an 8-bit/channel RGB images
+sub to_rgb16 {
+  my $self = shift;
+  my $result;
+
+  unless (defined wantarray) {
+    my @caller = caller;
+    warn "to_rgb16() called in void context - to_rgb8() returns the converted image at $caller[1] line $caller[2]\n";
+    return;
+  }
+
+  if ($self->{IMG}) {
+    $result = Imager->new;
+    $result->{IMG} = i_img_to_rgb16($self->{IMG})
+      or undef $result;
+  }
+
+  return $result;
+}
+
 sub addcolors {
   my $self = shift;
   my %opts = (colors=>[], @_);
@@ -2826,25 +2846,32 @@ sub setpixel {
   if (ref $x && ref $y) {
     unless (@$x == @$y) {
       $self->{ERRSTR} = 'length of x and y mismatch';
-      return undef;
+      return;
     }
+    my $set = 0;
     if ($color->isa('Imager::Color')) {
       for my $i (0..$#{$opts{'x'}}) {
-        i_ppix($self->{IMG}, $x->[$i], $y->[$i], $color);
+        i_ppix($self->{IMG}, $x->[$i], $y->[$i], $color)
+         or ++$set;
       }
     }
     else {
       for my $i (0..$#{$opts{'x'}}) {
-        i_ppixf($self->{IMG}, $x->[$i], $y->[$i], $color);
+        i_ppixf($self->{IMG}, $x->[$i], $y->[$i], $color)
+         or ++$set;
       }
     }
+    $set or return;
+    return $set;
   }
   else {
     if ($color->isa('Imager::Color')) {
-      i_ppix($self->{IMG}, $x, $y, $color);
+      i_ppix($self->{IMG}, $x, $y, $color)
+       and return;
     }
     else {
-      i_ppixf($self->{IMG}, $x, $y, $color);
+      i_ppixf($self->{IMG}, $x, $y, $color)
+       and return;
     }
   }
 

diff --git a/img16.c b/img16.c
index 4ef1b89381c4679f2e2bbd76f7f5d0ea7837dbf8..e486c012f53d891c006e0a851545acc8f364bbbc 100644 (file)
--- a/img16.c
+++ b/img16.c
@@ -253,7 +253,7 @@ i_img_to_rgb16(i_img *im) {
 static int i_ppix_d16(i_img *im, int x, int y, const i_color *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -273,7 +273,7 @@ static int i_ppix_d16(i_img *im, int x, int y, const i_color *val) {
 static int i_gpix_d16(i_img *im, int x, int y, i_color *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -286,7 +286,7 @@ static int i_gpix_d16(i_img *im, int x, int y, i_color *val) {
 static int i_ppixf_d16(i_img *im, int x, int y, const i_fcolor *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -306,7 +306,7 @@ static int i_ppixf_d16(i_img *im, int x, int y, const i_fcolor *val) {
 static int i_gpixf_d16(i_img *im, int x, int y, i_fcolor *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;

diff --git a/imgdouble.c b/imgdouble.c
index 903912ba1c76054244837574dc2ebbdd369cdbe6..8cc7b391bb2597583ecc6ce05ebe8a05d8d62a54 100644 (file)
--- a/imgdouble.c
+++ b/imgdouble.c
@@ -146,7 +146,7 @@ i_img *i_img_double_new(int x, int y, int ch) {
 static int i_ppix_ddoub(i_img *im, int x, int y, const i_color *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -166,7 +166,7 @@ static int i_ppix_ddoub(i_img *im, int x, int y, const i_color *val) {
 static int i_gpix_ddoub(i_img *im, int x, int y, i_color *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -179,7 +179,7 @@ static int i_gpix_ddoub(i_img *im, int x, int y, i_color *val) {
 static int i_ppixf_ddoub(i_img *im, int x, int y, const i_fcolor *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;
@@ -199,7 +199,7 @@ static int i_ppixf_ddoub(i_img *im, int x, int y, const i_fcolor *val) {
 static int i_gpixf_ddoub(i_img *im, int x, int y, i_fcolor *val) {
   int off, ch;
 
-  if (x < 0 || x >= im->xsize || y < 0 || y > im->ysize) 
+  if (x < 0 || x >= im->xsize || y < 0 || y >= im->ysize) 
     return -1;
 
   off = (x + y * im->xsize) * im->channels;

diff --git a/lib/Imager/Test.pm b/lib/Imager/Test.pm
index 181c8b3bc0ab65bb53096048df91755bcdea314f..287ac157c1d18d7bd16ec0d4750e610781df4a10 100644 (file)
--- a/lib/Imager/Test.pm
+++ b/lib/Imager/Test.pm
@@ -4,7 +4,7 @@ use Test::Builder;
 require Exporter;
 use vars qw(@ISA @EXPORT_OK);
 @ISA = qw(Exporter);
-@EXPORT_OK = qw(diff_text_with_nul test_image_raw test_image_16 test_image is_color3 is_color1 is_image is_image_similar);
+@EXPORT_OK = qw(diff_text_with_nul test_image_raw test_image_16 test_image is_color3 is_color1 is_image is_image_similar image_bounds_checks);
 
 sub diff_text_with_nul {
   my ($desc, $text1, $text2, @params) = @_;
@@ -185,6 +185,31 @@ sub is_image($$$) {
   return is_image_similar($left, $right, 0, $comment);
 }
 
+sub image_bounds_checks {
+  my $im = shift;
+
+  my $builder = Test::Builder->new;
+
+  $builder->ok(!$im->getpixel(x => -1, y => 0), 'bounds check get (-1, 0)');
+  $builder->ok(!$im->getpixel(x => 10, y => 0), 'bounds check get (10, 0)');
+  $builder->ok(!$im->getpixel(x => 0, y => -1), 'bounds check get (0, -1)');
+  $builder->ok(!$im->getpixel(x => 0, y => 10), 'bounds check get (0, 10)');
+  $builder->ok(!$im->getpixel(x => -1, y => 0), 'bounds check get (-1, 0) float');
+  $builder->ok(!$im->getpixel(x => 10, y => 0), 'bounds check get (10, 0) float');
+  $builder->ok(!$im->getpixel(x => 0, y => -1), 'bounds check get (0, -1) float');
+  $builder->ok(!$im->getpixel(x => 0, y => 10), 'bounds check get (0, 10) float');
+  my $black = Imager::Color->new(0, 0, 0);
+  require Imager::Color::Float;
+  my $blackf = Imager::Color::Float->new(0, 0, 0);
+  $builder->ok(!$im->setpixel(x => -1, y => 0, color => $black), 'bounds check set (-1, 0)');
+  $builder->ok(!$im->setpixel(x => 10, y => 0, color => $black), 'bounds check set (10, 0)');
+  $builder->ok(!$im->setpixel(x => 0, y => -1, color => $black), 'bounds check set (0, -1)');
+  $builder->ok(!$im->setpixel(x => 0, y => 10, color => $black), 'bounds check set (0, 10)');
+  $builder->ok(!$im->setpixel(x => -1, y => 0, color => $blackf), 'bounds check set (-1, 0) float');
+  $builder->ok(!$im->setpixel(x => 10, y => 0, color => $blackf), 'bounds check set (10, 0) float');
+  $builder->ok(!$im->setpixel(x => 0, y => -1, color => $blackf), 'bounds check set (0, -1) float');
+  $builder->ok(!$im->setpixel(x => 0, y => 10, color => $blackf), 'bounds check set (0, 10) float');
+}
 
 1;
 

diff --git a/t/t01introvert.t b/t/t01introvert.t
index a643fe5100619488afa500c9c5ba174145d6632b..0763f1e372eb7848a3653efaefa7d9ba2ab2fa53 100644 (file)
--- a/t/t01introvert.t
+++ b/t/t01introvert.t
@@ -3,12 +3,14 @@
 # to make sure we get expected values
 
 use strict;
-use Test::More tests=>196;
+use Test::More tests => 212;
 
 BEGIN { use_ok(Imager => qw(:handy :all)) }
 
 require "t/testtools.pl";
 
+use Imager::Test qw(image_bounds_checks);
+
 init_log("testout/t01introvert.log",1);
 
 my $im_g = Imager::ImgRaw::new(100, 101, 1);
@@ -482,6 +484,12 @@ cmp_ok(Imager->errstr, '=~', qr/channels must be between 1 and 4/,
   mask_tests($im, 0.005);
 }
 
+{ # check bounds checking
+  my $im = Imager->new(xsize => 10, ysize => 10);
+
+  image_bounds_checks($im);
+}
+
 sub check_add {
   my ($im, $color, $expected) = @_;
   my $index = Imager::i_addcolors($im, $color);

diff --git a/t/t021sixteen.t b/t/t021sixteen.t
index c0686383436c2baee540a0e31a92f578e8ce7dd3..c04f31f2fbf4c5577ac68af8941a56226556a14b 100644 (file)
--- a/t/t021sixteen.t
+++ b/t/t021sixteen.t
@@ -1,6 +1,6 @@
 #!perl -w
 use strict;
-use Test::More tests => 87;
+use Test::More tests => 103;
 
 BEGIN { use_ok(Imager=>qw(:all :handy)) }
 
@@ -9,7 +9,7 @@ init_log("testout/t021sixteen.log", 1);
 require "t/testtools.pl";
 
 use Imager::Color::Float;
-use Imager::Test qw(test_image is_image);
+use Imager::Test qw(test_image is_image image_bounds_checks);
 
 my $im_g = Imager::i_img_16_new(100, 101, 1);
 
@@ -160,3 +160,8 @@ cmp_ok(Imager->errstr, '=~', qr/channels must be between 1 and 4/,
   is($im16->bits, 16, "check bits");
   is_image($im, $im16, "check image data matches");
 }
+
+{ # bounds checks
+  my $im = Imager->new(xsize => 10, ysize => 10, bits => 16);
+  image_bounds_checks($im);
+}

diff --git a/t/t022double.t b/t/t022double.t
index f0fefdf0d09c1e21e8bcb2484c381946ce341ea0..403aef665ec1bb6ca3bf55105d5c9e8e58699471 100644 (file)
--- a/t/t022double.t
+++ b/t/t022double.t
@@ -1,12 +1,14 @@
 #!perl -w
 use strict;
-use Test::More tests => 81;
+use Test::More tests => 97;
 
 BEGIN { use_ok(Imager => qw(:all :handy)) }
 require "t/testtools.pl";
 
 init_log("testout/t022double.log", 1);
 
+use Imager::Test qw(image_bounds_checks);
+
 use Imager::Color::Float;
 
 my $im_g = Imager::i_img_double_new(100, 101, 1);
@@ -142,4 +144,7 @@ cmp_ok(Imager->errstr, '=~', qr/channels must be between 1 and 4/,
   mask_tests($im);
 }
 
-
+{ # bounds checking
+  my $im = Imager->new(xsize => 10, ysize=>10, bits=>'double');
+  image_bounds_checks($im);
+}

diff --git a/t/t023palette.t b/t/t023palette.t
index b002de94d13f001f99a4250d6d33ba8e19d619e1..ec464fb15c7e5b6744785ad2d87c663407849342 100644 (file)
--- a/t/t023palette.t
+++ b/t/t023palette.t
@@ -1,9 +1,11 @@
 #!perl -w
 # some of this is tested in t01introvert.t too
 use strict;
-use Test::More tests => 90;
+use Test::More tests => 107;
 BEGIN { use_ok("Imager"); }
 
+use Imager::Test qw(image_bounds_checks);
+
 sub isbin($$$);
 
 my $img = Imager->new(xsize=>50, ysize=>50, type=>'paletted');
@@ -287,6 +289,13 @@ cmp_ok(Imager->errstr, '=~', qr/Channels must be positive and <= 4/,
   is($pixels[2], 0, "check black pixel");
 }
 
+{ # check bounds checking
+  my $im = Imager->new(xsize => 10, ysize => 10, type=>'paletted');
+  ok($im->addcolors(colors => [ $black ]), "add color of pixel bounds check writes");
+
+  image_bounds_checks($im);
+}
+
 sub iscolor {
   my ($c1, $c2, $msg) = @_;