[PATCH v1] compositor-x11: fix title overflow in x11_backend_create_output

2016-06-04 Thread Benoit Gschwind
---
 src/compositor-x11.c | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 629b5f3..a518820 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -782,7 +782,7 @@ x11_backend_create_output(struct x11_backend *b, int x, int 
y,
 {
static const char name[] = "Weston Compositor";
static const char class[] = "weston-1\0Weston Compositor";
-   char title[32];
+   char * title = NULL;
struct x11_output *output;
xcb_screen_t *screen;
struct wm_normal_hints normal_hints;
@@ -800,11 +800,6 @@ x11_backend_create_output(struct x11_backend *b, int x, 
int y,
output_width = width * scale;
output_height = height * scale;
 
-   if (configured_name)
-   sprintf(title, "%s - %s", name, configured_name);
-   else
-   strcpy(title, name);
-
if (!no_input)
values[0] |=
XCB_EVENT_MASK_KEY_PRESS |
@@ -871,9 +866,25 @@ x11_backend_create_output(struct x11_backend *b, int x, 
int y,
}
 
/* Set window name.  Don't bother with non-EWMH WMs. */
-   xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
-   b->atom.net_wm_name, b->atom.utf8_string, 8,
-   strlen(title), title);
+   if (configured_name) {
+   ret = asprintf(&title, "%s - %s", name, configured_name);
+/* following asprintf manual, title is undefined on failure,
+ * thus force NULL */
+   if (ret < 0)
+   title = NULL;
+   } else {
+   title = strdup(name);
+   }
+
+   if(title) {
+   xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, 
output->window,
+   b->atom.net_wm_name, b->atom.utf8_string, 8,
+   strlen(title), title);
+   free(title);
+   } else {
+   return NULL;
+   }
+
xcb_change_property(b->conn, XCB_PROP_MODE_REPLACE, output->window,
b->atom.wm_class, b->atom.string, 8,
sizeof class, class);
-- 
2.7.3

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


[PATCH weston 1/2] compositor: add plugin-registry

2016-06-04 Thread Giulio Camuffo
From: Pekka Paalanen 

Implement a simple register and lookup for function tables. This is
intended for plugins to expose APIs to other plugins.

It has been very hard to arrange a plugin to be able to call into
another plugin without modifying Weston core to explicitly support each
case. This patch fixes that.

The tests all pass.

Signed-off-by: Pekka Paalanen 
---
 Makefile.am  |   8 +++
 src/compositor.c |   6 ++
 src/compositor.h |   2 +
 src/plugin-registry.c| 156 +++
 src/plugin-registry.h|  55 +++
 tests/plugin-registry-test.c | 101 
 6 files changed, 328 insertions(+)
 create mode 100644 src/plugin-registry.c
 create mode 100644 src/plugin-registry.h
 create mode 100644 tests/plugin-registry-test.c

diff --git a/Makefile.am b/Makefile.am
index d1d2178..7167b6e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -90,6 +90,8 @@ libweston_la_SOURCES =
\
src/noop-renderer.c \
src/pixman-renderer.c   \
src/pixman-renderer.h   \
+   src/plugin-registry.c   \
+   src/plugin-registry.h   \
src/timeline.c  \
src/timeline.h  \
src/timeline-object.h   \
@@ -235,6 +237,7 @@ libwestoninclude_HEADERS =  \
src/compositor-rdp.h\
src/compositor-wayland.h\
src/compositor-x11.h\
+   src/plugin-registry.h   \
src/timeline-object.h   \
shared/matrix.h \
shared/config-parser.h  \
@@ -1092,6 +1095,7 @@ shared_tests =\
zuctest
 
 module_tests = \
+   plugin-registry-test.la \
surface-test.la \
surface-global-test.la
 
@@ -1142,6 +1146,10 @@ noinst_PROGRAMS +=   \
 test_module_ldflags = \
-module -avoid-version -rpath $(libdir) $(COMPOSITOR_LIBS)
 
+plugin_registry_test_la_SOURCES = tests/plugin-registry-test.c
+plugin_registry_test_la_LDFLAGS = $(test_module_ldflags)
+plugin_registry_test_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
+
 surface_global_test_la_SOURCES = tests/surface-global-test.c
 surface_global_test_la_LDFLAGS = $(test_module_ldflags)
 surface_global_test_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
diff --git a/src/compositor.c b/src/compositor.c
index 3904ef0..8464f88 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -61,6 +61,7 @@
 #include "shared/timespec-util.h"
 #include "git-version.h"
 #include "version.h"
+#include "plugin-registry.h"
 
 #define DEFAULT_REPAINT_WINDOW 7 /* milliseconds */
 
@@ -4594,6 +4595,8 @@ weston_compositor_create(struct wl_display *display, void 
*user_data)
wl_list_init(&ec->axis_binding_list);
wl_list_init(&ec->debug_binding_list);
 
+   wl_list_init(&ec->plugin_api_list);
+
weston_plane_init(&ec->primary_plane, ec, 0, 0);
weston_compositor_stack_plane(ec, &ec->primary_plane, NULL);
 
@@ -4839,6 +4842,9 @@ weston_compositor_destroy(struct weston_compositor 
*compositor)
 
if (compositor->backend)
compositor->backend->destroy(compositor);
+
+   weston_plugin_api_destroy_list(compositor);
+
free(compositor);
 }
 
diff --git a/src/compositor.h b/src/compositor.h
index de8a3b6..027b3cd 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -782,6 +782,8 @@ struct weston_compositor {
 
struct weston_launcher *launcher;
 
+   struct wl_list plugin_api_list; /* struct weston_plugin_api::link */
+
uint32_t output_id_pool;
 
struct xkb_rule_names xkb_names;
diff --git a/src/plugin-registry.c b/src/plugin-registry.c
new file mode 100644
index 000..48c4220
--- /dev/null
+++ b/src/plugin-registry.c
@@ -0,0 +1,156 @@
+/*
+ * Copyright (C) 2016 DENSO CORPORATION
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED

[PATCH weston 2/2] xwayland: make the plugin usable by libweston compositors

2016-06-04 Thread Giulio Camuffo
This patch follows a similar approach to the taken to detach the backends
from weston. But instead of passing a configuration struct when loading the
plugin, we use the plugin API registry to register an API, and to get it
in the compositor side.
This API allows to spawn the Xwayland process in the compositor side, and
to deal with signal handling.
A new function is added in compositor.c to load and init the xwayland.so plugin.

Signed-off-by: Giulio Camuffo 
---
 Makefile.am|  14 ++-
 src/compositor.c   |  15 +++
 src/compositor.h   |   3 +
 src/main.c |  16 ++-
 src/weston.h   |   3 +
 xwayland/launcher.c| 259 +++--
 xwayland/weston-xwayland.c | 207 
 xwayland/xwayland-api.h| 134 +++
 xwayland/xwayland.h|   9 +-
 9 files changed, 492 insertions(+), 168 deletions(-)
 create mode 100644 xwayland/weston-xwayland.c
 create mode 100644 xwayland/xwayland-api.h

diff --git a/Makefile.am b/Makefile.am
index 7167b6e..2358000 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -143,7 +143,8 @@ bin_PROGRAMS += weston
 
 weston_LDFLAGS = -export-dynamic
 weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON   \
--DMODULEDIR='"$(moduledir)"'
+-DMODULEDIR='"$(moduledir)"' \
+-DXSERVER_PATH='"@XSERVER_PATH@"'
 weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
 weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) $(LIBINPUT_BACKEND_LIBS) \
@@ -152,7 +153,8 @@ weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
 weston_SOURCES =   \
src/main.c  \
src/weston-screenshooter.c  \
-   src/text-backend.c
+   src/text-backend.c  \
+   xwayland/weston-xwayland.c
 
 # Track this dependency explicitly instead of using BUILT_SOURCES.  We
 # add BUILT_SOURCES to CLEANFILES, but we want to keep git-version.h
@@ -958,7 +960,7 @@ endif
 
 if ENABLE_XWAYLAND
 
-module_LTLIBRARIES += xwayland.la
+libweston_module_LTLIBRARIES += xwayland.la
 
 xwayland_la_CPPFLAGS = \
-I$(top_builddir)/protocol  \
@@ -968,8 +970,7 @@ xwayland_la_CPPFLAGS =  \
-I$(top_builddir)/xwayland  \
-DDATADIR='"$(datadir)"'\
-DMODULEDIR='"$(moduledir)"'\
-   -DLIBEXECDIR='"$(libexecdir)"'  \
-   -DXSERVER_PATH='"@XSERVER_PATH@"'
+   -DLIBEXECDIR='"$(libexecdir)"'
 
 xwayland_la_LDFLAGS = -module -avoid-version
 xwayland_la_LIBADD =   \
@@ -989,6 +990,9 @@ xwayland_la_SOURCES =   \
xwayland/hash.c \
xwayland/hash.h \
shared/helpers.h
+
+libwestoninclude_HEADERS += xwayland/xwayland-api.h
+
 endif
 
 
diff --git a/src/compositor.c b/src/compositor.c
index 8464f88..bf8e5aa 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -4872,3 +4872,18 @@ weston_compositor_get_user_data(struct weston_compositor 
*compositor)
 {
return compositor->user_data;
 }
+
+WL_EXPORT int
+weston_compositor_load_xwayland(struct weston_compositor *compositor)
+{
+   int (*module_init)(struct weston_compositor *ec,
+  int *argc, char *argv[]);
+   int argc = 0;
+
+   module_init = weston_load_module("xwayland.so", "module_init");
+   if (!module_init)
+   return -1;
+   if (module_init(compositor, &argc, NULL) < 0)
+   return -1;
+   return 0;
+}
diff --git a/src/compositor.h b/src/compositor.h
index 027b3cd..f934aab 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -1705,6 +1705,9 @@ weston_seat_get_pointer(struct weston_seat *seat);
 struct weston_touch *
 weston_seat_get_touch(struct weston_seat *seat);
 
+int
+weston_compositor_load_xwayland(struct weston_compositor *compositor);
+
 #ifdef  __cplusplus
 }
 #endif
diff --git a/src/main.c b/src/main.c
index 733bf09..d17d941 100644
--- a/src/main.c
+++ b/src/main.c
@@ -751,11 +751,17 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
while (*p) {
end = strchrnul(p, ',');
snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
-   module_init = wet_load_module(buffer, "module_init");
-   if (!module_init)
-   return -1;
-   if (module_init(ec, argc, argv) < 0)
-   return -1;
+
+   if (strcmp(buffer, "xwayland.so") == 0) {
+   if (wet_load_xwayland(ec) < 0)
+   return -1;
+   } else {
+   module_init = wet_load_mod

[PATCH weston] compositor: allow to control the vt switching

2016-06-04 Thread Giulio Camuffo
A compositor may want to control the vt switching, for example to
ensure to have a lock screen before it. To enable that add a vfunc
that will be called when CTRL+ALT+FN is pressed. The default behavior
is to do the switching, but the user can change it by using the new
weston_compositor_set_vt_switcher() function, so that it can delay the
switching to later, by calling weston_compositor_activate_vt().

Signed-off-by: Giulio Camuffo 
---
 Makefile.am   |  2 +-
 src/compositor.c  | 18 ++
 src/compositor.h  | 39 +++
 src/launcher-direct.c |  8 
 src/launcher-impl.h   |  2 ++
 src/launcher-logind.c |  8 
 src/launcher-util.c   | 11 ++-
 src/main.c|  1 +
 8 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2358000..f6e0ee9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -66,7 +66,7 @@ libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
 libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
 libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
-   $(LIBINPUT_BACKEND_LIBS) libshared.la
+   $(LIBINPUT_BACKEND_LIBS) libshared.la libsession-helper.la
 libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION}
 
 libweston_la_SOURCES = \
diff --git a/src/compositor.c b/src/compositor.c
index bf8e5aa..938894b 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -54,6 +54,8 @@
 #include "timeline.h"
 
 #include "compositor.h"
+#include "launcher-impl.h"
+#include "launcher-util.h"
 #include "scaler-server-protocol.h"
 #include "presentation-time-server-protocol.h"
 #include "shared/helpers.h"
@@ -4492,6 +4494,22 @@ compositor_bind(struct wl_client *client,
 }
 
 WL_EXPORT int
+weston_compositor_activate_vt(struct weston_compositor *compositor, int vt)
+{
+   if (compositor->launcher)
+   return weston_launcher_activate_vt(compositor->launcher, vt);
+   return -1;
+}
+
+WL_EXPORT void
+weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
+ weston_compositor_vt_switcher_func_t switcher)
+{
+   if (compositor->launcher)
+   compositor->launcher->vt_switcher = switcher;
+}
+
+WL_EXPORT int
 weston_environment_get_fd(const char *env)
 {
char *e, *end;
diff --git a/src/compositor.h b/src/compositor.h
index f934aab..472e482 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -1369,6 +1369,45 @@ void
 weston_compositor_set_default_pointer_grab(struct weston_compositor 
*compositor,
const struct weston_pointer_grab_interface *interface);
 
+/**
+ * Request a vt switch for the compositor.
+ *
+ * This will only work if the compositor is running as the owner of
+ * the session.
+ *
+ * \param launcher The compositor instance.
+ * \param vt The vt to switch to.
+ *
+ * Returns 0 on success, -1 otherwise.
+ *
+ * \sa weston_compositor_set_vt_switcher
+ */
+int
+weston_compositor_activate_vt(struct weston_compositor *compositor, int vt);
+
+typedef void (*weston_compositor_vt_switcher_func_t)(
+   struct weston_compositor *compositor, int vt);
+/**
+ * Set the vt switcher for the compositor.
+ *
+ * If the compositor is the owner of the session, the CTRL+ALT+FN key
+ * combinations will trigger a vt switch. The default behavior is to do
+ * the switching immediately, but some compositors may want to make sure to
+ * e.g. draw a lock screen before doing the switch.
+ * This function allows to register a custom vt switcher so that the actual
+ * vt switch can be controlled by calling \a weston_compositor_activate_vt.
+ *
+ * \param compositor The compositor instance.
+ * \param switcher The vt switcher function, which will be called when a
+ * CTRL+ALT+FN key combination is pressed, carrying the
+ * requested vt.
+ *
+ * \sa weston_compositor_activate_vt
+ */
+void
+weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
+ weston_compositor_vt_switcher_func_t 
switcher);
+
 int
 weston_environment_get_fd(const char *env);
 
diff --git a/src/launcher-direct.c b/src/launcher-direct.c
index 29d9c28..fdf8b55 100644
--- a/src/launcher-direct.c
+++ b/src/launcher-direct.c
@@ -305,6 +305,13 @@ launcher_direct_destroy(struct weston_launcher 
*launcher_base)
free(launcher);
 }
 
+static int
+launcher_direct_get_vt(struct weston_launcher *base)
+{
+   struct launcher_direct *launcher = wl_container_of(base, launcher, 
base);
+   return launcher->tty;
+}
+
 struct launcher_interface launcher_direct_iface = {
launcher_direct_connect,
launcher_direct_destroy,
@@ -312,4 +319,5 @@ struct launcher_interface launcher_direct_iface = {
launcher_direct_close,
launcher_direct_activate_vt,
launcher_direct_restore,
+   launc

[PATCH weston v2] compositor: allow to control the vt switching

2016-06-04 Thread Giulio Camuffo
A compositor may want to control the vt switching, for example to
ensure to have a lock screen before it. To enable that add a vfunc
that will be called when CTRL+ALT+FN is pressed. The default behavior
is to do the switching, but the user can change it by using the new
weston_compositor_set_vt_switcher() function, so that it can delay the
switching to later, by calling weston_compositor_activate_vt().

Signed-off-by: Giulio Camuffo 
---
 Makefile.am  |  2 +-
 src/compositor.c | 18 ++
 src/compositor.h | 39 +++
 src/launcher-direct.c| 12 
 src/launcher-impl.h  |  2 ++
 src/launcher-logind.c|  8 
 src/launcher-util.c  | 11 ++-
 src/launcher-weston-launch.c | 12 
 src/main.c   |  1 +
 9 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2358000..f6e0ee9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -66,7 +66,7 @@ libweston_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
 libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
 libweston_la_LIBADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) -lm $(CLOCK_GETTIME_LIBS) \
-   $(LIBINPUT_BACKEND_LIBS) libshared.la
+   $(LIBINPUT_BACKEND_LIBS) libshared.la libsession-helper.la
 libweston_la_LDFLAGS = -release ${LIBWESTON_ABI_VERSION}
 
 libweston_la_SOURCES = \
diff --git a/src/compositor.c b/src/compositor.c
index bf8e5aa..938894b 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -54,6 +54,8 @@
 #include "timeline.h"
 
 #include "compositor.h"
+#include "launcher-impl.h"
+#include "launcher-util.h"
 #include "scaler-server-protocol.h"
 #include "presentation-time-server-protocol.h"
 #include "shared/helpers.h"
@@ -4492,6 +4494,22 @@ compositor_bind(struct wl_client *client,
 }
 
 WL_EXPORT int
+weston_compositor_activate_vt(struct weston_compositor *compositor, int vt)
+{
+   if (compositor->launcher)
+   return weston_launcher_activate_vt(compositor->launcher, vt);
+   return -1;
+}
+
+WL_EXPORT void
+weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
+ weston_compositor_vt_switcher_func_t switcher)
+{
+   if (compositor->launcher)
+   compositor->launcher->vt_switcher = switcher;
+}
+
+WL_EXPORT int
 weston_environment_get_fd(const char *env)
 {
char *e, *end;
diff --git a/src/compositor.h b/src/compositor.h
index f934aab..472e482 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -1369,6 +1369,45 @@ void
 weston_compositor_set_default_pointer_grab(struct weston_compositor 
*compositor,
const struct weston_pointer_grab_interface *interface);
 
+/**
+ * Request a vt switch for the compositor.
+ *
+ * This will only work if the compositor is running as the owner of
+ * the session.
+ *
+ * \param launcher The compositor instance.
+ * \param vt The vt to switch to.
+ *
+ * Returns 0 on success, -1 otherwise.
+ *
+ * \sa weston_compositor_set_vt_switcher
+ */
+int
+weston_compositor_activate_vt(struct weston_compositor *compositor, int vt);
+
+typedef void (*weston_compositor_vt_switcher_func_t)(
+   struct weston_compositor *compositor, int vt);
+/**
+ * Set the vt switcher for the compositor.
+ *
+ * If the compositor is the owner of the session, the CTRL+ALT+FN key
+ * combinations will trigger a vt switch. The default behavior is to do
+ * the switching immediately, but some compositors may want to make sure to
+ * e.g. draw a lock screen before doing the switch.
+ * This function allows to register a custom vt switcher so that the actual
+ * vt switch can be controlled by calling \a weston_compositor_activate_vt.
+ *
+ * \param compositor The compositor instance.
+ * \param switcher The vt switcher function, which will be called when a
+ * CTRL+ALT+FN key combination is pressed, carrying the
+ * requested vt.
+ *
+ * \sa weston_compositor_activate_vt
+ */
+void
+weston_compositor_set_vt_switcher(struct weston_compositor *compositor,
+ weston_compositor_vt_switcher_func_t 
switcher);
+
 int
 weston_environment_get_fd(const char *env);
 
diff --git a/src/launcher-direct.c b/src/launcher-direct.c
index 29d9c28..0ff99a4 100644
--- a/src/launcher-direct.c
+++ b/src/launcher-direct.c
@@ -305,6 +305,17 @@ launcher_direct_destroy(struct weston_launcher 
*launcher_base)
free(launcher);
 }
 
+static int
+launcher_direct_get_vt(struct weston_launcher *base)
+{
+   struct launcher_direct *launcher = wl_container_of(base, launcher, 
base);
+   struct stat s;
+   if (fstat(launcher->tty, &s) < 0)
+   return -1;
+
+   return minor(s.st_rdev);
+}
+
 struct launcher_interface launcher_direct_iface = {
launcher_direct_connect,
launc

Re: [PATCH] scanner: Fix reported executable name to 'wayland-scanner'

2016-06-04 Thread Bryce Harrington
On Fri, Jun 03, 2016 at 06:34:38PM -0500, Yong Bakos wrote:
> Hi,
> 
> On Jun 2, 2016, at 8:16 PM, Jonas Ådahl  wrote:
> > 
> > On Thu, Jun 02, 2016 at 02:42:44PM -0700, Bryce Harrington wrote:
> >> 'wayland-scanner -v' (correctly) reports the program as named
> >> "wayland-scanner", but 'wayland-scanner -h' was inconsistent, referring
> >> to it as './scanner'.
> >> 
> >> Signed-off-by: Bryce Harrington 
> 
> Bryce, good catch! But...
> 
> 
> > I guess we could also pass argv and use argv[0], but this works as well.
> > 
> > Reviewed-by: Jonas Ådahl 
> 
> 
> I don't follow this at all, because...
> 
> 
> >> ---
> >> src/scanner.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/src/scanner.c b/src/scanner.c
> >> index 5f06e8e..ae2326f 100644
> >> --- a/src/scanner.c
> >> +++ b/src/scanner.c
> >> @@ -57,8 +57,8 @@ enum side {
> >> static int
> >> usage(int ret)
> >> {
> >> -  fprintf(stderr, "usage: ./scanner [OPTION] 
> >> [client-header|server-header|code]"
> >> -  " [input_file output_file]\n");
> >> +  fprintf(stderr, "usage: wayland-scanner [OPTION] 
> >> [client-header|server-header|code]"
> >> +  " [input_file output_file]\n", argv[0]);
> 
> argv is out of scope here in `usage`. This won't compile without changing
> the parameter list, and the passed args at all call sites.

Sorry, I'd been experimenting with using argv initially, but realized
it'd have to be plumbed in as an arg to usage() (and to
scanner_version()), and opted to keep it simple and inline the program
name like scanner_version() does.

> Also, shouldn't the diff be more like (notice the %s format specifier):
> 
> + fprintf(stderr, "usage: %s [OPTION] [client-header|server-header|code]"
> + " [input_file output_file]\n", argv[0]);

> >>fprintf(stderr, "\n");
> >>fprintf(stderr, "Converts XML protocol descriptions supplied on "
> >>"stdin or input file to client\n"
> >> -- 
> >> 1.9.1
> 
> Regards,
> yong
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protocols v3] Add screensaver idle inhibitor protocol

2016-06-04 Thread Bryce Harrington
On Fri, Jun 03, 2016 at 10:39:24AM -0500, Yong Bakos wrote:
> >>> + This destroys the inhibit manager.
> > 
> > Good addition. Should probably say how destroying the manager affects
> > the inhibitors created from it (no effect at all)?
> 
> Agreed w/ pq. But, is the inhibit manager a /singleton/ global?

On the client side yes.

> >>> +  
> >>> +
> >>> +
> >>> +
> >>> +  
> >>> + Create a new inhibitor object associated with the given surface.
> >>> +  
> >>> +  
> >>> +   >>> +summary="the surface that inhibits the idle behavior"/>
> >>> +
> 
> Does it make sense to associate inhibitors to surfaces, rather than clients?
> See below.
> 
> 
> >>> +
> >>> +  
> >>> +
> >>> +  
> >>> +
> >>> +  An idle inhibitor prevents the output that the associated surface 
> >>> is
> >>> +  visible on from being blanked, dimmed, locked, set to power save, 
> >>> or
> >>> +  otherwise obscured due to lack of user interaction.  Any active
> >>> +  screensaver processes are also temporarily blocked from displaying.
> >>> +
> >>> +  If the surface is destroyed, unmapped, becomes occluded or 
> >>> otherwise
> >>> +  loses visibility, the screen behavior is restored to normal; if the
> >>> +  surface subsequently regains visibility the inhibitor takes effect 
> >>> once
> >>> +  again.
> 
> I don't believe this is a good choice. Imagine the case of a surface-less
> 'inhibitor daemon.' There may be no visible surface (is my thinking out
> of scope here?). Imagine another case, that of a "caffeine" widget. This
> widget's surface would be hidden when another app is fullscreen.

This use case did get discussed a bit in previous rounds.  But such
tools were felt to essentially be just crude workarounds for lack of
proper API level control over things.  Sort of in the same class as why
we don't provide an xrandr-equivalent tool for configuring the monitors
- don't provide tools to work around bad designs, just fix the design so
such tools are superfluous.  :-)

> Furthermore, I don't believe that inhibition should be coupled to outputs.
> See below.
> 
> 
> >>> +
> >>> +  Note that in the case of a compound window (a surface with
> >>> +  sub-surfaces), the inhibitor effects should be inherited from the 
> >>> top
> >>> +  level surface.  
> >> 
> >> Does this mean that if we have a subsurface with an inhibitor object,
> >> and for example an xdg_surface without one, the subsurface would still
> >> not inhibit the idle behavior, as it'd inherit the behavior?
> >> 
> >> If it should work like this, maybe we should make it an error to create
> >> an idle inhibitor on a subsurface. If it should inhibit, I think it
> >> needs to be clarified.
> > 
> > Right, I was thinking something like: Sub-surfaces that do not have an
> > inhibitor associated inherit the inhibition behaviour from their
> > parent. This applies recursively.
> > 
> > To clarify what I mean, lets consider an artificial case like this:
> > 
> > wl_surface A is a top-level, only on output 1.
> > wl_surface B is a sub-surface to A, only on output 2.
> > wl_surface C is a sub-surface to B, only on output 3.
> > 
> > wl_surface B has an inhibitor associated. All surfaces are completely
> > visible etc. on their own outputs.
> > 
> > Output 1 will go to powersave: no surface is inhibiting it.
> > 
> > Output 2 does not go to powersave: B inhibitor is in effect.
> > 
> > Output 3 does not go to powersave: C inherits B's inhibitor behaviour.
> 
> I don't believe that inhibition should /only/ be coupled to outputs.
> In the case of a "caffeine" widget or daemon, the use case is to
> prevent /all/ outputs from idling, regardless of what output the widget's
> surface resides upon.

You're right in gathering that you missed some earlier discussion on
this.  :-)  In the original RFC this protocol did provide an
"inhibit idle on all-outputs" type function but in earlier reviews there
just wasn't a solid use case for that so I took it out.

I believe the reason software like you mention operate on all monitors
is just that there isn't a convenient API for doing the per-output level
control.  Back in the day I suppose it was unusual to have more than one
output, so having finer grained control over screensavers and power
saving modes probably didn't make much sense.

Bryce

> > This raises a corner-case: If surface B becomes completely occluded so
> > that the compositor considers it is no longer inhibiting, what happens
> > with output 3? Should it powersave or not?
> > 
> > That is a strange use case but still possible. I'm not sure what should
> > happen. Should it behave like C had its own inhibitor, or should the
> > inheritance take also the effectiveness of the inhibitor on B?
> > 
> > I suppose you could pick either way.
> > 
> >>> +
> >>> +
> >>> +
> >>> +  
> >>> + This removes the inhibitor effect from the associated wl_surface.
> >>> +  
> >>> +
> >>> +
> >>> +  
> >>>