Re: [cp-patches] [patch] fix BE image issues

[Andreas Tobler]

Tom Tromey wrote:
> > > > > > "Andreas" == Andreas Tobler <address@hidden> writes:
>
>
> Andreas> 2005-07-24  Andreas Tobler  <address@hidden>
> Andreas> * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkImage.c
> Andreas> (Java_gnu_java_awt_peer_gtk_GtkImage_setPixels): Adjust BE image
> Andreas> handling.
>
> Andreas> +#ifdef WORDS_BIGENDIAN
> Andreas> +  for(j = 0 ; j < width * height; j++)
> Andreas> +   src[j] = SWAPU32((unsigned)src[j]);
> Andreas> +#endif
>
> I'm not so sure that this patch is correct.  It modifies 'src' in
> place.  It seems to me that instead we would want to only byte swap
> the data that we send to gtk.  (Not that I know much about this
> area.  I just didn't want this message to go unreplied-to...)

Ok, I agree and it makes sense to me.

> Also, I think it would be more efficient to do the swapping at the
> same time we are doing the copying, instead of swapping first and then
> copying.  That is, put this code into the loop (and only use memcpy as
> an optimization for LE machines).

Thanks for the feedback here and the clarification on IRC.
Here a new trial, tested on ppc ans sparc solaris.

> BTW I think a similar performance tweak should be applied in
> gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:area_updated_cb()

For this I could prepare a patch, but unfortunately I can not test.

Andreas

Index: classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkImage.c
===================================================================
RCS file: 
/cvs/gcc/gcc/libjava/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkImage.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 gnu_java_awt_peer_gtk_GtkImage.c
--- classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkImage.c 
16 Jul 2005 00:30:51 -0000      1.1.1.1
+++ classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkImage.c 
16 Aug 2005 17:06:45 -0000
@@ -176,7 +176,7 @@
   int width, height, rowstride;
   guchar *pixeldata;
   jint *src_array_iter, *src;
-  int i;
+  int i,j;

   gdk_threads_enter ();

@@ -184,15 +184,26 @@
   height = gdk_pixbuf_get_height (pixbuf);
   rowstride = gdk_pixbuf_get_rowstride (pixbuf);

-  src = src_array_iter =
+  src = src_array_iter =
     (*env)->GetIntArrayElements (env, pixels, NULL);

   pixeldata = gdk_pixbuf_get_pixels (pixbuf);
   for(i = 0 ; i < height; i++)
     {
+#ifdef WORDS_BIGENDIAN
+      /* Convert and copy the native BigEndian pixels to the 32-bit
+        AABBGGRR format the Java uses.  */
+      for (j = 0; j < width; j++)
+       {
+         unsigned int *pix_conv = (unsigned int *) pixeldata;
+         *pix_conv = SWAPU32((unsigned)src[j]);
+         pixeldata += 4;
+       }
+#else
       memcpy((void *)pixeldata, (void *)src, width * 4);
-      src += width;
       pixeldata += rowstride;
+#endif
+      src += width;
     }

   (*env)->ReleaseIntArrayElements (env, pixels, src_array_iter, 0);