Bug 27239 - Images shown with wrong color on powerpc (big endian)
Summary: Images shown with wrong color on powerpc (big endian)
Alias: None
Product: classpath
Classification: Unclassified
Component: awt (show other bugs)
Version: 0.90
: P3 normal
Target Milestone: ---
Assignee: Andreas Tobler
Depends on:
Reported: 2006-04-21 17:02 UTC by Bernhard Reiter
Modified: 2006-08-24 09:28 UTC (History)
2 users (show)

See Also:
Known to work: 0.92
Known to fail: 0.90 0.91
Last reconfirmed: 2006-07-03 21:59:09

Screenshot of "jamvm PR24116 drop-1.jpg" run. Demonstrating color problems. (16.16 KB, image/png)
2006-04-21 17:05 UTC, Bernhard Reiter
Screenshot of Go applet on ppc, showing color problems. (66.84 KB, image/png)
2006-04-21 17:07 UTC, Bernhard Reiter
Simplified testcase (928 bytes, text/x-java)
2006-04-30 14:34 UTC, Bernhard Reiter
Debug function to display the contents of a pixmap. (466 bytes, patch)
2006-05-01 03:27 UTC, Bernhard Reiter
Details | Diff
Proposed (unelegant) fix. (1012 bytes, patch)
2006-05-01 03:30 UTC, Bernhard Reiter
Details | Diff
Sven's patch. (903 bytes, patch)
2006-05-05 01:30 UTC, Sven de Marothy
Details | Diff
Sven's actual patch. (876 bytes, patch)
2006-05-05 01:41 UTC, Sven de Marothy
Details | Diff
a testimage with RGBA and drop-1 in the middle (13.57 KB, image/png)
2006-08-24 07:51 UTC, Bernhard Reiter
compare both testimage with 0.91-3 and 0.92 on ppc (67.21 KB, image/png)
2006-08-24 07:52 UTC, Bernhard Reiter

Note You need to log in before you can comment on or make changes to this bug.
Description Bernhard Reiter 2006-04-21 17:02:16 UTC
Failure, observed:
        Images from the go stones and avatars have the wrong colors
        for the go applets from games.yahoo.com/go

        Especially the black stones are almost invisible and there is
        a white transparancy when they are displayed.

                This works on ia86 (little endian),
                but not on ppc (big endian).

                The bug can be seen with jamvm 1.4.2
                classpath-0.90 with Mouse events patch from http://cvs.savannah.gnu.org/viewcvs/classpath/classpath/java/awt/LightweightDispatcher.java.diff?tr1=1.2&tr2=1.3&r1=text&r2=text

        Hypothesis: A bug in the image handling code regarding endianess.

Meanwhile I could reproduce this with the testcase
        from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24116

I will attach a screenshot.
The test case uses different methods to draw the image.
As not all tiles have wrong colors, possibly only one methods is broken.

Further ideas, attempts:
Search for
        find native/jni/gtk-peer  -name "*.c" | xargs grep -i "<<"
        find native/jni/gtk-peer  -name "*.c" | xargs grep -i ">>"

        finds bitshifts in

        A look in gnu_java_awt_peer_gtk_GdkPixbufDecoder.c reveals that
        there are comments about the endianess.

        grep -i endian *
        finds mentions in

        Inspection in gnu_java_awt_peer_gtk_GdkPixbufDecoder.c shows
        that Java_gnu_java_awt_peer_gtk_GdkPixbufDecoder_streamImage()

        smells as there is a comment that RGB is in host memory order,
        but no special handling regarding endianess.
Comment 1 Bernhard Reiter 2006-04-21 17:05:26 UTC
Created attachment 11311 [details]
Screenshot of "jamvm PR24116 drop-1.jpg" run. Demonstrating color problems.
Comment 2 Bernhard Reiter 2006-04-21 17:07:48 UTC
Created attachment 11312 [details]
Screenshot of Go applet on ppc, showing color problems.
Comment 3 Bernhard Reiter 2006-04-30 14:34:11 UTC
Created attachment 11349 [details]
Simplified testcase
Comment 4 Bernhard Reiter 2006-04-30 14:35:10 UTC
I have simplified the testcase and attaching PR27239.java
the problem seems to be within BufferedImage.
I can reproduce this with kaffe and jamvm.
Comment 5 Bernhard Reiter 2006-05-01 03:27:32 UTC
Created attachment 11351 [details]
Debug function to display the contents of a pixmap.

I suggest to add this funcation and the corresponding prototype.
It is very helpful when debugging this, as this function can be directly
called from gdb.
I have created a mini image with eight pixels so I could watch it.
Comment 6 Bernhard Reiter 2006-05-01 03:30:16 UTC
Created attachment 11352 [details]
Proposed (unelegant) fix.

This is an unelegant fix to the problem.
It seems like Java_gnu_java_awt_peer_gtk_GtkImage_setPixels()
did not fully take into account the different ways of saving colors
between java and gdk.

An elegant fix would refactor these transformations into helper functions,
as similiar funcationality is inside of gnu_java_awt_peer_gtk_GdkPixbufDecoder.c
Comment 7 Sven de Marothy 2006-05-05 01:30:51 UTC
Created attachment 11379 [details]
Sven's patch.
Comment 8 Sven de Marothy 2006-05-05 01:32:36 UTC
Yes, I think this was reported before, but good to have a bug on it.
It's my bug, I guess, since I implemented GtkImage. But I blame GTK for not documenting what pixel format they use anywhere. :)

I submitted what I feel is a better patch for this (also reimplementing a bit of GtkImageConsumer in fewer lines of code). But since I don't have a big-endian machine, I can't test this. Does it work?
Comment 9 Sven de Marothy 2006-05-05 01:39:31 UTC
Comment on attachment 11379 [details]
Sven's patch.

>Index: gnu/java/awt/peer/gtk/GtkImage.java
>RCS file: /sources/classpath/classpath/gnu/java/awt/peer/gtk/GtkImage.java,v
>retrieving revision 1.27
>diff -U3 -r1.27 GtkImage.java
>--- gnu/java/awt/peer/gtk/GtkImage.java	20 Mar 2006 14:42:37 -0000	1.27
>+++ gnu/java/awt/peer/gtk/GtkImage.java	5 May 2006 01:26:28 -0000
>@@ -53,6 +53,7 @@
> import java.util.Vector;
> import java.io.ByteArrayOutputStream;
> import java.io.BufferedInputStream;
>+import java.nio.ByteOrder;
> import java.net.URL;
> import gnu.classpath.Pointer;
>@@ -113,13 +114,27 @@
>   ImageProducer source;
>   /*
>-   * The 32-bit AABBGGRR format the GDK uses.
>+   * The 32-bit format the GDK uses.
>    */
>-  static ColorModel nativeModel = new DirectColorModel(32, 
>-						       0x000000FF,
>-						       0x0000FF00,
>-						       0x00FF0000,
>-						       0xFF000000);
>+  static ColorModel nativeModel;
>+  static 
>+  {
>+    // Little endian - AABBGGRR
>+    if( ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN )
>+      nativeModel = new DirectColorModel(32, 
>+					 0x000000FF,
>+					 0x0000FF00,
>+					 0x00FF0000,
>+					 0xFF000000);
>+    else
>+      // Big endian - RRGGBBAA
>+      nativeModel = new DirectColorModel(32, 
>+					 0xFF000000,
>+					 0x00FF0000,
>+					 0x0000FF00,
>+					 0x000000FF);
>+  }
>   /**
>    * Returns a copy of the pixel data as a java array.
>Index: gnu/java/awt/peer/gtk/GtkImageConsumer.java
>RCS file: /sources/classpath/classpath/gnu/java/awt/peer/gtk/GtkImageConsumer.java,v
>retrieving revision 1.4
>diff -U3 -r1.4 GtkImageConsumer.java
>--- gnu/java/awt/peer/gtk/GtkImageConsumer.java	26 Jul 2005 01:53:25 -0000	1.4
>+++ gnu/java/awt/peer/gtk/GtkImageConsumer.java	5 May 2006 01:26:28 -0000
>@@ -126,12 +126,12 @@
> 	    {
> 	      // get in AARRGGBB and convert to AABBGGRR
>	      int pix = cm.getRGB(pixels[offset + (i * scansize) + x + j]);
>-	      byte b = (byte)(pix & 0xFF);
>-	      byte r = (byte)(((pix & 0x00FF0000) >> 16) & 0xFF);
>-	      pix &= 0xFF00FF00;
>-	      pix |= ((b & 0xFF) << 16);
>-	      pix |= (r & 0xFF);
>-	      pixelCache[(y + i) * this.width + x + j] = pix;
>+	      pixelCache[(y + i) * this.width + x + j] = 
>+		GtkImage.nativeModel.getDataElement(new int[]
>+		    { (((pix & 0x00FF0000) >> 16) & 0xFF),
>+		      (((pix & 0x0000FF00) >> 8) & 0xFF),
>+		      (pix & 0xFF),
>+		      (((pix & 0xFF000000) >> 24) & 0xFF)}, 0);
> 	    }
>       }
>   }
Comment 10 Sven de Marothy 2006-05-05 01:41:45 UTC
Created attachment 11380 [details]
Sven's actual patch.

Gah! That's not what I wanted to do! Ignore the above. Here's the WORKING patch.
Comment 11 Bernhard Reiter 2006-05-06 08:54:26 UTC
thanks for commenting and working on an improved patch.
I will give it a look soon. Currently I am at GNU/Linuxtag in Wiesbaden.

When chatting with Mark I am not sure anymore
that this bug actually only occurs on ppc.
The missing back and forth encoding between java representation
and gdk representation could also affect little-endian platforms.
Thus the question: What were your results with the colors on little-endian?

As to gdk documentation, if they do not describe it,
I think it should be filed as bug.
There is http://developer.gnome.org/doc/API/2.0/gdk-pixbuf/gdk-pixbuf-gdk-pixbuf.html
and around this page giving some documentation about how the pixels are encoded.

While browsing the documentation I have also found:
might be interesting.

Comment 12 Bernhard Reiter 2006-05-06 09:22:32 UTC
gtkimagendian.patch from 2006-05-05 01:41
did not fix the bug for me. 

I guess you have seen the comments in
there probably are other places where this would need to be clarified, too.
Comment 13 Bernhard Reiter 2006-05-06 09:24:41 UTC
Note that my unelegant patch fixes several potential weaknesses,
like that there is not different treatment of image with and without alpha channels. This question cannot be answered by the GTK/GDK documentation
as it depends on classpath usage.
Comment 14 Sven de Marothy 2006-05-06 15:19:30 UTC
Note that your testcase uses BufferedImage and your patch is for GtkImage. That doesn't add up. Is your problem with BufferedImage, GtkImage or both? The GtkImage bug is the one I believe had been reported before.

As for your other issue: It is not a potential issue, because the setPixels method expects the data to be in the nativeModel color model.

If there are problems converting the pixels to the native format, this should be done via nativeModel, and not directly in the code. That's what nativeModel is for, after all. 
Comment 15 Mark Wielaard 2006-05-07 23:33:45 UTC
The color bug issue Bernard and I discussed was the "blue" golden gate bridge in ww2d (x86) as shown at: http://gnu.wildebeest.org/diary/index.php?p=156
Comment 16 Sven de Marothy 2006-05-22 09:06:24 UTC
What happens if you change nativeModel to:

  static ColorModel nativeModel = new DirectColorModel(32, 

In GtkImage? Does it work? 
Comment 17 Bernhard Reiter 2006-05-22 10:38:04 UTC
No, it does not help. Colors are still screwed.
Comment 18 Andreas Tobler 2006-06-28 20:53:22 UTC
Could you try with todays cvs? I commited a patch which should improve the situation.

Comment 19 Bernhard Reiter 2006-06-29 08:26:10 UTC
> Could you try with todays cvs? I commited a patch which should improve the
> situation.

Thanks for the improvement and the notification.
I will try, but it might take a bit of time, as I can only once in a while give it some attentions.
A patch that also applies against and old version is fastest to try for me,
but CVS is also fine.
Comment 20 Andreas Tobler 2006-06-29 22:05:20 UTC
ok, need to find a 0.90 version and then backport the patch, although I remember having seen the stuff working on 0.91.
Comment 21 Andreas Tobler 2006-07-03 21:58:53 UTC
Ok, I found some time to analyze the situation. The version you reported this bug against is ancient. We try to avoid bug fixing for older releases. In this case I guess 0.91 (the actual release) contains the same bug.

Backporting my 'fix' would be a bit difficult since a lot in this area changed dramatically. (On the good way).

I can try to fix it for 0.91 if you like.
On the otherhand I observed that the test program uses BufferedImage.TYPE_INT_RGB.

If you change this to BufferedImage.TYPE_INT_ARGB you can actually see the correct image even if it is a simple RGB. The ww2d-goldengate.png for example is a RGBA picture. Doing the ARGB change shows me the 'right' colors. Taking a RGB also works.

On the current cvs tree I can take the unmodified PR27239.java and it works for both. Means I do not have to change RGB to ARGB.

Let me know how important it is for you to have a fix for 0.91.
Comment 22 Bernhard Reiter 2006-07-04 12:22:02 UTC
> Ok, I found some time to analyze the situation. The version you reported this
> bug against is ancient. 

Thanks for the additional information.

> Let me know how important it is for you to have a fix for 0.91.

It is of low importance. It only would have made testing easier for me.
I will do testing once I get to it, which might be a while.

Comment 23 Bernhard Reiter 2006-08-24 07:51:20 UTC
Created attachment 12119 [details]
a testimage with RGBA and drop-1 in the middle
Comment 24 Bernhard Reiter 2006-08-24 07:52:23 UTC
Created attachment 12120 [details]
compare both testimage with 0.91-3 and 0.92 on ppc
Comment 25 Bernhard Reiter 2006-08-24 07:55:55 UTC
Good work! The situation is much improved! This issue can be resolved.

Got around testing it.
(It was more difficult than I have thought because I could not find out
how to make jamvm in Debian unstable point to a different set of classpaths.
So I had to throw together a Debian package.)

Look at PR27239-test-with-0.92.png to see the difference between 0.91-3 (Debian) and 0.92.

Comment 26 Bernhard Reiter 2006-08-24 09:27:14 UTC
Unfortunately fixing the PR27239.java bug did not fix the Yahoo applet bug,
thus I have made a new report #28832.