diff --git a/Changes b/Changes index e2c26a3d..3879ee0a 100644 --- a/Changes +++ b/Changes @@ -775,6 +775,14 @@ Revision history for Perl extension Imager. built from CVS on Win32) - Makefile.PL should now handle INCLUDE or LIB with spaces in them correctly on Win32. + - the pnm reader read maxval for ppm/pgm files and then ignored it, + it's now validated (0 < maxval < 65536) and used to scale + samples. Note that binary ppm/pgm files (P6/P5) with maxval > + 255 result in an error, since I didn't want to add new features + just yet, just get the code that's there working correctly. + Thanks to Elthek on rhizo for reporting this and help in + tracking it down. + - added a bunch of tests for reading pnm files. ================================================================= diff --git a/MANIFEST b/MANIFEST index d4cbfc21..15774a8e 100644 --- a/MANIFEST +++ b/MANIFEST @@ -138,6 +138,12 @@ testimg/expected.gif testimg/gimpgrad A GIMP gradient file testimg/junk.ppm testimg/loccmap.gif +testimg/maxval.ppm +testimg/maxval_0.ppm +testimg/maxval_256.ppm +testimg/maxval_4095_asc.ppm +testimg/maxval_65536.ppm +testimg/maxval_asc.ppm testimg/nocmap.gif testimg/palette.png testimg/palette_out.png diff --git a/TODO b/TODO index 11d836fe..ef0b9a4a 100644 --- a/TODO +++ b/TODO @@ -87,6 +87,7 @@ New Features: (handles TIFF and GIF) - write_multi() to save other multi-image types, (handles TIFF and GIF) + - pnm binary formats support multiple images per file - compose channels - build a new image based on channels from several images diff --git a/pnm.c b/pnm.c index a4e1753b..9dccf991 100644 --- a/pnm.c +++ b/pnm.c @@ -229,11 +229,11 @@ i_readpnm_wiol(io_glue *ig, int length) { int type; int x, y, ch; int width, height, maxval, channels, pcount; + int rounder; char *cp; unsigned char *uc; mbuf buf; i_color val; - int mult; i_clear_error(); @@ -331,7 +331,25 @@ i_readpnm_wiol(io_glue *ig, int length) { mm_log((1, "i_readpnm: error reading maxval\n")); return NULL; } + + if (maxval == 0) { + i_push_error(0, "maxval is zero - invalid pnm file"); + mm_log((1, "i_readpnm: maxval is zero, invalid pnm file\n")); + return NULL; + } + else if (maxval > 65535) { + i_push_errorf(0, "maxval of %d is over 65535 - invalid pnm file", + maxval); + mm_log((1, "i_readpnm: maxval of %d is over 65535 - invalid pnm file\n")); + return NULL; + } + else if (type >= 4 && maxval > 255) { + i_push_errorf(0, "maxval of %d is over 255 - not currently supported by Imager for binary pnm", maxval); + mm_log((1, "i_readpnm: maxval of %d is over 255 - not currently supported by Imager for binary pnm\n", maxval)); + return NULL; + } } else maxval=1; + rounder = maxval / 2; if (!(cp = gnext(&buf)) || !misspace(*cp)) { i_push_error(0, "garbage in header, invalid PNM file"); @@ -350,11 +368,10 @@ i_readpnm_wiol(io_glue *ig, int length) { case 1: /* Ascii types */ case 2: case 3: - mult = type == 1 ? 255 : 1; for(y=0;ychannels > 2) { chan_count = 3; for (x = 0; x < imgs[imgn]->xsize; ++x) { + printf("bumped entry %d\n", MED_CUT_INDEX(line[x])); ++colors[MED_CUT_INDEX(line[x])].count; } } diff --git a/t/t104ppm.t b/t/t104ppm.t index ba07a602..809e0da2 100644 --- a/t/t104ppm.t +++ b/t/t104ppm.t @@ -1,6 +1,9 @@ +#!perl -w use Imager ':all'; +require "t/testtools.pl"; +use strict; -print "1..13\n"; +print "1..43\n"; init_log("testout/t104ppm.log",1); @@ -16,7 +19,7 @@ i_arc($img,75,75,30,0,361,$red); i_conv($img,[0.1, 0.2, 0.4, 0.2, 0.1]); my $fh = openimage(">testout/t104.ppm"); -$IO = Imager::io_new_fd(fileno($fh)); +my $IO = Imager::io_new_fd(fileno($fh)); i_writeppm_wiol($img, $IO) or die "Cannot write testout/t104.ppm\n"; close($fh); @@ -25,12 +28,12 @@ print "ok 1\n"; $IO = Imager::io_new_bufchain(); i_writeppm_wiol($img, $IO) or die "Cannot write to bufchain"; -$data = Imager::io_slurp($IO); +my $data = Imager::io_slurp($IO); print "ok 2\n"; $fh = openimage("testout/t104.ppm"); $IO = Imager::io_new_fd( fileno($fh) ); -$cmpimg = i_readpnm_wiol($IO,-1) || die "Cannot read testout/t104.ppm\n"; +my $cmpimg = i_readpnm_wiol($IO,-1) || die "Cannot read testout/t104.ppm\n"; close($fh); print "ok 3\n"; @@ -52,7 +55,7 @@ i_arc($gimg, 75, 75, 30, 0, 361, $white); open FH, "> testout/t104_gray.pgm" or die "Cannot create testout/t104_gray.pgm: $!\n"; binmode FH; -my $IO = Imager::io_new_fd(fileno(FH)); +$IO = Imager::io_new_fd(fileno(FH)); i_writeppm_wiol($gimg, $IO) or print "not "; print "ok 6\n"; close FH; @@ -74,6 +77,100 @@ check_gray(11, Imager::i_get_pixel($ooim->{IMG}, 0, 1), 0); check_gray(12, Imager::i_get_pixel($ooim->{IMG}, 1, 0), 0); check_gray(13, Imager::i_get_pixel($ooim->{IMG}, 1, 1), 255); +{ + # https://rt.cpan.org/Ticket/Display.html?id=7465 + # the pnm reader ignores the maxval that it reads from the pnm file + my $maxval = Imager->new; + $maxval->read(file=>"testimg/maxval.ppm") or print "not "; + print "ok 14 # read testimg/maxval.ppm\n"; + + # this image contains three pixels, with each sample from 0 to 63 + # the pixels are (63, 63, 63), (32, 32, 32) and (31, 31, 0) + + # check basic parameters + $maxval->getchannels == 3 or print "not "; + print "ok 15 # channel count\n"; + $maxval->getwidth == 3 or print "not "; + print "ok 16 # width\n"; + $maxval->getheight == 1 or print "not "; + print "ok 17 # height\n"; + + # check the pixels + my ($white, $grey, $green) = $maxval->getpixel('x'=>[0,1,2], 'y'=>[0,0,0]) + or print "not "; + print "ok 18 # fetch pixels\n"; + check_color(19, $white, 255, 255, 255, "white pixel"); + check_color(20, $grey, 130, 130, 130, "grey pixel"); + check_color(21, $green, 125, 125, 0, "green pixel"); + + # and do the same for ASCII images + my $maxval_asc = Imager->new; + $maxval_asc->read(file=>"testimg/maxval_asc.ppm") or print "not "; + print "ok 22 # read testimg/maxval_asc.ppm\n"; + + # this image contains three pixels, with each sample from 0 to 63 + # the pixels are (63, 63, 63), (32, 32, 32) and (31, 31, 0) + + # check basic parameters + $maxval_asc->getchannels == 3 or print "not "; + print "ok 23 # channel count\n"; + $maxval_asc->getwidth == 3 or print "not "; + print "ok 24 # width\n"; + $maxval_asc->getheight == 1 or print "not "; + print "ok 25 # height\n"; + + # check the pixels + my ($white_asc, $grey_asc, $green_asc) = $maxval_asc->getpixel('x'=>[0,1,2], 'y'=>[0,0,0]) + or print "not "; + print "ok 26 # fetch pixels\n"; + check_color(27, $white_asc, 255, 255, 255, "white asc pixel"); + check_color(28, $grey_asc, 130, 130, 130, "grey asc pixel"); + check_color(29, $green_asc, 125, 125, 0, "green asc pixel"); +} + +{ # previously we didn't validate maxval at all, make sure it's + # validated now + my $maxval0 = Imager->new; + $maxval0->read(file=>'testimg/maxval_0.ppm') and print "not "; + print "ok 30 # reading maxval 0 image\n"; + print "# ", $maxval0->errstr, "\n"; + $maxval0->errstr =~ /maxval is zero - invalid pnm file/ + or print "not "; + print "ok 31 # error expected from reading maxval_0.ppm\n"; + + my $maxval65536 = Imager->new; + $maxval65536->read(file=>'testimg/maxval_65536.ppm') and print "not "; + print "ok 32 # reading maxval 65536 image\n"; + print "# ",$maxval65536->errstr, "\n"; + $maxval65536->errstr =~ /maxval of 65536 is over 65535 - invalid pnm file/ + or print "not "; + print "ok 33 # error expected from reading maxval_65536.ppm\n"; + + # maxval of 256 is valid, but Imager can't handle it yet in binary files + my $maxval256 = Imager->new; + $maxval256->read(file=>'testimg/maxval_256.ppm') and print "not "; + print "ok 34 # reading maxval 256 image\n"; + print "# ",$maxval256->errstr,"\n"; + $maxval256->errstr =~ /maxval of 256 is over 255 - not currently supported by Imager/ + or print "not "; + print "ok 35 # error expected from reading maxval_256.ppm\n"; + + # make sure we handle maxval > 255 for ascii + my $maxval4095asc = Imager->new; + okn(36, $maxval4095asc->read(file=>'testimg/maxval_4095_asc.ppm'), + "read maxval_4095_asc.ppm"); + okn(37, $maxval4095asc->getchannels == 3, "channels"); + okn(38, $maxval4095asc->getwidth == 3, "width"); + okn(39, $maxval4095asc->getheight == 1, "height"); + + my ($white, $grey, $green) = $maxval4095asc->getpixel('x'=>[0,1,2], 'y'=>[0,0,0]) + or print "not "; + print "ok 40 # fetch pixels\n"; + check_color(41, $white, 255, 255, 255, "white 4095 pixel"); + check_color(42, $grey, 128, 128, 128, "grey 4095 pixel"); + check_color(43, $green, 127, 127, 0, "green 4095 pixel"); +} + sub openimage { my $fname = shift; local(*FH); @@ -101,3 +198,15 @@ sub check_gray { print "not ok $num # $g doesn't match $gray\n"; } } + +sub check_color { + my ($num, $c, $red, $green, $blue, $note) = @_; + + my ($r, $g, $b) = $c->rgba; + if ($r == $red && $g == $green && $b == $blue) { + print "ok $num # $note\n"; + } + else { + print "not ok $num # ($r, $g, $b) doesn't match ($red, $green, $blue)\n"; + } +} diff --git a/testimg/maxval.ppm b/testimg/maxval.ppm new file mode 100644 index 00000000..60863c57 Binary files /dev/null and b/testimg/maxval.ppm differ diff --git a/testimg/maxval_0.ppm b/testimg/maxval_0.ppm new file mode 100644 index 00000000..3402f40f Binary files /dev/null and b/testimg/maxval_0.ppm differ diff --git a/testimg/maxval_256.ppm b/testimg/maxval_256.ppm new file mode 100644 index 00000000..a9f89b1c Binary files /dev/null and b/testimg/maxval_256.ppm differ diff --git a/testimg/maxval_4095_asc.ppm b/testimg/maxval_4095_asc.ppm new file mode 100644 index 00000000..d68c9969 --- /dev/null +++ b/testimg/maxval_4095_asc.ppm @@ -0,0 +1,4 @@ +P3 +3 1 +4095 +4095 4095 4095 2048 2048 2048 2047 2047 0 diff --git a/testimg/maxval_65536.ppm b/testimg/maxval_65536.ppm new file mode 100644 index 00000000..57453fe7 Binary files /dev/null and b/testimg/maxval_65536.ppm differ diff --git a/testimg/maxval_asc.ppm b/testimg/maxval_asc.ppm new file mode 100644 index 00000000..6be10bb9 --- /dev/null +++ b/testimg/maxval_asc.ppm @@ -0,0 +1,4 @@ +P3 +3 1 +63 +63 63 63 32 32 32 31 31 0