Re: GdkDrawable -> GObject

[Tim Janik <timj gtk org>]

On 17 May 2000, Havoc Pennington wrote:

> 
> Hi,
> 
> I started over on the GObject thing and this time did the hard part
> first; it works for me now, at least with testgtk.
> 
> Summary:
>  - GdkDrawable is a fully virtual object, that is, it has no 
>    data members, and all its methods are virtual pretty much
>  - GdkWindow is a subclass
>  - the instance/class structs are called GdkWindowObject[Class], for 
>    legacy reasons
>  - GdkPixmap is a subclass, its instance/class is GdkPixmapObject
>  - All functions take GdkWindow/GdkPixmap which are typedefs for 
>    GdkWindow; thus GDK_WINDOW() and GDK_PIXMAP() return the 
>    typedef. This means GDK_WINDOW(window)->struct_field 
>    doesn't work, but all the fields are private anyway.
>  - platform-specific code provides the GdkDrawable subclasses
>    GdkWindowImpl and GdkPixmapImpl; only the _get_type() 
>    functions of these are exported to cross-platform GDK
>  - GdkWindow and GdkPixmap are implemented by delegating 
>    much of their functionality to the Impl variants
>  - the X port shares code between GdkWindowImpl and  
>    GdkPixmapImpl using a GdkDrawableImpl, but other ports
>    do not need to do this

i'll have to look at the type system stuff more detailed once you committed
your changes to cvs. however, after talking to owen a bit on irc, and given
that you gave your changes some checking ;) i think you should commit what
you have, module minor bits outlined below.

> 
> Havoc

[other mail]
> Hmm, I suppose I should have gzipped that. Apologies for those in
> Europe.

apology not accepted ;) i'm pretty thankfull you included that in the
email and forced me to go through that patch (most of it at least).
developers get what the deserve for being on a developers list ;)


ok, first off, i'm a stupid moron. i outlined very elaborate why we
should use parent_class = g_type_class_peek (PARENT_TYPE) with the new
object system, completely overlooking that getting PARENT_TYPE wrong is
one of the most commonly made derivation mistakes and that we have
appropriate tools at hand now to circumvent that. so my modified
recommendation is to use:

  parent_class = g_type_class_peek_parent (class);

in class_init() implementations. havoc, i'd apprechiate if you had
enough patience to do this for the gdk stuff, i'll go over the gtk
widgets soon and do this.


>  gpointer
>  gdk_drawable_get_data (GdkDrawable   *drawable,
>  		       const gchar   *key)
> -{
> -  return g_dataset_get_data (drawable, key);
> -}
> -
> -GdkDrawableType
> -gdk_drawable_get_type (GdkDrawable *drawable)
>  {
> -  g_return_val_if_fail (drawable != NULL, (GdkDrawableType) -1);
> +  g_return_val_if_fail (GDK_IS_DRAWABLE (drawable), NULL);
>    
> -  return GDK_DRAWABLE_TYPE (drawable);
> +  return g_object_get_qdata (G_OBJECT (drawable),
> +                             g_quark_from_string (key));
>  }

use g_quark_try_string (key) for get_data implementations (it won't allocate
a new id if string isn't found and get_qdata implementations handle 0
quarks fine).


>  GdkDrawable*
>  gdk_drawable_ref (GdkDrawable *drawable)
>  {
> -  GdkDrawablePrivate *private = (GdkDrawablePrivate *)drawable;
> -  g_return_val_if_fail (drawable != NULL, NULL);
> -  
> -  private->ref_count += 1;
> +  g_return_val_if_fail (GDK_IS_DRAWABLE (drawable), NULL);
> +
> +  g_object_ref (G_OBJECT (drawable));
> +
>    return drawable;
>  }

i rather have you do the inconvenient thing here (which is really
an exception to our usual casting conventions) of:

  return (GdkDrawable*) g_object_ref (G_OBJECT (drawable));

because g_object_ref() may return NULL from a g_return_if_fail()
statement and after that, NULL will reliably produce a segfault,
while some outdated pointer with ref_count==0 might not.
(this is not meant to mean that people should check for NULL from
_ref() calls, i'm just saying: if we are about to die, let us die
and prevent possible zombie actions ;))


> @@ -313,7 +349,7 @@
>  {
>    g_return_if_fail (window != NULL);
>    
> -  window->user_data = user_data;
> +  ((GdkWindowObject*)window)->user_data = user_data;
>  }
>  
>  void
> @@ -322,49 +358,54 @@
>  {
>    g_return_if_fail (window != NULL);
>    
> -  *data = window->user_data;
> +  *data = ((GdkWindowObject*)window)->user_data;
>  }

GdkWindowObject.user_data has to die. we've got GObjects around the place
now, so there should simply be gdk_{window|drawable}_{set|get}_user_data ()
functions that use quarked data for the user_data storage, much like
gtk_object_{set|get}_user_data() are just compatibility wrappers nowadays.


> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtksocket.c,v
> retrieving revision 1.17
> retrieving revision 1.17.2.1
> diff -u -r1.17 -r1.17.2.1
> --- gtk/gtksocket.c	2000/05/12 15:25:47	1.17
> +++ gtk/gtksocket.c	2000/05/17 23:29:05	1.17.2.1
> @@ -155,7 +155,8 @@
>  
>    gdk_error_trap_push ();
>    
> -  if (socket->plug_window && socket->plug_window->user_data)
> +  if (socket->plug_window &&
> +      ((GdkWindowObject *)socket->plug_window)->user_data)
>      {
>        /*
>  	GtkWidget *child_widget;

code like this is broken (and was in the first place), there has always
been gdk_window_get_user_data(); direct GdkWindow->user_data access is cruel,
hackish and unsupported ;)


---
ciaoTJ