From 6a9807e8b103cb490e973b6612f91a1f2fb873f4 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Mon, 30 Jan 2012 20:42:50 +1100 Subject: [PATCH] re-work XS handling of channel lists --- Imager.pm | 12 +++---- Imager.xs | 88 ++++++++++++++---------------------------------- t/t01introvert.t | 14 ++++---- t/t021sixteen.t | 10 +++--- typemap | 28 +++++++++++++++ 5 files changed, 71 insertions(+), 81 deletions(-) diff --git a/Imager.pm b/Imager.pm index 8e232ba6..14a224dc 100644 --- a/Imager.pm +++ b/Imager.pm @@ -3328,14 +3328,14 @@ sub getsamples { my $offset = $opts{offset}; if ($opts{type} eq '8bit') { my @samples = i_gsamp($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, - $opts{y}, @{$opts{channels}}) + $opts{y}, $opts{channels}) or return; @{$target}[$offset .. $offset + @samples - 1] = @samples; return scalar(@samples); } elsif ($opts{type} eq 'float') { my @samples = i_gsampf($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, - $opts{y}, @{$opts{channels}}); + $opts{y}, $opts{channels}); @{$target}[$offset .. $offset + @samples - 1] = @samples; return scalar(@samples); } @@ -3345,7 +3345,7 @@ sub getsamples { my @data; my $count = i_gsamp_bits($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, $opts{y}, $bits, $target, - $offset, @{$opts{channels}}); + $offset, $opts{channels}); unless (defined $count) { $self->_set_error(Imager->_error_as_msg); return; @@ -3361,18 +3361,18 @@ sub getsamples { else { if ($opts{type} eq '8bit') { return i_gsamp($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, - $opts{y}, @{$opts{channels}}); + $opts{y}, $opts{channels}); } elsif ($opts{type} eq 'float') { return i_gsampf($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, - $opts{y}, @{$opts{channels}}); + $opts{y}, $opts{channels}); } elsif ($opts{type} =~ /^(\d+)bit$/) { my $bits = $1; my @data; i_gsamp_bits($self->{IMG}, $opts{x}, $opts{x}+$opts{width}, - $opts{y}, $bits, \@data, 0, @{$opts{channels}}) + $opts{y}, $bits, \@data, 0, $opts{channels}) or return; return @data; } diff --git a/Imager.xs b/Imager.xs index 4274c38e..edc5e4b1 100644 --- a/Imager.xs +++ b/Imager.xs @@ -29,6 +29,12 @@ extern "C" { #include "imperl.h" +/* used to represent channel lists parameters */ +typedef struct i_channel_list_tag { + int *channels; + int count; +} i_channel_list; + /* Allocate memory that will be discarded when mortals are discarded. @@ -3213,27 +3219,19 @@ i_img_virtual(im) Imager::ImgRaw im void -i_gsamp(im, l, r, y, ...) +i_gsamp(im, l, r, y, channels) Imager::ImgRaw im i_img_dim l i_img_dim r i_img_dim y + i_channel_list channels PREINIT: - int *chans; - int chan_count; i_sample_t *data; i_img_dim count, i; PPCODE: - if (items < 5) - croak("No channel numbers supplied to g_samp()"); if (l < r) { - chan_count = items - 4; - chans = mymalloc(sizeof(int) * chan_count); - for (i = 0; i < chan_count; ++i) - chans[i] = SvIV(ST(i+4)); - data = mymalloc(sizeof(i_sample_t) * (r-l) * chan_count); /* XXX: memleak? */ - count = i_gsamp(im, l, r, y, data, chans, chan_count); - myfree(chans); + data = mymalloc(sizeof(i_sample_t) * (r-l) * channels.count); /* XXX: memleak? */ + count = i_gsamp(im, l, r, y, data, channels.channels, channels.count); if (GIMME_V == G_ARRAY) { EXTEND(SP, count); for (i = 0; i < count; ++i) @@ -3253,7 +3251,7 @@ i_gsamp(im, l, r, y, ...) } undef_neg_int -i_gsamp_bits(im, l, r, y, bits, target, offset, ...) +i_gsamp_bits(im, l, r, y, bits, target, offset, channels) Imager::ImgRaw im i_img_dim l i_img_dim r @@ -3261,9 +3259,8 @@ i_gsamp_bits(im, l, r, y, bits, target, offset, ...) int bits AV *target STRLEN offset + i_channel_list channels PREINIT: - int *chans; - int chan_count; unsigned *data; i_img_dim count, i; CODE: @@ -3271,13 +3268,8 @@ i_gsamp_bits(im, l, r, y, bits, target, offset, ...) if (items < 8) croak("No channel numbers supplied to g_samp()"); if (l < r) { - chan_count = items - 7; - chans = mymalloc(sizeof(int) * chan_count); - for (i = 0; i < chan_count; ++i) - chans[i] = SvIV(ST(i+7)); - data = mymalloc(sizeof(unsigned) * (r-l) * chan_count); - count = i_gsamp_bits(im, l, r, y, data, chans, chan_count, bits); - myfree(chans); + data = mymalloc(sizeof(unsigned) * (r-l) * channels.count); + count = i_gsamp_bits(im, l, r, y, data, channels.channels, channels.count, bits); for (i = 0; i < count; ++i) { av_store(target, i+offset, newSVuv(data[i])); } @@ -3291,42 +3283,22 @@ i_gsamp_bits(im, l, r, y, bits, target, offset, ...) RETVAL undef_neg_int -i_psamp_bits(im, l, y, bits, channels_sv, data_av, data_offset = 0, pixel_count = -1) +i_psamp_bits(im, l, y, bits, channels, data_av, data_offset = 0, pixel_count = -1) Imager::ImgRaw im i_img_dim l i_img_dim y int bits - SV *channels_sv + i_channel_list channels AV *data_av int data_offset int pixel_count PREINIT: - int chan_count; - int *channels; STRLEN data_count; size_t data_used; unsigned *data; ptrdiff_t i; CODE: i_clear_error(); - if (SvOK(channels_sv)) { - AV *channels_av; - if (!SvROK(channels_sv) || SvTYPE(SvRV(channels_sv)) != SVt_PVAV) { - croak("channels is not an array ref"); - } - channels_av = (AV *)SvRV(channels_sv); - chan_count = av_len(channels_av) + 1; - if (chan_count < 1) { - croak("i_psamp_bits: no channels provided"); - } - channels = mymalloc(sizeof(int) * chan_count); - for (i = 0; i < chan_count; ++i) - channels[i] = SvIV(*av_fetch(channels_av, i, 0)); - } - else { - chan_count = im->channels; - channels = NULL; - } data_count = av_len(data_av) + 1; if (data_offset < 0) { @@ -3336,22 +3308,20 @@ i_psamp_bits(im, l, y, bits, channels_sv, data_av, data_offset = 0, pixel_count croak("data_offset greater than number of samples supplied"); } if (pixel_count == -1 || - data_offset + pixel_count * chan_count > data_count) { - pixel_count = (data_count - data_offset) / chan_count; + data_offset + pixel_count * channels.count > data_count) { + pixel_count = (data_count - data_offset) / channels.count; } - data_used = pixel_count * chan_count; + data_used = pixel_count * channels.count; data = mymalloc(sizeof(unsigned) * data_count); for (i = 0; i < data_used; ++i) data[i] = SvUV(*av_fetch(data_av, data_offset + i, 0)); - RETVAL = i_psamp_bits(im, l, l + pixel_count, y, data, channels, - chan_count, bits); + RETVAL = i_psamp_bits(im, l, l + pixel_count, y, data, channels.channels, + channels.count, bits); if (data) myfree(data); - if (channels) - myfree(channels); OUTPUT: RETVAL @@ -3430,27 +3400,19 @@ i_ppixf(im, x, y, cl) Imager::Color::Float cl void -i_gsampf(im, l, r, y, ...) +i_gsampf(im, l, r, y, channels) Imager::ImgRaw im i_img_dim l i_img_dim r i_img_dim y + i_channel_list channels PREINIT: - int *chans; - int chan_count; i_fsample_t *data; i_img_dim count, i; PPCODE: - if (items < 5) - croak("No channel numbers supplied to g_sampf()"); if (l < r) { - chan_count = items - 4; - chans = mymalloc(sizeof(int) * chan_count); - for (i = 0; i < chan_count; ++i) - chans[i] = SvIV(ST(i+4)); - data = mymalloc(sizeof(i_fsample_t) * (r-l) * chan_count); - count = i_gsampf(im, l, r, y, data, chans, chan_count); - myfree(chans); + data = mymalloc(sizeof(i_fsample_t) * (r-l) * channels.count); + count = i_gsampf(im, l, r, y, data, channels.channels, channels.count); if (GIMME_V == G_ARRAY) { EXTEND(SP, count); for (i = 0; i < count; ++i) diff --git a/t/t01introvert.t b/t/t01introvert.t index 69b43a2a..2b29909a 100644 --- a/t/t01introvert.t +++ b/t/t01introvert.t @@ -73,11 +73,11 @@ ok(!grep($_ != $red_idx, @pals[0..49]), "check for red"); ok(!grep($_ != $blue_idx, @pals[50..99]), "check for blue"); is(Imager::i_gpal($im_pal, 0, 100, 0), "\0" x 50 . "\2" x 50, "gpal in scalar context"); -my @samp = Imager::i_gsamp($im_pal, 0, 100, 0, 0, 1, 2); +my @samp = Imager::i_gsamp($im_pal, 0, 100, 0, [ 0, 1, 2 ]); is(@samp, 300, "gsamp count in list context"); my @samp_exp = ((255, 0, 0) x 50, (0, 0, 255) x 50); is_deeply(\@samp, \@samp_exp, "gsamp list deep compare"); -my $samp = Imager::i_gsamp($im_pal, 0, 100, 0, 0, 1, 2); +my $samp = Imager::i_gsamp($im_pal, 0, 100, 0, [ 0, 1, 2 ]); is(length($samp), 300, "gsamp scalar length"); is($samp, "\xFF\0\0" x 50 . "\0\0\xFF" x 50, "gsamp scalar bytes"); @@ -125,7 +125,7 @@ is(Imager::i_img_type($im_pal), 0, "pal img shouldn't be paletted now"); is_color3($colors[1], 0, 255, 0, "still green"); is_color3($colors[2], 0, 0, 255, "still blue"); is_color3($colors[3], 0, 0, 0, "still black"); - is(Imager::i_gsamp($im_pal2, 0, 100, 0, 0, 1, 2), + is(Imager::i_gsamp($im_pal2, 0, 100, 0, [ 0, 1, 2 ]), "\0\xFF\0\0\0\0"."\xFF\0\0" x 48 . "\0\0\xFF" x 50, "colors are still correct"); } @@ -403,16 +403,16 @@ cmp_ok(Imager->errstr, '=~', qr/channels must be between 1 and 4/, [ map $_->rgba, Imager::i_glinf($im, 2, 9, 4) ], "check colors were written"); - is_deeply([ Imager::i_gsamp($im, 0, 10, 0, (0, 3)) ], + is_deeply([ Imager::i_gsamp($im, 0, 10, 0, [ 0, 3 ]) ], [ (0, 0) x 5, (255, 255), (0, 0) x 4 ], "i_gsamp list context"); is("0000" x 5 . "FFFF" . "0000" x 4, - uc unpack("H*", Imager::i_gsamp($im, 0, 10, 0, (0, 3))), + uc unpack("H*", Imager::i_gsamp($im, 0, 10, 0, [ 0, 3 ])), "i_gsamp scalar context"); - is_deeply([ Imager::i_gsampf($im, 2, 9, 4, 0, 2, 3) ], + is_deeply([ Imager::i_gsampf($im, 2, 9, 4, [ 0, 2, 3 ]) ], [ (1.0, 0, 1.0) x 3, (0, 1.0, 1.0) x 2, (0, 0, 0), (1.0, 1.0, 1.0) ], "i_gsampf - list context"); - is_deeply([ unpack("d*", Imager::i_gsampf($im, 2, 9, 4, 0, 2, 3)) ], + is_deeply([ unpack("d*", Imager::i_gsampf($im, 2, 9, 4, [ 0, 2, 3 ])) ], [ (1.0, 0, 1.0) x 3, (0, 1.0, 1.0) x 2, (0, 0, 0), (1.0, 1.0, 1.0) ], "i_gsampf - scalar context"); print "# end low-level scan-line function tests\n"; diff --git a/t/t021sixteen.t b/t/t021sixteen.t index 5315bc88..90e82e18 100644 --- a/t/t021sixteen.t +++ b/t/t021sixteen.t @@ -56,25 +56,25 @@ test_colorf_glin($im_rgb, 0, 1, "added some green in the middle"); { my @samples; - is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, 0 .. 2), 12, + is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, [ 0 .. 2 ]), 12, "i_gsamp_bits all channels - count") or print "# ", Imager->_error_as_msg(), "\n"; is_deeply(\@samples, [ 65535, 0, 0, 65535, 0, 0, 0, 65535, 0, 0, 65535, 0 ], "check samples retrieved"); @samples = (); - is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, 0, 2), 8, + is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, [ 0, 2 ]), 8, "i_gsamp_bits some channels - count") or print "# ", Imager->_error_as_msg(), "\n"; is_deeply(\@samples, [ 65535, 0, 65535, 0, 0, 0, 0, 0 ], "check samples retrieved"); # fail gsamp - is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, 0, 3), undef, + is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 16, \@samples, 0, [ 0, 3 ]), undef, "i_gsamp_bits fail bad channel"); is(Imager->_error_as_msg(), 'No channel 3 in this image', 'check message'); - is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 17, \@samples, 0, 0, 2), 8, + is(Imager::i_gsamp_bits($im_rgb, 18, 22, 1, 17, \@samples, 0, [ 0, 2 ]), 8, "i_gsamp_bits succeed high bits"); is($samples[0], 131071, "check correct with high bits"); @@ -90,7 +90,7 @@ test_colorf_glin($im_rgb, 0, 1, 12, "write 16-bit samples") or print "# ", Imager->_error_as_msg(), "\n"; @samples = (); - is(Imager::i_gsamp_bits($im_rgb, 18, 22, 2, 16, \@samples, 0, 0 .. 2), 12, + is(Imager::i_gsamp_bits($im_rgb, 18, 22, 2, 16, \@samples, 0, [ 0 .. 2 ]), 12, "read them back") or print "# ", Imager->_error_as_msg(), "\n"; is_deeply(\@samples, \@wr_samples, "check they match"); diff --git a/typemap b/typemap index 91e20960..20980004 100644 --- a/typemap +++ b/typemap @@ -30,6 +30,8 @@ off_t T_OFF_T # STRLEN isn't in the default typemap in older perls STRLEN T_UV +i_channel_list T_IM_CHANNEL_LIST + ############################################################################# INPUT T_PTR_NULL @@ -92,6 +94,32 @@ T_PTROBJ_INV T_OFF_T $var = i_sv_off_t(aTHX_ $arg); +T_IM_CHANNEL_LIST + SvGETMAGIC($arg); + if (SvOK($arg)) { + AV *channels_av; + int i; + if (!SvROK($arg) || SvTYPE(SvRV($arg)) != SVt_PVAV) { + croak(\"$var is not an array ref\"); + } + channels_av = (AV *)SvRV($arg); + $var.count = av_len(channels_av) + 1; + if ($var.count < 1) { + croak(\"$pname: no channels provided\"); + } + $var.channels = malloc_temp(aTHX_ sizeof(int) * $var.count); + for (i = 0; i < $var.count; ++i) { + SV **entry = av_fetch(channels_av, i, 0); + $var.channels[i] = entry ? SvIV(*entry) : 0; + } + } + else { + /* assumes we have an image */ + $var.count = im->channels; + $var.channels = NULL; + } + + ############################################################################# OUTPUT T_IV_U -- 2.39.5