After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 114913 - TARGA (TGA) 16 images with alpha channel are displayed strangely
TARGA (TGA) 16 images with alpha channel are displayed strangely
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
1.x
Other All
: Normal normal
: 1.2
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2003-06-11 09:44 UTC by Yohei Honda
Modified: 2004-02-04 08:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Sample Image (generated by "Lineage") (600.02 KB, image/x-targa)
2003-06-12 05:27 UTC, Yohei Honda
Details

Description Yohei Honda 2003-06-11 09:44:41 UTC
When I open a TARGA 16 image with alpha channel (alpha channel bits = 1), 
I obtain an image which is strangely transparent and whose width is 
shorter than expected. This problem is seen on The GIMP 1.3.15, too. 
 
It seems that the TGA plugin allocates RGBA buffer, but treats it as RGB 
buffer in reading images (see upsample function in tga.c). 
I wrote patches for that problem, but hasn't been tested so much. 
 
Here is th patches: 
http://www.asahi-net.or.jp/~ka7y-hnd/tmp/tga.c-1.2.4.diff 
http://www.asahi-net.or.jp/~ka7y-hnd/tmp/tga.c-1.3.15.diff  
 
----- (tga.c-1.2.4.diff) ----- 
--- tga.c.orig	Sun Feb  3 06:33:43 2002 
+++ tga.c	Wed Jun 11 16:45:50 2003 
@@ -743,7 +743,8 @@ 
 upsample (guchar *dest, 
 	  guchar *src, 
 	  guint   width, 
-	  guint   bytes) 
+	  guint   bytes, 
+	  guint8  alphaBits) 
 { 
   gint x; 
  
@@ -758,7 +759,16 @@ 
       dest[2] =  ((src[0] << 3) & 0xf8); 
       dest[2] += (dest[2] >> 5); 
  
-      dest += 3; 
+      /* alphaBits takes 0 or 1. */ 
+      if (alphaBits == 1) 
+        { 
+	  dest[3] = (src[1] & 0x80)? 0: 255; 
+	  dest += 4; 
+	} 
+      else 
+        { 
+	  sest += 3; 
+	} 
       src += bytes; 
     } 
 } 
@@ -822,7 +832,7 @@ 
     { 
       if (info->bpp == 16 || info->bpp == 15) 
 	{ 
-	  upsample (row, buffer, info->width, info->bytes); 
+	  upsample (row, buffer, info->width, info->bytes, 
info->alphaBits); 
 	} 
       else 
 	{ 
@@ -898,7 +908,7 @@ 
           else if (info->colorMapSize == 24) 
             bgr2rgb(gimp_cmap, tga_cmap, info->colorMapLength, 
cmap_bytes, 0); 
           else if (info->colorMapSize == 16 || info->colorMapSize == 15) 
-            upsample(gimp_cmap, tga_cmap, info->colorMapLength, 
cmap_bytes); 
+            upsample(gimp_cmap, tga_cmap, info->colorMapLength, 
cmap_bytes, info->alphaBits); 
  
         } 
       else 
----- end -----
Comment 1 Sven Neumann 2003-06-11 10:20:21 UTC
Adding Nick Lamb to Cc:  Perhaps he can have a look at the patch.
Comment 2 Sven Neumann 2003-06-11 10:21:27 UTC
Setting milestone to 1.2.5 so that we at least consider to apply the
patch before the next release.
Comment 3 Dave Neary 2003-06-11 15:41:57 UTC
This bug seems to be caused by RGBA images with 15 or 16 bits per
pixel... not sure how that works, maybe Nick could comment?

In any case, it seems different to bug #65534, which is about
behaviour when the bits per pixel is 32. The patch looks more or less
OK to me, though...

Cheers,
Dave.
Comment 4 Raphaël Quinet 2003-06-11 17:27:24 UTC
I don't know if Nick Lamb will be able to comment because the e-mails
to his address have been bouncing.  That was some time ago, maybe this
is fixed now.  Nick, are you still alive?  :-)

Anyway, I am reasonably familiar with the TGA format and the GIMP TGA
plug-in.  The patch looks OK to me.  The only problem is a typo in the
1.2.4 version of the patch: "sest" instead of "dest".  I plan to apply
this patch tomorrow evening, unless Nick or someone else objects.  Or
maybe I will wait until Friday, to give a bit more time for comments.
Comment 5 Sven Neumann 2003-06-11 17:36:53 UTC
I'd prefer a switch rather than this lenghty

 if () else if () else if ()  ...

construct, but basically the patch looks good. I'm a bit surprised
about the following line:

 dest[3] = (src[1] & 0x80)? 0: 255;

So if the bit is set, the alpha value is 0 (full transparency)? Is
that correct?
Comment 6 Yohei Honda 2003-06-12 05:15:23 UTC
TGA files I have are generated by the game "Lineage."  
These header says that they have one bit alpha channel, and alpha  
bits of any pixel are 0.  
  
If alpha value is set to 255 when the bit is 1 and to 0 otherwise, I  
will obtain completely transparent image.  
Comment 7 Yohei Honda 2003-06-12 05:27:52 UTC
Created attachment 17478 [details]
Sample Image (generated by "Lineage")
Comment 8 Dave Neary 2003-06-12 08:59:01 UTC
Ummm - my reading of the tga spec is that in 15 or 16 bit true-colour,
there is no alpha channel. Here's the relevant passage...

Color Map Specification...

Field 4.3: Colour map entry size.
 
When working with VDA or VDA/D cards it is preferred that you select
16 bits (5 bits per primary with 1 bit to select interrupt control)
and set the 16th bit to 0 so that the interrupt bit is disabled.  Even
if this field is set to 15 bits (5 bits per primary) you must still
parse the color map data 16 bits at a time and ignore the 16th bit.

So it seems to me like this is exactly what the GIMP is doing. Is some
other coder using that 16th bit (actually, the first bit) to code
alpha? If that's the case, then the patch looks grand. It seems a
little hacky though... especially if there was never intended to be
alpha on 16 bit per pixel tgas.

Dave.
Comment 9 Dave Neary 2003-06-12 18:07:51 UTC
As I was saying on IRC, I am pretty sure that this behaviour is wrong
with respect to the TGA standard. However, I would be happy to see the
patch go in as long as it doesn't break other loaders (namely, ones
which use that bit as a hardware interrupt). If anyone ever comes up
with a case of the GIMP not doing the correct thing in that case
(which seems unlikely, since currently the bit is ignored), we'd have
to roll back the change. 

But since we currently ignore that bit, and since it only gets
considered if the global "I have an alpha channel" bit is set, I would
be happy enough with the patch going in.

That said, I don't think we should adopt this behaviour for TGA save
routines. If someone wants a targa with alpha, they'll have to go 32
bits per channel (IMHO).

Cheers,
Dave.
Comment 10 Dave Neary 2003-06-12 18:10:23 UTC
That is, of course, supposed to be 32 bits per pixel.

Dave.
Comment 11 Sven Neumann 2003-06-12 18:20:49 UTC
I think I can follow that reasoning. Go ahead and commit it then.

I'd still prefer if you would use a switch as it would improve code
readability...
Comment 12 Raphaël Quinet 2003-06-12 19:46:02 UTC
As I said in a previous comment, I would prefer to wait until tomorrow
before applying the patch, just in case Nick could pop up here and
comment.

Regarding the TGA save part, this does not apply to the GIMP plug-in
because it cannot save Targa 15/16 images anyway.
Comment 13 Dave Neary 2003-06-12 19:54:51 UTC
Slightly modified patch (using a switch) applied to 1.2 and head
branches. Closing bug.

2003-06-12  Dave Neary  <bolsh@gimp.org>

        * plug-ins/common/tga.c: Applied a patch from Yohei Honda
        <yoh2@d2.dion.ne.jp> to handle 16 bit TGAs which use the
        spare bit to do alpha. Closes bug #114913.
Comment 14 Dave Neary 2003-06-12 19:57:21 UTC
Ooops! Mid-air collision with Raphael's comment. 

The patch is now in CVS. I think it's safe enough. If there is any
reason to do so, we can always back the change out. As it is, for
programs who write images this way, it is quite badly broken. For
images that don't, the patch shouldn't change anything.

Cheers,
Dave.
Comment 15 Raphaël Quinet 2004-02-04 08:25:10 UTC
Setting correct resolution for this bug report...