From 618a3282ea8fbe37bda2659ff2d093a6e7880c8a Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Fri, 18 Feb 2011 18:09:19 +1100 Subject: [PATCH] test coverage and fix pass for compose() went from ~10% coverage to 100% coverage including both 8-bit and double/sample code paths. --- Changes | 10 ++ Imager.pm | 16 ++-- MANIFEST | 1 + compose.im | 46 +++++++-- image.c | 2 +- t/t62compose.t | 254 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 316 insertions(+), 13 deletions(-) create mode 100644 t/t62compose.t diff --git a/Changes b/Changes index 03fcc4f4..5d8a44cf 100644 --- 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 =========== diff --git a/Imager.pm b/Imager.pm index b2575c13..ed2b098f 100644 --- 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; diff --git a/MANIFEST b/MANIFEST index c26ccd79..c159baa4 100644 --- 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 diff --git a/compose.im b/compose.im index 5e0c236a..d7e0e81e 100644 --- a/compose.im +++ b/compose.im @@ -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 85a305e4..229bccbd 100644 --- a/image.c +++ b/image.c @@ -1165,7 +1165,7 @@ i_img_diffd(i_img *im1,i_img *im2) { yb=(im1->ysizeysize)?im1->ysize:im2->ysize; chb=(im1->channelschannels)?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 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; +} -- 2.39.5