- some TGA images weren't being detected correctly as TGA images
authorTony Cook <tony@develop=help.com>
Tue, 25 Mar 2008 11:29:06 +0000 (11:29 +0000)
committerTony Cook <tony@develop=help.com>
Tue, 25 Mar 2008 11:29:06 +0000 (11:29 +0000)
   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
MANIFEST
t/t1000files.t
t/t108tga.t
tga.c

diff --git a/Changes b/Changes
index b635fe3..26cc877 100644 (file)
--- 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
index 3f5aa5e..2d66ce9 100644 (file)
--- 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
index 041d5d2..66d3a98 100644 (file)
@@ -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(<<TGA, "tga", "TGA");
 00 00 FF 00 00 00 95 00 00 00 FF 00 00 00 95 00
 TGA
 
+probe_ok(<<TGA, "tga", "TGA 32-bit");
+00 00 0A 00 00 00 00 00 00 00 00 00 0A 00 0A 00
+20 08 84 00 00 00 00 84 FF FF FF FF 84 00 00 00
+00 84 FF FF FF FF 84 00 00 00 00 84 FF FF FF FF
+TGA
+
 probe_ok(<<ICO, "ico", "Windows Icon");
 00 00 01 00 02 00 20 20 10 00 00 00 00 00 E8 02
 00 00 26 00 00 00 20 20 00 00 00 00 00 00 A8 08
index 164da50..ca6d02b 100644 (file)
@@ -1,8 +1,8 @@
 #!perl -w
 use Imager qw(:all);
 use strict;
-use Test::More tests=>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 67594ee..96e1f94 100644 (file)
--- 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);