Re: [PATCH xwayland 1/3] xwayland: Separate DamagePtr into separate window data

2019-01-16 Thread Adam Jackson
On Tue, 2019-01-15 at 23:21 +0100, Carlos Garnacho wrote:
> This will be dissociated in future commits to handle the cases
> where windows are being realized before there is a compositor
> handling redirection.
> 
> In that case, we still want the DamagePtr to be registered upfront
> on RealizeWindowProc before a corresponding xwl_window might be
> created. Most notably, it cannot be lazily created on
> SetWindowPixmapProc as damage accounting gets broken. The downside
> of this is that we may create no-op DamagePtrs for windows that are
> not redirected eventually.

I don't think this downside ever happens. For rootless mode we only
create the xwl_window for windows that are already RedirectDrawManual,
and the changes you're making to xwl_realize_window are well after that
check.

> 
> @@ -552,18 +597,9 @@ xwl_realize_window(WindowPtr window)
>  
>  wl_surface_set_user_data(xwl_window->surface, xwl_window);
>  
> -xwl_window->damage =
> -DamageCreate(damage_report, damage_destroy, DamageReportNonEmpty,
> - FALSE, screen, xwl_window);
> -if (xwl_window->damage == NULL) {
> -ErrorF("Failed creating damage\n");
> -goto err_surf;
> -}
> -
>  compRedirectWindow(serverClient, window, CompositeRedirectManual);
>  
> -DamageRegister(>drawable, xwl_window->damage);
> -DamageSetReportAfterOp(xwl_window->damage, TRUE);
> +register_damage(window);

register_damage() can fail, you've thrown away the error checking here.

- ajax

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xwayland 1/3] xwayland: Separate DamagePtr into separate window data

2019-01-15 Thread Carlos Garnacho
This will be dissociated in future commits to handle the cases
where windows are being realized before there is a compositor
handling redirection.

In that case, we still want the DamagePtr to be registered upfront
on RealizeWindowProc before a corresponding xwl_window might be
created. Most notably, it cannot be lazily created on
SetWindowPixmapProc as damage accounting gets broken. The downside
of this is that we may create no-op DamagePtrs for windows that are
not redirected eventually.

Signed-off-by: Carlos Garnacho 
---
 hw/xwayland/xwayland.c | 69 --
 hw/xwayland/xwayland.h |  1 -
 2 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 289683b6e..7f70b950d 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -132,6 +132,7 @@ ddxProcessArgument(int argc, char *argv[], int i)
 static DevPrivateKeyRec xwl_window_private_key;
 static DevPrivateKeyRec xwl_screen_private_key;
 static DevPrivateKeyRec xwl_pixmap_private_key;
+static DevPrivateKeyRec xwl_damage_private_key;
 
 static struct xwl_window *
 xwl_window_get(WindowPtr window)
@@ -374,8 +375,14 @@ xwl_cursor_confined_to(DeviceIntPtr device,
 static void
 damage_report(DamagePtr pDamage, RegionPtr pRegion, void *data)
 {
-struct xwl_window *xwl_window = data;
-struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
+WindowPtr window = data;
+struct xwl_window *xwl_window = xwl_window_get(window);
+struct xwl_screen *xwl_screen;
+
+if (!xwl_window)
+return;
+
+xwl_screen = xwl_window->xwl_screen;
 
 #ifdef GLAMOR_HAS_GBM
 if (xwl_window->present_flipped) {
@@ -397,6 +404,44 @@ damage_destroy(DamagePtr pDamage, void *data)
 {
 }
 
+static Bool
+register_damage(WindowPtr window)
+{
+DamagePtr damage;
+
+damage = DamageCreate(damage_report, damage_destroy, DamageReportNonEmpty,
+  FALSE, window->drawable.pScreen, window);
+if (damage == NULL) {
+ErrorF("Failed creating damage\n");
+return FALSE;
+}
+
+DamageRegister(>drawable, damage);
+DamageSetReportAfterOp(damage, TRUE);
+
+dixSetPrivate(>devPrivates, _damage_private_key, damage);
+
+return TRUE;
+}
+
+static void
+unregister_damage(WindowPtr window)
+{
+DamagePtr damage;
+
+damage = dixLookupPrivate(>devPrivates, _damage_private_key);
+DamageUnregister(damage);
+DamageDestroy(damage);
+
+dixSetPrivate(>devPrivates, _damage_private_key, NULL);
+}
+
+static DamagePtr
+window_get_damage(WindowPtr window)
+{
+return dixLookupPrivate(>devPrivates, _damage_private_key);
+}
+
 static void
 shell_surface_ping(void *data,
struct wl_shell_surface *shell_surface, uint32_t serial)
@@ -552,18 +597,9 @@ xwl_realize_window(WindowPtr window)
 
 wl_surface_set_user_data(xwl_window->surface, xwl_window);
 
-xwl_window->damage =
-DamageCreate(damage_report, damage_destroy, DamageReportNonEmpty,
- FALSE, screen, xwl_window);
-if (xwl_window->damage == NULL) {
-ErrorF("Failed creating damage\n");
-goto err_surf;
-}
-
 compRedirectWindow(serverClient, window, CompositeRedirectManual);
 
-DamageRegister(>drawable, xwl_window->damage);
-DamageSetReportAfterOp(xwl_window->damage, TRUE);
+register_damage(window);
 
 dixSetPrivate(>devPrivates, _window_private_key, xwl_window);
 xorg_list_init(_window->link_damage);
@@ -627,8 +663,7 @@ xwl_unrealize_window(WindowPtr window)
 
 wl_surface_destroy(xwl_window->surface);
 xorg_list_del(_window->link_damage);
-DamageUnregister(xwl_window->damage);
-DamageDestroy(xwl_window->damage);
+unregister_damage(window);
 if (xwl_window->frame_callback)
 wl_callback_destroy(xwl_window->frame_callback);
 
@@ -690,7 +725,7 @@ xwl_window_post_damage(struct xwl_window *xwl_window)
 
 assert(!xwl_window->frame_callback);
 
-region = DamageRegion(xwl_window->damage);
+region = DamageRegion(window_get_damage(xwl_window->window));
 pixmap = (*xwl_screen->screen->GetWindowPixmap) (xwl_window->window);
 
 #ifdef XWL_HAS_GLAMOR
@@ -727,7 +762,7 @@ xwl_window_post_damage(struct xwl_window *xwl_window)
 wl_callback_add_listener(xwl_window->frame_callback, _listener, 
xwl_window);
 
 wl_surface_commit(xwl_window->surface);
-DamageEmpty(xwl_window->damage);
+DamageEmpty(window_get_damage(xwl_window->window));
 
 xorg_list_del(_window->link_damage);
 }
@@ -960,6 +995,8 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 return FALSE;
 if (!dixRegisterPrivateKey(_pixmap_private_key, PRIVATE_PIXMAP, 0))
 return FALSE;
+if (!dixRegisterPrivateKey(_damage_private_key, PRIVATE_WINDOW, 0))
+return FALSE;
 
 dixSetPrivate(>devPrivates, _screen_private_key, xwl_screen);
 xwl_screen->screen = pScreen;
diff --git a/hw/xwayland/xwayland.h