Fix frame timing prediction algorithm

* compositor.h (struct _RoleFuncs): Replace subframe with
`subsurface_update'.
* dmabuf.c (ExistingModifier):
* fns.c (RebalanceBusfault): Pacify various compiler warnings.
* frame_clock.c (struct _FrameClock, PostEndFrame)
(XLFrameClockHandleFrameEvent, XLMakeFrameClockForWindow): Fix
presentation logic.  First, store absolute presentation
timestamps.  Secondly, use the frame_delay to adjust timestamps.
* icon_surface.c (Subframe): Delete function.
(SubsurfaceUpdate): New function.
(EndSubframe): Delete function.
(XLGetIconSurface): Change attached functions.
* picture_renderer.c (struct _PictureTarget, RecordBufferActivity)
(HandleActivityEvent, DestroyRenderTarget)
(FillBoxesWithTransparency, Composite, FinishRender)
(picture_render_funcs): Send roundtrip event only once, and
flush upon FinishRender.
* process.c (ProcessPendingDescriptions, RunProcess,
ProcessPoll): Don't block signals where unnecessary.
* seat.c (struct _Seat, CursorFromRole, ReleaseSeat, Subframe)
(EndSubframe, MakeCurrentCursor, HandleBind, MakeSeatForDevicePair)
(StartResizeTracking):
* subsurface.c (Subframe, EndSubframe, Commit, Teardown)
(GetSubsurface):
* surface.c (HandleScaleChanged):
* xdg_surface.c (CheckFrame, Commit, Subframe, SubsurfaceUpdate)
(EndSubframe, XLGetXdgSurface):
* xdg_toplevel.c (Unmap): Adjust for above changes, and fix some
compiler warnings.

compositor.h

@@ -1071,8 +1071,7 @@ struct _RoleFuncs
 
   /* These are optional.  */
   Bool (*early_commit) (Surface *, Role *);
-  Bool (*subframe) (Surface *, Role *);
+  void (*subsurface_update) (Surface *, Role *);
-  void (*end_subframe) (Surface *, Role *);
   Window (*get_window) (Surface *, Role *);
   void (*get_resize_dimensions) (Surface *, Role *, int *, int *);
   void (*post_resize) (Surface *, Role *, int, int, int, int);

dmabuf.c

@@ -182,6 +182,12 @@ ExistingModifier (BufferParams *params, uint32_t *current_hi,
 {
   int i, count;
 
+  /* Pacify -Wmaybe-uninitialized under -O2.  count is only non-zero
+     if both return arguments are initialized, but GCC thinks
+     otherwise.  */
+  *current_hi = 0;
+  *current_lo = 0;
+
   for (i = 0, count = 0; i < ArrayElements (params->entries); ++i)
     {
       if (params->entries[i].fd != -1)

fns.c

@@ -593,6 +593,8 @@ RebalanceBusfault (Busfault **tree)
 	    RotateLeft (tree);
 	  else
 	    {
+	      XLAssert ((*tree)->left->right != NULL);
+
 	      RotateRight (&(*tree)->left);
 	      RotateLeft (tree);
 	    }
@@ -611,6 +613,8 @@ RebalanceBusfault (Busfault **tree)
 	    RotateRight (tree);
 	  else
 	    {
+	      XLAssert ((*tree)->right->left != NULL);
+
 	      RotateLeft (&(*tree)->right);
 	      RotateRight (tree);
 	    }

frame_clock.c

@@ -30,10 +30,6 @@ enum
   {
     /* 150ms.  */
     MaxPresentationAge	  = 150000,
-    /* 5000 microseconds.  This arbitrary value is the longest it
-       normally takes for a sync counter update to be processed by the
-       compositor.  */
-    PresentationThreshold = 5000,
   };
 
 /* Whether or not the compositor supports frame synchronization.  */
@@ -74,10 +70,6 @@ struct _FrameClock
      and the value of the last frame that was marked as complete.  */
   uint64_t next_frame_id, finished_frame_id;
 
-  /* Whether or not we are waiting for a frame to be completely
-     painted.  */
-  Bool in_frame;
-
   /* A timer used as a fake synchronization source if frame
      synchronization is not supported.  */
   Timer *static_frame_timer;
@@ -85,6 +77,38 @@ struct _FrameClock
   /* A timer used to end the next frame.  */
   Timer *end_frame_timer;
 
+    /* Callback run when the frame is frozen.  */
+  void (*freeze_callback) (void *);
+
+  /* Data for that callback.  */
+  void *freeze_callback_data;
+
+  /* The wanted configure value.  */
+  uint64_t configure_id;
+
+  /* The time the last frame was drawn.  */
+  uint64_t last_frame_time;
+
+  /* Any pending frame synchronization counter value, or 0.  */
+  uint64_t pending_sync_value;
+
+  /* The last frame drawn for whom a _NET_WM_FRAME_TIMINGS message has
+     not yet arrived.  */
+  uint64_t frame_timings_id;
+
+  /* The time the frame at frame_timings_id was drawn.  Used to
+     compute the presentation time.  */
+  uint64_t frame_timings_drawn_time;
+
+  /* The last known presentation time.  */
+  uint64_t last_presentation_time;
+
+  /* The refresh interval.  */
+  uint32_t refresh_interval;
+
+  /* The delay between the start of vblank and the redraw point.  */
+  uint32_t frame_delay;
+
   /* Whether or not configury is in progress, and whether or not this
      is frozen, and whether or not the frame shouldn't actually be
      unfrozen until EndFrame.  */
@@ -93,30 +117,13 @@ struct _FrameClock
   /* Whether or not EndFrame was called after StartFrame.  */
   Bool end_frame_called;
 
-  /* The wanted configure value.  */
+  /* Whether or not we are waiting for a frame to be completely
-  uint64_t configure_id;
+     painted.  */
-
+  Bool in_frame;
-  /* The time the last frame was drawn.  */
-  uint64_t last_frame_time;
-
-  /* The presentation time.  */
-  int32_t presentation_time;
-
-  /* The refresh interval.  */
-  uint32_t refresh_interval;
 
   /* Whether or not this frame clock should try to predict
      presentation times, in order to group frames together.  */
   Bool predict_refresh;
-
-  /* Callback run when the frame is frozen.  */
-  void (*freeze_callback) (void *);
-
-  /* Data for that callback.  */
-  void *freeze_callback_data;
-
-  /* Any pending frame synchronization counter value, or 0.  */
-  uint64_t pending_sync_value;
 };
 
 struct _CursorClockCallback
@@ -283,17 +290,20 @@ PostEndFrame (FrameClock *clock)
   XLAssert (clock->end_frame_timer == NULL);
 
   if (!clock->refresh_interval
-      || !clock->presentation_time)
+      || !clock->last_presentation_time)
     return;
 
   /* Obtain the monotonic clock time.  */
   clock_gettime (CLOCK_MONOTONIC, &current_time);
 
-  /* Calculate the time by which the next frame must be drawn.  It is
+  /* target is now the time the last frame was presented.  This is the
-     a multiple of the refresh rate with the vertical blanking
+     end of a vertical blanking period. */
-     period added.  */
+  target = clock->last_presentation_time;
-  target = clock->last_frame_time + clock->presentation_time;
+
+  /* now is the current time.  */
   now = HighPrecisionTimestamp (&current_time);
+
+  /* There is no additional offset to add to the time.  */
   additional = 0;
 
   /* If now is more than UINT32_MAX * 1000, then this timestamp may
@@ -313,9 +323,9 @@ PostEndFrame (FrameClock *clock)
 
   /* If the last time the frame time was obtained was that long ago,
      return immediately.  */
-  if (now - clock->last_frame_time >= MaxPresentationAge)
+  if (now - clock->last_presentation_time >= MaxPresentationAge)
     {
-      if ((fallback - clock->last_frame_time) <= MaxPresentationAge)
+      if ((fallback - clock->last_presentation_time) <= MaxPresentationAge)
 	{
 	  /* Some compositors wrap around once the X server time
 	     overflows the 32-bit Time type.  If now happens to be
@@ -331,23 +341,21 @@ PostEndFrame (FrameClock *clock)
 	return;
     }
 
+  /* Keep adding the refresh interval until target becomes the
+     presentation time of a frame in the future.  */
+
   while (target < now)
     {
       if (IntAddWrapv (target, clock->refresh_interval, &target))
 	return;
     }
 
-  /* Use 5000 microseconds before the presentation time, or 3/4th of
+  /* The vertical blanking period itself can't actually be computed
-     it if it is less than 5000.  Any more and we risk the counter
+     based on available data.  However, frame_delay must be inside the
-     value change signalling the end of the frame arriving after the
+     vertical blanking period for it to make any sense, so use it to
-     presentation deadline.  */
+     compute the deadline instead.  Add about 100 us to the frame
-  if (clock->presentation_time > PresentationThreshold)
+     delay to compensate for the roundtrip time.  */
-    target = target - (clock->presentation_time - PresentationThreshold);
+  target -= clock->frame_delay - 100;
-  else
-    /* However, if the presentation time is less than 5000
-       microseconds, use 3/4ths of it.  This computation seems to be a
-       good enough fallback.  */
-    target = target - (clock->presentation_time / 4 * 3);
 
   /* Add the remainder of now if it was probably truncated by the
      compositor.  */
@@ -627,22 +635,52 @@ XLFrameClockHandleFrameEvent (FrameClock *clock, XEvent *event)
 	  /* Run any frame callbacks, since drawing has finished.  */
 	  clock->in_frame = False;
 	  RunFrameCallbacks (clock);
+
+	  if (clock->frame_timings_id == -1)
+	    {
+	      /* Wait for the frame's presentation time to arrive,
+		 unless we are already waiting on a previous
+		 frame.  */
+	      clock->frame_timings_id = value;
+
+	      /* Also save the frame drawn time.  */
+	      clock->frame_timings_drawn_time = clock->last_frame_time;
+	    }
 	}
     }
 
   if (event->xclient.message_type == _NET_WM_FRAME_TIMINGS)
     {
+      /* Check that the frame timings are up to date.  */
+      low = event->xclient.data.l[0] & 0xffffffff;
+      high = event->xclient.data.l[1] & 0xffffffff;
+      value = low | (high << 32);
+
+      if (value != clock->frame_timings_id)
+	/* They are not.  */
+	return;
+
+      /* The timings message has arrived, so clear
+	 frame_timings_id.  */
+      clock->frame_timings_id = -1;
+
+      /* And set the last known presentation time.  */
+      clock->last_presentation_time = (clock->frame_timings_drawn_time
+				       + event->xclient.data.l[2]);
+
       /* Save the presentation time and refresh interval.  There is no
 	 need to mask these values, since they are being put into
 	 (u)int32_t.  */
-      clock->presentation_time = event->xclient.data.l[2];
       clock->refresh_interval = event->xclient.data.l[3];
+      clock->frame_delay = event->xclient.data.l[4];
 
-      if (clock->refresh_interval & (1U << 31))
+      if (clock->refresh_interval & (1U << 31)
+	  || clock->frame_delay == 0x80000000)
 	{
 	  /* This means frame timing information is unavailable.  */
-	  clock->presentation_time = 0;
 	  clock->refresh_interval = 0;
+	  clock->frame_delay = 0;
+	  clock->last_presentation_time = 0;
 	}
     }
 
@@ -702,6 +740,9 @@ XLMakeFrameClockForWindow (Window window)
   clock = XLCalloc (1, sizeof *clock);
   clock->next_frame_id = 0;
 
+  /* Set this to an invalid value.  */
+  clock->frame_timings_id = -1;
+
   XLOutputGetMinRefresh (&default_refresh_rate);
 
   XSyncIntToValue (&initial_value, 0);

icon_surface.c

@@ -342,8 +342,8 @@ Commit (Surface *surface, Role *role)
     MaybeUnmapWindow (icon);
 }
 
-static Bool
+static void
-Subframe (Surface *surface, Role *role)
+SubsurfaceUpdate (Surface *surface, Role *role)
 {
   IconSurface *icon;
 
@@ -354,20 +354,12 @@ Subframe (Surface *surface, Role *role)
       /* A frame is already in progress; schedule another one for
 	 later.  */
       icon->state |= StateLateFrame;
-      return False;
+      return;
     }
 
   /* I guess subsurface updates don't count as urgent frames?  */
   XLFrameClockStartFrame (icon->clock, False);
-  return True;
+  SubcompositorUpdate (icon->subcompositor);
-}
-
-static void
-EndSubframe (Surface *surface, Role *role)
-{
-  IconSurface *icon;
-
-  icon = IconSurfaceFromRole (role);
   XLFrameClockEndFrame (icon->clock);
 }
 
@@ -394,8 +386,7 @@ XLGetIconSurface (Surface *surface)
   role->role.funcs.teardown = Teardown;
   role->role.funcs.setup = Setup;
   role->role.funcs.release_buffer = ReleaseBuffer;
-  role->role.funcs.subframe = Subframe;
+  role->role.funcs.subsurface_update = SubsurfaceUpdate;
-  role->role.funcs.end_subframe = EndSubframe;
   role->role.funcs.get_window = GetWindow;
 
   /* Make an override-redirect window to use as the icon surface.  */

picture_renderer.c

@@ -212,6 +212,9 @@ struct _PictureTarget
 
   /* List of present completion callbacks.  */
   PresentCompletionCallback completion_callbacks;
+
+  /* List of buffers that were used in the course of an update.  */
+  XLList *buffers_used;
 };
 
 struct _DrmFormatInfo
@@ -485,14 +488,10 @@ FindBufferActivityRecord (PictureBuffer *buffer, PictureTarget *target)
 /* Record buffer activity involving the given buffer and target.  */
 
 static void
-RecordBufferActivity (RenderBuffer src, RenderTarget dest)
+RecordBufferActivity (PictureBuffer *buffer, PictureTarget *target,
+		      uint64_t roundtrip_id)
 {
   BufferActivityRecord *record;
-  PictureBuffer *buffer;
-  PictureTarget *target;
-
-  buffer = src.pointer;
-  target = dest.pointer;
 
   /* Try to find an existing record.  */
   record = FindBufferActivityRecord (buffer, target);
@@ -531,7 +530,7 @@ RecordBufferActivity (RenderBuffer src, RenderTarget dest)
       record->target = target;
     }
 
-  record->id = SendRoundtripMessage ();
+  record->id = roundtrip_id;
 }
 
 static void
@@ -614,33 +613,29 @@ UnlinkActivityRecord (BufferActivityRecord *record)
 static void
 HandleActivityEvent (uint64_t counter)
 {
-  BufferActivityRecord *record;
+  BufferActivityRecord *record, *last;
 
   /* Look through the global activity list for a record matching
      counter.  */
   record = all_activity.global_next;
   while (record != &all_activity)
     {
-      if (record->id == counter)
+      last = record;
-	break;
-
       record = record->global_next;
+
+      if (last->id == counter)
+	{
+	  /* Remove the record.  Then, run any callbacks pertaining to
+	     it.  This code mandates that there only be a single
+	     activity record for each buffer-target combination on the
+	     global list at any given time.  */
+	  UnlinkActivityRecord (last);
+	  MaybeRunIdleCallbacks (last->buffer, last->target);
+
+	  /* Free the record.  */
+	  XLFree (last);
+	}
     }
-
-  /* If record is all_activity (meaning no matching record was found),
-     return.  */
-  if (record == &all_activity)
-    return;
-
-  /* Remove the record.  Then, run any callbacks pertaining to it.
-     This code mandates that there only be a single activity record
-     for each buffer-target combination on the global list at any
-     given time.  */
-  UnlinkActivityRecord (record);
-  MaybeRunIdleCallbacks (record->buffer, record->target);
-
-  /* Free the record.  */
-  XLFree (record);
 }
 
 
@@ -916,6 +911,10 @@ DestroyRenderTarget (RenderTarget target)
 
   pict_target = target.pointer;
 
+  /* Assert that there are no more buffers left in the active buffer
+     list.  */
+  XLAssert (pict_target->buffers_used == NULL);
+
   if (pict_target->presentation_window)
     {
       XPresentFreeInput (compositor.display,
@@ -994,6 +993,9 @@ FillBoxesWithTransparency (RenderTarget target, pixman_box32_t *boxes,
   else
     rects = XLMalloc (sizeof *rects * nboxes);
 
+  /* Pacify GCC.  */
+  memset (rects, 0, sizeof *rects * nboxes);
+
   for (i = 0; i < nboxes; ++i)
     {
       rects[i].x = BoxStartX (boxes[i]) - min_x;
@@ -1189,8 +1191,41 @@ Composite (RenderBuffer buffer, RenderTarget target,
 		    /* dst-x, dst-y, width, height.  */
 		    x, y, width, height);
 
-  /* Record pending buffer activity.  */
+  /* Record pending buffer activity; the roundtrip message is then
-  RecordBufferActivity (buffer, target);
+     sent later.  */
+  picture_target->buffers_used
+    = XLListPrepend (picture_target->buffers_used, picture_buffer);
+}
+
+static void
+FinishRender (RenderTarget target, pixman_region32_t *damage)
+{
+  XLList *tem, *last;
+  PictureTarget *pict_target;
+  uint64_t roundtrip_id;
+
+  /* Finish rendering.  This function then sends a single roundtrip
+     message and records buffer activity for each buffer involved in
+     the update based on that.  */
+
+  roundtrip_id = SendRoundtripMessage ();
+  pict_target = target.pointer;
+  tem = pict_target->buffers_used;
+  while (tem)
+    {
+      last = tem;
+      tem = tem->next;
+
+      /* Record buffer activity on this one buffer.  */
+      RecordBufferActivity (last->data, pict_target,
+			    roundtrip_id);
+
+      /* Free the list element.  */
+      XLFree (last);
+    }
+
+  /* Clear buffers_used.  */
+  pict_target->buffers_used = NULL;
 }
 
 static int
@@ -1449,6 +1484,7 @@ static RenderFuncs picture_render_funcs =
     .fill_boxes_with_transparency = FillBoxesWithTransparency,
     .clear_rectangle = ClearRectangle,
     .composite = Composite,
+    .finish_render = FinishRender,
     .target_age = TargetAge,
     .import_fd_fence = ImportFdFence,
     .wait_fence = WaitFence,

process.c

@@ -198,11 +198,12 @@ RunNext (ProcessQueue *queue)
 }
 
 static void
-ProcessPendingDescriptions (void)
+ProcessPendingDescriptions (Bool need_block)
 {
   ProcessQueue *queue;
 
-  Block (NULL);
+  if (need_block)
+    Block (NULL);
 
   for (queue = all_queues; queue; queue = queue->next)
     {
@@ -210,7 +211,8 @@ ProcessPendingDescriptions (void)
 	RunNext (queue);
     }
 
-  Unblock ();
+  if (need_block)
+    Unblock ();
 }
 
 static void
@@ -373,7 +375,7 @@ RunProcess (ProcessQueue *queue, char **arguments)
   queue->descriptions.next = desc;
 
   /* Process pending process descriptions.  */
-  ProcessPendingDescriptions ();
+  ProcessPendingDescriptions (True);
 }
 
 ProcessQueue *
@@ -405,7 +407,7 @@ ProcessPoll (struct pollfd *fds, nfds_t nfds,
      be run by ProcessPendingDescriptions.  If it arrives after, then
      ppoll will be interrupted with EINTR.  */
   Block (&oldset);
-  ProcessPendingDescriptions ();
+  ProcessPendingDescriptions (False);
   rc = ppoll (fds, nfds, timeout, &oldset);
   Unblock ();
 

seat.c

@@ -472,6 +472,9 @@ struct _Seat
   /* The time of the last key event sent.  */
   Time its_depress_time;
 
+  /* The name of the seat.  */
+  char *name;
+
   /* The grab surface.  While it exists, events for different clients
      will be reported relative to it.  */
   Surface *grab_surface;
@@ -584,12 +587,6 @@ static TextInputFuncs *input_funcs;
 
 #define CursorFromRole(role)	((SeatCursor *) (role))
 
-/* Subcompositor targets used inside cursor subframes.  */
-static RenderTarget cursor_subframe_target;
-
-/* Its associated pixmap.  */
-static Pixmap cursor_subframe_pixmap;
-
 
 
 static Bool
@@ -1037,6 +1034,7 @@ ReleaseSeat (Seat *seat)
   FreeDestroyListeners (seat);
   FreeModifierCallbacks (seat);
 
+  XLFree (seat->name);
   XLFree (seat->key_pressed);
   XLFree (seat);
 }
@@ -1277,120 +1275,16 @@ ReleaseBuffer (Surface *surface, Role *role, ExtBuffer *buffer)
   XLReleaseBuffer (buffer);
 }
 
-static Bool
-Subframe (Surface *surface, Role *role)
-{
-  RenderTarget target;
-  Pixmap pixmap;
-  int min_x, min_y, max_x, max_y, width, height, x, y;
-  int index;
-  Bool need_clear;
-  SeatCursor *cursor;
-
-  cursor = CursorFromRole (role);
-
-  if (SubcompositorIsEmpty (cursor->subcompositor))
-    /* The subcompositor is empty.  Don't set up the cursor
-       pixmap.  */
-    return False;
-
-  /* First, compute the bounds of the subcompositor.  */
-  SubcompositorBounds (cursor->subcompositor,
-		       &min_x, &min_y, &max_x, &max_y);
-
-  /* Then, its width and height.  */
-  width = max_x - min_x + 1;
-  height = max_y - min_y + 1;
-
-  /* If the cursor hotspot extends outside width and height, extend
-     the picture.  */
-  ComputeHotspot (cursor, min_x, min_y, &x, &y);
-
-  if (x < 0 || y < 0 || x >= width || y >= height)
-    {
-      if (x >= width)
-	width = x;
-      if (y >= width)
-	height = y;
-
-      if (x < 0)
-	width += -x;
-      if (y < 0)
-	height += -y;
-
-      need_clear = True;
-    }
-  else
-    need_clear = False;
-
-  if (cursor->cursor_ring)
-    /* If the width or height of the cursor ring changed, resize its
-       contents.  */
-    ResizeCursorRing (cursor->cursor_ring, width, height);
-  else
-    /* Otherwise, there is not yet a cursor ring.  Create one.  */
-    cursor->cursor_ring = MakeCursorRing (width, height);
-
-  /* Get an unused cursor from the cursor ring.  */
-  index = GetUnusedCursor (cursor->cursor_ring);
-  XLAssert (index != CursorRingBusy);
-
-  /* Set it as the cursor being used.  */
-  cursor->cursor_ring->used = index;
-
-  /* Get the target and pixmap.  */
-  target = cursor->cursor_ring->targets[index];
-  pixmap = cursor->cursor_ring->pixmaps[index];
-
-  /* If the bounds extend beyond the subcompositor, clear the
-     picture.  */
-  if (need_clear)
-    RenderClearRectangle (target, 0, 0, width, height);
-
-  /* Garbage the subcompositor, since cursor contents are not
-     preserved.  */
-  SubcompositorGarbage (cursor->subcompositor);
-
-  /* Set the right transform if the hotspot is negative.  */
-  SubcompositorSetProjectiveTransform (cursor->subcompositor,
-				       MAX (0, -x), MAX (0, -x));
-
-  /* Attach the rendering target.  */
-  SubcompositorSetTarget (cursor->subcompositor, &target);
-
-  /* Set the subframe target and pixmap to the target and pixmap in
-     use.  */
-  cursor_subframe_target = target;
-  cursor_subframe_pixmap = pixmap;
-
-  /* Return True to let the drawing proceed.  */
-  return True;
-}
-
 static void
-EndSubframe (Surface *surface, Role *role)
+SubsurfaceUpdate (Surface *surface, Role *role)
 {
   SeatCursor *cursor;
-  int min_x, min_y, max_x, max_y;
 
   cursor = CursorFromRole (role);
 
-  if (cursor_subframe_pixmap != None)
+  /* A desync subsurface's contents changed.  Update the cursor
-    {
+     again.  */
-      /* First, compute the bounds of the subcompositor.  */
+  UpdateCursorFromSubcompositor (cursor);
-      SubcompositorBounds (cursor->subcompositor,
-			   &min_x, &min_y, &max_x, &max_y);
-
-      /* Apply the cursor.  */
-      ApplyCursor (cursor, cursor_subframe_target, min_x, min_y);
-
-      /* Finally, clear the target.  */
-      SubcompositorSetTarget (cursor->subcompositor, NULL);
-    }
-  else
-    ApplyEmptyCursor (cursor);
-
-  cursor_subframe_pixmap = None;
 }
 
 static void
@@ -1422,8 +1316,7 @@ MakeCurrentCursor (Seat *seat, Surface *surface, int x, int y)
   role->role.funcs.teardown = Teardown;
   role->role.funcs.setup = Setup;
   role->role.funcs.release_buffer = ReleaseBuffer;
-  role->role.funcs.subframe = Subframe;
+  role->role.funcs.subsurface_update = SubsurfaceUpdate;
-  role->role.funcs.end_subframe = EndSubframe;
 
   /* Set up the subcompositor.  */
 
@@ -1759,8 +1652,6 @@ HandleBind (struct wl_client *client, void *data,
 	    uint32_t version, uint32_t id)
 {
   struct wl_resource *resource;
-  char *name;
-  size_t length;
   Seat *seat;
 
   seat = data;
@@ -1779,14 +1670,8 @@ HandleBind (struct wl_client *client, void *data,
   wl_seat_send_capabilities (resource, (WL_SEAT_CAPABILITY_POINTER
 					| WL_SEAT_CAPABILITY_KEYBOARD));
 
-  length = snprintf (NULL, 0, "X11 master device %d %d",
-		     seat->master_pointer, seat->master_keyboard);
-  name = alloca (length + 1);
-  snprintf (name, length + 1, "X11 master device %d %d",
-	    seat->master_pointer, seat->master_keyboard);
-
   if (wl_resource_get_version (resource) > 2)
-    wl_seat_send_name (resource, name);
+    wl_seat_send_name (resource, seat->name);
 
   RetainSeat (data);
 }
@@ -1835,6 +1720,7 @@ MakeSeatForDevicePair (int master_keyboard, int master_pointer,
   seat = XLCalloc (1, sizeof *seat);
   seat->master_keyboard = master_keyboard;
   seat->master_pointer = master_pointer;
+  seat->name = XLStrdup (pointer_info->name);
   seat->global = wl_global_create (compositor.wl_display,
 				   &wl_seat_interface, 8,
 				   seat, HandleBind);
@@ -4853,6 +4739,9 @@ StartResizeTracking (Seat *seat, Surface *surface, uint32_t serial,
 {
   WhatEdge type;
 
+  /* Seat cannot be NULL here, but -Wanalyzer disagrees.  */
+  XLAssert (seat != NULL);
+
   if (seat != IdentifySeat (&type, serial))
     return False;
 

subsurface.c

@@ -618,34 +618,19 @@ AfterParentCommit (Surface *surface, void *data)
   subsurface->pending_substate.flags = 0;
 }
 
-static Bool
-Subframe (Surface *surface, Role *role)
-{
-  Subsurface *subsurface;
-
-  subsurface = SubsurfaceFromRole (role);
-
-  if (!subsurface->parent || !subsurface->parent->role
-      || !subsurface->parent->role->funcs.subframe)
-    return True;
-
-  return subsurface->parent->role->funcs.subframe (subsurface->parent,
-						   subsurface->parent->role);
-}
-
 static void
-EndSubframe (Surface *surface, Role *role)
+SubsurfaceUpdate (Surface *surface, Role *role)
 {
   Subsurface *subsurface;
 
   subsurface = SubsurfaceFromRole (role);
 
   if (!subsurface->parent || !subsurface->parent->role
-      || !subsurface->parent->role->funcs.end_subframe)
+      || !subsurface->parent->role->funcs.subsurface_update)
     return;
 
-  subsurface->parent->role->funcs.end_subframe (subsurface->parent,
+  subsurface->parent->role->funcs.subsurface_update (subsurface->parent,
-						subsurface->parent->role);
+						     subsurface->parent->role);
 }
 
 static Window
@@ -704,15 +689,9 @@ Commit (Surface *surface, Role *role)
 
   if (!subsurface->synchronous)
     {
-      /* If the surface is asynchronous, draw this subframe now.
+      /* Tell the parent that a subsurface changed.  It should then do
-	 Otherwise it is synchronous, so we should wait for the
+	 whatever is appropriate to update the subsurface.  */
-	 toplevel to end the frame.  */
+      SubsurfaceUpdate (surface, role);
-
-      if (Subframe (surface, role))
-	{
-	  SubcompositorUpdate (subcompositor);
-	  EndSubframe (surface, role);
-	}
 
       /* If the size changed, update the outputs this surface is in
 	 the scanout area of.  */
@@ -845,14 +824,11 @@ Teardown (Surface *surface, Role *role)
 
       /* According to the spec, this removal should take effect
 	 immediately.  */
-      if (subcompositor
+      if (subcompositor)
-	  && Subframe (surface, role))
+        SubsurfaceUpdate (surface, role);
-	{
-	  SubcompositorUpdate (subcompositor);
-	  EndSubframe (surface, role);
-	}
     }
 
+  /* Destroy the backing data of the subsurface.  */
   DestroyBacking (subsurface);
 
   /* Update whether or not idle inhibition should continue.  */
@@ -971,8 +947,7 @@ GetSubsurface (struct wl_client *client, struct wl_resource *resource,
   subsurface->role.funcs.teardown = Teardown;
   subsurface->role.funcs.setup = Setup;
   subsurface->role.funcs.release_buffer = ReleaseBuffer;
-  subsurface->role.funcs.subframe = Subframe;
+  subsurface->role.funcs.subsurface_update = SubsurfaceUpdate;
-  subsurface->role.funcs.end_subframe = EndSubframe;
   subsurface->role.funcs.early_commit = EarlyCommit;
   subsurface->role.funcs.get_window = GetWindow;
   subsurface->role.funcs.rescale = Rescale;

surface.c

@@ -818,20 +818,10 @@ HandleScaleChanged (void *data, int new_scale)
      attached.  */
   subcompositor = ViewGetSubcompositor (surface->view);
 
-  if (subcompositor)
+  if (subcompositor
-    {
+      && surface->role
-      /* When updating stuff out-of-band, a subframe must be started
+      && surface->role->funcs.subsurface_update)
-	 around the update.  */
+    surface->role->funcs.subsurface_update (surface, surface->role);
-
-      if (surface->role && surface->role->funcs.subframe
-	  && surface->role->funcs.subframe (surface, surface->role))
-	{
-	  SubcompositorUpdate (subcompositor);
-
-	  if (surface->role && surface->role->funcs.end_subframe)
-	    surface->role->funcs.end_subframe (surface, surface->role);
-	}
-    }
 
   /* The scale has changed, so pointer confinement must be redone.  */
   XLPointerConstraintsReconfineSurface (surface);

xdg_surface.c

@@ -630,6 +630,33 @@ IsRoleMapped (XdgRole *role)
 					     role->impl);
 }
 
+/* Check if a frame can be drawn.  Return True if it is okay to call
+   SubcompositorUpdate, or schedule a frame after the current frame is
+   drawn and return False.  */
+
+static Bool
+CheckFrame (XdgRole *role)
+{
+  if (XLFrameClockFrameInProgress (role->clock))
+    {
+      if (XLFrameClockCanBatch (role->clock))
+	/* But if we can squeeze the frame inside the vertical
+	   blanking period, or a frame is in progress but EndFrame has
+	   not yet been called, go ahead.  */
+	return True;
+
+      role->state |= StateLateFrame;
+      role->state &= ~StatePendingFrameCallback;
+
+      if (role->state & StateWaitingForAckConfigure)
+	role->state &= ~StateLateFrameAcked;
+
+      return False;
+    }
+
+  return True;
+}
+
 static void
 Commit (Surface *surface, Role *role)
 {
@@ -682,42 +709,21 @@ Commit (Surface *surface, Role *role)
       xdg_role->state &= ~StateWaitingForAckCommit;
     }
 
-  /* If the window is unmapped, skip all of this code!  Once the
-     window is mapped again, the compositor will send _NET_FRAME_DRAWN
-     should a frame still be in progress.  */
-  if (!IsRoleMapped (xdg_role))
-    goto start_drawing;
-
   /* If the frame clock is frozen but we are no longer waiting for the
      configure event to be acknowledged by the client, unfreeze the
      frame clock.  */
   if (!(xdg_role->state & StateWaitingForAckConfigure))
     Unfreeze (xdg_role);
 
-  /* A frame is already in progress, so instead say that an urgent
+  /* Now, check if a frame can be drawn, or schedule a frame to be
-     update is needed immediately after the frame completes.  In any
+     drawn after this one completes.  */
-     case, don't run frame callbacks upon buffer release anymore.  */
-  if (XLFrameClockFrameInProgress (xdg_role->clock))
-    {
-      if (XLFrameClockCanBatch (xdg_role->clock))
-	/* But if we can squeeze the frame inside the vertical
-	   blanking period, or a frame is in progress but EndFrame has
-	   not yet been called, go ahead.  */
-	goto start_drawing;
 
-      xdg_role->state |= StateLateFrame;
+  if (!CheckFrame (xdg_role))
-      xdg_role->state &= ~StatePendingFrameCallback;
+    /* The frame cannot be drawn, because the compositor has not yet
-
+       drawn a previous frame.  */
-      if (xdg_role->state & StateWaitingForAckConfigure)
+    return;
-	xdg_role->state &= ~StateLateFrameAcked;
-      else
-	xdg_role->state |= StateLateFrameAcked;
-
-      return;
-    }
-
- start_drawing:
 
+  /* The frame can be drawn, so update the window contents now.  */
   SubcompositorUpdate (xdg_role->subcompositor);
 
   /* Do not end frames explicitly.  Instead, wait for the
@@ -871,52 +877,26 @@ ReleaseBuffer (Surface *surface, Role *role, ExtBuffer *buffer)
     }
 }
 
-static Bool
+static void
-Subframe (Surface *surface, Role *role)
+SubsurfaceUpdate (Surface *surface, Role *role)
 {
   XdgRole *xdg_role;
 
   xdg_role = XdgRoleFromRole (role);
 
-  /* If the frame clock is frozen, return False.  */
+  /* If the frame clock is frozen, don't update anything.  */
   if (XLFrameClockIsFrozen (xdg_role->clock))
-    {
+    return;
-      /* However, run frame callbacks.  */
-      RunFrameCallbacksConditionally (xdg_role);
-      return False;
-    }
 
-  /* Similarly, return False if the role is unmapped.  */
+  /* If a frame is already in progress, return, but schedule a frame
-  if (!IsRoleMapped (xdg_role))
+     to be drawn later.  */
-    return False;
 
-  /* If a frame is already in progress, return False.  Then, require a
+  if (!CheckFrame (xdg_role))
-     late frame.  */
+    /* The frame cannot be drawn.  */
-  if (XLFrameClockFrameInProgress (xdg_role->clock))
+    return;
-    {
-      if (XLFrameClockCanBatch (xdg_role->clock))
-	/* But if we can squeeze the frame inside the vertical
-	   blanking period, or a frame is in progress but EndFrame has
-	   not yet been called, go ahead.  */
-	return True;
 
-      xdg_role->state |= StateLateFrame;
+  /* The frame can be drawn, so update the window contents.  */
-      xdg_role->state &= ~StatePendingFrameCallback;
+  SubcompositorUpdate (xdg_role->subcompositor);
-
-      if (xdg_role->state & StateWaitingForAckConfigure)
-	xdg_role->state &= ~StateLateFrameAcked;
-
-      return False;
-    }
-
-  return True;
-}
-
-static void
-EndSubframe (Surface *surface, Role *role)
-{
-  /* Don't end the frame here; instead, wait for the frame callback to
-     note that drawing the frame has finished.  */
 }
 
 static Window
@@ -1573,8 +1553,7 @@ XLGetXdgSurface (struct wl_client *client, struct wl_resource *resource,
   role->role.funcs.teardown = Teardown;
   role->role.funcs.setup = Setup;
   role->role.funcs.release_buffer = ReleaseBuffer;
-  role->role.funcs.subframe = Subframe;
+  role->role.funcs.subsurface_update = SubsurfaceUpdate;
-  role->role.funcs.end_subframe = EndSubframe;
   role->role.funcs.get_window = GetWindow;
   role->role.funcs.get_resize_dimensions = GetResizeDimensions;
   role->role.funcs.post_resize = PostResize;

xdg_toplevel.c

@@ -1173,7 +1173,8 @@ Unmap (XdgToplevel *toplevel)
   toplevel->min_width = 0;
   toplevel->min_height = 0;
 
-  memset (&toplevel->state, 0, sizeof toplevel->states);
+  memset (&toplevel->toplevel_state, 0,
+	  sizeof toplevel->toplevel_state);
 
   /* If there is a pending configure timer, remove it.  */
   if (toplevel->configuration_timer)