Re: [patch] Allows linking with gtk+2.0 and gtk+3.0

["Daniel P. Berrange" <dan berrange com>]

On Tue, Jun 29, 2010 at 10:18:28PM -0300, Jonh Wendell wrote:
> Hi there.
> 
> I plan to use gtk+3 in vinagre, but I can't do this with gtk-vnc being
> linked to gtk+2. As there is a demand to keep gtk-vnc depending on gtk+
> <= 2.18 for a while, I think that might be a good approach to have a
> configure flag to specify which gtk+ version to link. This approach is
> being used in other GNOME libraries, for instance, vte (gnome-terminal
> widget).
> 
> So, vinagre 3.0 (in gnome 3.0) would require gtk-vnc to be compiled with
> the flag '--with-gtk=3.0'.
> 
> In the future, when gtk-vnc can rely on gtk+3, we will remove the
> configure stuff and the [few] ifdef's.
> 
> Comments? OK to push?
> 
> Thanks,
> -- 
> Jonh Wendell
> http://www.bani.com.br

> >From 0efbde2e0c8a1ae3e432b448d4df7c4337d4461e Mon Sep 17 00:00:00 2001
> From: Jonh Wendell <jwendell gnome org>
> Date: Tue, 29 Jun 2010 17:56:27 -0300
> Subject: [PATCH 1/2] Fixed GOBJECT flags
> X-Spam-Status: Yes, hits=1.000000 required=0.900000, tests=bmf
> X-Spam-Flag: YES
> 
> ---
>  configure.ac |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 3b79c30..0075708 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -102,8 +102,8 @@ VERSION_SCRIPT_FLAGS=-Wl,--version-script=
>  AC_SUBST(VERSION_SCRIPT_FLAGS)
>  
>  PKG_CHECK_MODULES(GOBJECT, gobject-2.0 >= $GOBJECT_REQUIRED)
> -AC_SUBST(GTK_CFLAGS)
> -AC_SUBST(GTK_LIBS)
> +AC_SUBST(GOBJECT_CFLAGS)
> +AC_SUBST(GOBJECT_LIBS)
>  
>  PKG_CHECK_MODULES(GDK_PIXBUF, gdk-pixbuf-2.0 >= $GDK_PIXBUF_REQUIRED)
>  AC_SUBST(GDK_PIXBUF_CFLAGS)
> -- 


ACK, this one looks fine - i've got the same change locally myself.

>  	default:
>  		g_assert_not_reached ();
>  	}
>  
> +	#if GTK_CHECK_VERSION (2, 90, 0)
> +	fmt.byte_order = gdk_visual_get_byte_order (v) == GDK_LSB_FIRST ? G_BIG_ENDIAN : G_LITTLE_ENDIAN;
> +	#else
> +	fmt.byte_order = v->byte_order == GDK_LSB_FIRST ? G_BIG_ENDIAN : G_LITTLE_ENDIAN;
> +	#endif
> +

The API docs say this method is 'Since 2.22', so why is the check
doing 2.90 instead of 2.22 ?


>  	VNC_DEBUG ("Set depth color to %d (%d bpp)", fmt.depth, fmt.bits_per_pixel);
>  	if (!vnc_connection_set_pixel_format(priv->conn, &fmt))
>  		return FALSE;
> @@ -1998,22 +2000,31 @@ GdkPixbuf *vnc_display_get_pixbuf(VncDisplay *obj)
>  	VncDisplayPrivate *priv = obj->priv;
>  	GdkPixbuf *pixbuf;
>  	GdkImage *image;
> +	gint w, h;
>  
>  	if (!priv->conn ||
>  	    !vnc_connection_is_initialized(priv->conn))
>  		return NULL;
>  
>  	image = vnc_image_framebuffer_get_image(priv->fb);
> +	#if GTK_CHECK_VERSION(2, 90, 0)
> +	w = gdk_image_get_width (image);
> +	h = gdk_image_get_height (image);
> +	#else
> +	w = image->width;
> +	h = image->height;
> +	#endif

Same with these new methods

>  
>  	pixbuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB, FALSE, 8,
> -				image->width, image->height);
> +				w,
> +				h);
>  
>  	if (!gdk_pixbuf_get_from_image(pixbuf,
>  				       image,
>  				       gdk_colormap_get_system(),
>  				       0, 0, 0, 0,
> -				       image->width,
> -				       image->height))
> +				       w,
> +				       h))
>  		return NULL;
>  
>  	return pixbuf;
> diff --git a/src/vncdisplay.h b/src/vncdisplay.h
> index dc043d1..2ed2a6c 100644
> --- a/src/vncdisplay.h
> +++ b/src/vncdisplay.h
> @@ -22,7 +22,7 @@
>  #ifndef VNC_DISPLAY_H
>  #define VNC_DISPLAY_H
>  
> -#include <gtk/gtkdrawingarea.h>
> +#include <gtk/gtk.h>
>  #include <glib.h>
>  #include <vncgrabsequence.h>
>  
> diff --git a/src/vncimageframebuffer.c b/src/vncimageframebuffer.c
> index 1cae118..b8f39c3 100644
> --- a/src/vncimageframebuffer.c
> +++ b/src/vncimageframebuffer.c
> @@ -22,6 +22,7 @@
>  #include <config.h>
>  
>  #include <string.h>
> +#include <gtk/gtk.h>
>  
>  #include "vncimageframebuffer.h"
>  #include "vncutil.h"
> @@ -132,33 +133,66 @@ VncImageFramebuffer *vnc_image_framebuffer_new(GdkImage *image,
>  					       const VncPixelFormat *remoteFormat)
>  {
>          VncPixelFormat localFormat;
> +        guint32 red_mask, green_mask, blue_mask;
> +        gint red_shift, green_shift, blue_shift, w, h;
> +        guint16 bpp, bpl, depth;
> +        gpointer pixels;
> +        GdkByteOrder byte_order;
> +
> +	#if GTK_CHECK_VERSION (2, 90, 0)
> +	GdkVisual *visual = gdk_image_get_visual (image);
> +	gdk_visual_get_red_pixel_details (visual, &red_mask, &red_shift, NULL);
> +	gdk_visual_get_green_pixel_details (visual, &green_mask, &green_shift, NULL);
> +	gdk_visual_get_blue_pixel_details (visual, &blue_mask, &blue_shift, NULL);
> +	bpp = gdk_image_get_bytes_per_pixel (image);
> +	depth = gdk_image_get_depth (image);
> +	byte_order = gdk_image_get_byte_order (image);
> +	bpl = gdk_image_get_bytes_per_line (image);
> +	w = gdk_image_get_width (image);
> +	h = gdk_image_get_height (image);
> +	pixels = gdk_image_get_pixels (image);
> +	#else
> +	red_mask = image->visual->red_mask;
> +	red_shift = image->visual->red_shift;
> +	green_mask = image->visual->green_mask;
> +	green_shift = image->visual->green_shift;
> +	blue_mask = image->visual->blue_mask;
> +	blue_shift = image->visual->blue_shift;
> +	bpp = image->bpp;
> +	depth = image->depth;
> +	byte_order = image->byte_order;
> +	bpl = image->bpl;
> +	w = image->width;
> +	h = image->height;
> +	pixels = image->mem;
> +	#endif
>  
>  	VNC_DEBUG("Visual mask: %3d %3d %3d\n      shift: %3d %3d %3d",
> -		   image->visual->red_mask,
> -		   image->visual->green_mask,
> -		   image->visual->blue_mask,
> -		   image->visual->red_shift,
> -		   image->visual->green_shift,
> -		   image->visual->blue_shift);
> -
> -	localFormat.red_max = image->visual->red_mask >> image->visual->red_shift;
> -	localFormat.green_max = image->visual->green_mask >> image->visual->green_shift;
> -	localFormat.blue_max = image->visual->blue_mask >> image->visual->blue_shift;
> -	localFormat.red_shift = image->visual->red_shift;
> -	localFormat.green_shift = image->visual->green_shift;
> -	localFormat.blue_shift = image->visual->blue_shift;
> -	localFormat.depth = image->depth;
> -	localFormat.bits_per_pixel = image->bpp * 8;
> -	localFormat.byte_order = image->byte_order == GDK_LSB_FIRST ? G_LITTLE_ENDIAN : G_BIG_ENDIAN;
> -
> -	memset(image->mem, 0, image->bpl * image->height);
> +		   red_mask,
> +		   green_mask,
> +		   blue_mask,
> +		   red_shift,
> +		   green_shift,
> +		   blue_shift);
> +
> +	localFormat.red_max = red_mask >> red_shift;
> +	localFormat.green_max = green_mask >> green_shift;
> +	localFormat.blue_max = blue_mask >> blue_shift;
> +	localFormat.red_shift = red_shift;
> +	localFormat.green_shift = green_shift;
> +	localFormat.blue_shift = blue_shift;
> +	localFormat.depth = depth;
> +	localFormat.bits_per_pixel = bpp * 8;
> +	localFormat.byte_order = byte_order == GDK_LSB_FIRST ? G_LITTLE_ENDIAN : G_BIG_ENDIAN;
> +
> +	memset(pixels, 0, bpl * h);
>  
>  	return VNC_IMAGE_FRAMEBUFFER(g_object_new(VNC_TYPE_IMAGE_FRAMEBUFFER,
>  						  "image", image,
> -						  "buffer", (guint8 *)image->mem,
> -						  "width", image->width,
> -						  "height", image->height,
> -						  "rowstride", image->bpl,
> +						  "buffer", (guint8 *)pixels,
> +						  "width", w,
> +						  "height", h,
> +						  "rowstride", bpl,
>  						  "local-format", &localFormat,
>  						  "remote-format", remoteFormat,
>  						  NULL));

The general idea looks ok but the version checks all seem to need
adjusting to me

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|