- 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

diff --git a/Changes b/Changes
index b635fe3891249211514b3448ddd435259560ec8c..26cc877fbd4552b0c51335ac8db4c849d46109b3 100644 (file)
--- a/Changes
+++ b/Changes
@@ -29,6 +29,18 @@ Imager 0.63 - unreleased
    similarly.
    https://rt.cpan.org/Ticket/Display.html?id=29879
 
    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
 Bug fixes:
 
  - Imager::Matrix2d->translate() now only requires one of the x or y

diff --git a/MANIFEST b/MANIFEST
index 3f5aa5e74c0074d6d32f95abbb93fd7637d456b8..2d66ce91d00dac6b37f76c8a8d5286d31b2edc6a 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
 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
 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 041d5d2599245b4f7ed30a133bfa2afb03792ecb..66d3a98da2439ae78e7f16e3220a7f3c8814fcdf 100644 (file)
--- a/t/t1000files.t
+++ b/t/t1000files.t
@@ -4,7 +4,7 @@
 # the file format
 
 use strict;
 # the file format
 
 use strict;

-use Test::More tests => 32;
+use Test::More tests => 33;

 use Imager;
 
 Imager::init_log("testout/t1000files.log", 1);
 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
 
 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
 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

diff --git a/t/t108tga.t b/t/t108tga.t
index 164da50e35f43123fefe8fc9b239de806b0bdcfa..ca6d02bafd6f0a6d9eb9ac262d316a8b1ffac274 100644 (file)
--- a/t/t108tga.t
+++ b/t/t108tga.t
@@ -1,8 +1,8 @@
 #!perl -w
 use Imager qw(:all);
 use strict;
 #!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);
 
 
 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");
 }
 
   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;
 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);
 sub create_test_image {
 
   my $green  = i_color_new(0,255,0,255);

diff --git a/tga.c b/tga.c
index 67594ee3a2a72b9491105759ab0837e2d8642c6b..96e1f9444ce2817cf5e4c7c40d464fdcf70e5a82 100644 (file)
--- a/tga.c
+++ b/tga.c
@@ -107,27 +107,31 @@ bpp_to_bytes(unsigned int bpp) {
 /*
 =item bpp_to_channels(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
 
    bpp - bits per pixel

+   attr_bit_count - number of attribute bits

 
 =cut
 */
 
 static
 int
 
 =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;
   switch (bpp) {
   case 8:
     return 1;

+  case 16:
+    if (attr_bit_count == 1)
+      return 4;

   case 15:
     return 3;
   case 15:
     return 3;

-  case 16:
-    return 4;
+  case 32:
+    if (attr_bit_count == 8)
+      return 4;

   case 24:
     return 3;
   case 24:
     return 3;

-  case 32:
-    return 4;

   }
   return 0;
 }
   }
   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.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;
     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 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 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;
     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
   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;
        }
       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 : 
   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;
     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;
   tga_dest dest;
   unsigned char headbuf[18];
   unsigned int bitspp;

+  unsigned int attr_bits = 0;

   
   int mapped;
 
   
   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);
   
   i_clear_error();
   io_glue_commit_types(ig);

-  
+

   switch (img->channels) {
   case 1:
     bitspp = 8;
   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;
     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.");
     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.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);
 
 
   tga_header_pack(&header, headbuf);