Re: [gui] [PATCH] Implement GtkFramePeer.setIconImage()

[Bryce McKinlay <mckinlay at redhat dot com>]

David Jee wrote:

> Index: gnu/java/awt/peer/gtk/GtkFramePeer.java
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/gnu/java/awt/peer/gtk/GtkFramePeer.java,v
> retrieving revision 1.17.2.4
> diff -u -r1.17.2.4 GtkFramePeer.java
> --- gnu/java/awt/peer/gtk/GtkFramePeer.java	30 Jul 2004 20:00:15 -0000	1.17.2.4
> +++ gnu/java/awt/peer/gtk/GtkFramePeer.java	5 Aug 2004 13:52:57 -0000
> @@ -47,6 +47,7 @@
> import java.awt.Rectangle;
> import java.awt.Window;
> import java.awt.event.PaintEvent;
> +import java.awt.image.ColorModel;
> import java.awt.peer.FramePeer;
> import java.awt.peer.MenuBarPeer;
>
> @@ -146,16 +147,35 @@
>     setIconImage(frame.getIconImage());
>   }
>
> -  native void nativeSetIconImage (GdkPixbufDecoder decoder);
> +  native void nativeSetIconImageFromDecoder (GdkPixbufDecoder decoder);
> +  native void nativeSetIconImageFromData (int[] pixels, int width, int height);
>   public void setIconImage (Image image) 
>   {
>       if (image != null)
>         {
>           GtkImage img = (GtkImage) image;
>           // FIXME: Image should be loaded, but if not, do image loading here.
> -          // Also, can the image source be anything else than GdkPixbufDecoder?
> -          if (img.isLoaded() && img.getSource() instanceof GdkPixbufDecoder)
> -            nativeSetIconImage((GdkPixbufDecoder) img.getSource());
> +          if (img.isLoaded())
> +            {
> +              if (img.getSource() instanceof GdkPixbufDecoder)
> +                {
> +                  nativeSetIconImageFromDecoder((GdkPixbufDecoder) img.getSource());
> +                }
> +              else
> +                {
> +                  int[] pixels = img.getPixelCache();
> +                  ColorModel model = img.getColorModel();
> +                  int[] data = new int[pixels.length * 4];
> +                  for (int i = 0; i < pixels.length; i++)
> +                    {
> +                      data[i * 4] = model.getRed(pixels[i]);
> +                      data[i * 4 + 1] = model.getGreen(pixels[i]);
> +                      data[i * 4 + 2] = model.getBlue(pixels[i]);
> +                      data[i * 4 + 3] = model.getAlpha(pixels[i]);
> +                    }
> +                  nativeSetIconImageFromData(data, img.getWidth(null), img.getHeight(null));
> +                }
> +            }
>         }
>   }
>  


Hi David

This seems pretty inefficient because we are copying the image data 
twice - once into the data[] array (which is inefficient in itself 
because it is using 4 ints, and 4 method calls, for every pixel!), and 
then again in nativeSetIconImageFromData() to copy back into the native 
Gdk format.

In this case (setImageIcon) its not a big deal, because this code path 
is probably unlikely to be used in real application, and isn't likely to 
be called often enough to be a performance problem anyway.

However, that we need to do this does suggest a need to re-design the 
GtkImage stuff somewhat - ideally there should be no copying required at 
all to draw images that were loaded by our toolkit, because GtkImage 
ought to already have a pix buf containing the image in the native 
format, and Gtk can draw directly from that. Perhaps thats what 
GdkPixbufDecoder is already, but in that case, the 'Decoder' name is a 
bit confusing!

Regards

Bryce