Improve handling of pointer unlock events

* compositor.h: Update prototypes.
* idle_inhibit.c (DestroyIdleInhibitManager): New function.
(idle_inhibit_manager_impl): Add missing op handler.
* seat.c (struct _Seat): Remove pointer unlock surface.  Add
last_seen_subcompositor.
(ReleaseSeat): Release the last seen subcompositor callback.
(ClearPointerUnlockSurface, SwapUnlockSurface): Delete
functions.
(HandleSubcompositorDestroy): New function.
(EnteredSurface): Stop calling SwapUnlockSurface.
(DispatchEntryExit): Set the last seen subcompositor.
(DispatchMotion, CancelGrab1, CancelGrab): Use the last seen
subcompositor to decide where to unlock instead.
(LockSurfaceFocus, DispatchButton, DispatchGesturePinch)
(DispatchGestureSwipe): Stop calling SwapUnlockSurface.
(FakePointerEdge, ForceEntry): Stop calling SwapUnlockSurface.

* subcompositor.c (struct _SubcompositorDestroyCallback): New
struct.
(struct _Subcompositor): New field `destroy_callbacks'.
(MakeSubcompositor): Initialize new field.
(SubcompositorFree): Run and free destroy callbacks.
(SubcompositorOnDestroy, SubcompositorRemoveDestroyCallback):
New functions.
* tests/run_tests.sh:
* tests/select_test.c (verify_sample_text_multiple): Fix
ommissions and typos.

compositor.h

@@ -757,6 +757,7 @@ extern void XLInitShm (void);
 typedef struct _View View;
 typedef struct _List List;
 typedef struct _Subcompositor Subcompositor;
+typedef struct _SubcompositorDestroyCallback SubcompositorDestroyCallback;
 
 typedef enum _FrameMode FrameMode;
 
@@ -812,6 +813,10 @@ extern void SubcompositorFree (Subcompositor *);
 
 extern void SubcompositorFreeze (Subcompositor *);
 extern void SubcompositorUnfreeze (Subcompositor *);
+extern SubcompositorDestroyCallback *SubcompositorOnDestroy (Subcompositor *,
+							     void (*) (void *),
+							     void *);
+extern void SubcompositorRemoveDestroyCallback (SubcompositorDestroyCallback *);
 
 extern void ViewSetSubcompositor (View *, Subcompositor *);
 

idle_inhibit.c

@@ -308,8 +308,16 @@ CreateInhibitor (struct wl_client *client, struct wl_resource *resource,
 				  inhibitor, HandleResourceDestroy);
 }
 
+static void
+DestroyIdleInhibitManager (struct wl_client *client,
+			   struct wl_resource *resource)
+{
+  wl_resource_destroy (resource);
+}
+
 static struct zwp_idle_inhibit_manager_v1_interface idle_inhibit_manager_impl =
   {
+    .destroy = DestroyIdleInhibitManager,
     .create_inhibitor = CreateInhibitor,
   };
 

seat.c

@@ -447,16 +447,18 @@ struct _Seat
   /* The destroy callback attached to that surface.  */
   DestroyCallback *last_button_press_surface_callback;
 
-  /* The surface that the pointer focus should be unlocked to once the
-     mouse pointer is removed.  */
-  Surface *pointer_unlock_surface;
-
-  /* The destroy callback attached to that surface.  */
-  DestroyCallback *pointer_unlock_surface_callback;
-
   /* Unmap callback used for cancelling the grab.  */
   UnmapCallback *grab_unmap_callback;
 
+  /* The subcompositor that the mouse pointer is inside.  */
+  Subcompositor *last_seen_subcompositor;
+
+  /* The window for that subcompositor.  */
+  Window last_seen_subcompositor_window;
+
+  /* The destroy callback for the subcompositor.  */
+  SubcompositorDestroyCallback *subcompositor_callback;
+
   /* How many times the grab is held on this seat.  */
   int grab_held;
 
@@ -1039,9 +1041,6 @@ ReleaseSeat (Seat *seat)
   if (seat->last_seen_surface_callback)
     XLSurfaceCancelRunOnFree (seat->last_seen_surface_callback);
 
-  if (seat->pointer_unlock_surface_callback)
-    XLSurfaceCancelRunOnFree (seat->pointer_unlock_surface_callback);
-
   if (seat->last_button_press_surface_callback)
     XLSurfaceCancelRunOnFree (seat->last_button_press_surface_callback);
 
@@ -1063,6 +1062,9 @@ ReleaseSeat (Seat *seat)
   if (seat->data_source_destroy_callback)
     XLDataSourceCancelDestroyCallback (seat->data_source_destroy_callback);
 
+  if (seat->subcompositor_callback)
+    SubcompositorRemoveDestroyCallback (seat->subcompositor_callback);
+
   if (seat->grab_window != None)
     XDestroyWindow (compositor.display, seat->grab_window);
 
@@ -3345,17 +3347,6 @@ SendButton (Seat *seat, Surface *surface, Time time,
     }
 }
 
-static void
-ClearPointerUnlockSurface (void *data)
-{
-  Seat *seat;
-
-  seat = data;
-
-  seat->pointer_unlock_surface = NULL;
-  seat->pointer_unlock_surface_callback = NULL;
-}
-
 static void
 ClearGrabSurface (void *data)
 {
@@ -3391,27 +3382,6 @@ SwapGrabSurface (Seat *seat, Surface *surface)
     }
 }
 
-static void
-SwapUnlockSurface (Seat *seat, Surface *surface)
-{
-  if (seat->pointer_unlock_surface == surface)
-    return;
-
-  if (seat->pointer_unlock_surface)
-    {
-      XLSurfaceCancelRunOnFree (seat->pointer_unlock_surface_callback);
-      seat->pointer_unlock_surface_callback = NULL;
-      seat->pointer_unlock_surface = NULL;
-    }
-
-  if (surface)
-    {
-      seat->pointer_unlock_surface = surface;
-      seat->pointer_unlock_surface_callback
-	= XLSurfaceRunOnFree (surface, ClearPointerUnlockSurface, seat);
-    }
-}
-
 static void
 ClearLastSeenSurface (void *data)
 {
@@ -3459,11 +3429,8 @@ EnteredSurface (Seat *seat, Surface *surface, Time time,
   Time gesture_time;
 
   if (seat->grab_held && surface != seat->last_seen_surface)
-    {
     /* If the seat is grabbed, delay this for later.  */
-      SwapUnlockSurface (seat, surface);
     return;
-    }
 
   if (seat->last_seen_surface == surface)
     return;
@@ -3633,6 +3600,16 @@ TranslateGrabPosition (Seat *seat, Window window, double *event_x,
   return;
 }
 
+static void
+HandleSubcompositorDestroy (void *data)
+{
+  Seat *seat;
+
+  seat = data;
+  seat->last_seen_subcompositor = NULL;
+  seat->subcompositor_callback = NULL;
+}
+
 static void
 DispatchEntryExit (Subcompositor *subcompositor, XIEnterEvent *event)
 {
@@ -3645,6 +3622,38 @@ DispatchEntryExit (Subcompositor *subcompositor, XIEnterEvent *event)
   if (!seat)
     return;
 
+  if (event->mode != XINotifyGrab
+      && event->mode != XINotifyUngrab)
+    {
+      /* This is not an event generated by grab activation or
+	 deactivation.  Set the last seen subcompositor, or clear it
+	 on XI_Leave.  The last seen subcompositor is used to
+	 determine the surface to which a grab will be released.  */
+
+      if (event->evtype == XI_Leave
+	  || subcompositor != seat->last_seen_subcompositor)
+	{
+	  if (seat->last_seen_subcompositor)
+	    SubcompositorRemoveDestroyCallback (seat->subcompositor_callback);
+
+	  seat->last_seen_subcompositor = NULL;
+	  seat->subcompositor_callback = NULL;
+
+	  if (event->evtype == XI_Enter)
+	    {
+	      /* Attach the new subcompositor.  */
+	      seat->last_seen_subcompositor = subcompositor;
+	      seat->subcompositor_callback
+		= SubcompositorOnDestroy (subcompositor,
+					  HandleSubcompositorDestroy,
+					  seat);
+
+	      /* Also set the window used.  */
+	      seat->last_seen_subcompositor_window = event->event;
+	    }
+	}
+    }
+
   if (event->mode == XINotifyUngrab
       && seat->grab_surface)
     /* Any explicit grab was released, so release the grab surface as
@@ -3991,12 +4000,7 @@ DispatchMotion (Subcompositor *subcompositor, XIDeviceEvent *xev)
 				      xev->event_y);
 
   if (seat->grab_held)
-    {
-      /* If the grab is held, make the surface underneath the pointer
-	 the pending unlock surface.  */
-      SwapUnlockSurface (seat, actual_dispatch);
     dispatch = seat->last_seen_surface;
-    }
   else
     dispatch = actual_dispatch;
 
@@ -4106,41 +4110,48 @@ GetXButton (int detail)
 }
 
 static void
-CancelGrab (Seat *seat, Time time, Window source,
+CancelGrab1 (Seat *seat, Subcompositor *subcompositor,
-	    double x, double y)
+	     Time time, double x, double y)
 {
-  Window target;
+  Surface *surface;
 
+  /* Look up the surface under subcompositor at x, y and enter it, in
+     response to implicit grab termination.  */
+
+  surface = FindSurfaceUnder (subcompositor, x, y);
+
+  if (surface)
+    TransformToSurface (surface, x, y, &x, &y);
+
+  EnteredSurface (seat, surface, time, x, y, False);
+}
+
+static void
+CancelGrab (Seat *seat, Time time, Window source, double x,
+	    double y)
+{
   if (!seat->grab_held)
     return;
 
   if (--seat->grab_held)
     return;
 
-  if (seat->pointer_unlock_surface)
+  /* More or less how this works: translate x and y from source to
+     last_seen_subcompositor_window, and look up the surface in the
+     last_seen_subcompositor, if present.  */
+  if (seat->last_seen_subcompositor)
     {
-      target = XLWindowFromSurface (seat->pointer_unlock_surface);
+      /* Avoid translating coordinates if not necessary.  */
-
+      if (source != seat->last_seen_subcompositor_window)
-      if (target == None)
+	TranslateCoordinates (source, seat->last_seen_subcompositor_window,
-	/* If the window is gone, make the target surface NULL.  */
-	SwapUnlockSurface (seat, NULL);
-      else
-	{
-	  if (source != target)
-	    /* If the source is something other than the target,
-	       translate the coordinates to the target.  */
-	    TranslateCoordinates (source, target, x, y, &x, &y);
-
-	  /* Finally, translate the coordinates to the target
-	     view.  */
-	  TransformToSurface (seat->pointer_unlock_surface,
 			      x, y, &x, &y);
-	}
-    }
 
-  EnteredSurface (seat, seat->pointer_unlock_surface,
+      /* And cancel the grab.  */
-		  time, x, y, False);
+      CancelGrab1 (seat, seat->last_seen_subcompositor, time, x, y);
-  SwapUnlockSurface (seat, NULL);
+    }
+  else
+    /* Otherwise, just leave the surface.  */
+    EnteredSurface (seat, NULL, time, 0, 0, False);
 
   /* Cancel the unmap callback.  */
   XLSurfaceCancelUnmapCallback (seat->grab_unmap_callback);
@@ -4177,12 +4188,8 @@ LockSurfaceFocus (Seat *seat)
 
   seat->grab_held++;
 
-  /* Initially, make the focus revert back to the last seen
-     surface.  */
   if (seat->grab_held == 1)
     {
-      SwapUnlockSurface (seat, seat->last_seen_surface);
-
       /* Also cancel the grab upon the surface being unmapped.  */
       callback = XLSurfaceRunAtUnmap (seat->last_seen_surface,
 				      HandleGrabUnmapped, seat);
@@ -4261,12 +4268,7 @@ DispatchButton (Subcompositor *subcompositor, XIDeviceEvent *xev)
 				      xev->event_y);
 
   if (seat->grab_held)
-    {
-      /* If the grab is held, make the surface underneath the pointer
-	 the pending unlock surface.  */
-      SwapUnlockSurface (seat, actual_dispatch);
     dispatch = seat->last_seen_surface;
-    }
   else
     dispatch = actual_dispatch;
 
@@ -4496,12 +4498,7 @@ DispatchGesturePinch (Subcompositor *subcompositor, XIGesturePinchEvent *pinch)
 				      pinch->event_y);
 
   if (seat->grab_held)
-    {
-      /* If the grab is held, make the surface underneath the pointer
-	 the pending unlock surface.  */
-      SwapUnlockSurface (seat, actual_dispatch);
     dispatch = seat->last_seen_surface;
-    }
   else
     dispatch = actual_dispatch;
 
@@ -4666,12 +4663,7 @@ DispatchGestureSwipe (Subcompositor *subcompositor, XIGestureSwipeEvent *swipe)
 				      swipe->event_y);
 
   if (seat->grab_held)
-    {
-      /* If the grab is held, make the surface underneath the pointer
-	 the pending unlock surface.  */
-      SwapUnlockSurface (seat, actual_dispatch);
     dispatch = seat->last_seen_surface;
-    }
   else
     dispatch = actual_dispatch;
 
@@ -5100,10 +5092,7 @@ FakePointerEdge (Seat *seat, Surface *target, uint32_t serial,
      operation completes.  */
 
   if (seat->grab_held)
-    {
-      SwapUnlockSurface (seat, NULL);
     CancelGrabEarly (seat);
-    }
 
   /* Set the surface as the surface undergoing resize.  */
   seat->resize_surface = target;
@@ -5702,9 +5691,6 @@ ForceEntry (Seat *seat, Window source, double x, double y)
       target = XLWindowFromSurface (surface);
 
       if (target == None)
-	/* If the window is gone, make the target surface NULL.  */
-	SwapUnlockSurface (seat, NULL);
-      else
 	{
 	  if (source != target)
 	    /* If the source is something other than the target,

subcompositor.c

@@ -296,6 +296,18 @@ struct _View
   double fract_x, fract_y;
 };
 
+struct _SubcompositorDestroyCallback
+{
+  /* Function run upon the subcompositor being destroyed.  */
+  void (*destroy_func) (void *);
+
+  /* Data for that function.  */
+  void *data;
+
+  /* The next and last callbacks.  */
+  SubcompositorDestroyCallback *next, *last;
+};
+
 struct _Subcompositor
 {
   /* List of all inferiors in compositing order.  */
@@ -304,6 +316,9 @@ struct _Subcompositor
   /* Toplevel children of this subcompositor.  */
   List *children, *last_children;
 
+  /* List of destroy callbacks.  */
+  SubcompositorDestroyCallback destroy_callbacks;
+
   /* Target this subcompositor draws to.  */
   RenderTarget target;
 
@@ -560,6 +575,12 @@ MakeSubcompositor (void)
   subcompositor->last = subcompositor->inferiors;
   subcompositor->last_children = subcompositor->children;
 
+  /* Initialize the list of destroy callbacks.  */
+  subcompositor->destroy_callbacks.next
+    = &subcompositor->destroy_callbacks;
+  subcompositor->destroy_callbacks.last
+    = &subcompositor->destroy_callbacks;
+
   /* Initialize the buffers used to store previous damage.  */
   pixman_region32_init (&subcompositor->prior_damage[0]);
   pixman_region32_init (&subcompositor->prior_damage[1]);
@@ -3115,6 +3136,8 @@ SubcompositorSetProjectiveTransform (Subcompositor *subcompositor,
 void
 SubcompositorFree (Subcompositor *subcompositor)
 {
+  SubcompositorDestroyCallback *next, *last;
+
   /* It isn't valid to call this function with children attached.  */
   XLAssert (subcompositor->children->next
 	    == subcompositor->children);
@@ -3124,6 +3147,19 @@ SubcompositorFree (Subcompositor *subcompositor)
   XLFree (subcompositor->children);
   XLFree (subcompositor->inferiors);
 
+  /* Run each destroy callback.  */
+
+  next = subcompositor->destroy_callbacks.next;
+  while (next != &subcompositor->destroy_callbacks)
+    {
+      last = next;
+      next = next->next;
+
+      /* Call the function and free the callback.  */
+      last->destroy_func (last->data);
+      XLFree (last);
+    }
+
   /* Finalize the buffers used to store previous damage.  */
   pixman_region32_fini (&subcompositor->prior_damage[0]);
   pixman_region32_fini (&subcompositor->prior_damage[1]);
@@ -3259,3 +3295,33 @@ SubcompositorHeight (Subcompositor *subcompositor)
 {
   return subcompositor->max_y - subcompositor->min_y + 1;
 }
+
+SubcompositorDestroyCallback *
+SubcompositorOnDestroy (Subcompositor *subcompositor,
+			void (*destroy_func) (void *), void *data)
+{
+  SubcompositorDestroyCallback *callback;
+
+  callback = XLCalloc (1, sizeof *callback);
+
+  /* Link the callback onto the subcompositor.  */
+  callback->next = subcompositor->destroy_callbacks.next;
+  callback->last = &subcompositor->destroy_callbacks;
+  subcompositor->destroy_callbacks.next->last = callback;
+  subcompositor->destroy_callbacks.next = callback;
+
+  /* Add the func and data.  */
+  callback->destroy_func = destroy_func;
+  callback->data = data;
+
+  return callback;
+}
+
+void
+SubcompositorRemoveDestroyCallback (SubcompositorDestroyCallback *callback)
+{
+  callback->last->next = callback->next;
+  callback->next->last = callback->last;
+
+  XLFree (callback);
+}

tests/run_tests.sh

@@ -57,6 +57,8 @@ declare -a vfb_tests=(
     select_test
 )
 
+make -C . "${vfb_tests[@]}" select_helper select_helper_multiple
+
 echo "Compositor for vfb tests started at ${WAYLAND_DISPLAY}"
 
 for test_executable in "${vfb_tests[@]}"

tests/select_test.c

@@ -276,7 +276,7 @@ verify_sample_text_multiple (Time time)
       if (!dup2 (pipefds[1], 1))
 	exit (1);
 
-      execlp ("./select_helper_multiple", "./select_helper",
+      execlp ("./select_helper_multiple", "./select_helper_multiple",
 	      display_string, time_buffer, "STRING", "UTF8_STRING",
 	      NULL);
       exit (1);