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. Observations: 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 gnu_java_awt_peer_gtk_GdkPixbufDecoder.c gnu_java_awt_peer_gtk_GdkGraphics.c gnu_java_awt_peer_gtk_GtkImage.c gnu_java_awt_peer_gtk_GtkToolkit.c A look in gnu_java_awt_peer_gtk_GdkPixbufDecoder.c reveals that there are comments about the endianess. grep -i endian * finds mentions in gnu_java_awt_peer_gtk_GdkPixbufDecoder.c gnu_java_awt_peer_gtk_GdkRobotPeer.c 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.
Created attachment 11311 [details] Screenshot of "jamvm PR24116 drop-1.jpg" run. Demonstrating color problems.
Created attachment 11312 [details] Screenshot of Go applet on ppc, showing color problems.
Created attachment 11349 [details] Simplified testcase
I have simplified the testcase and attaching PR27239.java the problem seems to be within BufferedImage. I can reproduce this with kaffe and jamvm.
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.
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
Created attachment 11379 [details] Sven's patch.
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 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); > } > } > }
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.
Sven, 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: http://developer.gnome.org/doc/API/2.0/glib/glib-Byte-Order-Macros.html might be interesting.
gtkimagendian.patch from 2006-05-05 01:41 did not fix the bug for me. I guess you have seen the comments in gnu_java_awt_peer_gtk_GdkPixbufDecoder.c there probably are other places where this would need to be clarified, too.
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.
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.
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
What happens if you change nativeModel to: static ColorModel nativeModel = new DirectColorModel(32, 0x00FF0000, 0x0000FF00, 0x000000FF, 0xFF000000); In GtkImage? Does it work?
No, it does not help. Colors are still screwed.
Could you try with todays cvs? I commited a patch which should improve the situation. Thanks. Andreas
> 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.
ok, need to find a 0.90 version and then backport the patch, although I remember having seen the stuff working on 0.91.
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. Thanks, Andreas
> 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. Bernhard
Created attachment 12119 [details] a testimage with RGBA and drop-1 in the middle
Created attachment 12120 [details] compare both testimage with 0.91-3 and 0.92 on ppc
Andreas: 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. Thanks, Bernhard
Unfortunately fixing the PR27239.java bug did not fix the Yahoo applet bug, thus I have made a new report #28832.