From c5a24cca02c56aa4a58ea76109227f3ccffaee0a Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Tue, 25 Mar 2008 11:29:06 +0000 Subject: [PATCH] - some TGA images weren't being detected correctly as TGA images https://rt.cpan.org/Ticket/Display.html?id=32925 - handling of the left-over bit for 16-bit/pixel TGA images has been changed to match the behaviour of the GIMP. Previously the bit being set was treated as an opaque pixel, but one user reported a problem with loading such an image. I haven't been able to find any tools beyond the GIMP that handle alpha-channel 16-bit TGAs, so I'll match it's behaviour. See issue 114913 in the GIMP's bugzilla. http://rt.cpan.org/Ticket/Display.html?id=32926 --- Changes | 12 ++++++++++++ MANIFEST | 1 + t/t1000files.t | 8 +++++++- t/t108tga.t | 29 +++++++++++++++++++++++++---- tga.c | 38 +++++++++++++++++++++++++------------- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/Changes b/Changes index b635fe38..26cc877f 100644 --- a/Changes +++ b/Changes @@ -29,6 +29,18 @@ Imager 0.63 - unreleased similarly. https://rt.cpan.org/Ticket/Display.html?id=29879 + - some TGA images weren't being detected correctly as TGA images + https://rt.cpan.org/Ticket/Display.html?id=32925 + + - handling of the left-over bit for 16-bit/pixel TGA images has been + changed to match the behaviour of the GIMP. Previously the bit + being set was treated as an opaque pixel, but one user reported a + problem with loading such an image. I haven't been able to find any + tools beyond the GIMP that handle alpha-channel 16-bit TGAs, so + I'll match it's behaviour. See issue 114913 in the GIMP's + bugzilla. + http://rt.cpan.org/Ticket/Display.html?id=32926 + Bug fixes: - Imager::Matrix2d->translate() now only requires one of the x or y diff --git a/MANIFEST b/MANIFEST index 3f5aa5e7..2d66ce91 100644 --- a/MANIFEST +++ b/MANIFEST @@ -279,6 +279,7 @@ t/tr18561b.t tags.c testimg/209_yonge.jpg Regression test: #17981 testimg/alpha.tif Alpha scaling test image +testimg/alpha16.tga 16-bit/pixel TGA with alpha "channel" RT 32926 testimg/bad1oflow.bmp 1-bit/pixel, overflow integer on 32-bit machines testimg/bad1wid0.bmp 1-bit/pixel, zero width testimg/bad24comp.bmp 24-bit/pixel, bad compression diff --git a/t/t1000files.t b/t/t1000files.t index 041d5d25..66d3a98d 100644 --- a/t/t1000files.t +++ b/t/t1000files.t @@ -4,7 +4,7 @@ # the file format use strict; -use Test::More tests => 32; +use Test::More tests => 33; use Imager; Imager::init_log("testout/t1000files.log", 1); @@ -79,6 +79,12 @@ probe_ok(<40; -BEGIN { require "t/testtools.pl"; } +use Test::More tests=>46; +use Imager::Test qw(is_color4 is_image); init_log("testout/t108tga.log",1); @@ -130,6 +130,29 @@ is($compressed, 1, "check compressed tag"); ok(grep($_ eq 'tga', Imager->write_types), "check tga in write types"); } +{ # Issue #32926 + # a sample image was read as all transparent + # it had bitsperpixel = 16 and atribute channel set to 1, so it + # should have an alpha channel. + # So we'll do what the gimp does and treat a zero value as opaque. + + my $im = Imager->new; + ok($im->read(file => 'testimg/alpha16.tga'), + "read 16-bit/pixel alpha image"); + my $c1 = $im->getpixel('x' => 0, 'y' => 0); + is_color4($c1, 0, 0, 0, 0, "check transparent pixel"); + my $c2 = $im->getpixel('x' => 19, 'y' => 0); + is_color4($c2, 255, 0, 0, 255, "check opaque pixel"); + + # since this has an effect on writing too, write,it, read it, check it + my $data; + ok($im->write(data => \$data, type => 'tga', wierdpack => 1), + "write 16-bit/pixel w/alpha"); + my $im2 = Imager->new; + ok($im2->read(data => $data), "read it back"); + is_image($im, $im2, "check they match"); +} + sub write_test { my ($im, $filename, $wierdpack, $compress, $idstring) = @_; local *FH; @@ -170,8 +193,6 @@ sub read_test { } } - - sub create_test_image { my $green = i_color_new(0,255,0,255); diff --git a/tga.c b/tga.c index 67594ee3..96e1f944 100644 --- a/tga.c +++ b/tga.c @@ -107,27 +107,31 @@ bpp_to_bytes(unsigned int bpp) { /* =item bpp_to_channels(bpp) -Convert bits per pixel into channels in the image +Convert bits per pixel and the number of attribute bits into channels +in the image bpp - bits per pixel + attr_bit_count - number of attribute bits =cut */ static int -bpp_to_channels(unsigned int bpp) { +bpp_to_channels(unsigned int bpp, int attr_bit_count) { switch (bpp) { case 8: return 1; + case 16: + if (attr_bit_count == 1) + return 4; case 15: return 3; - case 16: - return 4; + case 32: + if (attr_bit_count == 8) + return 4; case 24: return 3; - case 32: - return 4; } return 0; } @@ -167,7 +171,7 @@ color_unpack(unsigned char *buf, int bytepp, i_color *val) { val->rgba.r = (buf[1] & 0x7c) << 1; val->rgba.g = ((buf[1] & 0x03) << 6) | ((buf[0] & 0xe0) >> 2); val->rgba.b = (buf[0] & 0x1f) << 3; - val->rgba.a = (buf[1] & 0x80) ? 255 : 0; + val->rgba.a = (buf[1] & 0x80) ? 0 : 255; val->rgba.r |= val->rgba.r >> 5; val->rgba.g |= val->rgba.g >> 5; val->rgba.b |= val->rgba.b >> 5; @@ -211,13 +215,18 @@ color_pack(unsigned char *buf, int bitspp, i_color *val) { case 8: buf[0] = val->gray.gray_color; break; + case 16: + buf[0] = (val->rgba.b >> 3); + buf[0] |= (val->rgba.g & 0x38) << 2; + buf[1] = (val->rgba.r & 0xf8)>> 1; + buf[1] |= (val->rgba.g >> 6); + buf[1] |= val->rgba.a > 0x7f ? 0 : 0x80; + break; case 15: buf[0] = (val->rgba.b >> 3); buf[0] |= (val->rgba.g & 0x38) << 2; buf[1] = (val->rgba.r & 0xf8)>> 1; buf[1] |= (val->rgba.g >> 6); - case 16: - buf[1] |= val->rgba.a & 0x80; break; case 24: buf[0] = val->rgb.b; @@ -340,7 +349,7 @@ tga_header_verify(unsigned char headbuf[18]) { case 2: /* Uncompressed, rgb images */ case 10: /* Compressed, rgb images */ if (header.bitsperpixel != 15 && header.bitsperpixel != 16 - && header.bitsperpixel != 24 && header.bitsperpixel != 23) + && header.bitsperpixel != 24 && header.bitsperpixel != 32) return 0; break; } @@ -722,7 +731,8 @@ i_readtga_wiol(io_glue *ig, int length) { case 9: /* Compressed, color-mapped images */ if ((channels = bpp_to_channels(mapped ? header.colourmapdepth : - header.bitsperpixel))) break; + header.bitsperpixel, + header.imagedescriptor & 0xF))) break; i_push_error(0, "Targa Image has none of 15/16/24/32 pixel layout"); if (idstring) myfree(idstring); return NULL; @@ -817,6 +827,7 @@ i_writetga_wiol(i_img *img, io_glue *ig, int wierdpack, int compress, char *idst tga_dest dest; unsigned char headbuf[18]; unsigned int bitspp; + unsigned int attr_bits = 0; int mapped; @@ -838,7 +849,7 @@ i_writetga_wiol(i_img *img, io_glue *ig, int wierdpack, int compress, char *idst i_clear_error(); io_glue_commit_types(ig); - + switch (img->channels) { case 1: bitspp = 8; @@ -856,6 +867,7 @@ i_writetga_wiol(i_img *img, io_glue *ig, int wierdpack, int compress, char *idst break; case 4: bitspp = wierdpack ? 16 : 32; + attr_bits = wierdpack ? 1 : 8; break; default: i_push_error(0, "Targa only handles 1,3 and 4 channel images."); @@ -875,7 +887,7 @@ i_writetga_wiol(i_img *img, io_glue *ig, int wierdpack, int compress, char *idst header.width = img->xsize; header.height = img->ysize; header.bitsperpixel = mapped ? 8 : bitspp; - header.imagedescriptor = (1<<5); /* normal order instead of upside down */ + header.imagedescriptor = (1<<5) | attr_bits; /* normal order instead of upside down */ tga_header_pack(&header, headbuf); -- 2.39.5