Hi

I'm testing wayland protocol with e. And I found a problem with e_comp_wl. As I mentioned on the subject of this email, all wl_surface and wl_subsurface's requests are ignored until e_client is created.

Is it intended behavior or a bug? It looks bug to me.

In case of weston, weston_surface surface request data. And, weston_surface is created when wl_compositor.create_surface is called. It makes possible all data is safe. However, because enlightenment creates e_clientat some points after create_surface is called, it loses client's requests. looks e_client is not proper position to save surface request data. Of course, enlightenment is different with weston. But I think losing(ignoring) client request is problem obviously, isn't it?

In _e_comp_wl_compositor_cb_surface_create(), enlightenment creates e_pixmap. Can we move surface request data from E_Client(E_Comp_Wl_Client_Data, actually) to E_Pixmap?

I'm attaching my patch to help you understand what I am thinking. This patch only considers wl_surface's requests as example. Maybe we need to consider wl_subsurface's requests and more things. And also, to help you understand this problem, you can find a bug by following below steps.

1) run enlightenment as wayland server.
2) run weston-flower
3) move cursor over weston-flower

result: no cursor image shown. It disappears. If you move cursor outside of weston-flower and move it over weston-flower again, now you can see cursor. (There is another problem here, cursor doesn't move)

Regards
Boram
>From 3a279e889296aab5c99c9e678b252b06a2115c74 Mon Sep 17 00:00:00 2001
From: Boram Park <[email protected]>
Date: Thu, 26 Mar 2015 11:06:44 +0900
Subject: [PATCH] move pending data from E_Comp_Wl_Client_Data to E_Pixmap

Summary:
e_pixmap is created when surface is created. e_client is created
when shell surface is created or cursor surface is set. Before
shell(cursor) surface is created, all surface data can't be saved
because e_client doesn't exist. To avoid this problem, saving pending
data of surface to e_pixmap will be best than saving to e_client.
---
 src/bin/e_comp_wl.c |  144 +++++++++++++++++++++++++++++++--------------------
 src/bin/e_comp_wl.h |    1 -
 src/bin/e_pixmap.c  |   16 ++++++
 src/bin/e_pixmap.h  |    1 +
 4 files changed, 106 insertions(+), 56 deletions(-)

diff --git a/src/bin/e_comp_wl.c b/src/bin/e_comp_wl.c
index 9be9493..61909a9 100644
--- a/src/bin/e_comp_wl.c
+++ b/src/bin/e_comp_wl.c
@@ -1010,7 +1010,8 @@ _e_comp_wl_surface_state_commit(E_Client *ec, E_Comp_Wl_Surface_State *state)
 
    if (state->new_attach)
      {
-        _e_comp_wl_surface_state_size_update(ec, &ec->comp_data->pending);
+        E_Comp_Wl_Surface_State *pending = e_pixmap_pending_get(ec->pixmap);
+        _e_comp_wl_surface_state_size_update(ec, pending);
 
         if (ec->changes.pos)
           e_comp_object_frame_xy_adjust(ec->frame, ec->x, ec->y, &x, &y);
@@ -1157,12 +1158,10 @@ static void
 _e_comp_wl_surface_cb_attach(struct wl_client *client EINA_UNUSED, struct wl_resource *resource, struct wl_resource *buffer_resource, int32_t sx, int32_t sy)
 {
    E_Pixmap *ep;
-   E_Client *ec;
    E_Comp_Wl_Buffer *buffer = NULL;
+   E_Comp_Wl_Surface_State *pending;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
-   if (!(ec = e_pixmap_client_get(ep))) return;
-   if (e_object_is_del(E_OBJECT(ec))) return;
 
    if (buffer_resource)
      {
@@ -1174,36 +1173,37 @@ _e_comp_wl_surface_cb_attach(struct wl_client *client EINA_UNUSED, struct wl_res
           }
      }
 
-   _e_comp_wl_surface_state_buffer_set(&ec->comp_data->pending, buffer);
+   pending = e_pixmap_pending_get(ep);
+
+   _e_comp_wl_surface_state_buffer_set(pending, buffer);
 
-   ec->comp_data->pending.sx = sx;
-   ec->comp_data->pending.sy = sy;
-   ec->comp_data->pending.new_attach = EINA_TRUE;
+   pending->sx = sx;
+   pending->sy = sy;
+   pending->new_attach = EINA_TRUE;
 }
 
 static void
 _e_comp_wl_surface_cb_damage(struct wl_client *client EINA_UNUSED, struct wl_resource *resource, int32_t x, int32_t y, int32_t w, int32_t h)
 {
    E_Pixmap *ep;
-   E_Client *ec;
    Eina_Rectangle *dmg = NULL;
+   E_Comp_Wl_Surface_State *pending;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
-   if (!(ec = e_pixmap_client_get(ep))) return;
-   if (e_object_is_del(E_OBJECT(ec))) return;
-
    if (!(dmg = eina_rectangle_new(x, y, w, h))) return;
 
-   ec->comp_data->pending.damages =
-     eina_list_append(ec->comp_data->pending.damages, dmg);
+   pending = e_pixmap_pending_get(ep);
+   pending->damages = eina_list_append(pending->damages, dmg);
 }
 
 static void
 _e_comp_wl_frame_cb_destroy(struct wl_resource *resource)
 {
+   E_Pixmap *ep;
    E_Client *ec;
 
-   if (!(ec = wl_resource_get_user_data(resource))) return;
+   if (!(ep = wl_resource_get_user_data(resource))) return;
+   if (!(ec = e_pixmap_client_get(ep))) return;
    if (e_object_is_del(E_OBJECT(ec))) return;
 
    ec->comp_data->frames =
@@ -1214,12 +1214,10 @@ static void
 _e_comp_wl_surface_cb_frame(struct wl_client *client, struct wl_resource *resource, uint32_t callback)
 {
    E_Pixmap *ep;
-   E_Client *ec;
    struct wl_resource *res;
+   E_Comp_Wl_Surface_State *pending;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
-   if (!(ec = e_pixmap_client_get(ep))) return;
-   if (e_object_is_del(E_OBJECT(ec))) return;
 
    /* create frame callback */
    if (!(res =
@@ -1229,21 +1227,21 @@ _e_comp_wl_surface_cb_frame(struct wl_client *client, struct wl_resource *resour
         return;
      }
 
-   wl_resource_set_implementation(res, NULL, ec, _e_comp_wl_frame_cb_destroy);
+   pending = e_pixmap_pending_get(ep);
+   pending->frames = eina_list_prepend(pending->frames, res);
 
-   ec->comp_data->pending.frames =
-     eina_list_prepend(ec->comp_data->pending.frames, res);
+   wl_resource_set_implementation(res, NULL, NULL, _e_comp_wl_frame_cb_destroy);
 }
 
 static void
 _e_comp_wl_surface_cb_opaque_region_set(struct wl_client *client EINA_UNUSED, struct wl_resource *resource, struct wl_resource *region_resource)
 {
    E_Pixmap *ep;
-   E_Client *ec;
+   E_Comp_Wl_Surface_State *pending;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
-   if (!(ec = e_pixmap_client_get(ep))) return;
-   if (e_object_is_del(E_OBJECT(ec))) return;
+
+   pending = e_pixmap_pending_get(ep);
 
    if (region_resource)
      {
@@ -1252,14 +1250,14 @@ _e_comp_wl_surface_cb_opaque_region_set(struct wl_client *client EINA_UNUSED, st
         if (!(tmp = wl_resource_get_user_data(region_resource)))
           return;
 
-        eina_tiler_union(ec->comp_data->pending.opaque, tmp);
+        eina_tiler_union(pending->opaque, tmp);
      }
    else
      {
-        if (ec->comp_data->pending.opaque)
+        if (pending->opaque)
           {
-             eina_tiler_clear(ec->comp_data->pending.opaque);
-             /* eina_tiler_free(ec->comp_data->pending.opaque); */
+             eina_tiler_clear(pending->opaque);
+             /* eina_tiler_free(pending->opaque); */
           }
      }
 }
@@ -1268,11 +1266,11 @@ static void
 _e_comp_wl_surface_cb_input_region_set(struct wl_client *client EINA_UNUSED, struct wl_resource *resource, struct wl_resource *region_resource)
 {
    E_Pixmap *ep;
-   E_Client *ec;
+   E_Comp_Wl_Surface_State *pending;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
-   if (!(ec = e_pixmap_client_get(ep))) return;
-   if (e_object_is_del(E_OBJECT(ec))) return;
+
+   pending = e_pixmap_pending_get(ep);
 
    if (region_resource)
      {
@@ -1281,12 +1279,21 @@ _e_comp_wl_surface_cb_input_region_set(struct wl_client *client EINA_UNUSED, str
         if (!(tmp = wl_resource_get_user_data(region_resource)))
           return;
 
-        eina_tiler_union(ec->comp_data->pending.input, tmp);
+        eina_tiler_union(pending->input, tmp);
      }
    else
      {
-        eina_tiler_rect_add(ec->comp_data->pending.input,
-                            &(Eina_Rectangle){0, 0, ec->client.w, ec->client.h});
+        E_Client *ec = e_pixmap_client_get(ep);
+
+        if (!ec || e_object_is_del(E_OBJECT(ec)))
+           eina_tiler_clear(pending->input);
+        else
+           /* Do we need below? if region_resource is null, does it mean that
+            * user wants to set whole client area to input region? In case of
+            * opaque_region, if NULL, opaque region is cleared.
+            */
+           eina_tiler_rect_add(pending->input,
+                               &(Eina_Rectangle){0, 0, ec->client.w, ec->client.h});
      }
 }
 
@@ -1298,6 +1305,10 @@ _e_comp_wl_surface_cb_commit(struct wl_client *client EINA_UNUSED, struct wl_res
    Eina_List *l;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
+
+   /* ec should be created before commit. if no ec, it means that a surface
+    * isn't ready to be shown on screen.
+    */
    if (!(ec = e_pixmap_client_get(ep))) return;
    if (e_object_is_del(E_OBJECT(ec))) return;
 
@@ -1341,10 +1352,15 @@ static void
 _e_comp_wl_surface_destroy(struct wl_resource *resource)
 {
    E_Pixmap *ep;
+   E_Comp_Wl_Surface_State *pending;
    E_Client *ec;
 
    if (!(ep = wl_resource_get_user_data(resource))) return;
 
+   /* finish the pending state of pixmap */
+   pending = e_pixmap_pending_get(ep);
+   _e_comp_wl_surface_state_finish(pending);
+
    /* try to get the e_client from this pixmap */
    if (!(ec = e_pixmap_client_get(ep)))
      return;
@@ -1362,6 +1378,7 @@ _e_comp_wl_compositor_cb_surface_create(struct wl_client *client, struct wl_reso
 {
    struct wl_resource *res;
    E_Pixmap *ep;
+   E_Comp_Wl_Surface_State *pending;
    uint64_t win;
    pid_t pid;
 
@@ -1398,6 +1415,10 @@ _e_comp_wl_compositor_cb_surface_create(struct wl_client *client, struct wl_reso
    /* set reference to pixmap so we can fetch it later */
    wl_resource_set_user_data(res, ep);
 
+   /* initialize the pending state of pixmap */
+   pending = e_pixmap_pending_get(ep);
+   _e_comp_wl_surface_state_init(pending, 0, 0);
+
    /* emit surface create signal */
    wl_signal_emit(&e_comp->wl_comp_data->signals.surface.create, res);
 }
@@ -1598,6 +1619,7 @@ _e_comp_wl_subsurface_commit_to_cache(E_Client *ec)
 {
    E_Comp_Client_Data *cdata;
    E_Comp_Wl_Subsurf_Data *sdata;
+   E_Comp_Wl_Surface_State *pending;
    struct wl_resource *cb;
    Eina_List *l;
    Eina_Iterator *itr;
@@ -1608,43 +1630,45 @@ _e_comp_wl_subsurface_commit_to_cache(E_Client *ec)
 
    DBG("Subsurface Commit to Cache");
 
+   pending = e_pixmap_pending_get(ec->pixmap);
+
    /* move pending damage to cached */
-   EINA_LIST_FOREACH(cdata->pending.damages, l, rect)
-     eina_list_move(&sdata->cached.damages, &cdata->pending.damages, rect);
+   EINA_LIST_FOREACH(pending->damages, l, rect)
+     eina_list_move(&sdata->cached.damages, &pending->damages, rect);
 
-   if (cdata->pending.new_attach)
+   if (pending->new_attach)
      {
         sdata->cached.new_attach = EINA_TRUE;
         _e_comp_wl_surface_state_buffer_set(&sdata->cached,
-                                            cdata->pending.buffer);
+                                            pending->buffer);
         e_comp_wl_buffer_reference(&sdata->cached_buffer_ref,
-                                   cdata->pending.buffer);
+                                   pending->buffer);
      }
 
-   sdata->cached.sx = cdata->pending.sx;
-   sdata->cached.sy = cdata->pending.sy;
-   /* sdata->cached.buffer = cdata->pending.buffer; */
-   sdata->cached.new_attach = cdata->pending.new_attach;
+   sdata->cached.sx = pending->sx;
+   sdata->cached.sy = pending->sy;
+   /* sdata->cached.buffer = pending->buffer; */
+   sdata->cached.new_attach = pending->new_attach;
 
    /* _e_comp_wl_surface_state_buffer_set(&cdata->pending, NULL); */
-   /* cdata->pending.sx = 0; */
-   /* cdata->pending.sy = 0; */
-   /* cdata->pending.new_attach = EINA_FALSE; */
+   /* pending->sx = 0; */
+   /* pending->sy = 0; */
+   /* pending->new_attach = EINA_FALSE; */
 
-   /* copy cdata->pending.opaque into sdata->cached.opaque */
-   itr = eina_tiler_iterator_new(cdata->pending.opaque);
+   /* copy pending->opaque into sdata->cached.opaque */
+   itr = eina_tiler_iterator_new(pending->opaque);
    EINA_ITERATOR_FOREACH(itr, rect)
      eina_tiler_rect_add(sdata->cached.opaque, rect);
    eina_iterator_free(itr);
 
    /* repeat for input */
-   itr = eina_tiler_iterator_new(cdata->pending.input);
+   itr = eina_tiler_iterator_new(pending->input);
    EINA_ITERATOR_FOREACH(itr, rect)
      eina_tiler_rect_add(sdata->cached.input, rect);
    eina_iterator_free(itr);
 
-   EINA_LIST_FOREACH(cdata->pending.frames, l, cb)
-     eina_list_move(&sdata->cached.frames, &cdata->pending.frames, cb);
+   EINA_LIST_FOREACH(pending->frames, l, cb)
+     eina_list_move(&sdata->cached.frames, &pending->frames, cb);
 
    sdata->cached.has_data = EINA_TRUE;
 }
@@ -1991,6 +2015,7 @@ static void
 _e_comp_wl_client_cb_new(void *data EINA_UNUSED, E_Client *ec)
 {
    uint64_t win;
+   E_Comp_Wl_Surface_State *pending;
 
    /* make sure this is a wayland client */
    if (e_pixmap_type_get(ec->pixmap) != E_PIXMAP_TYPE_WL) return;
@@ -2014,7 +2039,9 @@ _e_comp_wl_client_cb_new(void *data EINA_UNUSED, E_Client *ec)
 
    wl_signal_init(&ec->comp_data->destroy_signal);
 
-   _e_comp_wl_surface_state_init(&ec->comp_data->pending, ec->w, ec->h);
+   pending = e_pixmap_pending_get(ec->pixmap);
+   pending->sx = ec->w;
+   pending->sy = ec->h;
 
    /* set initial client properties */
    ec->argb = EINA_TRUE;
@@ -2044,6 +2071,7 @@ _e_comp_wl_client_cb_del(void *data EINA_UNUSED, E_Client *ec)
 {
    /* Eina_Rectangle *dmg; */
    struct wl_resource *cb;
+   E_Comp_Wl_Surface_State *pending;
 
    /* make sure this is a wayland client */
    if (e_pixmap_type_get(ec->pixmap) != E_PIXMAP_TYPE_WL) return;
@@ -2066,7 +2094,8 @@ _e_comp_wl_client_cb_del(void *data EINA_UNUSED, E_Client *ec)
 
    wl_signal_emit(&ec->comp_data->destroy_signal, &ec->comp_data->surface);
 
-   _e_comp_wl_surface_state_finish(&ec->comp_data->pending);
+   pending = e_pixmap_pending_get(ec->pixmap);
+   _e_comp_wl_surface_state_finish(pending);
 
    e_comp_wl_buffer_reference(&ec->comp_data->buffer_ref, NULL);
 
@@ -2619,6 +2648,8 @@ e_comp_wl_surface_create(struct wl_client *client, int version, uint32_t id)
 EINTERN void
 e_comp_wl_surface_attach(E_Client *ec, E_Comp_Wl_Buffer *buffer)
 {
+   E_Comp_Wl_Surface_State *pending;
+
    e_comp_wl_buffer_reference(&ec->comp_data->buffer_ref, buffer);
 
    /* set usable early because shell module checks this */
@@ -2626,13 +2657,16 @@ e_comp_wl_surface_attach(E_Client *ec, E_Comp_Wl_Buffer *buffer)
    e_pixmap_resource_set(ec->pixmap, buffer);
    e_pixmap_dirty(ec->pixmap);
 
-   _e_comp_wl_surface_state_size_update(ec, &ec->comp_data->pending);
+   pending = e_pixmap_pending_get(ec->pixmap);
+   _e_comp_wl_surface_state_size_update(ec, pending);
 }
 
 EINTERN Eina_Bool
 e_comp_wl_surface_commit(E_Client *ec)
 {
-   _e_comp_wl_surface_state_commit(ec, &ec->comp_data->pending);
+   E_Comp_Wl_Surface_State *pending = e_pixmap_pending_get(ec->pixmap);
+
+   _e_comp_wl_surface_state_commit(ec, pending);
 
    /* schedule repaint */
    if (e_pixmap_refresh(ec->pixmap))
diff --git a/src/bin/e_comp_wl.h b/src/bin/e_comp_wl.h
index 686de07..7a874eb 100644
--- a/src/bin/e_comp_wl.h
+++ b/src/bin/e_comp_wl.h
@@ -244,7 +244,6 @@ struct _E_Comp_Wl_Client_Data
      } shell;
 
    E_Comp_Wl_Buffer_Ref buffer_ref;
-   E_Comp_Wl_Surface_State pending;
 
    Eina_List *frames;
 
diff --git a/src/bin/e_pixmap.c b/src/bin/e_pixmap.c
index a558e91..4dcafcf 100644
--- a/src/bin/e_pixmap.c
+++ b/src/bin/e_pixmap.c
@@ -32,6 +32,7 @@ struct _E_Pixmap
 #endif
 
 #if defined(HAVE_WAYLAND_CLIENTS) || defined(HAVE_WAYLAND_ONLY)
+   E_Comp_Wl_Surface_State pending;
    E_Comp_Wl_Buffer_Ref buffer_ref;
    struct wl_listener buffer_destroy_listener;
    void *data;
@@ -547,6 +548,21 @@ e_pixmap_resource_set(E_Pixmap *cp, void *resource)
 #endif
 }
 
+EAPI void*
+e_pixmap_pending_get(E_Pixmap *cp)
+{
+   EINA_SAFETY_ON_NULL_RETURN_VAL(cp, NULL);
+   if (cp->type != E_PIXMAP_TYPE_WL)
+     {
+        CRI("ACK");
+        return NULL;
+     }
+#if defined(HAVE_WAYLAND_CLIENTS) || defined(HAVE_WAYLAND_ONLY)
+   return &cp->pending;
+#endif
+   return NULL;
+}
+
 EAPI Ecore_Window
 e_pixmap_parent_window_get(E_Pixmap *cp)
 {
diff --git a/src/bin/e_pixmap.h b/src/bin/e_pixmap.h
index 1bf3878..29d765d 100644
--- a/src/bin/e_pixmap.h
+++ b/src/bin/e_pixmap.h
@@ -18,6 +18,7 @@ EAPI E_Pixmap *e_pixmap_new(E_Pixmap_Type type, ...);
 EAPI E_Pixmap_Type e_pixmap_type_get(const E_Pixmap *cp);
 EAPI void *e_pixmap_resource_get(E_Pixmap *cp);
 EAPI void e_pixmap_resource_set(E_Pixmap *cp, void *resource);
+EAPI void *e_pixmap_pending_get(E_Pixmap *cp);
 EAPI void e_pixmap_parent_window_set(E_Pixmap *cp, Ecore_Window win);
 EAPI void e_pixmap_visual_cmap_set(E_Pixmap *cp, void *visual, unsigned int cmap);
 EAPI unsigned int e_pixmap_failures_get(const E_Pixmap *cp);
-- 
1.7.9.5

------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to