Re: Pointer/screen problems, part two

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> > Question on gdk_event_new()
> >  
> >  GdkEvent *gdk_event_new (void);
> > 
> > or:
> > 
> >  GdkEvent *gdk_event_new (GdkEventType type);
> > 
> > ? Advantages of the second:
> > 
> >  - We could theoretically allocate the event short
> >    for smaller events.
> >  - We can guarantee that the event is initialized
> >    properly for the particular event type.
> >  - It corresponds better to the idea that events
> >    "derive" from GdkEvent. Changing the type of a
> >    GdkEvent is conceptually like turning a GtkButton
> >    into an a GtkEntry.
> > 
> > Still, it would cause problems for fill-in interfaces
> > that work like gdk_event_translate() (inside GDK we
> > could clearly use an internal interface). So on the 
> > balance, preference is the simple first form.
> 
> really? after you outlined three good reasons for the second
> form? you have my vote for gdk_event_new (GdkEventType type)
> since i'd like to conceptually think of it as a typed object.

OK, did it that way. Patch attached implements 
gdk_event_new (GdkEventType type) and cleans up uses of
stack allocated GdkEvents in gtk/ and tests/.

If you read through the patch, you'll notice a lot of:

 event->motion.window = g_object_ref (window);

Type stuff. (I'm hoping I didn't miss any, since if I
did, it will make for a nasty-to-track down crash.)

This possibly implies that gdk_event_set_window() might
be a good thing to have.

Regards,
                                        Owen

Index: gdk/gdkevents.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkevents.c,v
retrieving revision 1.56
diff -u -p -r1.56 gdkevents.c
--- gdk/gdkevents.c	25 Sep 2002 07:23:52 -0000	1.56
+++ gdk/gdkevents.c	2 Oct 2002 19:45:59 -0000
@@ -256,6 +256,7 @@ gdk_event_put (GdkEvent *event)
 }
 
 static GMemChunk *event_chunk = NULL;
+static GHashTable *event_hash = NULL;
 
 GdkEvent*
 _gdk_event_new (void)
@@ -263,17 +264,30 @@ _gdk_event_new (void)
   GdkEventPrivate *new_event;
   
   if (event_chunk == NULL)
-    event_chunk = g_mem_chunk_new ("events",
-				   sizeof (GdkEventPrivate),
-				   4096,
-				   G_ALLOC_AND_FREE);
+    {
+      event_chunk = g_mem_chunk_new ("events",
+				     sizeof (GdkEventPrivate),
+				     4096,
+				     G_ALLOC_AND_FREE);
+      event_hash = g_hash_table_new (g_direct_hash, NULL);
+    }
   
   new_event = g_chunk_new (GdkEventPrivate, event_chunk);
   new_event->flags = 0;
+  new_event->screen = NULL;
+
+  g_hash_table_insert (event_hash, new_event, GUINT_TO_POINTER (1));
   
   return (GdkEvent*) new_event;
 }
 
+static gboolean
+gdk_event_is_allocated (GdkEvent *event)
+{
+  if (event_hash)
+    return g_hash_table_lookup (event_hash, event) != NULL;
+}
+ 
 /**
  * gdk_event_copy:
  * @event: a #GdkEvent
@@ -399,7 +413,8 @@ gdk_event_free (GdkEvent *event)
     default:
       break;
     }
-  
+
+  g_hash_table_remove (event_chunk, event);
   g_mem_chunk_free (event_chunk, event);
 }
 
@@ -736,6 +751,59 @@ gdk_event_get_axis (GdkEvent   *event,
     return FALSE;
 
   return gdk_device_get_axis (device, axes, axis_use, value);
+}
+
+/**
+ * gdk_event_set_screen:
+ * @event: a #GdkEvent
+ * @screen: a #GdkScreen
+ * 
+ * Sets the screen for @event to @screen. The event must
+ * have been allocated by GTK+, for instance, by
+ * gdk_event_copy().
+ **/
+void
+gdk_event_set_screen (GdkEvent  *event,
+		      GdkScreen *screen)
+{
+  GdkEventPrivate *private;
+  
+  g_return_if_fail (gdk_event_is_allocated (event));
+
+  private = (GdkEventPrivate *)event;
+  
+  private->screen = screen;
+}
+
+/**
+ * gdk_event_get_screen:
+ * @event: a #GdkEvent
+ * 
+ * Returns the screen for the event. The screen is
+ * typically the screen for event->any.window, but
+ * for events such as mouse events, it is the screen
+ * where the the pointer was when the event occurs -
+ * that is, the screen which has the root window 
+ * to which event->motion.x_root and
+ * event->motion.y_root are relative.
+ * 
+ * Return value: the screen for the event
+ **/
+GdkScreen *
+gdk_event_get_screen (GdkEvent *event)
+{
+  if (gdk_event_is_allocated (event))
+    {
+      GdkEventPrivate *private = (GdkEventPrivate *)event;
+
+      if (private->screen)
+	return private->screen;
+    }
+
+  if (event->any.window)
+    return gdk_drawable_get_screen (event->any.window);
+
+  return NULL;
 }
 
 /**
Index: gdk/gdkevents.h
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkevents.h,v
retrieving revision 1.17
diff -u -p -r1.17 gdkevents.h
--- gdk/gdkevents.h	25 Sep 2002 19:16:39 -0000	1.17
+++ gdk/gdkevents.h	2 Oct 2002 19:45:59 -0000
@@ -470,6 +470,10 @@ void	  gdk_event_handler_set 	(GdkEventF
 					 gpointer        data,
 					 GDestroyNotify  notify);
 
+void       gdk_event_set_screen (GdkEvent  *event,
+				 GdkScreen *screen);
+GdkScreen *gdk_event_get_screen (GdkEvent  *event);
+
 void	  gdk_set_show_events		(gboolean	 show_events);
 gboolean  gdk_get_show_events		(void);
 
Index: gdk/x11/gdkevents-x11.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/x11/gdkevents-x11.c,v
retrieving revision 1.94
diff -u -p -r1.94 gdkevents-x11.c
--- gdk/x11/gdkevents-x11.c	25 Sep 2002 19:16:40 -0000	1.94
+++ gdk/x11/gdkevents-x11.c	2 Oct 2002 19:46:03 -0000
@@ -499,6 +499,19 @@ generate_focus_event (GdkWindow *window,
   gdk_event_put (&event);
 }
 
+static void
+set_screen_from_root (GdkDisplay *display,
+		      GdkEvent   *event,
+		      Window      xrootwin)
+{
+  GdkScreen *screen;
+
+  screen = _gdk_x11_display_screen_for_xrootwin (display, xrootwin);
+  g_assert (screen);
+
+  gdk_event_set_screen (event, screen);
+}
+
 static gboolean
 gdk_event_translate (GdkDisplay *display,
 		     GdkEvent   *event,
@@ -817,6 +830,9 @@ gdk_event_translate (GdkDisplay *display
 	  event->scroll.y_root = (gfloat)xevent->xbutton.y_root;
 	  event->scroll.state = (GdkModifierType) xevent->xbutton.state;
 	  event->scroll.device = display->core_pointer;
+
+	  set_screen_from_root (display, event, xevent->xbutton.root);
+	  
           break;
           
         default:
@@ -832,6 +848,8 @@ gdk_event_translate (GdkDisplay *display
 	  event->button.button = xevent->xbutton.button;
 	  event->button.device = display->core_pointer;
 	  
+	  set_screen_from_root (display, event, xevent->xbutton.root);
+
 	  _gdk_event_button_generate (display, event);
           break;
 	}
@@ -872,6 +890,8 @@ gdk_event_translate (GdkDisplay *display
       event->button.state = (GdkModifierType) xevent->xbutton.state;
       event->button.button = xevent->xbutton.button;
       event->button.device = display->core_pointer;
+
+      set_screen_from_root (display, event, xevent->xbutton.root);
       
       break;
       
@@ -902,6 +922,8 @@ gdk_event_translate (GdkDisplay *display
       event->motion.is_hint = xevent->xmotion.is_hint;
       event->motion.device = display->core_pointer;
       
+      set_screen_from_root (display, event, xevent->xmotion.root);
+      
       break;
       
     case EnterNotify:
@@ -954,6 +976,8 @@ gdk_event_translate (GdkDisplay *display
       event->crossing.x_root = xevent->xcrossing.x_root;
       event->crossing.y_root = xevent->xcrossing.y_root;
       
+      set_screen_from_root (display, event, xevent->xcrossing.root);
+      
       /* Translate the crossing mode into Gdk terms.
        */
       switch (xevent->xcrossing.mode)
@@ -1040,6 +1064,8 @@ gdk_event_translate (GdkDisplay *display
       event->crossing.y = xevent->xcrossing.y + yoffset;
       event->crossing.x_root = xevent->xcrossing.x_root;
       event->crossing.y_root = xevent->xcrossing.y_root;
+      
+      set_screen_from_root (display, event, xevent->xcrossing.root);
       
       /* Translate the crossing mode into Gdk terms.
        */