Minor fixes to EGL renderer and toplevel resizing

* compositor.h (struct _RenderFuncs): Accept damage in
finish_render.
* egl.c (EglInitFuncs): Load eglSwapBuffersWithDamageEXT.
(FinishRender): Accept damage and swap only damaged area if
possible.
(UpdateBuffer): Always update textures.
* explicit_synchronization.c (XLSyncRelease): Fix comment.
* renderer.c (RenderFinishRender): Accept damage argument.
* subcompositor.c (ViewWidth, ViewHeight): Fix computation of
destination width and height when there is a content scale;
those values have already been adjusted into window coordinates.
(SubcompositorUpdate, SubcompositorExpose): Pass damage to
RenderFinishRender.
* surface.c (ApplyDamage): Fix some aspects of damage
calculation.
* xdg_surface.c (IsRoleMapped): Fix crash when destroying client
with subsurfaces.
(Commit, NoteBounds, ResizeForMap, XLXdgRoleSendConfigure)
(XLXdgRoleSetBoundsSize, XLXdgRoleResizeForMap): Really fix
window state reverting to maximized during unmaximization.  This
had two causes: the first was that the window geometry would be
set even before ack_configure, which was fixed by moving setting
the window geometry into NoteBounds, and the second was that
NoteBounds would sometimes resize the window back to its old
dimensions if a commit (or subsurface update) happened between
SetBoundsSize and the configure event arriving, also confusing
the window manager.
* xdg_toplevel.c (NoteConfigureTime): Fix coding style.
(HandleConfigureEvent): Call SetBoundsSize before posting the
configure event.

compositor.h

@@ -287,8 +287,9 @@ struct _RenderFuncs
   void (*composite) (RenderBuffer, RenderTarget, Operation, int, int,
 		     int, int, int, int, DrawParams *);
 
-  /* Finish rendering, and swap changes to display.  May be NULL.  */
+  /* Finish rendering, and swap changes in given damage to display.
-  void (*finish_render) (RenderTarget);
+     May be NULL.  */
+  void (*finish_render) (RenderTarget, pixman_region32_t *);
 
   /* Return the age of the target.  Value is a number not less than
      -1, describing the "age" of the contents of the target.
@@ -414,7 +415,7 @@ extern void RenderFillBoxesWithTransparency (RenderTarget, pixman_box32_t *,
 extern void RenderClearRectangle (RenderTarget, int, int, int, int);
 extern void RenderComposite (RenderBuffer, RenderTarget, Operation, int,
 			     int, int, int, int, int, DrawParams *);
-extern void RenderFinishRender (RenderTarget);
+extern void RenderFinishRender (RenderTarget, pixman_region32_t *);
 extern int RenderTargetAge (RenderTarget);
 extern RenderFence RenderImportFdFence (int, Bool *);
 extern void RenderWaitFence (RenderFence);

egl.c

@@ -334,6 +334,7 @@ static PFNEGLCLIENTWAITSYNCKHRPROC IClientWaitSync;
 static PFNEGLGETSYNCATTRIBKHRPROC IGetSyncAttrib;
 static PFNEGLWAITSYNCKHRPROC IWaitSync;
 static PFNEGLDUPNATIVEFENCEFDANDROIDPROC IDupNativeFenceFD;
+static PFNEGLSWAPBUFFERSWITHDAMAGEEXTPROC ISwapBuffersWithDamage;
 
 /* The EGL display handle.  */
 static EGLDisplay egl_display;
@@ -527,6 +528,8 @@ EglInitFuncs (void)
   LoadProc (GetSyncAttrib, "KHR", "EGL_KHR_fence_sync");
   LoadProc (WaitSync, "KHR", "EGL_KHR_wait_sync");
   LoadProc (DupNativeFenceFD, "ANDROID", "EGL_ANDROID_native_fence_sync");
+  LoadProc (SwapBuffersWithDamage, "EXT",
+	    "EGL_EXT_swap_buffers_with_damage");
 }
 
 static void
@@ -1441,17 +1444,39 @@ Composite (RenderBuffer buffer, RenderTarget target,
 }
 
 static void
-FinishRender (RenderTarget target)
+FinishRender (RenderTarget target, pixman_region32_t *damage)
 {
   EglTarget *egl_target;
+  EGLint *rects;
+  int nboxes, i;
+  pixman_box32_t *boxes;
 
   egl_target = target.pointer;
 
   if (egl_target->flags & IsPixmap)
     glFinish ();
-  else
+  else if (!ISwapBuffersWithDamage || !damage)
     /* This should also do glFinish.  */
     eglSwapBuffers (egl_display, egl_target->surface);
+  else
+    {
+      /* Do a swap taking the buffer damage into account.  First,
+	 convert the damage into cartesian coordinates.  */
+      boxes = pixman_region32_rectangles (damage, &nboxes);
+      rects = alloca (nboxes * 4 * sizeof *damage);
+
+      for (i = 0; i < nboxes; ++i)
+	{
+	  rects[i * 4 + 0] = boxes[i].x1;
+	  rects[i * 4 + 1] = egl_target->height - boxes[i].y2;
+	  rects[i * 4 + 2] = boxes[i].x2 - boxes[i].x1;
+	  rects[i * 4 + 3] = boxes[i].y2 - boxes[i].y1;
+	}
+
+      /* Next, swap buffers with the damage.  */
+      ISwapBuffersWithDamage (egl_display, egl_target->surface,
+			      rects, nboxes);
+    }
 }
 
 static int
@@ -2324,15 +2349,16 @@ UpdateBuffer (RenderBuffer buffer, pixman_region32_t *damage,
        upload the contents.  */
     EnsureTexture (egl_buffer);
   else if (!damage)
-    {
+    /* Upload all the contents to the buffer's texture if the buffer
-      /* Upload all the contents to the buffer's texture if the buffer
+       type requires manual updates.  Buffers backed by EGLImages do
-	 type requires manual updates.  Buffers backed by EGLImages do
+       not appear to need updates, since updates to the EGLImage are
-	 not need updates, since updates to the EGLImage are
+       automatically reflected in the texture.
-	 automatically reflected in the texture.  */
 
-      if (egl_buffer->u.type == ShmBuffer)
+       However, someone on #dri says calling
-	UpdateTexture (egl_buffer);
+       glEGLImageTargetTexture2DOES is still required and not doing so
-    }
+       may cause certain drivers to stop working in the future.  So
+       still do it for buffers backed by EGLImages.  */
+    UpdateTexture (egl_buffer);
   else if (pixman_region32_not_empty (damage))
     {
       switch (egl_buffer->u.type)
@@ -2344,8 +2370,9 @@ UpdateBuffer (RenderBuffer buffer, pixman_region32_t *damage,
 					params);
 	  break;
 
-	default:
+	case DmaBufBuffer:
-	  /* These buffers need no updates.  */
+	  /* See comment in !damage branch.  */
+	  UpdateTexture (egl_buffer);
 	  break;
 	}
     }

explicit_synchronization.c

@@ -404,10 +404,10 @@ XLSyncRelease (SyncRelease *release)
   Bool error;
   int fd;
 
-  /* Optimization ideas.  Every time a buffer is attached with a
+  /* Optimization ideas.  Every time an shm buffer is attached with a
      release object, create new finish_fence for the buffer, that is
-     signalled whenever the contents are drawn.  Then, just use that
+     signalled whenever the contents are uploaded.  Then, just use
-     fence here instead.  */
+     that fence here instead.  */
 
   error = False;
   fd = RenderGetFinishFence (&error);

renderer.c

@@ -139,10 +139,10 @@ RenderComposite (RenderBuffer source, RenderTarget target, Operation op,
 }
 
 void
-RenderFinishRender (RenderTarget target)
+RenderFinishRender (RenderTarget target, pixman_region32_t *damage)
 {
   if (render_funcs.finish_render)
-    render_funcs.finish_render (target);
+    render_funcs.finish_render (target, damage);
 }
 
 int

subcompositor.c

@@ -1634,8 +1634,7 @@ ViewWidth (View *view)
        view->dest_height can be fractional values.  When that happens,
        we simply use the ceiling and rely on the renderer to DTRT with
        scaling.  */
-    return ceil (view->dest_width
+    return ceil (view->dest_width);
-		 * GetContentScale (view->scale));
 
   width = XLBufferWidth (view->buffer);
 
@@ -1658,8 +1657,7 @@ ViewHeight (View *view)
        view->dest_height can be fractional values.  When that happens,
        we simply use the ceiling and rely on the renderer to DTRT with
        scaling.  */
-    return ceil (view->dest_height
+    return ceil (view->dest_height);
-		 * GetContentScale (view->scale));
 
   height = XLBufferHeight (view->buffer);
 
@@ -2504,7 +2502,12 @@ SubcompositorUpdate (Subcompositor *subcompositor)
   while (list != subcompositor->inferiors);
 
   /* Swap changes to display.  */
-  RenderFinishRender (subcompositor->target);
+
+  if (IsGarbaged (subcompositor) || age < 0 || age >= 3)
+    RenderFinishRender (subcompositor->target, NULL);
+  else
+    /* Swap changes to display based on the update region.  */
+    RenderFinishRender (subcompositor->target, &update_region);
 
  complete:
 
@@ -2699,7 +2702,7 @@ SubcompositorExpose (Subcompositor *subcompositor, XEvent *event)
   while (list != subcompositor->inferiors);
 
   /* Swap changes to display.  */
-  RenderFinishRender (subcompositor->target);
+  RenderFinishRender (subcompositor->target, NULL);
 }
 
 void

surface.c

@@ -836,12 +836,20 @@ ApplyDamage (Surface *surface)
   pixman_region32_t temp;
   int scale;
   float x_factor, y_factor;
+  double src_width, src_height;
+  double dest_width, dest_height;
 
   scale = GetEffectiveScale (surface->current_state.buffer_scale);
 
+  /* If no surface was attached, just return.  */
+  if (!surface->current_state.buffer)
+    return;
+
   /* N.B. that this must come after the scale is applied.  */
 
-  if (scale || surface->current_state.src_x != -1
+  if (scale || (surface->current_state.src_x != -1
+		&& surface->current_state.src_x != 0.0
+		&& surface->current_state.src_y != 0.0)
       || surface->current_state.dest_width != -1)
     {
       pixman_region32_init (&temp);
@@ -855,15 +863,33 @@ ApplyDamage (Surface *surface)
 
       /* If a viewport dest size is set, add that to the scale as
 	 well.  */
-      if (surface->current_state.src_width != -1)
+      if (surface->current_state.dest_width != -1)
 	{
-	  x_factor += (float) (surface->current_state.src_width
+	  if (surface->current_state.src_width != -1)
-			       / surface->current_state.dest_width);
+	    {
-	  y_factor += (float) (surface->current_state.src_height
+	      src_width = surface->current_state.src_width;
-			       / surface->current_state.dest_height);
+	      src_height = surface->current_state.src_height;
+	    }
+	  else
+	    {
+	      src_width = XLBufferWidth (surface->current_state.buffer);
+	      src_height = XLBufferHeight (surface->current_state.buffer);
+
+	      /* Now scale these buffer dimensions down by the buffer
+		 scale, so they can be turned into surface
+		 coordinates.  */
+	      src_width /= surface->current_state.buffer_scale;
+	      src_height /= surface->current_state.buffer_scale;
+	    }
+
+	  dest_width = surface->current_state.dest_width;
+	  dest_height = surface->current_state.dest_height;
+
+	  x_factor *= (float) (src_width / dest_width);
+	  y_factor *= (float) (src_height / dest_height);
 	}
 
-      if (x_factor != 1.0f && y_factor != 1.0f)
+      if (x_factor != 1.0f || y_factor != 1.0f)
 	XLScaleRegion (&temp, &surface->current_state.damage,
 		       x_factor, y_factor);
 
@@ -879,8 +905,7 @@ ApplyDamage (Surface *surface)
       pixman_region32_fini (&temp);
     }
   else
-    ViewDamage (surface->view,
+    ViewDamage (surface->view, &surface->current_state.damage);
-		&surface->current_state.damage);
 }
 
 static void

xdg_surface.c

@@ -38,6 +38,8 @@ enum
     StateWaitingForAckCommit	= (1 << 4),
     StateLateFrameAcked		= (1 << 5),
     StateMaybeConfigure		= (1 << 6),
+    StateDirtyFrameExtents	= (1 << 7),
+    StateTemporaryBounds	= (1 << 8),
   };
 
 typedef struct _XdgRole XdgRole;
@@ -631,6 +633,11 @@ Unfreeze (XdgRole *role)
 static Bool
 IsRoleMapped (XdgRole *role)
 {
+  if (!role->impl)
+    /* This can happen when destroying subsurfaces as part of client
+       cleanup.  */
+    return False;
+
   return role->impl->funcs.is_window_mapped (&role->role,
 					     role->impl);
 }
@@ -655,23 +662,36 @@ Commit (Surface *surface, Role *role)
 	= xdg_role->pending_state.window_geometry_width;
       xdg_role->current_state.window_geometry_height
 	= xdg_role->pending_state.window_geometry_height;
+
+#ifdef DEBUG_GEOMETRY_CALCULATION
+      fprintf (stderr, "Client set window geometry to: [%d %d %d %d]\n",
+	       xdg_role->current_state.window_geometry_x,
+	       xdg_role->current_state.window_geometry_y,
+	       xdg_role->current_state.window_geometry_width,
+	       xdg_role->current_state.window_geometry_height);
+#endif
+
+      /* Now, clear the "pending window geometry" flag.  */
+      xdg_role->state &= ~StatePendingWindowGeometry;
+
+      /* Next, set the "dirty frame extents" flag; this is then used
+	 to update the window geometry the next time the window is
+	 resized.  */
+      xdg_role->state |= StateDirtyFrameExtents;
     }
 
   xdg_role->impl->funcs.commit (role, surface,
 				xdg_role->impl);
 
-  if (xdg_role->state & StatePendingWindowGeometry)
-    {
-      if (xdg_role->impl->funcs.handle_geometry_change)
-	xdg_role->impl->funcs.handle_geometry_change (role,
-						      xdg_role->impl);
-
-      xdg_role->state &= ~StatePendingWindowGeometry;
-    }
-
   /* This flag means no commit has happened after an
      ack_configure.  */
-  xdg_role->state &= ~StateWaitingForAckCommit;
+  if (!(xdg_role->state & StateWaitingForAckConfigure))
+    {
+#ifdef DEBUG_GEOMETRY_CALCULATION
+      fprintf (stderr, "Client aknowledged commit\n");
+#endif
+      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
@@ -1102,12 +1122,19 @@ NoteBounds (void *data, int min_x, int min_y,
        still in effect.  */
     return;
 
+  if (role->state & StateTemporaryBounds)
+    return;
+
   /* Avoid resizing the window should its actual size not have
      changed.  */
 
   bounds_width = max_x - min_x + 1;
   bounds_height = max_y - min_y + 1;
 
+#ifdef DEBUG_GEOMETRY_CALCULATION
+  fprintf (stderr, "Noticed bounds: %d %d\n", bounds_width, bounds_height);
+#endif
+
   if (role->bounds_width != bounds_width
       || role->bounds_height != bounds_height)
     {
@@ -1121,6 +1148,18 @@ NoteBounds (void *data, int min_x, int min_y,
 						  bounds_width,
 						  bounds_height);
 
+      if (role->state & StateDirtyFrameExtents)
+	{
+	  /* Only handle window geometry changes once a commit happens
+	     and the window is really resized.  */
+
+	  if (role->impl->funcs.handle_geometry_change)
+	    role->impl->funcs.handle_geometry_change (&role->role,
+						      role->impl);
+
+	  role->state &= ~StateDirtyFrameExtents;
+	}
+
       XResizeWindow (compositor.display, role->window,
 		     bounds_width, bounds_height);
       run_reconstrain_callbacks = True;
@@ -1183,7 +1222,38 @@ ResizeForMap (XdgRole *role)
 
   SubcompositorBounds (role->subcompositor, &min_x,
 		       &min_y, &max_x, &max_y);
+
+  /* At this point, we are probably still waiting for ack_commit; as a
+     result, NoteBounds will not really resize the window.  */
   NoteBounds (role, min_x, min_y, max_x, max_y);
+
+#ifdef DEBUG_GEOMETRY_CALCULATION
+  fprintf (stderr, "ResizeForMap: %d %d\n",
+	   max_x - min_x + 1, max_y - min_y + 1);
+#endif
+
+  if (role->state & StateDirtyFrameExtents)
+    {
+      /* Only handle window geometry changes once a commit
+	 happens.  */
+
+      if (role->impl->funcs.handle_geometry_change)
+	role->impl->funcs.handle_geometry_change (&role->role,
+						  role->impl);
+
+      role->state &= ~StateDirtyFrameExtents;
+    }
+
+  /* Resize the window pre-map.  This should generate a
+     ConfigureNotify event once the resize completes.  */
+  XResizeWindow (compositor.display, role->window,
+		 max_x - min_x + 1, max_y - min_y + 1);
+
+  if (role->impl->funcs.note_window_resized)
+    role->impl->funcs.note_window_resized (&role->role,
+					   role->impl,
+					   max_x - min_x + 1,
+					   max_y - min_y + 1);
 }
 
 static void
@@ -1470,6 +1540,9 @@ XLXdgRoleSendConfigure (Role *role, uint32_t serial)
   xdg_role->state |= StateWaitingForAckConfigure;
   xdg_role->state |= StateWaitingForAckCommit;
 
+  /* See the comment under XLXdgRoleSetBoundsSize.  */
+  xdg_role->state &= ~StateTemporaryBounds;
+
   /* We know know that the ConfigureNotify event following any
      _NET_WM_SYNC_REQUEST event was accepted, so clear the maybe
      configure flag.  */
@@ -1573,6 +1646,17 @@ XLXdgRoleSetBoundsSize (Role *role, int bounds_width, int bounds_height)
   xdg_role = XdgRoleFromRole (role);
   xdg_role->bounds_width = bounds_width;
   xdg_role->bounds_height = bounds_height;
+
+  /* Now, a temporary bounds_width and bounds_height has been
+     recorded.  This means that if a configure event has not yet been
+     delivered, then any subsequent SubcompositorUpdate will cause
+     NoteBounds to resize back to the old width and height, confusing
+     the window manager and possibly causing it to maximize us.
+
+     Set a flag that tells NoteBounds to abstain from resizing the
+     window.  This flag is then cleared once a configure event is
+     delivered, or the next time the role is mapped.  */
+  xdg_role->state |= StateTemporaryBounds;
 }
 
 void
@@ -1690,6 +1774,12 @@ XLXdgRoleResizeForMap (Role *role)
   XdgRole *xdg_role;
 
   xdg_role = XdgRoleFromRole (role);
+
+  /* Clear the StateTemporaryBounds flag; it should not persist after
+     mapping, as a configure event is no longer guaranteed to be sent
+     if the toplevel is unmapped immediately after
+     XLXdgRoleSetBoundsSize.  */
+  xdg_role->state &= ~StateTemporaryBounds;
   ResizeForMap (xdg_role);
 }
 

xdg_toplevel.c

@@ -432,7 +432,6 @@ NoteConfigureTime (Timer *timer, void *data, struct timespec time)
 
   toplevel = data;
 
-
   /* If only the window state changed, call SendStates.  */
   if (!(toplevel->state & StatePendingConfigureSize))
     SendStates (toplevel);
@@ -1330,6 +1329,12 @@ HandleConfigureEvent (XdgToplevel *toplevel, XEvent *event)
 			 event->xconfigure.height))
     WriteStates (toplevel);
 
+  /* Also set the bounds width and height to avoid resizing the
+     window.  */
+  XLXdgRoleSetBoundsSize (toplevel->role,
+			  toplevel->width,
+			  toplevel->height);
+
   if (!MaybePostDelayedConfigure (toplevel, StatePendingConfigureSize))
     {
       XLXdgRoleCalcNewWindowSize (toplevel->role,
@@ -1346,12 +1351,6 @@ HandleConfigureEvent (XdgToplevel *toplevel, XEvent *event)
   toplevel->configure_width = toplevel->width;
   toplevel->configure_height = toplevel->height;
 
-  /* Also set the bounds width and height to avoid resizing
-     the window.  */
-  XLXdgRoleSetBoundsSize (toplevel->role,
-			  toplevel->width,
-			  toplevel->height);
-
   RecordStateSize (toplevel);
 
   return True;