[PATCH weston 0/2] xwm: Dump properties.

2018-05-04 Thread Fabien Lahoudere
This series adds code to help debug by dumping properties of type CARDINAL and
WINDOW.

Pekka Paalanen (2):
  xwm: dump properties of type CARDINAL
  xwm: dump properties of type WINDOW

 xwayland/window-manager.c | 70 ++-
 1 file changed, 69 insertions(+), 1 deletion(-)

-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/2] xwm: dump properties of type CARDINAL

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Add code to dump CARDINAL arrays from properties. Useful for debugging.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 xwayland/window-manager.c | 66 ++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 06370b7..061ce17 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -27,6 +27,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -377,6 +378,67 @@ xcb_cursor_library_load_cursor(struct weston_wm *wm, const 
char *file)
return cursor;
 }
 
+static unsigned
+dump_cardinal_array_elem(FILE *fp, unsigned format,
+void *arr, unsigned len, unsigned ind)
+{
+   const char *comma;
+
+   /* If more than 16 elements, print 0-14, ..., last */
+   if (ind > 14 && ind < len - 1) {
+   fprintf(fp, ", ...");
+   return len - 1;
+   }
+
+   comma = ind ? ", " : "";
+
+   switch (format) {
+   case 32:
+   fprintf(fp, "%s%" PRIu32, comma, ((uint32_t *)arr)[ind]);
+   break;
+   case 16:
+   fprintf(fp, "%s%" PRIu16, comma, ((uint16_t *)arr)[ind]);
+   break;
+   case 8:
+   fprintf(fp, "%s%" PRIu8, comma, ((uint8_t *)arr)[ind]);
+   break;
+   default:
+   fprintf(fp, "%s???", comma);
+   }
+
+   return ind + 1;
+}
+
+static void
+dump_cardinal_array(xcb_get_property_reply_t *reply)
+{
+   unsigned i = 0;
+   FILE *fp;
+   void *arr;
+   char *str = NULL;
+   size_t size = 0;
+
+   assert(reply->type == XCB_ATOM_CARDINAL);
+
+   fp = open_memstream(&str, &size);
+   if (!fp)
+   return;
+
+   arr = xcb_get_property_value(reply);
+
+   fprintf(fp, "[");
+   while (i < reply->value_len)
+   i = dump_cardinal_array_elem(fp, reply->format,
+arr, reply->value_len, i);
+   fprintf(fp, "]");
+
+   if (fclose(fp) != 0)
+   return;
+
+   wm_log_continue("%s\n", str);
+   free(str);
+}
+
 void
 dump_property(struct weston_wm *wm,
  xcb_atom_t property, xcb_get_property_reply_t *reply)
@@ -403,7 +465,7 @@ dump_property(struct weston_wm *wm,
incr_value = xcb_get_property_value(reply);
wm_log_continue("%d\n", *incr_value);
} else if (reply->type == wm->atom.utf8_string ||
- reply->type == wm->atom.string) {
+  reply->type == wm->atom.string) {
text_value = xcb_get_property_value(reply);
if (reply->value_len > 40)
len = 40;
@@ -424,6 +486,8 @@ dump_property(struct weston_wm *wm,
width +=  wm_log_continue("%s", name);
}
wm_log_continue("\n");
+   } else if (reply->type == XCB_ATOM_CARDINAL) {
+   dump_cardinal_array(reply);
} else {
wm_log_continue("huh?\n");
}
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 2/2] xwm: dump properties of type WINDOW

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Very useful for TRANSIENT_FOR property debugging.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 xwayland/window-manager.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 061ce17..2b3defb 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -446,6 +446,7 @@ dump_property(struct weston_wm *wm,
int32_t *incr_value;
const char *text_value, *name;
xcb_atom_t *atom_value;
+   xcb_window_t *window_value;
int width, len;
uint32_t i;
 
@@ -488,6 +489,9 @@ dump_property(struct weston_wm *wm,
wm_log_continue("\n");
} else if (reply->type == XCB_ATOM_CARDINAL) {
dump_cardinal_array(reply);
+   } else if (reply->type == XCB_ATOM_WINDOW && reply->format == 32) {
+   window_value = xcb_get_property_value(reply);
+   wm_log_continue("win %u\n", *window_value);
} else {
wm_log_continue("huh?\n");
}
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 0/1] Improve override-redirect debug messages

2018-05-04 Thread Fabien Lahoudere
This patch is an addition to https://patchwork.freedesktop.org/patch/211161/.

Pekka Paalanen (1):
  xwm: improve override-redirect debug messages

 xwayland/window-manager.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/1] xwm: improve override-redirect debug messages

2018-05-04 Thread Fabien Lahoudere
From: Pekka Paalanen 

Explicitly say if O-R is yes or no, to not have to remember which
messages possibly printed it.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 

Conflicts:
xwayland/window-manager.c
---
 xwayland/window-manager.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 32a756c..0ac8639 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -747,14 +747,25 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, 
xcb_generic_event_t *eve
const struct weston_desktop_xwayland_interface *xwayland_api =
wm->server->compositor->xwayland_interface;
struct weston_wm_window *window;
+   const char *or_str = " unk";
 
-   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
+   if (wm_lookup_window(wm, configure_notify->window, &window)) {
+   if (!!configure_notify->override_redirect == 
!!window->override_redirect)
+   or_str = ".";
+   else
+   or_str = " BUG!";
+   } else {
+   window = NULL;
+   }
+
+   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d, O-R: %s%s\n",
   configure_notify->window,
   configure_notify->x, configure_notify->y,
   configure_notify->width, configure_notify->height,
-  configure_notify->override_redirect ? ", override" : "");
+  configure_notify->override_redirect ? "yes" : "no",
+  or_str);
 
-   if (!wm_lookup_window(wm, configure_notify->window, &window))
+   if (!window)
return;
 
window->x = configure_notify->x;
@@ -1136,8 +1147,8 @@ weston_wm_handle_map_notify(struct weston_wm *wm, 
xcb_generic_event_t *event)
return;
}
 
-   wm_log("XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
-  map_notify->override_redirect ? ", override" : "");
+   wm_log("XCB_MAP_NOTIFY (window %d) O-R : %s\n", map_notify->window,
+  map_notify->override_redirect ? "yes" : "no");
 
if (!wm_lookup_window(wm, map_notify->window, &window))
return;
@@ -1449,11 +1460,11 @@ weston_wm_handle_create_notify(struct weston_wm *wm, 
xcb_generic_event_t *event)
xcb_create_notify_event_t *create_notify =
(xcb_create_notify_event_t *) event;
 
-   wm_log("XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height 
%d%s%s)\n",
+   wm_log("XCB_CREATE_NOTIFY (window %d, at (%d, %d), width %d, height %d, 
O-R: %s%s)\n",
   create_notify->window,
   create_notify->x, create_notify->y,
   create_notify->width, create_notify->height,
-  create_notify->override_redirect ? ", override" : "",
+  create_notify->override_redirect ? "yes" : "no",
   our_resource(wm, create_notify->window) ? ", ours" : "");
 
if (our_resource(wm, create_notify->window))
@@ -1493,11 +1504,11 @@ weston_wm_handle_reparent_notify(struct weston_wm *wm, 
xcb_generic_event_t *even
(xcb_reparent_notify_event_t *) event;
struct weston_wm_window *window;
 
-   wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d%s)\n",
+   wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d, O-R: 
%s)\n",
   reparent_notify->window,
   reparent_notify->parent,
   reparent_notify->event,
-  reparent_notify->override_redirect ? ", override" : "");
+  reparent_notify->override_redirect ? "yes" : "no");
 
if (reparent_notify->parent == wm->screen->root) {
weston_wm_window_create(wm, reparent_notify->window, 10, 10,
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/1] desktop-shell: detect stale shell surface outputs

2018-05-02 Thread Fabien Lahoudere
From: Semi Malinen 

When displays are hot (un)plugged, it may happen that
a shell surface is left with a stale pointer to an output
that has already been freed. Add an output destroy listener
to catch such situations and set the output pointer to NULL.

Signed-off-by: Semi Malinen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c | 29 +++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..b539431 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -128,6 +128,7 @@ struct shell_surface {
 
struct weston_output *fullscreen_output;
struct weston_output *output;
+   struct wl_listener output_destroy_listener;
 
struct surface_state {
bool fullscreen;
@@ -1934,6 +1935,17 @@ shell_surface_update_layer(struct shell_surface *shsurf)
 }
 
 static void
+notify_output_destroy(struct wl_listener *listener, void *data)
+{
+   struct shell_surface *shsurf =
+   container_of(listener,
+struct shell_surface, output_destroy_listener);
+
+   shsurf->output = NULL;
+   shsurf->output_destroy_listener.notify = NULL;
+}
+
+static void
 shell_surface_set_output(struct shell_surface *shsurf,
  struct weston_output *output)
 {
@@ -1948,6 +1960,18 @@ shell_surface_set_output(struct shell_surface *shsurf,
shsurf->output = es->output;
else
shsurf->output = get_default_output(es->compositor);
+
+   if (shsurf->output_destroy_listener.notify) {
+   wl_list_remove(&shsurf->output_destroy_listener.link);
+   shsurf->output_destroy_listener.notify = NULL;
+   }
+
+   if (!shsurf->output)
+   return;
+
+   shsurf->output_destroy_listener.notify = notify_output_destroy;
+   wl_signal_add(&shsurf->output->destroy_signal,
+ &shsurf->output_destroy_listener);
 }
 
 static void
@@ -1986,7 +2010,7 @@ unset_maximized(struct shell_surface *shsurf)
weston_desktop_surface_get_surface(shsurf->desktop_surface);
 
/* undo all maximized things here */
-   shsurf->output = get_default_output(surface->compositor);
+   shell_surface_set_output(shsurf, 
get_default_output(surface->compositor));
 
if (shsurf->saved_position_valid)
weston_view_set_position(shsurf->view,
@@ -2348,7 +2372,8 @@ desktop_surface_added(struct weston_desktop_surface 
*desktop_surface,
shsurf->fullscreen.black_view = NULL;
wl_list_init(&shsurf->fullscreen.transform.link);
 
-   shsurf->output = get_default_output(shsurf->shell->compositor);
+   shell_surface_set_output(
+   shsurf, get_default_output(shsurf->shell->compositor));
 
wl_signal_init(&shsurf->destroy_signal);
 
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/4] desktop-shell: handle NULL output in get_output_work_area()

2018-05-02 Thread Fabien Lahoudere
From: Pekka Paalanen 

This is a tentative crash fix for a case where there are no
enabled weston_outputs at all.

Let get_output_work_area() return a zero area if the given output is
NULL. If there is no output, there is no area. Unfortunately we cannot
return "no position" but have to use 0,0 instead.

In send_configure_for_surface(), this causes a maximized surface to
receive width=0 and height=0 in the configure event, which means the
client is free to choose the size. There is no correct size to send for
maximizing for no output.

In constrain_position(), this has no effect. The interactive move of a
surface is restricted to not go below the panel, so even if a user
managed to move a surface without an output, it just prevents the
surface moving beyond y=0.

In weston_view_set_initial_position(), get_output_work_area() will not
be called with NULL output anyway.

In set_maximized_position(), this makes it behave as if the output was
at 0,0 which is the default position of the first output.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..c1a551b 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -341,6 +341,15 @@ get_output_work_area(struct desktop_shell *shell,
 {
int32_t panel_width = 0, panel_height = 0;
 
+   if (!output) {
+   area->x = 0;
+   area->y = 0;
+   area->width = 0;
+   area->height = 0;
+
+   return;
+   }
+
area->x = output->x;
area->y = output->y;
 
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 0/4] Handle several NULL output pointer

2018-05-02 Thread Fabien Lahoudere
This is a series of fixes, for the case when Weston is running without any
connected outputs (weston_outputs).

Pekka Paalanen (4):
  desktop-shell: handle NULL output in get_output_work_area()
  desktop-shell: handle NULL output in center_on_output()
  desktop-shell: do not lower_fullscreen_layer(s, NULL)
  desktop-shell: survive NULL output in shell_configure_fullscreen()

 desktop-shell/shell.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 3/4] desktop-shell: do not lower_fullscreen_layer(s, NULL)

2018-05-02 Thread Fabien Lahoudere
From: Pekka Paalanen 

In activate, do not call lower_fullscreen_layer() at all if the output
is NULL. It should not do anything in that case, per the existing
comment.

This is a tentative crash fix for a case where there are no enabled
weston_outputs at all.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index e6ce20b..1edd37b 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -3735,7 +3735,8 @@ activate(struct desktop_shell *shell, struct weston_view 
*view,
 
/* Only demote fullscreen surfaces on the output of activated shsurf.
 * Leave fullscreen surfaces on unrelated outputs alone. */
-   lower_fullscreen_layer(shell, shsurf->output);
+   if (shsurf->output)
+   lower_fullscreen_layer(shell, shsurf->output);
 
weston_view_activate(view, seat, flags);
 
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 4/4] desktop-shell: survive NULL output in shell_configure_fullscreen()

2018-05-02 Thread Fabien Lahoudere
From: Pekka Paalanen 

Running 'weston-simple-egl -f -b' (fullscreen, unthrottled) caused a
crash in shell_ensure_fullscreen_black_view() due to
shsurf->fullscreen_output being NULL. Also shell_configure_fullscreen()
could crash on that condition.

Fix shell_configure_fullscreen() to bail out with minimal work if there
is no fullscreen_output.

It is unclear if anything will cause a reconfiguration when an output is
plugged in.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 1edd37b..b9fa42d 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2152,6 +2152,13 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
weston_layer_entry_insert(&shsurf->shell->fullscreen_layer.view_list,
  &shsurf->view->layer_link);
 
+   if (!shsurf->fullscreen_output) {
+   /* If there is no output, there's not much we can do.
+* Position the window somwhere, whatever. */
+   weston_view_set_position(shsurf->view, 0, 0);
+   return;
+   }
+
shell_ensure_fullscreen_black_view(shsurf);
 
surface_subsurfaces_boundingbox(surface, &surf_x, &surf_y,
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 2/4] desktop-shell: handle NULL output in center_on_output()

2018-05-02 Thread Fabien Lahoudere
From: Pekka Paalanen 

This is a tentative crash fix for a case where there are no enabled
weston_outputs at all.

If no output is given, just put the surface at 0,0. At least it should
become mostly visible if an output is plugged in, if not centered.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index c1a551b..e6ce20b 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4150,6 +4150,11 @@ center_on_output(struct weston_view *view, struct 
weston_output *output)
int32_t surf_x, surf_y, width, height;
float x, y;
 
+   if (!output) {
+   weston_view_set_position(view, 0, 0);
+   return;
+   }
+
surface_subsurfaces_boundingbox(view->surface, &surf_x, &surf_y, 
&width, &height);
 
x = output->x + (output->width - width) / 2 - surf_x / 2;
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH 1/1] libweston: add weston_view_set_output()

2018-04-26 Thread Fabien Lahoudere
From: Semi Malinen 

Instead of desktop shell assigning view outputs directly,
use a new method, weston_view_set_output(). The method can
set up an output destroy listener to make sure that views
do not have stale output pointers.

Without this patch it is possible to end up in a scenario
where, e.g. configure_static_view() accesses memory that
has already been freed. The scenario can be provoked by
repeatedly plugging and unplugging a display. The faulty
memory accesses are reported by valgrind.

Signed-off-by: Semi Malinen 
Signed-off-by: Fabien Lahoudere 
---
 desktop-shell/shell.c  | 11 +++
 libweston/compositor.c | 43 +--
 libweston/compositor.h |  4 
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index b846e30..1e153cb 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -580,7 +580,7 @@ create_focus_surface(struct weston_compositor *ec,
free(fsurf);
return NULL;
}
-   fsurf->view->output = output;
+   weston_view_set_output(fsurf->view, output);
fsurf->view->is_mapped = true;
 
weston_surface_set_size(surface, output->width, output->height);
@@ -2464,7 +2464,7 @@ map(struct desktop_shell *shell, struct shell_surface 
*shsurf,
shsurf->view->is_mapped = true;
if (shsurf->state.maximized) {
surface->output = shsurf->output;
-   shsurf->view->output = shsurf->output;
+   weston_view_set_output(shsurf->view, shsurf->output);
}
 
if (!shell->locked) {
@@ -2881,6 +2881,9 @@ configure_static_view(struct weston_view *ev, struct 
weston_layer *layer, int x,
 {
struct weston_view *v, *next;
 
+   if (!ev->output)
+   return;
+
wl_list_for_each_safe(v, next, &layer->view_list.link, layer_link.link) 
{
if (v->output == ev->output && v != ev) {
weston_view_unmap(v);
@@ -2970,7 +2973,7 @@ desktop_shell_set_background(struct wl_client *client,
surface->committed_private = shell;
weston_surface_set_label_func(surface, background_get_label);
surface->output = weston_head_from_resource(output_resource)->output;
-   view->output = surface->output;
+   weston_view_set_output(view, surface->output);
 
sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
if (sh_output->background_surface) {
@@ -3067,7 +3070,7 @@ desktop_shell_set_panel(struct wl_client *client,
surface->committed_private = shell;
weston_surface_set_label_func(surface, panel_get_label);
surface->output = weston_head_from_resource(output_resource)->output;
-   view->output = surface->output;
+   weston_view_set_output(view, surface->output);
 
sh_output = find_shell_output_from_weston_output(shell, 
surface->output);
if (sh_output->panel_surface) {
diff --git a/libweston/compositor.c b/libweston/compositor.c
index dbf61ef..9be66fc 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1021,6 +1021,45 @@ weston_surface_update_output_mask(struct weston_surface 
*es, uint32_t mask)
}
 }
 
+static void
+notify_view_output_destroy(struct wl_listener *listener, void *data)
+{
+   struct weston_view *view =
+   container_of(listener,
+struct weston_view, output_destroy_listener);
+
+   view->output = NULL;
+   view->output_destroy_listener.notify = NULL;
+}
+
+/** Set the primary output of the view
+ *
+ * \param view The view whose primary output to set
+ * \param output The new primary output for the view
+ *
+ * Set \a output to be the primary output of the \a view.
+ *
+ * Notice that the assignment may be temporary; the primary output could be
+ * automatically changed. Hence, one cannot rely on the value persisting.
+ *
+ * Passing NULL as /a output will set the primary output to NULL.
+ */
+WL_EXPORT void
+weston_view_set_output(struct weston_view *view, struct weston_output *output)
+{
+   if (view->output_destroy_listener.notify) {
+   wl_list_remove(&view->output_destroy_listener.link);
+   view->output_destroy_listener.notify = NULL;
+   }
+   view->output = output;
+   if (output) {
+   view->output_destroy_listener.notify =
+   notify_view_output_destroy;
+   wl_signal_add(&output->destroy_signal,
+ &view->output_destroy_listener);
+   }
+}
+
 /** Recalculate which output(s) the surface has views displayed on
  *
  * \param es  The surface to remap to outputs
@@ -1112,7 +1151,7 @@ weston_view_assign_output(struct weston_view *ev)
}
pixman_region32_

[PATCH weston 1/4] pixman,drm: do not composite previous damage

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Pixman-renderer uses a single internal shadow buffer. It is enough to
composite the current damage into shadow, but the copy to hw buffer
needs to include the previous damage because of double-buffering in
DRM-backend.

This patch lets pixman-renderer do exactly that without compositing also
the previous damage on DRM-renderer.

Arguably weston_output should not have field previous_damage to begin
with, because it implies double-buffering, which e.g. EGL does not
guarantee. It would be better for each backend explicitly always provide
any extra damage that should be copied to hw.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c  | 16 
 libweston/pixman-renderer.c | 35 ++-
 libweston/pixman-renderer.h |  7 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 23fd887..fa650c3 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1670,25 +1670,17 @@ drm_output_render_pixman(struct drm_output_state *state,
 {
struct drm_output *output = state->output;
struct weston_compositor *ec = output->base.compositor;
-   pixman_region32_t total_damage, previous_damage;
-
-   pixman_region32_init(&total_damage);
-   pixman_region32_init(&previous_damage);
-
-   pixman_region32_copy(&previous_damage, damage);
-
-   pixman_region32_union(&total_damage, damage, &output->previous_damage);
-   pixman_region32_copy(&output->previous_damage, &previous_damage);
 
output->current_image ^= 1;
 
pixman_renderer_output_set_buffer(&output->base,
  output->image[output->current_image]);
+   pixman_renderer_output_set_hw_extra_damage(&output->base,
+  &output->previous_damage);
 
-   ec->renderer->repaint_output(&output->base, &total_damage);
+   ec->renderer->repaint_output(&output->base, damage);
 
-   pixman_region32_fini(&total_damage);
-   pixman_region32_fini(&previous_damage);
+   pixman_region32_copy(&output->previous_damage, damage);
 
return drm_fb_ref(output->dumb[output->current_image]);
 }
diff --git a/libweston/pixman-renderer.c b/libweston/pixman-renderer.c
index f7366cf..cf43b15 100644
--- a/libweston/pixman-renderer.c
+++ b/libweston/pixman-renderer.c
@@ -41,6 +41,7 @@ struct pixman_output_state {
void *shadow_buffer;
pixman_image_t *shadow_image;
pixman_image_t *hw_buffer;
+   pixman_region32_t *hw_extra_damage;
 };
 
 struct pixman_surface_state {
@@ -555,15 +556,29 @@ copy_to_hw_buffer(struct weston_output *output, 
pixman_region32_t *region)
 
 static void
 pixman_renderer_repaint_output(struct weston_output *output,
-pixman_region32_t *output_damage)
+  pixman_region32_t *output_damage)
 {
struct pixman_output_state *po = get_output_state(output);
+   pixman_region32_t hw_damage;
 
-   if (!po->hw_buffer)
-   return;
+   if (!po->hw_buffer) {
+   po->hw_extra_damage = NULL;
+   return;
+   }
+
+   pixman_region32_init(&hw_damage);
+   if (po->hw_extra_damage) {
+   pixman_region32_union(&hw_damage,
+ po->hw_extra_damage, output_damage);
+   po->hw_extra_damage = NULL;
+   } else {
+   pixman_region32_copy(&hw_damage, output_damage);
+   }
 
repaint_surfaces(output, output_damage);
-   copy_to_hw_buffer(output, output_damage);
+
+   copy_to_hw_buffer(output, &hw_damage);
+   pixman_region32_fini(&hw_damage);
 
pixman_region32_copy(&output->previous_damage, output_damage);
wl_signal_emit(&output->frame_signal, output);
@@ -862,7 +877,8 @@ pixman_renderer_init(struct weston_compositor *ec)
 }
 
 WL_EXPORT void
-pixman_renderer_output_set_buffer(struct weston_output *output, pixman_image_t 
*buffer)
+pixman_renderer_output_set_buffer(struct weston_output *output,
+ pixman_image_t *buffer)
 {
struct pixman_output_state *po = get_output_state(output);
 
@@ -876,6 +892,15 @@ pixman_renderer_output_set_buffer(struct weston_output 
*output, pixman_image_t *
}
 }
 
+WL_EXPORT void
+pixman_renderer_output_set_hw_extra_damage(struct weston_output *output,
+  pixman_region32_t *extra_damage)
+{
+   struct pixman_output_state *po = get_output_state(output);
+
+   po->hw_extra_damage = extra_damage;
+}
+
 WL_EXPORT int
 pixman_renderer_output_create(struct weston_output *output)
 {
diff --git a/li

[PATCH weston 3/4] compositor-drm: expose global shadow flag for pixman

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Allow global control of the pixman shadow buffers. The compositor can
choose whether all output use or do not use a shadoe buffer with the
pixman renderer.

The option is added to the end of struct weston_drm_backend_config to
avoid bumping WESTON_DRM_BACKEND_CONFIG_VERSION.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c | 15 ---
 libweston/compositor-drm.h |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ce33905..a1df45b 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -261,6 +261,7 @@ struct drm_backend {
bool atomic_modeset;
 
int use_pixman;
+   bool use_pixman_shadow;
 
struct udev_input input;
 
@@ -4124,6 +4125,7 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
uint32_t format = output->gbm_format;
uint32_t pixman_format;
unsigned int i;
+   uint32_t flags = 0;
 
switch (format) {
case GBM_FORMAT_XRGB:
@@ -4151,9 +4153,14 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
goto err;
}
 
-   if (pixman_renderer_output_create(&output->base,
- PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0)
-   goto err;
+   if (b->use_pixman_shadow)
+   flags |= PIXMAN_RENDERER_OUTPUT_USE_SHADOW;
+
+   if (pixman_renderer_output_create(&output->base, flags) < 0)
+   goto err;
+ 
+   weston_log("DRM: output %s %s shadow framebuffer.\n", output->base.name,
+  b->use_pixman_shadow ? "uses" : "does not use");
 
pixman_region32_init_rect(&output->previous_damage,
  output->base.x, output->base.y, 
output->base.width, output->base.height);
@@ -5905,6 +5912,7 @@ drm_backend_create(struct weston_compositor *compositor,
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
+   b->use_pixman_shadow = config->use_pixman_shadow;
 
compositor->backend = &b->base;
 
@@ -6064,6 +6072,7 @@ err_compositor:
 static void
 config_init_to_defaults(struct weston_drm_backend_config *config)
 {
+   config->use_pixman_shadow = true;
 }
 
 WL_EXPORT int
diff --git a/libweston/compositor-drm.h b/libweston/compositor-drm.h
index 68f93ea..539 100644
--- a/libweston/compositor-drm.h
+++ b/libweston/compositor-drm.h
@@ -146,6 +146,9 @@ struct weston_drm_backend_config {
 * based on seat names and boot_vga to find the right device.
 */
char *specific_device;
+
+   /** Use shadow buffer if using Pixman-renderer. */
+   bool use_pixman_shadow;
 };
 
 #ifdef  __cplusplus
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 2/4] pixman: make shadow buffer optional

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Add a flag to pixman-renderer for initializing the output with a shadow
framebuffer. All backends were getting the shadow implcitly, so all
backends are modified to ask for the shadow explicitly.

Using a shadow buffer is usually beneficial, because read-modify-write
cycles (blending) into a scanout-capable buffer may be very slow. The
scanout framebuffer may also have reduced color depth, making blending
and read-back produce inferior results.

In some use cases though the shadow buffer might be just an extra copy
hurting more than it helps. Whether it helps or hurts depends on the
platform and the workload. Therefore let the backends control whether
pixman-renderer uses a shadow buffer for an output or not.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c  |  3 +-
 libweston/compositor-fbdev.c|  3 +-
 libweston/compositor-headless.c |  3 +-
 libweston/compositor-rdp.c  |  5 +--
 libweston/compositor-wayland.c  |  3 +-
 libweston/compositor-x11.c  |  6 ++--
 libweston/pixman-renderer.c | 68 -
 libweston/pixman-renderer.h |  6 +++-
 8 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index fa650c3..ce33905 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -4151,7 +4151,8 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
goto err;
}
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+ PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0)
goto err;
 
pixman_region32_init_rect(&output->previous_damage,
diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
index de40c21..a78f6fa 100644
--- a/libweston/compositor-fbdev.c
+++ b/libweston/compositor-fbdev.c
@@ -511,7 +511,8 @@ fbdev_output_enable(struct weston_output *base)
output->base.start_repaint_loop = fbdev_output_start_repaint_loop;
output->base.repaint = fbdev_output_repaint;
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+   PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 0)
goto out_hw_surface;
 
loop = wl_display_get_event_loop(backend->compositor->wl_display);
diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
index 6cb4f20..61a5bd9 100644
--- a/libweston/compositor-headless.c
+++ b/libweston/compositor-headless.c
@@ -172,7 +172,8 @@ headless_output_enable(struct weston_output *base)
 output->image_buf,
 
output->base.current_mode->width * 4);
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+   PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 0)
goto err_renderer;
 
pixman_renderer_output_set_buffer(&output->base,
diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
index 693f136..fd0651a 100644
--- a/libweston/compositor-rdp.c
+++ b/libweston/compositor-rdp.c
@@ -460,7 +460,7 @@ rdp_switch_mode(struct weston_output *output, struct 
weston_mode *target_mode)
output->current_mode->flags |= WL_OUTPUT_MODE_CURRENT;
 
pixman_renderer_output_destroy(output);
-   pixman_renderer_output_create(output);
+   pixman_renderer_output_create(output, 
PIXMAN_RENDERER_OUTPUT_USE_SHADOW);
 
new_shadow_buffer = pixman_image_create_bits(PIXMAN_x8r8g8b8, 
target_mode->width,
target_mode->height, 0, target_mode->width * 4);
@@ -546,7 +546,8 @@ rdp_output_enable(struct weston_output *base)
return -1;
}
 
-   if (pixman_renderer_output_create(&output->base) < 0) {
+   if (pixman_renderer_output_create(&output->base,
+ PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0) {
pixman_image_unref(output->shadow_surface);
return -1;
}
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 2473f08..56ed474 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -782,7 +782,8 @@ cleanup_window:
 static int
 wayland_output_init_pixman_renderer(struct wayland_output *output)
 {
-   return pixman_renderer_output_create(&output->base);
+   return pixman_renderer_output_create(&output->base,
+PIXMAN_RENDERER_OUTPUT_USE_SHADOW);
 }

[PATCH weston 4/4] main: add setting for DRM/pixman shadow framebuffer

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Allows to control the Pixman-renderer shadow framebuffer usage from
weston.ini. It defaults to enabled, and whether it is a good idea to
disable or not depends on the platform and the workload.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 compositor/main.c  | 3 +++
 man/weston-drm.man | 4 
 2 files changed, 7 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index f72d3ea..9a65e69 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1283,6 +1283,7 @@ load_drm_backend(struct weston_compositor *c,
struct weston_drm_backend_config config = {{ 0, }};
struct weston_config_section *section;
struct wet_compositor *wet = to_wet_compositor(c);
+   int use_shadow;
int ret = 0;
 
wet->drm_use_current_mode = false;
@@ -1303,6 +1304,8 @@ load_drm_backend(struct weston_compositor *c,
 NULL);
weston_config_section_get_uint(section, "pageflip-timeout",
   &config.pageflip_timeout, 0);
+   weston_config_section_get_bool(section, "pixman-shadow", &use_shadow, 
1);
+   config.use_pixman_shadow = use_shadow;
 
config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
config.base.struct_size = sizeof(struct weston_drm_backend_config);
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 75d7902..f488c87 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -79,6 +79,10 @@ Transform for the output, which can be rotated in 90-degree 
steps
 and possibly flipped. Possible values are
 .BR normal ", " 90 ", " 180 ", " 270 ", "
 .BR flipped ", " flipped-90 ", " flipped-180 ", and " flipped-270 .
+.TP
+\fBpixman-shadow\fR=\fIboolean\fR
+If using the Pixman-renderer, use shadow framebuffers. Defaults to
+.BR true .
 .
 .\" ***
 .SH OPTIONS
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 0/4] Optimize pixman renderer

2018-04-23 Thread Fabien Lahoudere
Optimizes pixman renderer by:
- optimizing compositing damage in DRM/pixman

  Reduce Weston's CPU usage by avoiding unnecessary compositing when updating 
  the shadow buffer in pixman-renderer, under the DRM backend. The test was a 
  proprietary graphical X11 application in demo mode. The effect is a drop of
  total system CPU usage from 0.41 to 0.33.

- optimizing shadow buffer usage

  The shadow framebuffer is an intermediate buffer where the scene is composited
  and then copied from the shadow to the actual hardware buffer. This extra step
  costs memory bandwidth compared to compositing directly into a hardware 
buffer.

  Weston's DRM-backend with the Pixman-renderer uses a shadow framebuffer by
  default. Especially on systems with dedicated VRAM, read-modify-write cycles
  (a.k.a blending) into the scanout-capable buffer can be very slow. Also the
  scanout pixel format may not be optimal for compositing. Therefore Weston 
takes
  the safe default to always use a shadow framebuffer.

  However, in our use case, the hardware does not have dedicated VRAM behind a
  relatively slow bus, and the graphical load has practically no blending. We 
  can reduce Weston's CPU usage quite a lot by not using the shadow frambuffer.
  
  We test on proprietary graphical X11 application in demo mode. Using perf we
  measure that this change improve weston CPU usage by 13% with dual display and
  11% with clone mode.

Pekka Paalanen (4):
  pixman,drm: do not composite previous damage
  pixman: make shadow buffer optional
  compositor-drm: expose global shadow flag for pixman
  main: add setting for DRM/pixman shadow framebuffer

 compositor/main.c   |  3 ++
 libweston/compositor-drm.c  | 30 +++--
 libweston/compositor-drm.h  |  3 ++
 libweston/compositor-fbdev.c|  3 +-
 libweston/compositor-headless.c |  3 +-
 libweston/compositor-rdp.c  |  5 ++-
 libweston/compositor-wayland.c  |  3 +-
 libweston/compositor-x11.c  |  6 ++-
 libweston/pixman-renderer.c | 99 -
 libweston/pixman-renderer.h | 13 +-
 man/weston-drm.man  |  4 ++
 11 files changed, 118 insertions(+), 54 deletions(-)

-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 0/4] Optimize pixman renderer

2018-04-23 Thread Fabien Lahoudere
Optimizes pixman renderer by:
- optimizing compositing damage in DRM/pixman

  Reduce Weston's CPU usage by avoiding unnecessary compositing when updating 
  the shadow buffer in pixman-renderer, under the DRM backend. The test was a 
  proprietary graphical X11 application in demo mode. The effect is a drop of
  total system CPU usage from 0.41 to 0.33.

- optimizing shadow buffer usage

  The shadow framebuffer is an intermediate buffer where the scene is composited
  and then copied from the shadow to the actual hardware buffer. This extra step
  costs memory bandwidth compared to compositing directly into a hardware 
buffer.

  Weston's DRM-backend with the Pixman-renderer uses a shadow framebuffer by
  default. Especially on systems with dedicated VRAM, read-modify-write cycles
  (a.k.a blending) into the scanout-capable buffer can be very slow. Also the
  scanout pixel format may not be optimal for compositing. Therefore Weston 
takes
  the safe default to always use a shadow framebuffer.

  However, in our use case, the hardware does not have dedicated VRAM behind a
  relatively slow bus, and the graphical load has practically no blending. We 
  can reduce Weston's CPU usage quite a lot by not using the shadow frambuffer.
  
  We test on proprietary graphical X11 application in demo mode. Using perf we
  measure that this change improve weston CPU usage by 13% with dual display and
  11% with clone mode.

Pekka Paalanen (4):
  pixman,drm: do not composite previous damage
  pixman: make shadow buffer optional
  compositor-drm: expose global shadow flag for pixman
  main: add setting for DRM/pixman shadow framebuffer

 compositor/main.c   |  3 ++
 libweston/compositor-drm.c  | 30 +++--
 libweston/compositor-drm.h  |  3 ++
 libweston/compositor-fbdev.c|  3 +-
 libweston/compositor-headless.c |  3 +-
 libweston/compositor-rdp.c  |  5 ++-
 libweston/compositor-wayland.c  |  3 +-
 libweston/compositor-x11.c  |  6 ++-
 libweston/pixman-renderer.c | 99 -
 libweston/pixman-renderer.h | 13 +-
 man/weston-drm.man  |  4 ++
 11 files changed, 118 insertions(+), 54 deletions(-)

-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 2/4] pixman: make shadow buffer optional

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Add a flag to pixman-renderer for initializing the output with a shadow
framebuffer. All backends were getting the shadow implcitly, so all
backends are modified to ask for the shadow explicitly.

Using a shadow buffer is usually beneficial, because read-modify-write
cycles (blending) into a scanout-capable buffer may be very slow. The
scanout framebuffer may also have reduced color depth, making blending
and read-back produce inferior results.

In some use cases though the shadow buffer might be just an extra copy
hurting more than it helps. Whether it helps or hurts depends on the
platform and the workload. Therefore let the backends control whether
pixman-renderer uses a shadow buffer for an output or not.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c  |  3 +-
 libweston/compositor-fbdev.c|  3 +-
 libweston/compositor-headless.c |  3 +-
 libweston/compositor-rdp.c  |  5 +--
 libweston/compositor-wayland.c  |  3 +-
 libweston/compositor-x11.c  |  6 ++--
 libweston/pixman-renderer.c | 68 -
 libweston/pixman-renderer.h |  6 +++-
 8 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index fa650c3..ce33905 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -4151,7 +4151,8 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
goto err;
}
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+ PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0)
goto err;
 
pixman_region32_init_rect(&output->previous_damage,
diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
index de40c21..a78f6fa 100644
--- a/libweston/compositor-fbdev.c
+++ b/libweston/compositor-fbdev.c
@@ -511,7 +511,8 @@ fbdev_output_enable(struct weston_output *base)
output->base.start_repaint_loop = fbdev_output_start_repaint_loop;
output->base.repaint = fbdev_output_repaint;
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+   PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 0)
goto out_hw_surface;
 
loop = wl_display_get_event_loop(backend->compositor->wl_display);
diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
index 6cb4f20..61a5bd9 100644
--- a/libweston/compositor-headless.c
+++ b/libweston/compositor-headless.c
@@ -172,7 +172,8 @@ headless_output_enable(struct weston_output *base)
 output->image_buf,
 
output->base.current_mode->width * 4);
 
-   if (pixman_renderer_output_create(&output->base) < 0)
+   if (pixman_renderer_output_create(&output->base,
+   PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 0)
goto err_renderer;
 
pixman_renderer_output_set_buffer(&output->base,
diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
index 693f136..fd0651a 100644
--- a/libweston/compositor-rdp.c
+++ b/libweston/compositor-rdp.c
@@ -460,7 +460,7 @@ rdp_switch_mode(struct weston_output *output, struct 
weston_mode *target_mode)
output->current_mode->flags |= WL_OUTPUT_MODE_CURRENT;
 
pixman_renderer_output_destroy(output);
-   pixman_renderer_output_create(output);
+   pixman_renderer_output_create(output, 
PIXMAN_RENDERER_OUTPUT_USE_SHADOW);
 
new_shadow_buffer = pixman_image_create_bits(PIXMAN_x8r8g8b8, 
target_mode->width,
target_mode->height, 0, target_mode->width * 4);
@@ -546,7 +546,8 @@ rdp_output_enable(struct weston_output *base)
return -1;
}
 
-   if (pixman_renderer_output_create(&output->base) < 0) {
+   if (pixman_renderer_output_create(&output->base,
+ PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0) {
pixman_image_unref(output->shadow_surface);
return -1;
}
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 2473f08..56ed474 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -782,7 +782,8 @@ cleanup_window:
 static int
 wayland_output_init_pixman_renderer(struct wayland_output *output)
 {
-   return pixman_renderer_output_create(&output->base);
+   return pixman_renderer_output_create(&output->base,
+PIXMAN_RENDERER_OUTPUT_USE_SHADOW);
 }

[PATCH weston 4/4] main: add setting for DRM/pixman shadow framebuffer

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Allows to control the Pixman-renderer shadow framebuffer usage from
weston.ini. It defaults to enabled, and whether it is a good idea to
disable or not depends on the platform and the workload.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 compositor/main.c  | 3 +++
 man/weston-drm.man | 4 
 2 files changed, 7 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index f72d3ea..9a65e69 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1283,6 +1283,7 @@ load_drm_backend(struct weston_compositor *c,
struct weston_drm_backend_config config = {{ 0, }};
struct weston_config_section *section;
struct wet_compositor *wet = to_wet_compositor(c);
+   int use_shadow;
int ret = 0;
 
wet->drm_use_current_mode = false;
@@ -1303,6 +1304,8 @@ load_drm_backend(struct weston_compositor *c,
 NULL);
weston_config_section_get_uint(section, "pageflip-timeout",
   &config.pageflip_timeout, 0);
+   weston_config_section_get_bool(section, "pixman-shadow", &use_shadow, 
1);
+   config.use_pixman_shadow = use_shadow;
 
config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
config.base.struct_size = sizeof(struct weston_drm_backend_config);
diff --git a/man/weston-drm.man b/man/weston-drm.man
index 75d7902..f488c87 100644
--- a/man/weston-drm.man
+++ b/man/weston-drm.man
@@ -79,6 +79,10 @@ Transform for the output, which can be rotated in 90-degree 
steps
 and possibly flipped. Possible values are
 .BR normal ", " 90 ", " 180 ", " 270 ", "
 .BR flipped ", " flipped-90 ", " flipped-180 ", and " flipped-270 .
+.TP
+\fBpixman-shadow\fR=\fIboolean\fR
+If using the Pixman-renderer, use shadow framebuffers. Defaults to
+.BR true .
 .
 .\" ***
 .SH OPTIONS
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston 1/4] pixman,drm: do not composite previous damage

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Pixman-renderer uses a single internal shadow buffer. It is enough to
composite the current damage into shadow, but the copy to hw buffer
needs to include the previous damage because of double-buffering in
DRM-backend.

This patch lets pixman-renderer do exactly that without compositing also
the previous damage on DRM-renderer.

Arguably weston_output should not have field previous_damage to begin
with, because it implies double-buffering, which e.g. EGL does not
guarantee. It would be better for each backend explicitly always provide
any extra damage that should be copied to hw.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c  | 16 
 libweston/pixman-renderer.c | 35 ++-
 libweston/pixman-renderer.h |  7 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 23fd887..fa650c3 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1670,25 +1670,17 @@ drm_output_render_pixman(struct drm_output_state *state,
 {
struct drm_output *output = state->output;
struct weston_compositor *ec = output->base.compositor;
-   pixman_region32_t total_damage, previous_damage;
-
-   pixman_region32_init(&total_damage);
-   pixman_region32_init(&previous_damage);
-
-   pixman_region32_copy(&previous_damage, damage);
-
-   pixman_region32_union(&total_damage, damage, &output->previous_damage);
-   pixman_region32_copy(&output->previous_damage, &previous_damage);
 
output->current_image ^= 1;
 
pixman_renderer_output_set_buffer(&output->base,
  output->image[output->current_image]);
+   pixman_renderer_output_set_hw_extra_damage(&output->base,
+  &output->previous_damage);
 
-   ec->renderer->repaint_output(&output->base, &total_damage);
+   ec->renderer->repaint_output(&output->base, damage);
 
-   pixman_region32_fini(&total_damage);
-   pixman_region32_fini(&previous_damage);
+   pixman_region32_copy(&output->previous_damage, damage);
 
return drm_fb_ref(output->dumb[output->current_image]);
 }
diff --git a/libweston/pixman-renderer.c b/libweston/pixman-renderer.c
index f7366cf..cf43b15 100644
--- a/libweston/pixman-renderer.c
+++ b/libweston/pixman-renderer.c
@@ -41,6 +41,7 @@ struct pixman_output_state {
void *shadow_buffer;
pixman_image_t *shadow_image;
pixman_image_t *hw_buffer;
+   pixman_region32_t *hw_extra_damage;
 };
 
 struct pixman_surface_state {
@@ -555,15 +556,29 @@ copy_to_hw_buffer(struct weston_output *output, 
pixman_region32_t *region)
 
 static void
 pixman_renderer_repaint_output(struct weston_output *output,
-pixman_region32_t *output_damage)
+  pixman_region32_t *output_damage)
 {
struct pixman_output_state *po = get_output_state(output);
+   pixman_region32_t hw_damage;
 
-   if (!po->hw_buffer)
-   return;
+   if (!po->hw_buffer) {
+   po->hw_extra_damage = NULL;
+   return;
+   }
+
+   pixman_region32_init(&hw_damage);
+   if (po->hw_extra_damage) {
+   pixman_region32_union(&hw_damage,
+ po->hw_extra_damage, output_damage);
+   po->hw_extra_damage = NULL;
+   } else {
+   pixman_region32_copy(&hw_damage, output_damage);
+   }
 
repaint_surfaces(output, output_damage);
-   copy_to_hw_buffer(output, output_damage);
+
+   copy_to_hw_buffer(output, &hw_damage);
+   pixman_region32_fini(&hw_damage);
 
pixman_region32_copy(&output->previous_damage, output_damage);
wl_signal_emit(&output->frame_signal, output);
@@ -862,7 +877,8 @@ pixman_renderer_init(struct weston_compositor *ec)
 }
 
 WL_EXPORT void
-pixman_renderer_output_set_buffer(struct weston_output *output, pixman_image_t 
*buffer)
+pixman_renderer_output_set_buffer(struct weston_output *output,
+ pixman_image_t *buffer)
 {
struct pixman_output_state *po = get_output_state(output);
 
@@ -876,6 +892,15 @@ pixman_renderer_output_set_buffer(struct weston_output 
*output, pixman_image_t *
}
 }
 
+WL_EXPORT void
+pixman_renderer_output_set_hw_extra_damage(struct weston_output *output,
+  pixman_region32_t *extra_damage)
+{
+   struct pixman_output_state *po = get_output_state(output);
+
+   po->hw_extra_damage = extra_damage;
+}
+
 WL_EXPORT int
 pixman_renderer_output_create(struct weston_output *output)
 {
diff --git a/li

[PATCH weston 3/4] compositor-drm: expose global shadow flag for pixman

2018-04-23 Thread Fabien Lahoudere
From: Pekka Paalanen 

Allow global control of the pixman shadow buffers. The compositor can
choose whether all output use or do not use a shadoe buffer with the
pixman renderer.

The option is added to the end of struct weston_drm_backend_config to
avoid bumping WESTON_DRM_BACKEND_CONFIG_VERSION.

Signed-off-by: Pekka Paalanen 
Signed-off-by: Fabien Lahoudere 
---
 libweston/compositor-drm.c | 15 ---
 libweston/compositor-drm.h |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index ce33905..a1df45b 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -261,6 +261,7 @@ struct drm_backend {
bool atomic_modeset;
 
int use_pixman;
+   bool use_pixman_shadow;
 
struct udev_input input;
 
@@ -4124,6 +4125,7 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
uint32_t format = output->gbm_format;
uint32_t pixman_format;
unsigned int i;
+   uint32_t flags = 0;
 
switch (format) {
case GBM_FORMAT_XRGB:
@@ -4151,9 +4153,14 @@ drm_output_init_pixman(struct drm_output *output, struct 
drm_backend *b)
goto err;
}
 
-   if (pixman_renderer_output_create(&output->base,
- PIXMAN_RENDERER_OUTPUT_USE_SHADOW) < 
0)
-   goto err;
+   if (b->use_pixman_shadow)
+   flags |= PIXMAN_RENDERER_OUTPUT_USE_SHADOW;
+
+   if (pixman_renderer_output_create(&output->base, flags) < 0)
+   goto err;
+ 
+   weston_log("DRM: output %s %s shadow framebuffer.\n", output->base.name,
+  b->use_pixman_shadow ? "uses" : "does not use");
 
pixman_region32_init_rect(&output->previous_damage,
  output->base.x, output->base.y, 
output->base.width, output->base.height);
@@ -5905,6 +5912,7 @@ drm_backend_create(struct weston_compositor *compositor,
b->compositor = compositor;
b->use_pixman = config->use_pixman;
b->pageflip_timeout = config->pageflip_timeout;
+   b->use_pixman_shadow = config->use_pixman_shadow;
 
compositor->backend = &b->base;
 
@@ -6064,6 +6072,7 @@ err_compositor:
 static void
 config_init_to_defaults(struct weston_drm_backend_config *config)
 {
+   config->use_pixman_shadow = true;
 }
 
 WL_EXPORT int
diff --git a/libweston/compositor-drm.h b/libweston/compositor-drm.h
index 68f93ea..539 100644
--- a/libweston/compositor-drm.h
+++ b/libweston/compositor-drm.h
@@ -146,6 +146,9 @@ struct weston_drm_backend_config {
 * based on seat names and boot_vga to find the right device.
 */
char *specific_device;
+
+   /** Use shadow buffer if using Pixman-renderer. */
+   bool use_pixman_shadow;
 };
 
 #ifdef  __cplusplus
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston] gl-renderer: Set pitch correctly for subsampled textures

2017-10-09 Thread Fabien Lahoudere
Hello

I test this patch and it fixes my problem.

Thanks

On Thu, 2017-10-05 at 15:36 +0100, Daniel Stone wrote:
> Multi-plane sub-sampled textures have partial width/height, e.g.
> YUV420/I420 has a full-size Y plane, followed by a half-width/height U
> plane, and a half-width/height V plane.
> 
> zwp_linux_dmabuf_v1 allows clients to pass an explicit pitch for each
> plane, but for wl_shm this must be inferred. gl-renderer was correctly
> accounting for the width and height when subsampling, but the pitch was
> being taken as the pitch for the first plane.
> 
> This does not match the requirements for GStreamer's waylandsink, in
> particular, as well as other clients. Fix the SHM upload path to
> correctly set the pitch for each plane, according to subsampling.
> 
> Tested with:
>   $ gst-launch-1.0 videotestsrc ! waylandsink
> 

Tested-by: Fabien Lahoudere 

> Signed-off-by: Daniel Stone 
> Fixes: fdeefe42418 ("gl-renderer: add support of WL_SHM_FORMAT_YUV420")
> Reported-by: Fabien Lahoudere 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103063
> Cc: Vincent Abriou 
> ---
>  libweston/gl-renderer.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 244ce309..40bf0bb6 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -1445,14 +1445,13 @@ gl_renderer_flush_damage(struct weston_surface 
> *surface)
>   goto done;
>   }
>  
> - glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch);
> -
>   if (gs->needs_full_upload) {
>   glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
>   glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
>   wl_shm_buffer_begin_access(buffer->shm_buffer);
>   for (j = 0; j < gs->num_textures; j++) {
>   glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
> + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / 
> gs->hsub[j]);
>   glTexImage2D(GL_TEXTURE_2D, 0,
>    gs->gl_format[j],
>    gs->pitch / gs->hsub[j],
> @@ -1477,6 +1476,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
>   glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, r.y1);
>   for (j = 0; j < gs->num_textures; j++) {
>   glBindTexture(GL_TEXTURE_2D, gs->textures[j]);
> + glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT, gs->pitch / 
> gs->hsub[j]);
>   glTexSubImage2D(GL_TEXTURE_2D, 0,
>   r.x1 / gs->hsub[j],
>   r.y1 / gs->vsub[j],
-- 
Fabien Lahoudere,
Embedded Linux developer at Collabora.

This message is intended for the use of only the person(s) ("intended 
recipient") to whom it is
addressed. It may contain information that is privileged and confidential. 
Accordingly any
dissemination, distribution, copying or other use of this message or any of its 
content by any
person other than the intended recipient may constitute a breach of civil or 
criminal law and is
strictly prohibited. If you are not the intended recipient, please contact the 
sender as soon as
possible.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[weston 1/1] calibrator: Make mouse button optional

2017-09-07 Thread Fabien Lahoudere
When calibrating touchscreen with weston-calibrator, you can use the mouse to
click on the cross which is recorded as a touch event. This event is used to
compute the final calibration of the touchscreen which results in invalid
touchscreen calibration and broken touchscreen behaviour.

In order to avoid to use the mouse in weston-calibrator, we disable mouse
operation by default and add a parameter "--enable-mouse" to enable it.

Signed-off-by: Fabien Lahoudere 
---
 clients/calibrator.c | 38 ++
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/clients/calibrator.c b/clients/calibrator.c
index 04c1cfc..778c23c 100644
--- a/clients/calibrator.c
+++ b/clients/calibrator.c
@@ -24,12 +24,14 @@
 #include "config.h"
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -218,7 +220,7 @@ redraw_handler(struct widget *widget, void *data)
 }
 
 static struct calibrator *
-calibrator_create(struct display *display)
+calibrator_create(struct display *display, bool enable_button)
 {
struct calibrator *calibrator;
 
@@ -233,7 +235,8 @@ calibrator_create(struct display *display)
 
calibrator->current_test = ARRAY_LENGTH(test_ratios) - 1;
 
-   widget_set_button_handler(calibrator->widget, button_handler);
+   if (enable_button)
+   widget_set_button_handler(calibrator->widget, button_handler);
widget_set_touch_down_handler(calibrator->widget, touch_handler);
widget_set_redraw_handler(calibrator->widget, redraw_handler);
 
@@ -250,13 +253,40 @@ calibrator_destroy(struct calibrator *calibrator)
free(calibrator);
 }
 
+static void
+help(const char *name)
+{
+   fprintf(stderr, "Usage: %s [args...]\n", name);
+   fprintf(stderr, "  -m, --enable-mouse   Enable mouse for testing 
the touchscreen\n");
+   fprintf(stderr, "  -h, --help  Display this help message\n");
+}
 
 int
 main(int argc, char *argv[])
 {
struct display *display;
struct calibrator *calibrator;
-
+   int c;
+   bool enable_mouse = 0;
+   struct option opts[] = {
+   { "enable-mouse", no_argument, NULL, 'm' },
+   { "help",no_argument,   NULL, 'h' },
+   { 0, 0, NULL,  0  }
+   };
+
+   while ((c = getopt_long(argc, argv, "mh", opts, NULL)) != -1) {
+   switch (c) {
+   case 'm':
+   enable_mouse = 1;
+   break;
+   case 'h':
+   help(argv[0]);
+   exit(EXIT_FAILURE);
+   default:
+   break;
+   }
+   }
+   
display = display_create(&argc, argv);
 
if (display == NULL) {
@@ -264,7 +294,7 @@ main(int argc, char *argv[])
return -1;
}
 
-   calibrator = calibrator_create(display);
+   calibrator = calibrator_create(display, enable_mouse);
 
if (!calibrator)
return -1;
-- 
1.8.3.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel