Minor improvements to cursor handling and picture renderer

* egl.c (TargetFromPixmap): Set new `IsPixmap' flag.
(FinishRender): Call glFinish on single-buffered pixmap surface.
* picture_renderer.c (HavePixmapFormat): New function.
(FindSupportedFormats): Check that the relevant pixmap format is
supported.
(DepthForFormat): Return bpp in new bpp argument.
(BufferFromShm): Adjust accordingly.
(GetScanlinePad): New function.
(Roundup): New macro.
(ValidateShmParams): Take into account bytes per pixel and
scanline pad.
(SetupMitShm): Require pixmap formats mandated by Wayland
specification.
(InitBufferFuncs): Initialize supported pixmap formats.
* seat.c (CursorRingElements, CursorRingBusy): New macros.
(struct _CursorRing): New struct.
(struct _SeatCursor): Assign cursor ring to seat cursor.
(MakeCursorRing, MaybeCreateCursor, GetUnusedCursor)
(FreeCursorRing, ResizCursorRing): New functions for managing a
ring of cursor render targets.
(UpdateCursorOutput, FreeCursor, UpdateCursorFromSubcompositor)
(ApplyEmptyCursor, Subframe, EndSubframe): Use cursors provided
by that ring to avoid creating new render targets while
animating cursors.

egl.c

@@ -151,6 +151,7 @@ struct _EglBuffer
 enum
   {
     SwapPreservesContents = 1,
+    IsPixmap 		  = 2,
   };
 
 struct _EglTarget
@@ -941,6 +942,11 @@ TargetFromPixmap (Pixmap pixmap)
   target->surface = ICreatePlatformPixmapSurface (egl_display, egl_config,
 						  &pixmap, NULL);
 
+  /* Mark the target as being a pixmap surface.  EGL pixmap surfaces
+     are always single-buffered, so we have to call glFinish
+     manually.  */
+  target->flags |= IsPixmap;
+
   /* Try enabling EGL_BUFFER_PRESERVED to preserve the color buffer
      post swap.  */
   if (TryPreserveOnSwap (target->surface))
@@ -1275,6 +1281,9 @@ FinishRender (RenderTarget target)
 
   egl_target = target.pointer;
 
+  if (egl_target->flags & IsPixmap)
+    glFinish ();
+  else
     /* This should also do glFinish.  */
     eglSwapBuffers (egl_display, egl_target->surface);
 }

picture_renderer.c

@@ -161,6 +161,12 @@ static Window round_trip_window;
 /* The opcode of the DRI3 extension.  */
 static int dri3_opcode;
 
+/* List of pixmap format values supported by the X server.  */
+static XPixmapFormatValues *x_formats;
+
+/* Number of those formats.  */
+static int num_x_formats;
+
 /* XRender and DRI3-based renderer.  A RenderTarget is just a
    Picture.  */
 
@@ -421,6 +427,21 @@ FindFormatMatching (XRenderPictFormat *format)
   return NULL;
 }
 
+static Bool
+HavePixmapFormat (int depth, int bpp)
+{
+  int i;
+
+  for (i = 0; i < num_x_formats; ++i)
+    {
+      if (x_formats[i].depth == depth
+	  && x_formats[i].bits_per_pixel == bpp)
+	return True;
+    }
+
+  return False;
+}
+
 static Bool
 FindSupportedFormats (void)
 {
@@ -443,6 +464,12 @@ FindSupportedFormats (void)
 
       info = FindFormatMatching (format);
 
+      /* See if the info's depth and bpp are supported.  */
+      if (info
+	  && !HavePixmapFormat (info->depth,
+				info->bits_per_pixel))
+	continue;
+
       if (info && !info->format)
 	info->format = format;
 
@@ -921,14 +948,16 @@ BufferFromDmaBufAsync (DmaBufAttributes *attributes,
 }
 
 static int
-DepthForFormat (uint32_t format)
+DepthForFormat (uint32_t format, int *bpp)
 {
   switch (format)
     {
     case WL_SHM_FORMAT_ARGB8888:
+      *bpp = 32;
       return 32;
 
     case WL_SHM_FORMAT_XRGB8888:
+      *bpp = 32;
       return 24;
 
     default:
@@ -959,9 +988,9 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
   xcb_shm_seg_t seg;
   Pixmap pixmap;
   Picture picture;
-  int fd, depth, format;
+  int fd, depth, format, bpp;
 
-  depth = DepthForFormat (attributes->format);
+  depth = DepthForFormat (attributes->format, &bpp);
   format = attributes->format;
 
   /* Duplicate the fd, since XCB closes file descriptors after sending
@@ -996,13 +1025,59 @@ BufferFromShm (SharedMemoryAttributes *attributes, Bool *error)
   return (RenderBuffer) picture;
 }
 
+static int
+GetScanlinePad (int depth)
+{
+  int i;
+
+  for (i = 0; i < num_x_formats; ++i)
+    {
+      if (x_formats[i].depth == depth)
+	return x_formats[i].scanline_pad;
+    }
+
+  /* This should never happen in practice.  */
+  return -1;
+}
+
+/* Roundup rounds up NBYTES to PAD.  PAD is a value that can appear as
+   the scanline pad.  Macro borrowed from Xlib, as usual for everyone
+   working with such images.  */
+#define Roundup(nbytes, pad) ((((nbytes) + ((pad) - 1)) / (pad)) * ((pad) >> 3))
+
 static Bool
 ValidateShmParams (uint32_t format, uint32_t width, uint32_t height,
 		   int32_t offset, int32_t stride, size_t pool_size)
 {
-  if (pool_size < offset || stride != width * 4
+  int bpp, depth;
-      || offset + stride * height > pool_size
+  long wanted_stride;
-      || offset < 0)
+  size_t total_size;
+
+  /* Obtain the depth and bpp.  */
+  depth = DepthForFormat (format, &bpp);
+  XLAssert (depth != -1);
+
+  /* If any signed values are negative, return.  */
+  if (offset < 0 || stride < 0)
+    return False;
+
+  /* Obtain width * bpp padded to the scanline pad.  Xlib or the X
+     server do not try to handle overflow here... */
+  wanted_stride = Roundup (width * (long) bpp,
+			   GetScanlinePad (depth));
+
+  /* Assume that size_t can hold int32_t.  */
+  total_size = offset;
+
+  /* Calculate the total data size.  */
+  if (IntMultiplyWrapv (stride, height, &total_size))
+    return False;
+
+  if (IntAddWrapv (offset, total_size, &total_size))
+    return False;
+
+  /* Verify that the stride is correct and the image fits.  */
+  if (stride != wanted_stride || total_size > pool_size)
     return False;
 
   return True;
@@ -1058,6 +1133,22 @@ SetupMitShm (void)
     }
 
   free (reply);
+
+  /* Now check that the mandatory image formats are supported.  */
+
+  if (!HavePixmapFormat (24, 32))
+    {
+      fprintf (stderr, "X server does not support pixmap format"
+	       " of depth 24 with 32 bits per pixel\n");
+      exit (1);
+    }
+
+  if (!HavePixmapFormat (32, 32))
+    {
+      fprintf (stderr, "X server does not support pixmap format"
+	       " of depth 32 with 32 bits per pixel\n");
+      exit (1);
+    }
 }
 
 static void
@@ -1067,6 +1158,17 @@ InitBufferFuncs (void)
   xcb_dri3_query_version_reply_t *reply;
   const xcb_query_extension_reply_t *ext;
 
+  /* Obtain the list of supported pixmap formats from the X
+     server.  */
+  x_formats = XListPixmapFormats (compositor.display, &num_x_formats);
+
+  if (!x_formats)
+    {
+      fprintf (stderr, "No pixmap formats could be retrieved from"
+	       " the X server\n");
+      return;
+    }
+
   /* Set up the MIT shared memory extension.  It is required to
      work.  */
   SetupMitShm ();

seat.c

@@ -104,6 +104,7 @@ typedef struct _ScrollValuator ScrollValuator;
 typedef struct _DestroyListener DestroyListener;
 typedef struct _DeviceInfo DeviceInfo;
 typedef struct _ModifierChangeCallback ModifierChangeCallback;
+typedef struct _CursorRing CursorRing;
 
 typedef enum _ResizeEdge ResizeEdge;
 typedef enum _WhatEdge WhatEdge;
@@ -166,6 +167,24 @@ static int resize_edges[] =
     ResizeAxisMove,
   };
 
+#define CursorRingElements	2
+#define CursorRingBusy		3
+
+struct _CursorRing
+{
+  /* The width and height of the RenderTargets within.  */
+  int width, height;
+
+  /* Array of render targets.  */
+  RenderTarget targets[CursorRingElements];
+
+  /* Array of pixmaps.  */
+  Pixmap pixmaps[CursorRingElements];
+
+  /* Index of target being used.  -1 means nothing is being used.  */
+  short used;
+};
+
 struct _DestroyListener
 {
   /* Function called when seat is destroyed.  */
@@ -202,6 +221,10 @@ struct _SeatCursor
   /* Whether or not this cursor is currently keeping the cursor clock
      active.  */
   Bool holding_cursor_clock;
+
+  /* Ring of render targets for cursors.  This allows updating the
+     cursor while not creating a new render target each time.  */
+  CursorRing *cursor_ring;
 };
 
 struct _ResizeDoneCallback
@@ -650,6 +673,107 @@ RetainSeat (Seat *seat)
   seat->refcount++;
 }
 
+static CursorRing *
+MakeCursorRing (int width, int height)
+{
+  CursorRing *ring;
+
+  ring = XLCalloc (1, sizeof *ring);
+  ring->width = width;
+  ring->height = height;
+  ring->used = -1;
+
+  return ring;
+}
+
+static void
+MaybeCreateCursor (CursorRing *ring, int index)
+{
+  XLAssert (index < CursorRingElements);
+
+  /* If the cursor has already been created, return.  */
+  if (ring->pixmaps[index])
+    return;
+
+  ring->pixmaps[index]
+    = XCreatePixmap (compositor.display,
+		     DefaultRootWindow (compositor.display),
+		     ring->width, ring->height,
+		     compositor.n_planes);
+  ring->targets[index]
+    = RenderTargetFromPixmap (ring->pixmaps[index]);
+}
+
+static int
+GetUnusedCursor (CursorRing *ring)
+{
+  int i;
+
+  for (i = 0; i < CursorRingElements; ++i)
+    {
+      if (ring->used != i)
+	{
+	  /* Create the cursor contents if they have not yet been
+	     created.  */
+	  MaybeCreateCursor (ring, i);
+
+	  return i;
+	}
+    }
+
+  return CursorRingBusy;
+}
+
+static void
+FreeCursorRing (CursorRing *ring)
+{
+  int i;
+
+  for (i = 0; i < CursorRingElements; ++i)
+    {
+      if (!ring->pixmaps[i])
+	/* This element wasn't created.  */
+	continue;
+
+      /* Free the target and pixmap.  */
+      RenderDestroyRenderTarget (ring->targets[i]);
+      XFreePixmap (compositor.display, ring->pixmaps[i]);
+    }
+
+  /* Free the ring itself.  */
+  XLFree (ring);
+}
+
+static void
+ResizeCursorRing (CursorRing *ring, int width, int height)
+{
+  int i;
+
+  if (width == ring->width && height == ring->height)
+    return;
+
+  /* Destroy the pixmaps currently in the cursor ring.  */
+
+  for (i = 0; i < CursorRingElements; ++i)
+    {
+      if (!ring->pixmaps[i])
+	/* This element wasn't created.  */
+	continue;
+
+      /* Free the target and pixmap.  */
+      RenderDestroyRenderTarget (ring->targets[i]);
+      XFreePixmap (compositor.display, ring->pixmaps[i]);
+
+      /* Mark this element as free.  */
+      ring->pixmaps[i] = None;
+    }
+
+  /* Reinitialize the cursor ring with new data.  */
+  ring->width = width;
+  ring->height = height;
+  ring->used = -1;
+}
+
 static void
 UpdateCursorOutput (SeatCursor *cursor, int root_x, int root_y)
 {
@@ -738,6 +862,10 @@ FreeCursor (SeatCursor *cursor)
 		    cursor->seat->master_pointer,
 		    window, InitDefaultCursor ());
 
+  /* And release the cursor ring.  */
+  if (cursor->cursor_ring)
+    FreeCursorRing (cursor->cursor_ring);
+
   /* Maybe release the cursor clock if it was active for this
      cursor.  */
   EndCursorClock (cursor);
@@ -912,10 +1040,10 @@ ApplyCursor (SeatCursor *cursor, RenderTarget target,
 static void
 UpdateCursorFromSubcompositor (SeatCursor *cursor)
 {
-  Pixmap pixmap;
   RenderTarget target;
   int min_x, min_y, max_x, max_y, width, height, x, y;
   Bool need_clear;
+  int index;
 
   /* First, compute the bounds of the subcompositor.  */
   SubcompositorBounds (cursor->subcompositor,
@@ -946,12 +1074,20 @@ UpdateCursorFromSubcompositor (SeatCursor *cursor)
   else
     need_clear = False;
 
-  /* This is unbelivably the only way to dynamically update the cursor
+  if (cursor->cursor_ring)
-     under X.  Rather inefficient with animated cursors.  */
+    /* If the width or height of the cursor ring changed, resize its
-  pixmap = XCreatePixmap (compositor.display,
+       contents.  */
-			  DefaultRootWindow (compositor.display),
+    ResizeCursorRing (cursor->cursor_ring, width, height);
-			  width, height, compositor.n_planes);
+  else
-  target = RenderTargetFromPixmap (pixmap);
+    /* 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);
+
+  /* Get the target and pixmap.  */
+  target = cursor->cursor_ring->targets[index];
 
   /* If the bounds extend beyond the subcompositor, clear the
      picture.  */
@@ -970,10 +1106,11 @@ UpdateCursorFromSubcompositor (SeatCursor *cursor)
   SubcompositorUpdate (cursor->subcompositor);
   SubcompositorSetTarget (cursor->subcompositor, NULL);
 
+  /* Apply the new cursor.  */
   ApplyCursor (cursor, target, min_x, min_y);
 
-  RenderDestroyRenderTarget (target);
+  /* Set it as the cursor being used.  */
-  XFreePixmap (compositor.display, pixmap);
+  cursor->cursor_ring->used = index;
 }
 
 static void
@@ -1000,6 +1137,10 @@ ApplyEmptyCursor (SeatCursor *cursor)
     XIDefineCursor (compositor.display,
 		    cursor->seat->master_pointer,
 		    window, InitDefaultCursor ());
+
+  if (cursor->cursor_ring)
+    /* This means no cursor in the ring is currently being used.  */
+    cursor->cursor_ring->used = -1;
 }
 
 static void
@@ -1071,6 +1212,7 @@ 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;
 
@@ -1110,12 +1252,24 @@ Subframe (Surface *surface, Role *role)
   else
     need_clear = False;
 
-  /* This is unbelivably the only way to dynamically update the cursor
+  if (cursor->cursor_ring)
-     under X.  Rather inefficient with animated cursors.  */
+    /* If the width or height of the cursor ring changed, resize its
-  pixmap = XCreatePixmap (compositor.display,
+       contents.  */
-			  DefaultRootWindow (compositor.display),
+    ResizeCursorRing (cursor->cursor_ring, width, height);
-			  width, height, compositor.n_planes);
+  else
-  target = RenderTargetFromPixmap (pixmap);
+    /* 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.  */
@@ -1159,19 +1313,12 @@ EndSubframe (Surface *surface, Role *role)
       /* Apply the cursor.  */
       ApplyCursor (cursor, cursor_subframe_target, min_x, min_y);
 
-      /* Then, free the temporary target.  */
-      RenderDestroyRenderTarget (cursor_subframe_target);
-
       /* Finally, clear the target.  */
       SubcompositorSetTarget (cursor->subcompositor, NULL);
     }
   else
     ApplyEmptyCursor (cursor);
 
-  if (cursor_subframe_pixmap)
-    XFreePixmap (compositor.display,
-		 cursor_subframe_pixmap);
-
   cursor_subframe_pixmap = None;
 }