Re: [PATCH weston v3] drm: port the drm backend to the new init api

[Quentin Glidic]

For now, I will just comment on the part I am not too happy with.

On 31/10/2015 12:08, Giulio Camuffo wrote:

[snip]
diff --git a/src/main.c b/src/main.c
index 292f8e0..bde27ee 100644
--- a/src/main.c
+++ b/src/main.c
@@ -47,6 +47,8 @@
  #include "git-version.h"
  #include "version.h"

+#include "compositor-drm.h"
+
  static struct wl_list child_process_list;
  static struct weston_compositor *segv_compositor;

@@ -666,12 +668,137 @@ load_backend_new(struct weston_compositor *compositor, 
const char *backend,
  }

  static int
+parse_modeline(const char *s, struct weston_drm_backend_modeline *mode)
+{
+   char hsync[16];
+   char vsync[16];
+   float fclock;
+
+   mode->flags = 0;
+
+   if (sscanf(s, "%f %hd %hd %hd %hd %hd %hd %hd %hd %15s %15s",
+  &fclock,
+  &mode->hdisplay,
+  &mode->hsync_start,
+  &mode->hsync_end,
+  &mode->htotal,
+  &mode->vdisplay,
+  &mode->vsync_start,
+  &mode->vsync_end,
+  &mode->vtotal, hsync, vsync) != 11)
+   return -1;
+
+   mode->clock = fclock * 1000;
+   if (strcmp(hsync, "+hsync") == 0)
+   mode->flags |= WESTON_DRM_BACKEND_MODELINE_FLAG_PHSYNC;
+   else if (strcmp(hsync, "-hsync") == 0)
+   mode->flags |= WESTON_DRM_BACKEND_MODELINE_FLAG_NHSYNC;
+   else
+   return -1;
+
+   if (strcmp(vsync, "+vsync") == 0)
+   mode->flags |= WESTON_DRM_BACKEND_MODELINE_FLAG_PVSYNC;
+   else if (strcmp(vsync, "-vsync") == 0)
+   mode->flags |= WESTON_DRM_BACKEND_MODELINE_FLAG_NVSYNC;
+   else
+   return -1;
+
+   return 0;
+}


As discussed on IRC, I think this part should stay in the drm backend. 
The rationale is that we probably want all libweston-based compositors 
to work with the exact same modeline format, with the same error 
messages and the same predictable behaviour.




+static void
+drm_configure_output(struct weston_compositor *c, const char *name,
+struct weston_drm_backend_output_config *config)
+{
+   struct weston_config *wc = weston_compositor_get_user_data(c);
+   struct weston_config_section *section;
+   char *s;
+   int scale;
+
+   section = weston_config_get_section(wc, "output", "name", name);
+   weston_config_section_get_string(section, "mode", &s, "preferred");
+   if (strcmp(s, "off") == 0)
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_OFF;
+   else if (strcmp(s, "preferred") == 0)
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_PREFERRED;
+   else if (strcmp(s, "current") == 0)
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_CURRENT;
+   else if (sscanf(s, "%dx%d", &config->base.width,
+   &config->base.height) == 2)
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_MODE;
+   else if (parse_modeline(s, &config->modeline) == 0)
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_MODELINE;
+   else {
+   weston_log("Invalid mode \"%s\" for output %s\n",
+  s, name);
+   config->type = WESTON_DRM_BACKEND_OUTPUT_TYPE_PREFERRED;
+   }
+   free(s);


I would like this part shared between the backend and the compositor.
As I said, the parsing of modeline should be in the backend, but also 
what I would call “light modeline”, i.e. "width x height". That put the 
“technical” part into the “technical code”.

Then you have a separate setting for on/off(/current).

The configure_output function would return a boolean, which indicates 
whether or not to activate that output. The modeline would be passed as 
a string (config->moduline) and could be NULL.
Returning FALSE obviously means that all the configuration will be 
ignored (and thus, you can return early, keeping everything to NULL) and 
the meaning of TRUE depends on the modeline:

- if it is a well-formed modeline (or “light modeline”), use it;
- if it is malformed, fallback to NULL;
- if it is NULL, use the preferred setting.

If “current” is really a needed setting (I do not know the use case it 
was added for), we can just use a 3-values enum as the return value:

- OFF = 0
- PREFERRED = 1
- CURRENT = -1
which will change the meaning of a NULL modeline.


What do you think?
If we agree on that, I can make a patch for this (as a follow-up to 
squash before the push maybe).


Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: question to wayland-protocols and configure error.

[Quentin Glidic]

On 20/11/2015 10:38, Christian Hartmann wrote:

2015-11-20 10:34 GMT+01:00 Christian Hartmann :

I got a configure error since this week:

checking for COMPOSITOR... yes
checking for WAYLAND_PROTOCOLS... no
configure: error: Package requirements (wayland-protocols >= 0.1.0)
were not met:

No package 'wayland-protocols' found

Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.

Alternatively, you may set the environment variables WAYLAND_PROTOCOLS_CFLAGS
and WAYLAND_PROTOCOLS_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.


On both test / developing machine, what I am missing and where can I
get these requested protocols pc files??

Any help or hint(s) would be appreciate :=)


and yes I always start to update at first wayland setup. but that did
not solved my issue, and I am already looking at it, but cannot find
these protocols yet


Hi,


This is a brand new repository that was created a few days ago.
You can find it here[0].
You simply have to install it before Weston (it does not matter if you 
install it before or after Wayland).
This repository is installing a set of protocols files for other 
projects to use them. Weston is the first user and most of the protocols 
that were waiting for stabilisation in Weston were moved to 
wayland-protocols.



Cheers,

--

Quentin “Sardem FF7” Glidic

[0] 

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

Re: [PATCH] install pkg-config file to /usr/share

[Quentin Glidic]

On 22/11/2015 13:31, Igor Gnatenko wrote:

It is arch-independent, so no need to install it to /usr/lib*

Signed-off-by: Igor Gnatenko 
---
  Makefile.am | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index a32e977..71b8799 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -15,5 +15,5 @@ dist_noinst_DATA =
\
$(sort $(foreach p,$(unstable_protocols),$(dir $p)README))  
\
$(NULL)

-pkgconfigdir = $(libdir)/pkgconfig
+pkgconfigdir = $(datadir)/pkgconfig
  pkgconfig_DATA = wayland-protocols.pc




Ideally, we should use PKG_NOARCH_INSTALLDIR (in configure.ac) and drop 
the *dir from Makefile.am.


If that is not wanted, this patch is fine:
Reviewed-by: Quentin Glidic 

(Sorry for the duplicate mail Igor.)

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v2] configure.ac: Fix compatibility for older pkg-config versions

[Quentin Glidic]

On 03/12/2015 23:45, Bryce Harrington wrote:

noarch_pkgconfigdir is not available on oldish pkg-config's.  Among
other things this affects Wayland's nightly auto-build Ubuntu 14.04
PPAs.

Signed-off-by: Bryce Harrington 
Cc: Pekka Paalanen 
Cc: Quentin Glidic 
Cc: Peter Hutterer 
---

Changes from v1
  - Document why the work around is needed and what obsoletes it.
  - Drop pkgconfigdir, which isn't strictly needed

  compat.m4| 12 
  configure.ac |  2 ++
  2 files changed, 14 insertions(+)
  create mode 100644 compat.m4

diff --git a/compat.m4 b/compat.m4
new file mode 100644
index 000..290ef03
--- /dev/null
+++ b/compat.m4
@@ -0,0 +1,12 @@
+dnl noarch_pkgconfigdir only available in pkg-config 0.27 and newer
+dnl http://lists.freedesktop.org/archives/pkg-config/2012-July/000875.html
+dnl Ubuntu 14.04 provides only pkg-config 0.26 so lacks this function.
+dnl
+dnl The Wayland project maintains automated builds for Ubuntu 14.04 in
+dnl a Launchpad PPA.  14.04 is a Long Term Support distro release, which
+dnl will reach EOL April 2019, however the Wayland PPA may stop targeting
+dnl it some time after the next LTS release (April 2016).
+m4_ifndef([PKG_NOARCH_INSTALLDIR], [AC_DEFUN([PKG_NOARCH_INSTALLDIR], [
+noarch_pkgconfigdir='${datadir}'/pkgconfig
+AC_SUBST([noarch_pkgconfigdir])
+])])
diff --git a/configure.ac b/configure.ac
index 93688d0..eeb95ef 100644
--- a/configure.ac
+++ b/configure.ac
@@ -11,6 +11,8 @@ AC_INIT([wayland-protocols],
  [wayland-protocols],
  [http://wayland.freedesktop.org/])

+m4_include(compat.m4)
+


This is really unusual to include a m4 file directly. aclocal is 
supposed to manage that for you. Not sure if you needed it, but in that 
case, you are probably missing something else (maybe AC_CONFIG_MACRO_DIR 
is required when using extra m4 files).




  AC_SUBST([WAYLAND_PROTOCOLS_VERSION], [wayland_protocols_version])

  AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz])




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH v3 wayland-protocols] configure.ac: Fix compatibility for older pkg-config versions

[Quentin Glidic]

On 04/12/2015 02:40, Bryce Harrington wrote:

noarch_pkgconfigdir is not available on oldish pkg-config's.  Among
other things this affects Wayland's nightly auto-build Ubuntu 14.04
PPAs.

Signed-off-by: Bryce Harrington 
Cc: Pekka Paalanen 
Cc: Quentin Glidic 
Cc: Peter Hutterer 
---
Changes from v2
- Place config.m4 into an m4 subdir
- Auto load m4's via AC_CONFIG_MACRO_DIR

Changes from v1
- Document why the work around is needed and what obsoletes it.
- Drop pkgconfigdir, which isn't strictly needed

  configure.ac |  2 ++
  m4/compat.m4 | 12 
  2 files changed, 14 insertions(+)
  create mode 100644 m4/compat.m4

diff --git a/configure.ac b/configure.ac
index 93688d0..c51b7fc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -11,6 +11,8 @@ AC_INIT([wayland-protocols],
  [wayland-protocols],
  [http://wayland.freedesktop.org/])

+AC_CONFIG_MACRO_DIR([m4])
+
  AC_SUBST([WAYLAND_PROTOCOLS_VERSION], [wayland_protocols_version])

  AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz])
diff --git a/m4/compat.m4 b/m4/compat.m4
new file mode 100644
index 000..290ef03
--- /dev/null
+++ b/m4/compat.m4
@@ -0,0 +1,12 @@
+dnl noarch_pkgconfigdir only available in pkg-config 0.27 and newer
+dnl http://lists.freedesktop.org/archives/pkg-config/2012-July/000875.html
+dnl Ubuntu 14.04 provides only pkg-config 0.26 so lacks this function.
+dnl
+dnl The Wayland project maintains automated builds for Ubuntu 14.04 in
+dnl a Launchpad PPA.  14.04 is a Long Term Support distro release, which
+dnl will reach EOL April 2019, however the Wayland PPA may stop targeting
+dnl it some time after the next LTS release (April 2016).
+m4_ifndef([PKG_NOARCH_INSTALLDIR], [AC_DEFUN([PKG_NOARCH_INSTALLDIR], [
+noarch_pkgconfigdir='${datadir}'/pkgconfig
+AC_SUBST([noarch_pkgconfigdir])
+])])



Perfect.

Reviewed-by: Quentin Glidic 

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] test: add make check

[Quentin Glidic]

On 22/12/2015 19:19, Derek Foreman wrote:

We can now test all the protocol files by running make check (or distcheck)
which will pass them through the scanner.

Signed-off-by: Derek Foreman 
---
Changes from v1:
Use #~/bin/sh -e and drop the && from each line in the script
Discover the scanner's location and use it

  .gitignore| 3 +++
  Makefile.am   | 7 +++
  configure.ac  | 6 ++
  tests/scan.sh | 5 +
  4 files changed, 21 insertions(+)
  create mode 100755 tests/scan.sh

diff --git a/.gitignore b/.gitignore
index e6f85d0..ca19ecf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,6 @@ missing
  *.pc
  autom4te.cache
  aclocal.m4
+*.trs
+*.log
+test-driver
diff --git a/Makefile.am b/Makefile.am
index 5926a41..dc44c28 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,3 +16,10 @@ dist_noinst_DATA =   
\
$(NULL)

  noarch_pkgconfig_DATA = wayland-protocols.pc
+
+EXTRA_DIST = tests/scan.sh


dist_check_SCRIPTS maybe? :-)



+
+TESTS = $(unstable_protocols)


Why not all the variables already?



+TEST_EXTENSIONS = .xml
+AM_TESTS_ENVIRONMENT = SCANNER='$(wayland_scanner)'; export SCANNER;
+XML_LOG_COMPILER = $(srcdir)/tests/scan.sh
diff --git a/configure.ac b/configure.ac
index c51b7fc..dc5d899 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,6 +15,12 @@ AC_CONFIG_MACRO_DIR([m4])

  AC_SUBST([WAYLAND_PROTOCOLS_VERSION], [wayland_protocols_version])

+AC_PATH_PROG([wayland_scanner], [wayland-scanner])


It makes me wonder… Do we expect wayland-scanner to generate 
arch-independent code? If so, it’s ok. If not, it should be AC_PATH_TOOL.




+if test x$wayland_scanner = x; then
+PKG_CHECK_MODULES(WAYLAND_SCANNER, [wayland-scanner])
+wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner 
wayland-scanner`


Ok, this is a good idea (and maybe am I the one originally writing it 
like that, or reviewing something similar) but it can lead to strange 
situations while cross-compiling.


If you cross-compile from x86 to arm64, with no scanner installed for 
x86, it will try to run the arm64 scanner, which is not possible.


I see the need for it in the “$HOME testing environment” but then we 
should at least check that $host == $build.




+fi
+
  AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz])

  AM_SILENT_RULES([yes])
diff --git a/tests/scan.sh b/tests/scan.sh
new file mode 100755
index 000..fd92bec
--- /dev/null
+++ b/tests/scan.sh
@@ -0,0 +1,5 @@
+#!/bin/sh -e
+
+$SCANNER client-header $1 /dev/null
+$SCANNER server-header $1 /dev/null
+$SCANNER code $1 /dev/null




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v200 wayland-protocols] test: add make check

[Quentin Glidic]

From: Derek Foreman 

We can now test all the protocol files by running make check (or distcheck)
which will pass them through the scanner.

Signed-off-by: Derek Foreman 
Reviewed-by: Quentin Glidic 
---

Just added AC_ARG_VAR to allow overriding the variable when calling configure.

 .gitignore|  3 +++
 Makefile.am   | 10 ++
 configure.ac  | 14 ++
 tests/scan.sh | 10 ++
 4 files changed, 37 insertions(+)
 create mode 100755 tests/scan.sh

diff --git a/.gitignore b/.gitignore
index e6f85d0..ca19ecf 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,6 @@ missing
 *.pc
 autom4te.cache
 aclocal.m4
+*.trs
+*.log
+test-driver
diff --git a/Makefile.am b/Makefile.am
index 5926a41..582b3f2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,6 +7,9 @@ unstable_protocols =
\
unstable/xdg-shell/xdg-shell-unstable-v5.xml
\
$(NULL)
 
+stable_protocols = 
\
+   $(NULL)
+
 nobase_dist_pkgdata_DATA = 
\
$(unstable_protocols)   
\
$(NULL)
@@ -16,3 +19,10 @@ dist_noinst_DATA =   
\
$(NULL)
 
 noarch_pkgconfig_DATA = wayland-protocols.pc
+
+dist_check_SCRIPTS = tests/scan.sh
+
+TESTS = $(unstable_protocols) $(stable_protocols)
+TEST_EXTENSIONS = .xml
+AM_TESTS_ENVIRONMENT = SCANNER='$(wayland_scanner)'; export SCANNER;
+XML_LOG_COMPILER = $(srcdir)/tests/scan.sh
diff --git a/configure.ac b/configure.ac
index c51b7fc..61693fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,6 +15,20 @@ AC_CONFIG_MACRO_DIR([m4])
 
 AC_SUBST([WAYLAND_PROTOCOLS_VERSION], [wayland_protocols_version])
 
+AC_CANONICAL_HOST
+AC_CANONICAL_BUILD
+
+AC_ARG_VAR([wayland_scanner], [The wayland-scanner executable])
+AC_PATH_PROG([wayland_scanner], [wayland-scanner])
+if test x$wayland_scanner = x; then
+if test x$host = x$build; then
+PKG_CHECK_MODULES(WAYLAND_SCANNER, [wayland-scanner])
+wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner 
wayland-scanner`
+else
+AC_MSG_WARN([You are cross compiling without wayland-scanner 
in your path.  make check will fail.])
+fi
+fi
+
 AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz])
 
 AM_SILENT_RULES([yes])
diff --git a/tests/scan.sh b/tests/scan.sh
new file mode 100755
index 000..15dd39f
--- /dev/null
+++ b/tests/scan.sh
@@ -0,0 +1,10 @@
+#!/bin/sh -e
+
+if [ "x$SCANNER" = "x" ] ; then
+   echo "No scanner present, test skipped." 1>&2
+   exit 77
+fi
+
+$SCANNER client-header $1 /dev/null
+$SCANNER server-header $1 /dev/null
+$SCANNER code $1 /dev/null
-- 
2.6.4

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

[PATCH wayland-protocols] Add notification area protocol version 1

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---

I hope the descriptions in the protocol are clear enough.

I have working compositor implementations for Weston (as a plugin), Orbment 
(plugin) and Sway,
and a working client implementation in eventd.

I want to make very clear that this protocol is intended for non-DEs 
compositors.
DEs are expected to implement notifications as an internal detail (e.g. 
providing D-Bus APIs).

Another important point: this protocol is not using global coordinates.
The compositor defines an output and an area which notifications are restricted 
to.
The recommended default value is the "primary" output, minus the possible 
panels.

I do not expect much notification daemons to port to Wayland and this protocol, 
but if any does,
I would rather see them using an existing protocol than having to do the 
compositor implementation
over and over.
Of course if this protocol is rejected, I will maintain it along with my 
compositor implementation
with a different namespace.

 Makefile.am|   1 +
 unstable/notification-area/README  |   4 +
 .../notification-area-unstable-v1.xml  | 128 +
 3 files changed, 133 insertions(+)
 create mode 100644 unstable/notification-area/README
 create mode 100644 unstable/notification-area/notification-area-unstable-v1.xml

diff --git a/Makefile.am b/Makefile.am
index 582b3f2..032a422 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5,6 +5,7 @@ unstable_protocols =
\
unstable/text-input/text-input-unstable-v1.xml  
\
unstable/input-method/input-method-unstable-v1.xml  
\
unstable/xdg-shell/xdg-shell-unstable-v5.xml
\
+   unstable/notification-area/notification-area-unstable-v1.xml\
$(NULL)
 
 stable_protocols = 
\
diff --git a/unstable/notification-area/README 
b/unstable/notification-area/README
new file mode 100644
index 000..a77bc43
--- /dev/null
+++ b/unstable/notification-area/README
@@ -0,0 +1,4 @@
+Protocol for DE-agnostic notification daemons
+
+Maintainers:
+Quentin Glidic 
diff --git a/unstable/notification-area/notification-area-unstable-v1.xml 
b/unstable/notification-area/notification-area-unstable-v1.xml
new file mode 100644
index 000..370034b
--- /dev/null
+++ b/unstable/notification-area/notification-area-unstable-v1.xml
@@ -0,0 +1,128 @@
+
+
+
+   Copyright © 2011-2016 Quentin "Sardem FF7" Glidic
+
+   Permission to use, copy, modify, distribute, and sell this
+   software and its documentation for any purpose is hereby granted
+   without fee, provided that the above copyright notice appear in
+   all copies and that both that copyright notice and this permission
+   notice appear in supporting documentation, and that the name of
+   the copyright holders not be used in advertising or publicity
+   pertaining to distribution of the software without specific,
+   written prior permission.  The copyright holders make no
+   representations about the suitability of this software for any
+   purpose.  It is provided "as is" without express or implied
+   warranty.
+
+   THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
+   SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+   FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
+   SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+   WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN
+   AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
+   ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
+   THIS SOFTWARE.
+
+
+
+   
+   The object is a singleton global.
+
+   This interface can only be bound once at the same time.
+   Any binding of this interface while already bound results
+   in a protocol error (bound).
+
+   This interface is intended for classic notification daemons which
+   display “bubbles”. These daemons are meant to be used in small
+   desktop environment or with highly-customizable compositors.
+   This interface must not be implemented where an integrated
+   notification system is desirable, like in complete desktop
+   environments as GNOME, KDE or EFL. Such an integrated
+
+   DE-independent daemons have an internal logic to place these
+   notifications on screen, which can be a really complex layout.
+   The compositor is in charge of providing a area for the
+   notification daemon to use. This is usually the screen area, minus
+   panels or other surfaces that should always be visible.
+   Th

Re: [PATCH weston] configure: Make libjpeg an optional dependency

[Quentin Glidic]

On 06/02/2016 06:07, Emmanuel Gil Peyrot wrote:

libjpeg is only used in shared/image-loader.c in weston, like libwebp
it doesn’t make sense to fail the entire build if it isn’t present, for
any reason.

I kept libpng a hard dependency in the image-loader because so many
other parts of weston depend on PNG, it is already mandatory from
everywhere else.

Signed-off-by: Emmanuel Gil Peyrot 
---
  configure.ac  |  8 
  shared/image-loader.c | 23 ---
  2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index bff6380..c42e32b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -311,13 +311,12 @@ fi
  AM_CONDITIONAL(ENABLE_VAAPI_RECORDER, test "x$have_libva" = xyes)


-AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], have_jpeglib=yes)
+AC_CHECK_LIB([jpeg], [jpeg_CreateDecompress], [have_jpeglib=yes], 
[have_jpeglib=no])


Please add a proper AC_ARG_ENABLE. I let you decide whether you want 
yes, no or auto as default.




  if test x$have_jpeglib = xyes; then
JPEG_LIBS="-ljpeg"
-else
-  AC_ERROR([libjpeg not found])
+  AC_SUBST(JPEG_LIBS)
+  AC_DEFINE([HAVE_JPEG], [1], [Have jpeg])
  fi
-AC_SUBST(JPEG_LIBS)

  PKG_CHECK_MODULES(CAIRO, [cairo])

@@ -676,6 +675,7 @@ AC_MSG_RESULT([

Colord Support  ${have_colord}
LCMS2 Support   ${have_lcms}
+   libjpeg Support ${have_jpeglib}
libwebp Support ${have_webp}
libunwind Support   ${have_libunwind}
VA H.264 encoding Support   ${have_libva}
diff --git a/shared/image-loader.c b/shared/image-loader.c
index ec75bd4..e6c0c7d 100644
--- a/shared/image-loader.c
+++ b/shared/image-loader.c
@@ -30,13 +30,16 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 

  #include "shared/helpers.h"
  #include "image-loader.h"

+#ifdef HAVE_JPEG
+#include 
+#endif
+
  #ifdef HAVE_WEBP
  #include 
  #endif
@@ -48,6 +51,14 @@ stride_for_width(int width)
  }

  static void
+pixman_image_destroy_func(pixman_image_t *image, void *data)
+{
+   free(data);
+}
+
+#ifdef HAVE_JPEG
+
+static void
  swizzle_row(JSAMPLE *row, JDIMENSION width)
  {
JSAMPLE *s;
@@ -68,12 +79,6 @@ error_exit(j_common_ptr cinfo)
longjmp(cinfo->client_data, 1);
  }

-static void
-pixman_image_destroy_func(pixman_image_t *image, void *data)
-{
-   free(data);
-}
-
  static pixman_image_t *
  load_jpeg(FILE *fp)
  {
@@ -132,6 +137,8 @@ load_jpeg(FILE *fp)
return pixman_image;
  }

+#endif
+
  static inline int
  multiply_alpha(int alpha, int color)
  {
@@ -363,7 +370,9 @@ struct image_loader {

  static const struct image_loader loaders[] = {
{ { 0x89, 'P', 'N', 'G' }, 4, load_png },
+#ifdef HAVE_JPEG
{ { 0xff, 0xd8 }, 2, load_jpeg },
+#endif
  #ifdef HAVE_WEBP
{ { 'R', 'I', 'F', 'F' }, 4, load_webp }
  #endif



Code is ok.

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[RFC weston 2/5] libweston: Move weston_load_module here

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 ivi-shell/ivi-layout.c   |  3 ++-
 lib/libweston.c  | 47 +++
 lib/libweston.h  |  2 ++
 src/compositor-drm.c |  9 +
 src/compositor-fbdev.c   |  3 ++-
 src/compositor-wayland.c |  5 +++--
 src/compositor-x11.c |  5 +++--
 src/compositor.c | 44 
 src/compositor.h |  3 ---
 src/main.c   |  4 ++--
 10 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index f7c4f27..fe2e47f 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -60,6 +60,7 @@
 #include 
 #include 
 
+#include "libweston.h"
 #include "compositor.h"
 #include "ivi-layout-export.h"
 #include "ivi-layout-private.h"
@@ -2853,7 +2854,7 @@ load_controller_modules(struct weston_compositor 
*compositor, const char *module
end = strchrnul(p, ',');
snprintf(buffer, sizeof buffer, "%.*s", (int)(end - p), p);
 
-   controller_module_init = weston_load_module(buffer, 
"controller_module_init");
+   controller_module_init = libweston_load_module(buffer, 
"controller_module_init");
if (!controller_module_init)
return -1;
 
diff --git a/lib/libweston.c b/lib/libweston.c
index 801d137..da21f8e 100644
--- a/lib/libweston.c
+++ b/lib/libweston.c
@@ -1,3 +1,7 @@
+
+#include 
+#include 
+
 #include "shared/zalloc.h"
 
 #include "libweston-internal.h"
@@ -22,6 +26,49 @@ libweston_uninit(struct libweston_context *context)
free(context);
 }
 
+WL_EXPORT void *
+libweston_load_module(const char *name, const char *entrypoint)
+{
+   const char *builddir = getenv("WESTON_BUILD_DIR");
+   char path[PATH_MAX];
+   void *module, *init;
+
+   if (name == NULL)
+   return NULL;
+
+   if (name[0] != '/') {
+   if (builddir)
+   snprintf(path, sizeof path, "%s/.libs/%s", builddir, 
name);
+   else
+   snprintf(path, sizeof path, "%s/%s", MODULEDIR, name);
+   } else {
+   snprintf(path, sizeof path, "%s", name);
+   }
+
+   module = dlopen(path, RTLD_NOW | RTLD_NOLOAD);
+   if (module) {
+   weston_log("Module '%s' already loaded\n", path);
+   dlclose(module);
+   return NULL;
+   }
+
+   weston_log("Loading module '%s'\n", path);
+   module = dlopen(path, RTLD_NOW);
+   if (!module) {
+   weston_log("Failed to load module: %s\n", dlerror());
+   return NULL;
+   }
+
+   init = dlsym(module, entrypoint);
+   if (!init) {
+   weston_log("Failed to lookup init function: %s\n", dlerror());
+   dlclose(module);
+   return NULL;
+   }
+
+   return init;
+}
+
 WL_EXPORT int
 libweston_load_backend(struct libweston_context *context, enum 
libweston_backend preffered)
 {
diff --git a/lib/libweston.h b/lib/libweston.h
index ca98df1..9d1bfc1 100644
--- a/lib/libweston.h
+++ b/lib/libweston.h
@@ -9,6 +9,8 @@ struct libweston_context;
 struct libweston_context *libweston_init(struct weston_compositor *compositor);
 void libweston_uninit(struct libweston_context *context);
 
+void *libweston_load_module(const char *name, const char *entrypoint);
+
 enum libweston_backend {
LIBWESTON_BACKEND_NONE = 0,
 };
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 8b9882e..04de95e 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -46,6 +46,7 @@
 #include 
 #include 
 
+#include "libweston.h"
 #include "shared/helpers.h"
 #include "shared/timespec-util.h"
 #include "libbacklight.h"
@@ -608,13 +609,13 @@ drm_output_set_gamma(struct weston_output *output_base,
 }
 
 /* Determine the type of vblank synchronization to use for the output.
- * 
+ *
  * The pipe parameter indicates which CRTC is in use.  Knowing this, we
  * can determine which vblank sequence type to use for it.  Traditional
  * cards had only two CRTCs, with CRTC 0 using no special flags, and
  * CRTC 1 using DRM_VBLANK_SECONDARY.  The first bit of the pipe
  * parameter indicates this.
- * 
+ *
  * Bits 1-5 of the pipe parameter are 5 bit wide pipe number between
  * 0-31.  If this is non-zero it indicates we're dealing with a
  * multi-gpu situation and we need to calculate the vblank sync
@@ -1525,8 +1526,8 @@ create_gbm_device(int fd)
 {
struct gbm_device *gbm;
 
-   gl_renderer = weston_load_module("gl-renderer.so",
-"gl_renderer_interface");
+   

[RFC weston 3/5] libweston: Add config getters API

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 Makefile.am  |   1 +
 lib/backend-config.c |  33 
 lib/libweston-internal.h |  10 
 lib/libweston.h  |  11 
 src/compositor.c | 137 +++
 src/compositor.h |   2 +
 src/main.c   |  12 +++--
 7 files changed, 201 insertions(+), 5 deletions(-)
 create mode 100644 lib/backend-config.c

diff --git a/Makefile.am b/Makefile.am
index 1cfe154..36078ba 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -103,6 +103,7 @@ noinst_LTLIBRARIES += libweston.la
 libweston_la_SOURCES = \
lib/libweston.h \
lib/libweston.c \
+   lib/backend-config.c \
$(NULL)
 libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
 libweston_la_LDFLAGS = \
diff --git a/lib/backend-config.c b/lib/backend-config.c
new file mode 100644
index 000..56e0936
--- /dev/null
+++ b/lib/backend-config.c
@@ -0,0 +1,33 @@
+
+#include "libweston-internal.h"
+#include "backend-config.h"
+
+WL_EXPORT void
+libweston_backend_config_set_user_data(struct libweston_context *context, void 
*user_data)
+{
+   context->backend_config.user_data = user_data;
+}
+
+WL_EXPORT void
+libweston_backend_config_set_bool_getter(struct libweston_context *context, 
libweston_backend_bool_getter getter)
+{
+   context->backend_config.bool_getter = getter;
+}
+
+WL_EXPORT void
+libweston_backend_config_set_string_getter(struct libweston_context *context, 
libweston_backend_string_getter getter)
+{
+   context->backend_config.string_getter = getter;
+}
+
+WL_EXPORT void
+libweston_backend_config_set_string_list_getter(struct libweston_context 
*context, libweston_backend_string_list_getter getter)
+{
+   context->backend_config.string_list_getter = getter;
+}
+
+WL_EXPORT void
+libweston_backend_config_set_int_getter(struct libweston_context *context, 
libweston_backend_int_getter getter)
+{
+   context->backend_config.int_getter = getter;
+}
diff --git a/lib/libweston-internal.h b/lib/libweston-internal.h
index 2e1b7c3..1e5267e 100644
--- a/lib/libweston-internal.h
+++ b/lib/libweston-internal.h
@@ -7,8 +7,18 @@
 
 int libweston_backend_init(struct libweston_context *context);
 
+struct backend_config_interface {
+   void *user_data;
+
+   libweston_backend_bool_getter bool_getter;
+   libweston_backend_string_getter string_getter;
+   libweston_backend_string_list_getter string_list_getter;
+   libweston_backend_int_getter int_getter;
+};
+
 struct libweston_context {
struct weston_compositor *compositor;
+   struct backend_config_interface backend_config;
 };
 
 #endif /* _LIBWESTON_INTERNAL_H_ */
diff --git a/lib/libweston.h b/lib/libweston.h
index 9d1bfc1..7a995b8 100644
--- a/lib/libweston.h
+++ b/lib/libweston.h
@@ -2,6 +2,7 @@
 #ifndef _LIBWESTON_H_
 #define _LIBWESTON_H_
 
+#include 
 #include "compositor.h"
 
 struct libweston_context;
@@ -16,4 +17,14 @@ enum libweston_backend {
 };
 int libweston_load_backend(struct libweston_context *context, enum 
libweston_backend preffered);
 
+typedef bool (*libweston_backend_bool_getter)(const char *section, const char 
*key, bool default_value, void *user_data);
+typedef char *(*libweston_backend_string_getter)(const char *section, const 
char *key, const char *default_value, void *user_data);
+typedef char **(*libweston_backend_string_list_getter)(const char *section, 
const char *key, const char * const *default_value, size_t *size, void 
*user_data);
+typedef int (*libweston_backend_int_getter)(const char *section, const char 
*key, int default_value, void *user_data);
+void libweston_backend_config_set_user_data(struct libweston_context *context, 
void *user_data);
+void libweston_backend_config_set_bool_getter(struct libweston_context 
*context, libweston_backend_bool_getter getter);
+void libweston_backend_config_set_string_getter(struct libweston_context 
*context, libweston_backend_string_getter getter);
+void libweston_backend_config_set_string_list_getter(struct libweston_context 
*context, libweston_backend_string_list_getter getter);
+void libweston_backend_config_set_int_getter(struct libweston_context 
*context, libweston_backend_int_getter getter);
+
 #endif /* _LIBWESTON_H_ */
diff --git a/src/compositor.c b/src/compositor.c
index 5b698d1..57c3612 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -4701,6 +4701,137 @@ timeline_key_binding_handler(struct weston_keyboard 
*keyboard, uint32_t time,
weston_timeline_open(compositor);
 }
 
+static bool
+backend_config_bool_getter(const char *section, const char *key, bool 
default_value, void *user_data)
+{
+   struct weston_compositor *compositor = user_data;
+   int r = default_value;
+
+   if (!section) {
+   const struct weston_option options[] = {
+ 

[RFC weston 4/5] drm: Move backend to libweston

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 Makefile.am   | 10 ++---
 {src => lib}/compositor-drm.c | 87 +++
 {src => lib}/libbacklight.c   |  0
 {src => lib}/libbacklight.h   |  0
 lib/libweston.c   | 23 
 lib/libweston.h   |  1 +
 src/main.c|  3 ++
 7 files changed, 71 insertions(+), 53 deletions(-)
 rename {src => lib}/compositor-drm.c (97%)
 rename {src => lib}/libbacklight.c (100%)
 rename {src => lib}/libbacklight.h (100%)

diff --git a/Makefile.am b/Makefile.am
index 36078ba..68997d8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -287,12 +287,12 @@ drm_backend_la_CFLAGS =   \
$(DRM_COMPOSITOR_CFLAGS)\
$(AM_CFLAGS)
 drm_backend_la_SOURCES =   \
-   src/compositor-drm.c\
+   lib/compositor-drm.c\
$(INPUT_BACKEND_SOURCES)\
shared/helpers.h\
shared/timespec-util.h  \
-   src/libbacklight.c  \
-   src/libbacklight.h
+   lib/libbacklight.c  \
+   lib/libbacklight.h
 
 if ENABLE_VAAPI_RECORDER
 drm_backend_la_SOURCES += src/vaapi-recorder.c src/vaapi-recorder.h
@@ -1325,8 +1325,8 @@ if BUILD_SETBACKLIGHT
 noinst_PROGRAMS += setbacklight
 setbacklight_SOURCES = \
tests/setbacklight.c\
-   src/libbacklight.c  \
-   src/libbacklight.h
+   lib/libbacklight.c  \
+   lib/libbacklight.h
 setbacklight_CFLAGS = $(AM_CFLAGS) $(SETBACKLIGHT_CFLAGS)
 setbacklight_LDADD = $(SETBACKLIGHT_LIBS)
 endif
diff --git a/src/compositor-drm.c b/lib/compositor-drm.c
similarity index 97%
rename from src/compositor-drm.c
rename to lib/compositor-drm.c
index 04de95e..5d3cd0c 100644
--- a/src/compositor-drm.c
+++ b/lib/compositor-drm.c
@@ -46,7 +46,7 @@
 #include 
 #include 
 
-#include "libweston.h"
+#include "libweston-internal.h"
 #include "shared/helpers.h"
 #include "shared/timespec-util.h"
 #include "libbacklight.h"
@@ -88,6 +88,7 @@ enum output_config {
 
 struct drm_backend {
struct weston_backend base;
+   struct libweston_context *context;
struct weston_compositor *compositor;
 
struct udev *udev;
@@ -2144,15 +2145,15 @@ setup_output_seat_constraint(struct drm_backend *b,
 }
 
 static int
-get_gbm_format_from_section(struct weston_config_section *section,
-   uint32_t default_value,
-   uint32_t *format)
+get_gbm_format_from_config(struct libweston_context *context,
+  const char *section,
+  uint32_t default_value,
+  uint32_t *format)
 {
char *s;
int ret = 0;
 
-   weston_config_section_get_string(section,
-"gbm-format", &s, NULL);
+   s = context->backend_config.string_getter(section, "gbm-format", NULL, 
context->backend_config.user_data);
 
if (s == NULL)
*format = default_value;
@@ -2296,7 +2297,7 @@ create_output_for_connector(struct drm_backend *b,
struct drm_output *output;
struct drm_mode *drm_mode, *next, *current;
struct weston_mode *m;
-   struct weston_config_section *section;
+   char section[39];
drmModeModeInfo crtc_mode, modeline;
int i, width, height, scale;
char *s;
@@ -2320,9 +2321,9 @@ create_output_for_connector(struct drm_backend *b,
output->base.serial_number = "unknown";
wl_list_init(&output->base.mode_list);
 
-   section = weston_config_get_section(b->compositor->config, "output", 
"name",
-   output->base.name);
-   weston_config_section_get_string(section, "mode", &s, "preferred");
+   snprintf(section, sizeof section, "output %s", output->base.name);
+
+   s = b->context->backend_config.string_getter(section, "mode", 
"preferred", b->context->backend_config.user_data);
if (strcmp(s, "off") == 0)
config = OUTPUT_CONFIG_OFF;
else if (strcmp(s, "preferred") == 0)
@@ -2340,20 +2341,21 @@ create_output_for_connector(struct drm_backend *b,
}
free(s);
 
-   weston_config_section_get_int(section, "scale", &scale, 1);
-   weston_config_section_get_string(section, "transform", &s, "normal");
+   scale = b->context->backend_config.int_getter(section, "scale", 1, 
b->context->backend_config.user_data);
+ 

[RFC weston 0/5] libweston: section+key=value backend config

[Quentin Glidic]

From: Quentin Glidic 

Here is my draft for the key/value backend config idea.
As I stated on IRC already, I think the key/value config is the nicer one.

First two patches are quite independent from the backend config topic,
but are needed to get the final config API from the start.

The main patch is the third one, especially the compositor.c part, which
demonstrates how this API would be used.

Patches 4 and 5 are just porting drm and x11 backends, for testing.

It seems the biggest problem of this way is “leaking” the whole configuration
to the backend. I do not think it is a problem, since I would trust libweston
and backends are hard-coded in it, so no wild backend would access that 
configuration.

Quentin Glidic (5):
  libweston: Initial dummy library
  libweston: Move weston_load_module here
  libweston: Add config getters API
  drm: Move backend to libweston
  x11: Move to libweston

 Makefile.am   |  26 --
 ivi-shell/ivi-layout.c|   3 +-
 lib/backend-config.c  |  33 
 {src => lib}/compositor-drm.c |  94 ++---
 {src => lib}/compositor-x11.c |  90 
 {src => lib}/libbacklight.c   |   0
 {src => lib}/libbacklight.h   |   0
 lib/libweston-internal.h  |  24 ++
 lib/libweston.c   | 101 +++
 lib/libweston.h   |  32 +++
 src/compositor-fbdev.c|   3 +-
 src/compositor-wayland.c  |   5 +-
 src/compositor.c  | 188 --
 src/compositor.h  |   6 +-
 src/main.c|  27 --
 15 files changed, 461 insertions(+), 171 deletions(-)
 create mode 100644 lib/backend-config.c
 rename {src => lib}/compositor-drm.c (97%)
 rename {src => lib}/compositor-x11.c (95%)
 rename {src => lib}/libbacklight.c (100%)
 rename {src => lib}/libbacklight.h (100%)
 create mode 100644 lib/libweston-internal.h
 create mode 100644 lib/libweston.c
 create mode 100644 lib/libweston.h

-- 
2.6.4

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

[RFC weston 1/5] libweston: Initial dummy library

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 Makefile.am  | 13 -
 lib/libweston-internal.h | 14 ++
 lib/libweston.c  | 29 +
 lib/libweston.h  | 17 +
 src/compositor.c |  7 +++
 src/compositor.h |  1 +
 src/main.c   |  6 ++
 7 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 lib/libweston-internal.h
 create mode 100644 lib/libweston.c
 create mode 100644 lib/libweston.h

diff --git a/Makefile.am b/Makefile.am
index 623621d..1cfe154 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,7 @@ all-local : weston.ini ivi-shell/weston.ini
 AM_CFLAGS = $(GCC_CFLAGS)
 
 AM_CPPFLAGS =  \
+   -I$(top_srcdir)/lib \
-I$(top_srcdir)/src \
-I$(top_builddir)/src   \
-I$(top_builddir)/clients   \
@@ -65,7 +66,7 @@ weston_LDFLAGS = -export-dynamic
 weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
 weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
 weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
-   $(DLOPEN_LIBS) -lm -lrt libshared.la
+   $(DLOPEN_LIBS) -lm -lrt libshared.la libweston.la
 
 weston_SOURCES =   \
src/git-version.h   \
@@ -97,6 +98,16 @@ weston_SOURCES = \
shared/platform.h   \
src/weston-egl-ext.h
 
+noinst_LTLIBRARIES += libweston.la
+
+libweston_la_SOURCES = \
+   lib/libweston.h \
+   lib/libweston.c \
+   $(NULL)
+libweston_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
+libweston_la_LDFLAGS = \
+   -version-info 0:0:0
+
 if SYSTEMD_NOTIFY_SUPPORT
 module_LTLIBRARIES += systemd-notify.la
 systemd_notify_la_LDFLAGS = -module -avoid-version
diff --git a/lib/libweston-internal.h b/lib/libweston-internal.h
new file mode 100644
index 000..2e1b7c3
--- /dev/null
+++ b/lib/libweston-internal.h
@@ -0,0 +1,14 @@
+
+#ifndef _LIBWESTON_INTERNAL_H_
+#define _LIBWESTON_INTERNAL_H_
+
+#include 
+#include "libweston.h"
+
+int libweston_backend_init(struct libweston_context *context);
+
+struct libweston_context {
+   struct weston_compositor *compositor;
+};
+
+#endif /* _LIBWESTON_INTERNAL_H_ */
diff --git a/lib/libweston.c b/lib/libweston.c
new file mode 100644
index 000..801d137
--- /dev/null
+++ b/lib/libweston.c
@@ -0,0 +1,29 @@
+#include "shared/zalloc.h"
+
+#include "libweston-internal.h"
+
+WL_EXPORT struct libweston_context *
+libweston_init(struct weston_compositor *compositor)
+{
+   struct libweston_context *context;
+
+   context = zalloc(sizeof(struct libweston_context));
+   if ( context == NULL )
+   return NULL;
+
+   context->compositor = compositor;
+
+   return context;
+}
+
+WL_EXPORT void
+libweston_uninit(struct libweston_context *context)
+{
+   free(context);
+}
+
+WL_EXPORT int
+libweston_load_backend(struct libweston_context *context, enum 
libweston_backend preffered)
+{
+   return -1;
+}
diff --git a/lib/libweston.h b/lib/libweston.h
new file mode 100644
index 000..ca98df1
--- /dev/null
+++ b/lib/libweston.h
@@ -0,0 +1,17 @@
+
+#ifndef _LIBWESTON_H_
+#define _LIBWESTON_H_
+
+#include "compositor.h"
+
+struct libweston_context;
+
+struct libweston_context *libweston_init(struct weston_compositor *compositor);
+void libweston_uninit(struct libweston_context *context);
+
+enum libweston_backend {
+   LIBWESTON_BACKEND_NONE = 0,
+};
+int libweston_load_backend(struct libweston_context *context, enum 
libweston_backend preffered);
+
+#endif /* _LIBWESTON_H_ */
diff --git a/src/compositor.c b/src/compositor.c
index 98efb4c..56eefc4 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -54,6 +54,7 @@
 #include "timeline.h"
 
 #include "compositor.h"
+#include "libweston.h"
 #include "scaler-server-protocol.h"
 #include "presentation_timing-server-protocol.h"
 #include "shared/helpers.h"
@@ -4720,6 +4721,10 @@ weston_compositor_create(struct wl_display *display, 
void *user_data)
if (!ec)
return NULL;
 
+   ec->libweston = libweston_init(ec);
+   if (!ec->libweston)
+   goto fail;
+
ec->wl_display = display;
ec->user_data = user_data;
wl_signal_init(&ec->destroy_signal);
@@ -5021,6 +5026,8 @@ weston_compositor_destroy(struct weston_compositor 
*compositor)
 
if (compositor->backend)
compositor->backend->destroy(compositor);
+   if (compositor->libweston)
+   libweston_uninit(compositor->libweston);
free(compositor);
 }
 
diff --git a/src/compositor.h b/src/compositor.h
index 58

[RFC weston 5/5] x11: Move to libweston

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 Makefile.am   |  2 +-
 {src => lib}/compositor-x11.c | 87 ---
 lib/libweston.c   |  2 +
 lib/libweston.h   |  1 +
 src/main.c|  2 +
 5 files changed, 39 insertions(+), 55 deletions(-)
 rename {src => lib}/compositor-x11.c (95%)

diff --git a/Makefile.am b/Makefile.am
index 68997d8..59e2130 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -260,7 +260,7 @@ x11_backend_la_CFLAGS = \
$(X11_COMPOSITOR_CFLAGS)\
$(AM_CFLAGS)
 x11_backend_la_SOURCES =   \
-   src/compositor-x11.c\
+   lib/compositor-x11.c\
shared/helpers.h
 endif
 
diff --git a/src/compositor-x11.c b/lib/compositor-x11.c
similarity index 95%
rename from src/compositor-x11.c
rename to lib/compositor-x11.c
index cac96cb..f71b813 100644
--- a/src/compositor-x11.c
+++ b/lib/compositor-x11.c
@@ -49,7 +49,7 @@
 
 #include 
 
-#include "libweston.h"
+#include "libweston-internal.h"
 #include "compositor.h"
 #include "gl-renderer.h"
 #include "pixman-renderer.h"
@@ -61,13 +61,9 @@
 
 #define DEFAULT_AXIS_STEP_DISTANCE wl_fixed_from_int(10)
 
-static int option_width;
-static int option_height;
-static int option_scale;
-static int option_count;
-
 struct x11_backend {
struct weston_backendbase;
+   struct libweston_context *context;
struct weston_compositor *compositor;
 
Display *dpy;
@@ -1564,19 +1560,22 @@ init_gl_renderer(struct x11_backend *b)
return ret;
 }
 static struct x11_backend *
-x11_backend_create(struct weston_compositor *compositor,
-  int fullscreen,
-  int no_input,
-  int use_pixman,
-  int *argc, char *argv[],
-  struct weston_config *config)
+x11_backend_create(struct libweston_context *context)
 {
+   struct weston_compositor *compositor = context->compositor;
struct x11_backend *b;
+   char **output_names;
+   size_t output_names_size;
struct x11_output *output;
-   struct weston_config_section *section;
+   bool fullscreen;
+   bool no_input;
+   int option_width;
+   int option_height;
+   int option_scale;
+   int option_count;
int i, x = 0, output_count = 0;
int width, height, scale, count;
-   const char *section_name;
+   char section[64];
char *name, *t, *mode;
uint32_t transform;
 
@@ -1586,6 +1585,7 @@ x11_backend_create(struct weston_compositor *compositor,
if (b == NULL)
return NULL;
 
+   b->context = context;
b->compositor = compositor;
if (weston_compositor_set_presentation_clock_software(compositor) < 0)
goto err_free;
@@ -1600,6 +1600,13 @@ x11_backend_create(struct weston_compositor *compositor,
if (xcb_connection_has_error(b->conn))
goto err_xdisplay;
 
+   fullscreen = b->context->backend_config.bool_getter(NULL, "fullscreen", 
false, b->context->backend_config.user_data);
+   option_width = b->context->backend_config.int_getter(NULL, "width", 0, 
b->context->backend_config.user_data);
+   option_height = b->context->backend_config.int_getter(NULL, "height", 
0, b->context->backend_config.user_data);
+   option_scale = b->context->backend_config.int_getter(NULL, "scale", 0, 
b->context->backend_config.user_data);
+   option_count = b->context->backend_config.int_getter(NULL, 
"output-count", 0, b->context->backend_config.user_data);
+   no_input = b->context->backend_config.bool_getter(NULL, "no-input", 
false, b->context->backend_config.user_data);
+
b->screen = x11_compositor_get_default_screen(b);
wl_array_init(&b->keys);
 
@@ -1612,7 +1619,7 @@ x11_backend_create(struct weston_compositor *compositor,
fullscreen = 0;
}
 
-   b->use_pixman = use_pixman;
+   b->use_pixman = b->context->backend_config.bool_getter(NULL, 
"use-pixman", false, b->context->backend_config.user_data);
if (b->use_pixman) {
if (pixman_renderer_init(compositor) < 0) {
weston_log("Failed to initialize pixman renderer for 
X11 backend\n");
@@ -1622,7 +1629,7 @@ x11_backend_create(struct weston_compositor *compositor,
else if (init_gl_renderer(b) < 0) {
goto err_xdisplay;
}
-   weston_log("Using %s renderer\n", use_pixman ? "pixman" : "gl");
+   weston_log("Using %s renderer\n", b->use_

Re: [RFC weston 4/5] drm: Move backend to libweston

[Quentin Glidic]

On 13/02/2016 11:22, Benoit Gschwind wrote:
>
> [snip]
>

Following a comment from Quentin Glidic on IRC, I read wrong the
snprintf (I readed printf ... a shame, my apology). Which make the patch
valid if output->base.name is not too long.


output->base.name is managed in the backend and is fixed-sized already.
The size I used is even way too large for the possible values here.


But I keep the following:



This case show that this approach is not very good, In that case the
developer must give a list of unbound outputs setup and key/value is not
well suited for that case. (there is some workaround, like using key
pattern).

This case also leave me to add that section/key/value is not a good
choice and you should stick to key/value.

I still prefer the opaque configuration structure with function to set
the structure content.


I am not sure how to understand that part (even with your precision on 
IRC) but I think it is relevant to remind something that I find really 
important (and not only for that specific comment):


We are designing the *backend* configuration API.
We agreed already that backends will be an internal implementation 
detail of libweston. As such, I (as a future libweston-based compositor 
writer) consider them as trusted, which means:

- They would not use getters to grab random parts of my configuration
- They should keep the options set “small” enough for end-users to write 
it in any text-based configuration format the compositor would like to 
use (Weston is currently using a KeyFile, which is a really common 
format and not really fitting for complex structures). I expect backends 
needing a really complex configuration to provide a GUI program, or use 
their own configuration file.
DRM is probably the most complex backend we will have for quite a long 
time (I consider any future “hardware-based” backend will have similar 
needs), and it fits well in the INI/KeyFile format.



In the current form, my patch has one little (IMO) 
non-backward-compatible change: it does not use a duplicated name 
"[output]" for the section with the "name=" key as the selector.
It is entirely doable if it is a requirement for this patch to be 
considered, but in this design, backends should use unique section 
names, since KeyFile parser (like GLib’s) can merge duplicate sections.



Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland-protocols] configure.ac: Don't use AC_CANONICAL_* macro calls

[Quentin Glidic]

On 18/02/2016 13:29, Jussi Kukkonen wrote:

There's nothing in the build that requires knowledge of target or
host architecture, yet the calls will fail if the host cpu is not
recognised (which happens when e.g. Yocto builds for "allarch").

Signed-off-by: Jussi Kukkonen 
---
  configure.ac | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index 90cce42..596093b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,9 +15,6 @@ AC_CONFIG_MACRO_DIR([m4])

  AC_SUBST([WAYLAND_PROTOCOLS_VERSION], [wayland_protocols_version])

-AC_CANONICAL_HOST
-AC_CANONICAL_BUILD
-
  AC_ARG_VAR([wayland_scanner], [The wayland-scanner executable])
  AC_PATH_PROG([wayland_scanner], [wayland-scanner])
  if test x$wayland_scanner = x; then



These two calls are needed, see a few lines below:
"if test x$host = x$build; then"
$host and $build are only populated if we call these macros.

Nacked-by: Quentin Glidic 

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland] build: fix ./configure --disable-dtd-validation

[Quentin Glidic]

On 29/02/2016 13:59, Pekka Paalanen wrote:

From: Pekka Paalanen 

When configured with --disable-dtd-validation:

   CPPASsrc/dtddata.o
src/dtddata.S: Assembler messages:
src/dtddata.S:39: Error: file not found: src/wayland.dtd.embed
Makefile:1520: recipe for target 'src/dtddata.o' failed

This is because the variable name used does not match the implicit
variable name in autoconf.

Fix the variable name, making both --disable-dtd-validation and
--enable-dtd-validation to what they should.

Do not try to build dtddata.S if dtd-validation is disabled. It depends
on wayland.dtd.embed which is created by configure only if
dtd-validation is enabled.

If not building dtddata.S, also make sure the extern definitions in
scanner.c are compiled out.

Bugzilla: https://bugs.gentoo.org/show_bug.cgi?id=575212
Reported-by: l...@gentoo.org
Signed-off-by: Pekka Paalanen 


Reviewed-by: Quentin Glidic 


--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] input: Implement wl_seat.release

[Quentin Glidic]

From: Quentin Glidic 

Avoid a crash because listener is NULL.

Signed-off-by: Quentin Glidic 
---
 src/input.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/input.c b/src/input.c
index 8c106dd..5d13b08 100644
--- a/src/input.c
+++ b/src/input.c
@@ -2230,10 +2230,17 @@ seat_get_touch(struct wl_client *client, struct 
wl_resource *resource,
   seat, unbind_resource);
 }
 
+static void
+seat_release(struct wl_client *client, struct wl_resource *resource)
+{
+   wl_resource_destroy(resource);
+}
+
 static const struct wl_seat_interface seat_interface = {
seat_get_pointer,
seat_get_keyboard,
seat_get_touch,
+   seat_release,
 };
 
 static void
-- 
2.7.2

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

[PATCH weston] compositor.h: Add shell_interface.get_output_work_area

[Quentin Glidic]

From: Quentin Glidic 

This will allow plugins to be aware of e.g. panels, to avoid covering
them with other surfaces.

Signed-off-by: Quentin Glidic 
---
 desktop-shell/shell.c | 4 +++-
 src/compositor.h  | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 24437d8..9b01f4f 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -453,10 +453,11 @@ get_output_panel_size(struct desktop_shell *shell,
 }
 
 static void
-get_output_work_area(struct desktop_shell *shell,
+get_output_work_area(void *data,
 struct weston_output *output,
 pixman_rectangle32_t *area)
 {
+   struct desktop_shell *shell = data;
int32_t panel_width = 0, panel_height = 0;
 
area->x = output->x;
@@ -6625,6 +6626,7 @@ module_init(struct weston_compositor *ec,
ec->shell_interface.set_window_geometry = set_window_geometry;
ec->shell_interface.set_maximized = shell_interface_set_maximized;
ec->shell_interface.set_pid = set_pid;
+   ec->shell_interface.get_output_work_area = get_output_work_area;
 
weston_layer_init(&shell->fullscreen_layer, &ec->cursor_layer.link);
weston_layer_init(&shell->panel_layer, &shell->fullscreen_layer.link);
diff --git a/src/compositor.h b/src/compositor.h
index 08b4059..d61a327 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -116,6 +116,7 @@ struct weston_shell_interface {
int32_t width, int32_t height);
void (*set_maximized)(struct shell_surface *shsurf);
void (*set_pid)(struct shell_surface *shsurf, pid_t pid);
+   void (*get_output_work_area)(void *shell, struct weston_output *output, 
pixman_rectangle32_t *area);
 };
 
 struct weston_animation {
-- 
2.7.2

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

[PATCH weston] weston.pc: Properly specify pixman and xkbcommon as Requires.private

[Quentin Glidic]

From: Quentin Glidic 

We include their header in compositor.h, but they are not required for
linking if plugins do not use them.

Signed-off-by: Quentin Glidic 
---
 src/weston.pc.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/weston.pc.in b/src/weston.pc.in
index 828cb1f..7a09938 100644
--- a/src/weston.pc.in
+++ b/src/weston.pc.in
@@ -8,4 +8,5 @@ pkglibexecdir=${libexecdir}/@PACKAGE@
 Name: Weston Plugin API
 Description: Header files for Weston plugin development
 Version: @WESTON_VERSION@
+Requires.private: pixman-1 xkbcommon
 Cflags: -I${includedir}
-- 
2.7.2

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

[PATCH weston v2] weston.pc: Properly specify Requires.private

[Quentin Glidic]

From: Quentin Glidic 

We include wayland-server.h, pixman.h and xkbcommon.h in compositor.h,
but they are not required for linking if the plugin doesn’t use them.

Signed-off-by: Quentin Glidic 
---

v2: Was missing wayland-server

src/weston.pc.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/weston.pc.in b/src/weston.pc.in
index 828cb1f..c560eb3 100644
--- a/src/weston.pc.in
+++ b/src/weston.pc.in
@@ -8,4 +8,5 @@ pkglibexecdir=${libexecdir}/@PACKAGE@
 Name: Weston Plugin API
 Description: Header files for Weston plugin development
 Version: @WESTON_VERSION@
+Requires.private: wayland-server pixman-1 xkbcommon
 Cflags: -I${includedir}
-- 
2.7.2

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

Re: [PATCH weston v2] weston.pc: Properly specify Requires.private

[Quentin Glidic]

On 06/04/2016 08:39, Bryce Harrington wrote:

[snip]
One thing I wonder is if any of these might need to become a Require
when we are providing a libweston library API?


From [1] which references [2], a lot of pkg-config files are doing it 
wrong. Requires should only be used in rare cases (though it appeared 
quite late in pkg-config, and most projects just didn’t update).
I believe the most common case is with GObject: the G_TYPE_CHECK_* 
macros are calling g_type_check_* functions which are libgobject-2.0.so 
symbols.


In Weston, we have no such thing, so we are safe.


[1] 
[2] 

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] weston-info: do not round refresh rates

[Quentin Glidic]

On 12/04/2016 10:13, Pekka Paalanen wrote:

From: Pekka Paalanen 

Weston-info was accidentally rounding refresh rates to integer Hz.

Fix it to print 3 decimals, as the protocol carries exactly that.

Reported-by: Michel Dänzer 
Cc: John Galt 
Signed-off-by: Pekka Paalanen 
---
 clients/weston-info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/weston-info.c b/clients/weston-info.c
index 8ba80c0..aa81816 100644
--- a/clients/weston-info.c
+++ b/clients/weston-info.c
@@ -226,7 +226,7 @@ print_output_info(void *data)
wl_list_for_each(mode, &output->modes, link) {
printf("\tmode:\n");

-   printf("\t\twidth: %d px, height: %d px, refresh: %.f Hz,\n",
+   printf("\t\twidth: %d px, height: %d px, refresh: %.3f Hz,\n",
   mode->width, mode->height,
   (float) mode->refresh / 1000);




Trivial enough, LGTM.
Reviewed-by: Quentin Glidic 

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 3/6] libweston-desktop/xwayland: window type XWAYLAND cannot have a parent

[Quentin Glidic]

On 24/11/2016 13:59, Pekka Paalanen wrote:

On Thu, 24 Nov 2016 12:29:26 +
Daniel Stone  wrote:


Hi,

On 24 November 2016 at 11:40, Pekka Paalanen  wrote:

Add an assert to ensure that a window of type XWAYLAND is never
attempted with a parent.

This essentially adding documentation.

Signed-off-by: Pekka Paalanen 
---
 libweston-desktop/xwayland.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index bd68bc6..0c4ff2b 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -74,6 +74,7 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
bool to_add = (parent == NULL && state != XWAYLAND);

assert(state != NONE);
+   assert(!parent || state != XWAYLAND);


From looking at the callers, it seems like it should be (!parent ||
state == TRANSIENT). I'm a huge fan of adding documentation via
asserts though!


Hi,

hm, Quentin said type XWAYLAND windows (mostly just override-redirect)
must not have a parent, and that's what I wanted to document since
XWAYLAND is the one with all the special handling.



That is exactly why I don’t like asserts as docs for such cases.

This is all internal API, and it is supposed to be used that way:
- TOPLEVEL, MAXIMIZED and FULLSCREEN are for toplevel windows, that the 
shell will know about
- XWAYLAND is for special parentless windows (mostly O-R, but currently 
we have some false positive matches)
- TRANSIENT is for grabless windows that the shell will never know 
about, and all of them will have a parent *and a position relative to 
this parent*


If a window has a parent and a relative positione, it must be linked to 
this parent. This way, any created view of the parent will get children 
views created too. Theoretically, an XWAYLAND window could have O-R 
children, if they do not have an absolute position, but they would have 
to be TRANSIENT.


Both your asserts are correct, but they do not “document” the same 
thing. :-)


Maybe it was a mistake to reuse as much code as possible here, since 
XWAYLAND is really special.


Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 6/6] libweston-desktop/xwayland: add is_mapped handling for XWAYLAND

[Quentin Glidic]

On 24/11/2016 12:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

The xwayland window type XWAYLAND is not handled by the shell at all,
instead libweston-desktop maps such surfaces itself. However, it forgot
to set weston_surface::is_mapped and weston_view::is_mapped.

weston_surface::is_mapped affects the behaviour of weston_view_unmap()
and weston_surface_attach().

weston_view::is_mapped affects the behaviour of weston_view_unmap() and
weston_view_destroy().

When manually mapping a window of type XWAYLAND, mark both the view and
surface as mapped. This follows the expections in libweston, even though
the meaning of is_mapped is not clearly defined for either surface or
view.

Also, when the XWAYLAND window is manually unmapped, unmap the
weston_surface. This updates weston_surface::is_mapped to reflect the
state. However, as a side-effect it will also unmap all sibling views,
should any exist.

Signed-off-by: Pekka Paalanen 
---
 libweston-desktop/xwayland.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index 7b8f6db..6fb566a 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -71,6 +71,7 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
 struct weston_desktop_surface 
*parent,
 int32_t x, int32_t y)
 {
+   struct weston_surface *surface_base;
bool to_add = (parent == NULL && state != XWAYLAND);

assert(state != NONE);
@@ -81,6 +82,8 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
return;
}

+   surface_base = weston_desktop_surface_get_surface(surface->surface);


The convention in libweston-desktop is to use "wsurface" for 
weston_surface (and "dsurface" for weston_desktop_surface in 
libweston-desktop shells).

And why didn’t you assign the variable directly?



+
if (surface->state != state) {
if (surface->state == XWAYLAND) {
assert(!surface->added);
@@ -88,6 +91,7 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
weston_desktop_surface_unlink_view(surface->view);
weston_view_destroy(surface->view);
surface->view = NULL;
+   weston_surface_unmap(surface_base);


If we define better what “mapped” means, maybe it would make sense to 
unmap a surface with no view?



Anyway, it is a good thing to do:
Reviewed-by: Quentin Glidic 

Cheers,



}

if (to_add) {
@@ -109,6 +113,8 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf

weston_layer_entry_insert(&surface->xwayland->layer.view_list,
  &surface->view->layer_link);
weston_view_set_position(surface->view, x, y);
+   surface->view->is_mapped = true;
+   surface_base->is_mapped = true;
}

surface->state = state;




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 5/6] libweston-desktop/xwayland: XWAYLAND surfaces are never 'added'

[Quentin Glidic]

On 24/11/2016 12:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Add documentation (asserts) that show that windows of types XWAYLAND are
never registered with the shell.

Signed-off-by: Pekka Paalanen 


Not a big fan of asserts in internal stuff, but if you feel like it’s 
needed:

Reviewed-by: Quentin Glidic 

Cheers,


---
 libweston-desktop/xwayland.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index f0cd1ed..7b8f6db 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -83,6 +83,8 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf

if (surface->state != state) {
if (surface->state == XWAYLAND) {
+   assert(!surface->added);
+
weston_desktop_surface_unlink_view(surface->view);
weston_view_destroy(surface->view);
surface->view = NULL;
@@ -100,6 +102,8 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
}

if (state == XWAYLAND) {
+   assert(!surface->added);
+
surface->view =

weston_desktop_surface_create_view(surface->surface);

weston_layer_entry_insert(&surface->xwayland->layer.view_list,




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 4/6] libweston-desktop/xwayland: clarify 'added' logic

[Quentin Glidic]

On 24/11/2016 12:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Setting to a constant is much easier to read and grep for than setting to
a computed variable.

There are no functional changes.

Signed-off-by: Pekka Paalanen 


It was clear to me at least. ;-)
Anyway:
Reviewed-by: Quentin Glidic 

Cheers,



---
 libweston-desktop/xwayland.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index 0c4ff2b..f0cd1ed 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -92,9 +92,11 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf

weston_desktop_surface_unset_relative_to(surface->surface);
weston_desktop_api_surface_added(surface->desktop,
 surface->surface);
+   surface->added = true;
} else if (surface->added) {
weston_desktop_api_surface_removed(surface->desktop,
   surface->surface);
+   surface->added = false;
}

if (state == XWAYLAND) {
@@ -106,7 +108,6 @@ weston_desktop_xwayland_surface_change_state(struct 
weston_desktop_xwayland_surf
}

surface->state = state;
-   surface->added = to_add;
}

if (parent != NULL)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 0/7] Initial Xwayland window positioning

[Quentin Glidic]

On 29/11/2016 16:11, Pekka Paalanen wrote:

From: Pekka Paalanen 

Many old X11 applications provide -geometry command line option that can be
used to initially position the window. Some obscure applications even rely on
this to work properly. Currently it does not work in Weston, the shell will
unconditionally pick a place of its own.

This series makes -geometry +x+y work.

First it sets up the plumbing needed to deliver initial position from
xwayland.so through libweston-desktop to the desktop shell.

However, shell using the initial position is not enough because Xwayland runs
wild with its wl_surface.commits. Commonly Xwayland does the first commit
before xwayland.so has set up the window geometry, causing the initial
placement to be wrong. It's a race. Xwayland also waits for frame callbacks so
the compositor cannot ignore the commit either, because Xwayland would never
commit again. Writing a completely separate path for Xwayland windows in Weston
core and the shell would be ugly the least and a maintenance nightmare. Hence
the race is suggested to be fixed with more X11 protocol, in the last patch, so
that the first commit from Xwayland happens when everything is actually ready.

A band-aid fix is to handle geometry changes on an already mapped window
attempting to keep the content in place, but because of a related race the
decorations are already drawn, so the window appears to jump rather than just
grow decorations. But, arguably on-the-fly geometry changes should be handled.

The jumping is fixed with the help of a new Xwayland feature:
_WAYLAND_ALLOW_COMMITS property. With that, we not only ensure the geometry is
set before the first commit, but also that the decorations are really fully
drawn.

The last is WIP, because when we investigated the original race, Daniel came up
with a plan described in https://phabricator.freedesktop.org/T7622 . I have not
yet done the XWM reorganization it calls for.

This patch series is also available at:
https://git.collabora.com/cgit/user/pq/weston.git/log/?h=x11-positioning-v1
with a couple more debug patches on top.

The Xwayland patch series needed for _XWAYLAND_ALLOW_COMMITS is:
https://patchwork.freedesktop.org/series/15904/
https://lists.x.org/archives/xorg-devel/2016-November/051913.html

A question was presented, how _XWAYLAND_ALLOW_COMMITS would interact with
_NET_WM_SYNC_REQUEST in both basic and extended forms:
https://lists.x.org/archives/xorg-devel/2016-November/051932.html


Thanks,
pq

Pekka Paalanen (7):
  xwayland: WM debug prints
  xwayland: add set_toplevel_with_position to internal API
  libweston-desktop: add set_xwayland_position API
  xwayland: detect initially positioned X11 windows
  shell: implement set_xwayland_position
  libweston-desktop/xwayland: react to geometry changes
  WIP xwayland: poke _XWAYLAND_ALLOW_COMMITS


Patches 3, 5 and 6 are:
Reviewed-by: Quentin Glidic 

Same goes for 2 and 4 with the comment addressed.

1 and 7 requires too much X11-enabled brain and xwayland.c knowledge for 
now.



Cheers,




 desktop-shell/shell.c  | 37 
 libweston-desktop/internal.h   |  5 +++
 libweston-desktop/libweston-desktop.c  | 10 +
 libweston-desktop/libweston-desktop.h  | 33 +++
 libweston-desktop/xwayland.c   | 18 
 xwayland/window-manager.c  | 77 +-
 xwayland/xwayland-internal-interface.h |  2 +
 xwayland/xwayland.h|  1 +
 8 files changed, 181 insertions(+), 2 deletions(-)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/7] xwayland: add set_toplevel_with_position to internal API

[Quentin Glidic]

On 29/11/2016 16:11, Pekka Paalanen wrote:

From: Pekka Paalanen 

Add a new entry to the internal interface between the xwayland plugin
and libweston-desktop (or any other desktop protocol implementation).
The new entry is identical to set_toplevel except it carries an absolute
position for the toplevel window.

Following patches will implement this new entry in
libweston-desktop and start using it in XWM.

Signed-off-by: Pekka Paalanen 
---
 xwayland/xwayland-internal-interface.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xwayland/xwayland-internal-interface.h 
b/xwayland/xwayland-internal-interface.h
index e771730..1096444 100644
--- a/xwayland/xwayland-internal-interface.h
+++ b/xwayland/xwayland-internal-interface.h
@@ -38,6 +38,8 @@ struct weston_desktop_xwayland_interface {
  struct weston_surface 
*surface,
  const struct 
weston_xwayland_client_interface *client);
void (*set_toplevel)(struct weston_desktop_xwayland_surface *shsurf);
+   void (*set_toplevel_with_position)(struct 
weston_desktop_xwayland_surface *shsurf,
+  int32_t x, int32_t y);


Why not "set_position"? It seems cleaner to me, and potentially usable 
for other types. Since this is an internal detail, we should be safe 
from bad code to call it too much.


Patch 4 would need an update then.

Cheers,



void (*set_parent)(struct weston_desktop_xwayland_surface *shsurf,
   struct weston_surface *parent);
void (*set_transient)(struct weston_desktop_xwayland_surface *shsurf,




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v4] libweston: Position layers in an absolute way

[Quentin Glidic]

From: Quentin Glidic 

Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.

Signed-off-by: Quentin Glidic 
Acked-by: Pekka Paalanen 
Reviewed-by: Daniel Stone 
Reviewed-by: Giulio Camuffo 
---

v4:
Switched weston_layer_init() args order
Port libweston-desktop/xwayland layer

 desktop-shell/input-panel.c |  6 ++--
 desktop-shell/shell.c   | 64 +
 fullscreen-shell/fullscreen-shell.c |  4 ++-
 ivi-shell/input-panel-ivi.c |  6 ++--
 ivi-shell/ivi-layout.c  |  4 ++-
 ivi-shell/ivi-shell.c   |  2 +-
 libweston-desktop/xwayland.c|  6 +++-
 libweston/compositor.c  | 52 ---
 libweston/compositor.h  | 70 +++--
 tests/weston-test.c |  3 +-
 10 files changed, 168 insertions(+), 49 deletions(-)

diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 58a4cd0..40a4092 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -115,8 +115,8 @@ show_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = true;
 
if (!shell->locked)
-   wl_list_insert(&shell->compositor->cursor_layer.link,
-  &shell->input_panel_layer.link);
+   weston_layer_set_position(&shell->input_panel_layer,
+ WESTON_LAYER_POSITION_TOP_UI);
 
wl_list_for_each_safe(ipsurf, next,
  &shell->input_panel.surfaces, link) {
@@ -141,7 +141,7 @@ hide_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = false;
 
if (!shell->locked)
-   wl_list_remove(&shell->input_panel_layer.link);
+   weston_layer_unset_position(&shell->input_panel_layer);
 
wl_list_for_each_safe(view, next,
  &shell->input_panel_layer.view_list.link,
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 3913f95..a9db10b 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -892,13 +892,13 @@ seat_destroyed(struct wl_listener *listener, void *data)
 }
 
 static struct workspace *
-workspace_create(void)
+workspace_create(struct desktop_shell *shell)
 {
struct workspace *ws = malloc(sizeof *ws);
if (ws == NULL)
return NULL;
 
-   weston_layer_init(&ws->layer, NULL);
+   weston_layer_init(&ws->layer, shell->compositor);
 
wl_list_init(&ws->focus_list);
wl_list_init(&ws->seat_destroyed_listener.link);
@@ -937,7 +937,7 @@ activate_workspace(struct desktop_shell *shell, unsigned 
int index)
struct workspace *ws;
 
ws = get_workspace(shell, index);
-   wl_list_insert(&shell->panel_layer.link, &ws->layer.link);
+   weston_layer_set_position(&ws->layer, WESTON_LAYER_POSITION_NORMAL);
 
shell->workspaces.current = index;
 }
@@ -3064,20 +3064,16 @@ resume_desktop(struct desktop_shell *shell)
 {
struct workspace *ws = get_current_workspace(shell);
 
-   wl_list_remove(&shell->lock_layer.link);
-   if (shell->showing_input_panels) {
-   wl_list_insert(&shell->compositor->cursor_layer.link,
-  &shell->input_panel_layer.link);
-   wl_list_insert(&shell->input_panel_layer.link,
-  &shell->fullscreen_layer.link);
-   } else {
-   wl_list_insert(&shell->compositor->cursor_layer.link,
-  &shell->fullscreen_layer.link);
-   }
-   wl_list_insert(&shell->fullscreen_layer.link,
-  &shell->panel_layer.link);
-   wl_list_insert(&shell->panel_layer.link,
-  &ws->layer.link),
+   weston_layer_unset_position(&shell->lock_layer);
+
+   if (shell->showing_input_panels)
+   weston_layer_set_position(&shell->input_panel_layer,
+ WESTON_LAYER_POSITION_TOP_UI);
+   weston_layer_set_position(&shell->fullscreen_layer,
+ WESTON_LAYER_POSITION_FULLSCREEN);
+   weston_layer_set_position(&shell->panel_layer,
+ WESTON_LAYER_POSITION_UI);
+   weston_layer_set_position(&ws->layer, WESTON_LAYER_POSITION_NORMAL);
 
restore_focus_state(shell, get_current_workspace(shell));
 
@@ -3757,13 +3753,14 @@ lock(struct de

Re: [PATCH weston v3] libweston: Position layers in an absolute way

[Quentin Glidic]

On 05/12/2016 16:29, Pekka Paalanen wrote:

On Mon, 11 Jul 2016 11:29:40 +0200
Quentin Glidic  wrote:


From: Quentin Glidic 

Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.

Signed-off-by: Quentin Glidic 
Acked-by: Pekka Paalanen 
---

v3:
 - Added weston_layer_unset_position
 - HIDDEN is now still rendered
   (Feature needed by Giulio Camuffo)
 - weston_layer now stores the weston_compositor pointer to avoid its
   need in set_position
 - Reordered weston_layer members (we break the ABI anyway)
 - weston_layer_init now only initialize the struct, you must
   call set_position to add it to the layer list
 - BACKGROUND is now 2 instead of 1, as these values are mainly meant
   for libweston modules. The compositor should simply use
   (BACKGROUND - 1) if it wants to support such modules with
   a fallback surface
   (Suggestion by Bill Spitzak)

v2:
 - Tiny commit message addition: added runtime behaviour comment.
 - Reworked (a lot) the enum comment to explain further the position
   values and their actual expected scope. I hope their are clear enough.
   Here are the biggest changes:
 - Added a BOTTOM_UI value for widgets and conky-like applications.
 - Changed BACKGROUND to be 1, as nothing should be under it
 - Added a comment about mandatory background

 desktop-shell/input-panel.c |  6 ++--
 desktop-shell/shell.c   | 64 +
 fullscreen-shell/fullscreen-shell.c |  4 ++-
 ivi-shell/input-panel-ivi.c |  6 ++--
 ivi-shell/ivi-layout.c  |  4 ++-
 ivi-shell/ivi-shell.c   |  2 +-
 libweston/compositor.c  | 52 ---
 libweston/compositor.h  | 70 +++--
 tests/weston-test.c |  3 +-
 9 files changed, 163 insertions(+), 48 deletions(-)




diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index c72f801..bdaad87 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c


I looks like these items still remain:


desktop-shell/shell.c=1048=finish_workspace_change_animation(struct 
desktop_shell *shell,
desktop-shell/shell.c:1068: 
wl_list_remove(&shell->workspaces.anim_from->layer.link);
desktop-shell/shell.c=1123=animate_workspace_change(struct desktop_shell *shell,
desktop-shell/shell.c:1150: wl_list_insert(from->layer.link.prev, 
&to->layer.link);
desktop-shell/shell.c=1160=update_workspace(struct desktop_shell *shell, 
unsigned int index,
desktop-shell/shell.c:1164: wl_list_insert(&from->layer.link, 
&to->layer.link);
desktop-shell/shell.c:1165: wl_list_remove(&from->layer.link);
desktop-shell/shell.c=1255=take_surface_to_workspace_by_seat(struct 
desktop_shell *shell,
desktop-shell/shell.c:1289: wl_list_remove(&to->layer.link);
desktop-shell/shell.c:1290: wl_list_insert(from->layer.link.prev, 
&to->layer.link);

They are all manipulating the weston_layer::link, so it seems to me
they should be fixed.


I think I overlooked all the changes for libweston-desktop, or some 
other patch that got applied since then.



diff --git a/libweston/compositor.c b/libweston/compositor.c
index 771f1c9..9d6bbf6 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2402,14 +2402,52 @@ weston_layer_entry_remove(struct weston_layer_entry 
*entry)
entry->layer = NULL;
 }

+
+/** Initialize the weston_layer struct.
+ *
+ * \param compositor The compositor instance
+ * \param layer The layer to initialize
+ */
 WL_EXPORT void
-weston_layer_init(struct weston_layer *layer, struct wl_list *below)
+weston_layer_init(struct weston_compositor *compositor,
+ struct weston_layer *layer)
 {
+   layer->compositor = compositor;
+   wl_list_init(&layer->link);
wl_list_init(&layer->view_list.link);
layer->view_list.layer = layer;
weston_layer_set_mask_infinite(layer);
-   if (below != NULL)
-   wl_list_insert(below, &layer->link);
+}
+
+/** Sets the position of the layer in the layer list.
+ *
+ * \param layer The layer to modify
+ * \param position The position the layer will be placed at


This should document that calling set_position twice without an unset
in between is not allowed.


+ */
+WL_EXPORT void
+weston_layer_set_position(struct weston_layer *layer,
+ enum weston_layer_position position)
+{
+   struct weston_layer *below;
+
+   layer->position = position;
+   wl_list_for_each_reverse(below, &layer->compositor->layer_list, link) {
+   if (below->position >= layer->position) {
+   wl_list_insert(&below->link

Re: [PATCH weston v2] libweston-desktop: don't crash when getting the pid for X clients

[Quentin Glidic]

On 08/12/2016 09:21, Giulio Camuffo wrote:

X client's don't have a wl_client associated with their
weston_desktop_client, so make sure to not use it.

Signed-off-by: Giulio Camuffo 


Perfect:
Reviewed-by: Quentin Glidic 

Cheers,


---

v2: use -1 as the pid unset value, and initialize the pid to 0 for xwayland
surfaces. This means the branch where we use the client is never reached
for xwayland, even if the pid is not set.

 libweston-desktop/surface.c  | 8 +++-
 libweston-desktop/xwayland.c | 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
index 2205107..d3be936 100644
--- a/libweston-desktop/surface.c
+++ b/libweston-desktop/surface.c
@@ -269,6 +269,8 @@ weston_desktop_surface_create(struct weston_desktop 
*desktop,
wsurface->committed = weston_desktop_surface_committed;
wsurface->committed_private = surface;

+   surface->pid = -1;
+
surface->surface_commit_listener.notify =
weston_desktop_surface_surface_committed;
wl_signal_add(&surface->surface->commit_signal,
@@ -590,7 +592,7 @@ weston_desktop_surface_get_pid(struct 
weston_desktop_surface *surface)
 {
pid_t pid;

-   if (surface->pid != 0) {
+   if (surface->pid != -1) {
pid = surface->pid;
} else {
struct weston_desktop_client *client =
@@ -598,6 +600,10 @@ weston_desktop_surface_get_pid(struct 
weston_desktop_surface *surface)
struct wl_client *wl_client =
weston_desktop_client_get_client(client);

+   /* wl_client should always be valid, because only in the
+* xwayland case it wouldn't be, but in that case we won't
+* reach here, as the pid is initialized to 0. */
+   assert(wl_client);
wl_client_get_credentials(wl_client, &pid, NULL, NULL);
}
return pid;
diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index c89248c..0741a4c 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -241,6 +241,8 @@ create_surface(struct weston_desktop_xwayland *xwayland,
wl_resource_add_destroy_listener(wsurface->resource,
 &surface->resource_destroy_listener);

+   weston_desktop_surface_set_pid(surface->surface, 0);
+
return surface;
 }





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2] libweston-desktop: don't crash when getting the pid for X clients

[Quentin Glidic]

On 08/12/2016 09:21, Giulio Camuffo wrote:

X client's don't have a wl_client associated with their
weston_desktop_client, so make sure to not use it.

Signed-off-by: Giulio Camuffo 


Pushed:
2295a62..f15320f  master -> master

Thanks,



---

v2: use -1 as the pid unset value, and initialize the pid to 0 for xwayland
surfaces. This means the branch where we use the client is never reached
for xwayland, even if the pid is not set.

 libweston-desktop/surface.c  | 8 +++-
 libweston-desktop/xwayland.c | 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
index 2205107..d3be936 100644
--- a/libweston-desktop/surface.c
+++ b/libweston-desktop/surface.c
@@ -269,6 +269,8 @@ weston_desktop_surface_create(struct weston_desktop 
*desktop,
wsurface->committed = weston_desktop_surface_committed;
wsurface->committed_private = surface;

+   surface->pid = -1;
+
surface->surface_commit_listener.notify =
weston_desktop_surface_surface_committed;
wl_signal_add(&surface->surface->commit_signal,
@@ -590,7 +592,7 @@ weston_desktop_surface_get_pid(struct 
weston_desktop_surface *surface)
 {
pid_t pid;

-   if (surface->pid != 0) {
+   if (surface->pid != -1) {
pid = surface->pid;
} else {
struct weston_desktop_client *client =
@@ -598,6 +600,10 @@ weston_desktop_surface_get_pid(struct 
weston_desktop_surface *surface)
struct wl_client *wl_client =
weston_desktop_client_get_client(client);

+   /* wl_client should always be valid, because only in the
+* xwayland case it wouldn't be, but in that case we won't
+* reach here, as the pid is initialized to 0. */
+   assert(wl_client);
wl_client_get_credentials(wl_client, &pid, NULL, NULL);
}
return pid;
diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index c89248c..0741a4c 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -241,6 +241,8 @@ create_surface(struct weston_desktop_xwayland *xwayland,
wl_resource_add_destroy_listener(wsurface->resource,
 &surface->resource_destroy_listener);

+   weston_desktop_surface_set_pid(surface->surface, 0);
+
return surface;
 }





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v4] libweston-desktop: fix stale ping when a wl_shell_surface is destroyed

[Quentin Glidic]

On 08/12/2016 16:20, Giulio Camuffo wrote:

When sending a ping event to a surface using the wl_shell interface,
if that surface is destroyed before we receive the pong we will never
receive it, even if the client is actually responsive, since the
interface does not exist anymore. So when the surface if destroyed
pretend it's a pong and reset the ping state.

Signed-off-by: Giulio Camuffo 


I made a few renames to match the current code.
I was going to push it, but I found a last issue, see below.



---

v3: store the ping serial in the surface instead of the client wrapper
v4: removed leftover change

 libweston-desktop/wl-shell.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
index 399139c..f75b022 100644
--- a/libweston-desktop/wl-shell.c
+++ b/libweston-desktop/wl-shell.c
@@ -57,6 +57,7 @@ struct weston_desktop_wl_shell_surface {
struct weston_desktop_seat *popup_seat;
enum weston_desktop_wl_shell_surface_state state;
struct wl_listener wl_surface_resource_destroy_listener;
+   uint32_t ping_serial;
 };

 static void
@@ -112,6 +113,7 @@ weston_desktop_wl_shell_surface_ping(struct 
weston_desktop_surface *dsurface,
 {
struct weston_desktop_wl_shell_surface *surface = user_data;

+   surface->ping_serial = serial;
wl_shell_surface_send_ping(surface->resource, serial);
 }

@@ -181,11 +183,27 @@ weston_desktop_wl_shell_change_state(struct 
weston_desktop_wl_shell_surface *sur
 }

 static void
+pong_client(struct weston_desktop_wl_shell_surface *surface)


weston_desktop_wl_shell_surface_pong_client


+{
+   struct weston_desktop_client *client =
+   weston_desktop_surface_get_client(surface->surface);
+
+   weston_desktop_client_pong(client, surface->ping_serial);
+   surface->ping_serial = 0;
+}
+
+static void
 weston_desktop_wl_shell_surface_destroy(struct weston_desktop_surface 
*dsurface,
void *user_data)
 {
struct weston_desktop_wl_shell_surface *surface = user_data;

+   /* If the surface being destroyed was the one that was pinged before
+* we need to fake a pong here, because it cannot answer the ping 
anymore,
+* even if the client is responsive. */
+   if (surface->ping_serial != 0)
+   pong_client(surface);
+
wl_list_remove(&surface->wl_surface_resource_destroy_listener.link);

weston_desktop_wl_shell_surface_maybe_ungrab(surface);
@@ -203,8 +221,10 @@ weston_desktop_wl_shell_surface_protocol_pong(struct 
wl_client *wl_client,
  uint32_t serial)
 {
struct weston_desktop_surface *surface = 
wl_resource_get_user_data(resource);


dsurface


+   struct weston_desktop_wl_shell_surface *wls =
+   weston_desktop_surface_get_implementation_data(surface);


surface


-   weston_desktop_client_pong(weston_desktop_surface_get_client(surface), 
serial);
+   pong_client(wls);


We should check that surface->ping_serial == serial somehow.
Maybe it is safe to ignore serial for now, as it fixes something, then 
add a check in a follow-up commit.


Thoughts? Pekka (aka assertman)?

Cheers,



 }

 static void




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v4] libweston-desktop: fix stale ping when a wl_shell_surface is destroyed

[Quentin Glidic]

On 08/12/2016 16:52, Giulio Camuffo wrote:

2016-12-08 16:46 GMT+01:00 Quentin Glidic :

On 08/12/2016 16:20, Giulio Camuffo wrote:


When sending a ping event to a surface using the wl_shell interface,
if that surface is destroyed before we receive the pong we will never
receive it, even if the client is actually responsive, since the
interface does not exist anymore. So when the surface if destroyed
pretend it's a pong and reset the ping state.

Signed-off-by: Giulio Camuffo 



I made a few renames to match the current code.
I was going to push it, but I found a last issue, see below.



---

v3: store the ping serial in the surface instead of the client wrapper
v4: removed leftover change

 libweston-desktop/wl-shell.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
index 399139c..f75b022 100644
--- a/libweston-desktop/wl-shell.c
+++ b/libweston-desktop/wl-shell.c
@@ -57,6 +57,7 @@ struct weston_desktop_wl_shell_surface {
struct weston_desktop_seat *popup_seat;
enum weston_desktop_wl_shell_surface_state state;
struct wl_listener wl_surface_resource_destroy_listener;
+   uint32_t ping_serial;
 };

 static void
@@ -112,6 +113,7 @@ weston_desktop_wl_shell_surface_ping(struct
weston_desktop_surface *dsurface,
 {
struct weston_desktop_wl_shell_surface *surface = user_data;

+   surface->ping_serial = serial;
wl_shell_surface_send_ping(surface->resource, serial);
 }

@@ -181,11 +183,27 @@ weston_desktop_wl_shell_change_state(struct
weston_desktop_wl_shell_surface *sur
 }

 static void
+pong_client(struct weston_desktop_wl_shell_surface *surface)



weston_desktop_wl_shell_surface_pong_client


+{
+   struct weston_desktop_client *client =
+   weston_desktop_surface_get_client(surface->surface);
+
+   weston_desktop_client_pong(client, surface->ping_serial);
+   surface->ping_serial = 0;
+}
+
+static void
 weston_desktop_wl_shell_surface_destroy(struct weston_desktop_surface
*dsurface,
void *user_data)
 {
struct weston_desktop_wl_shell_surface *surface = user_data;

+   /* If the surface being destroyed was the one that was pinged
before
+* we need to fake a pong here, because it cannot answer the ping
anymore,
+* even if the client is responsive. */
+   if (surface->ping_serial != 0)
+   pong_client(surface);
+

wl_list_remove(&surface->wl_surface_resource_destroy_listener.link);

weston_desktop_wl_shell_surface_maybe_ungrab(surface);
@@ -203,8 +221,10 @@ weston_desktop_wl_shell_surface_protocol_pong(struct
wl_client *wl_client,
  uint32_t serial)
 {
struct weston_desktop_surface *surface =
wl_resource_get_user_data(resource);



dsurface


+   struct weston_desktop_wl_shell_surface *wls =
+   weston_desktop_surface_get_implementation_data(surface);



surface


-
weston_desktop_client_pong(weston_desktop_surface_get_client(surface),
serial);
+   pong_client(wls);



We should check that surface->ping_serial == serial somehow.
Maybe it is safe to ignore serial for now, as it fixes something, then add a
check in a follow-up commit.


Well, but weston_desktop_client_pong() already checks that, so i don't
think we need to check it here too.


It checks that the last ping matches the passed serial. This (passed) 
serial should be the one the *client* sent. Here, we just pass 
->ping_serial, which is guaranteed to be the one 
weston_desktop_client_pong() is waiting for. But nothing prevents the 
client to send wl_shell_surface.pong(1337), and we wouldn’t catch these.







Thoughts? Pekka (aka assertman)?

Cheers,



 }

 static void




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v5] libweston: Position layers in an absolute way

[Quentin Glidic]

From: Quentin Glidic 

Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.

Signed-off-by: Quentin Glidic 
---
v5:   
- Made set_position/unset_position safe (+ doc)   
- Commented weston_compositor::layer_list member  
- Put weston-test layer on top
- Commented layer ordering
- Updated workspace layer code

 desktop-shell/input-panel.c |  6 +--
 desktop-shell/shell.c   | 79 -
 fullscreen-shell/fullscreen-shell.c |  4 +-
 ivi-shell/input-panel-ivi.c |  6 +--
 ivi-shell/ivi-layout.c  |  4 +-
 ivi-shell/ivi-shell.c   |  2 +-
 libweston-desktop/xwayland.c|  6 ++-
 libweston/compositor.c  | 62 ++---
 libweston/compositor.h  | 72 +++--
 tests/weston-test.c |  3 +-
 10 files changed, 187 insertions(+), 57 deletions(-)

diff --git a/desktop-shell/input-panel.c b/desktop-shell/input-panel.c
index 58a4cd0..40a4092 100644
--- a/desktop-shell/input-panel.c
+++ b/desktop-shell/input-panel.c
@@ -115,8 +115,8 @@ show_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = true;
 
if (!shell->locked)
-   wl_list_insert(&shell->compositor->cursor_layer.link,
-  &shell->input_panel_layer.link);
+   weston_layer_set_position(&shell->input_panel_layer,
+ WESTON_LAYER_POSITION_TOP_UI);
 
wl_list_for_each_safe(ipsurf, next,
  &shell->input_panel.surfaces, link) {
@@ -141,7 +141,7 @@ hide_input_panels(struct wl_listener *listener, void *data)
shell->showing_input_panels = false;
 
if (!shell->locked)
-   wl_list_remove(&shell->input_panel_layer.link);
+   weston_layer_unset_position(&shell->input_panel_layer);
 
wl_list_for_each_safe(view, next,
  &shell->input_panel_layer.view_list.link,
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 3913f95..161b3b5 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -892,13 +892,13 @@ seat_destroyed(struct wl_listener *listener, void *data)
 }
 
 static struct workspace *
-workspace_create(void)
+workspace_create(struct desktop_shell *shell)
 {
struct workspace *ws = malloc(sizeof *ws);
if (ws == NULL)
return NULL;
 
-   weston_layer_init(&ws->layer, NULL);
+   weston_layer_init(&ws->layer, shell->compositor);
 
wl_list_init(&ws->focus_list);
wl_list_init(&ws->seat_destroyed_listener.link);
@@ -937,7 +937,7 @@ activate_workspace(struct desktop_shell *shell, unsigned 
int index)
struct workspace *ws;
 
ws = get_workspace(shell, index);
-   wl_list_insert(&shell->panel_layer.link, &ws->layer.link);
+   weston_layer_set_position(&ws->layer, WESTON_LAYER_POSITION_NORMAL);
 
shell->workspaces.current = index;
 }
@@ -1018,6 +1018,9 @@ reverse_workspace_change_animation(struct desktop_shell 
*shell,
shell->workspaces.anim_dir = -1 * shell->workspaces.anim_dir;
shell->workspaces.anim_timestamp = 0;
 
+   weston_layer_set_position(&to->layer, WESTON_LAYER_POSITION_NORMAL);
+   weston_layer_set_position(&from->layer, WESTON_LAYER_POSITION_NORMAL-1);
+
weston_compositor_schedule_repaint(shell->compositor);
 }
 
@@ -1065,7 +1068,7 @@ finish_workspace_change_animation(struct desktop_shell 
*shell,
workspace_deactivate_transforms(to);
shell->workspaces.anim_to = NULL;
 
-   wl_list_remove(&shell->workspaces.anim_from->layer.link);
+   weston_layer_unset_position(&shell->workspaces.anim_from->layer);
 }
 
 static void
@@ -1147,7 +1150,8 @@ animate_workspace_change(struct desktop_shell *shell,
wl_list_insert(&output->animation_list,
   &shell->workspaces.animation.link);
 
-   wl_list_insert(from->layer.link.prev, &to->layer.link);
+   weston_layer_set_position(&to->layer, WESTON_LAYER_POSITION_NORMAL);
+   weston_layer_set_position(&from->layer, WESTON_LAYER_POSITION_NORMAL-1);
 
workspace_translate_in(to, 0);
 
@@ -1161,8 +1165,8 @@ update_workspace(struct desktop_shell *shell, unsigned 
int index,
 struct workspace *from, struct workspace *to)
 {
shell->workspaces.current = index;
-   wl_l

Re: [PATCH weston v3] libweston: Position layers in an absolute way

[Quentin Glidic]

On 05/12/2016 16:48, Quentin Glidic wrote:

On 05/12/2016 16:29, Pekka Paalanen wrote:

On Mon, 11 Jul 2016 11:29:40 +0200
Quentin Glidic  wrote:


From: Quentin Glidic 

>>> [snip]

+ */
+WL_EXPORT void
+weston_layer_set_position(struct weston_layer *layer,
+  enum weston_layer_position position)
+{
+struct weston_layer *below;
+
+layer->position = position;
+wl_list_for_each_reverse(below, &layer->compositor->layer_list, 
link) {

+if (below->position >= layer->position) {
+wl_list_insert(&below->link, &layer->link);
+return;
+}
+}
+wl_list_insert(layer->compositor->layer_list.next, &layer->link);


I think this should not have .next, should it? Now it's adding the new
layer below the top-most layer when it should become the top-most
layer, no?


Honestly I cannot remember why I used .next here… I vaguely remember 
something like layers are not ordered as one would think, but that’s 
all. I will try to re-figure it out.




FTR, you were right. It’s a leftover from one of the first version of 
this code. It did not cause any issue because this branch only triggers 
for the fade layer, so the list is empty, then the fade layer 
subsequently acts as a catch-all.



--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v3] libweston: Position layers in an absolute way

[Quentin Glidic]

On 05/12/2016 17:00, Pekka Paalanen wrote:

On Mon, 5 Dec 2016 16:48:38 +0100
Quentin Glidic  wrote:


On 05/12/2016 16:29, Pekka Paalanen wrote:

On Mon, 11 Jul 2016 11:29:40 +0200
Quentin Glidic  wrote:
  

From: Quentin Glidic 

Currently, layers’ order depends on the module loading order and it does
not survive runtime modifications (like shell locking/unlocking).
With this patch, modules can safely add their own layer at the expected
position in the stack, with runtime persistence.

Signed-off-by: Quentin Glidic 
Acked-by: Pekka Paalanen 
---

v3:
  - Added weston_layer_unset_position
  - HIDDEN is now still rendered
(Feature needed by Giulio Camuffo)
  - weston_layer now stores the weston_compositor pointer to avoid its
need in set_position
  - Reordered weston_layer members (we break the ABI anyway)
  - weston_layer_init now only initialize the struct, you must
call set_position to add it to the layer list
  - BACKGROUND is now 2 instead of 1, as these values are mainly meant
for libweston modules. The compositor should simply use
(BACKGROUND - 1) if it wants to support such modules with
a fallback surface
(Suggestion by Bill Spitzak)

v2:
  - Tiny commit message addition: added runtime behaviour comment.
  - Reworked (a lot) the enum comment to explain further the position
values and their actual expected scope. I hope their are clear enough.
Here are the biggest changes:
  - Added a BOTTOM_UI value for widgets and conky-like applications.
  - Changed BACKGROUND to be 1, as nothing should be under it
  - Added a comment about mandatory background

  desktop-shell/input-panel.c |  6 ++--
  desktop-shell/shell.c   | 64 +
  fullscreen-shell/fullscreen-shell.c |  4 ++-
  ivi-shell/input-panel-ivi.c |  6 ++--
  ivi-shell/ivi-layout.c  |  4 ++-
  ivi-shell/ivi-shell.c   |  2 +-
  libweston/compositor.c  | 52 ---
  libweston/compositor.h  | 70 +++--
  tests/weston-test.c |  3 +-
  9 files changed, 163 insertions(+), 48 deletions(-)
  
  

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index c72f801..bdaad87 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c


I looks like these items still remain:


desktop-shell/shell.c=1048=finish_workspace_change_animation(struct 
desktop_shell *shell,
desktop-shell/shell.c:1068: 
wl_list_remove(&shell->workspaces.anim_from->layer.link);
desktop-shell/shell.c=1123=animate_workspace_change(struct desktop_shell *shell,
desktop-shell/shell.c:1150: wl_list_insert(from->layer.link.prev, 
&to->layer.link);
desktop-shell/shell.c=1160=update_workspace(struct desktop_shell *shell, 
unsigned int index,
desktop-shell/shell.c:1164: wl_list_insert(&from->layer.link, 
&to->layer.link);
desktop-shell/shell.c:1165: wl_list_remove(&from->layer.link);
desktop-shell/shell.c=1255=take_surface_to_workspace_by_seat(struct 
desktop_shell *shell,
desktop-shell/shell.c:1289: wl_list_remove(&to->layer.link);
desktop-shell/shell.c:1290: wl_list_insert(from->layer.link.prev, 
&to->layer.link);

They are all manipulating the weston_layer::link, so it seems to me
they should be fixed.


I think I overlooked all the changes for libweston-desktop, or some
other patch that got applied since then.


I believe the workspace code has been there for... years? :-)

struct workspace embeds a struct weston_layer.


Sure, then I didn’t touch it because I didn’t break it. It’s now done in v5.



+}
+
+/** Hide a layer by taking it off the layer list.
+ *
+ * \param layer The layer to hide


This should document that calling this on a layer that is "not on the
list" is not allowed.

However, since init() does init the link, I wonder if you meant these
to be safe instead of disallow repeated calls?


I think set_position and unset_position should always be safe. That
would make then easier to use.
Maybe Giulio could help us on this choice?


FWIW, I would favour being safe than forbid in this case.


Nice, done in v5.



+ */
+WL_EXPORT void
+weston_layer_unset_position(struct weston_layer *layer)
+{
+   wl_list_remove(&layer->link);
  }

  WL_EXPORT void
@@ -4725,8 +4763,12 @@ weston_compositor_create(struct wl_display *display, 
void *user_data)
loop = wl_display_get_event_loop(ec->wl_display);
ec->idle_source = wl_event_loop_add_timer(loop, idle_handler, ec);

-   weston_layer_init(&ec->fade_layer, &ec->layer_list);
-   weston_layer_init(&ec->cursor_layer, &ec->fade_layer.link);
+   weston_layer_init(ec, &ec->fade_layer);
+   weston_layer_init(ec, &ec->cursor_layer);
+
+   weston_layer_set_position(&ec->fade_layer, WESTON_LAYER_P

[PATCH weston v2 2/5] libweston: Properly namespace modules entrypoint

[Quentin Glidic]

From: Quentin Glidic 

Use different functions so we cannot load a libweston common module in
weston directly or the other way around.

Signed-off-by: Quentin Glidic 
---
 compositor/cms-colord.c | 1 +
 compositor/systemd-notify.c | 1 +
 compositor/weston.h | 4 
 fullscreen-shell/fullscreen-shell.c | 1 +
 libweston/compositor.c  | 8 +++-
 libweston/compositor.h  | 3 +--
 tests/plugin-registry-test.c| 1 +
 tests/surface-global-test.c | 1 +
 tests/surface-screenshot.c  | 1 +
 tests/surface-test.c| 1 +
 xwayland/launcher.c | 3 +--
 11 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
index 152a734..ae3ef25 100644
--- a/compositor/cms-colord.c
+++ b/compositor/cms-colord.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "weston.h"
 #include "cms-helper.h"
 #include "shared/helpers.h"
 
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index 49e51f4..ce18ede 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -34,6 +34,7 @@
 #include "shared/string-helpers.h"
 #include "shared/zalloc.h"
 #include "compositor.h"
+#include "weston.h"
 
 struct systemd_notifier {
int watchdog_time;
diff --git a/compositor/weston.h b/compositor/weston.h
index bb04002..2e0417c 100644
--- a/compositor/weston.h
+++ b/compositor/weston.h
@@ -63,6 +63,10 @@ wet_get_config(struct weston_compositor *compositor);
 void *
 wet_load_module(const char *name, const char *entrypoint);
 
+int
+module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[]);
+
 int
 wet_load_xwayland(struct weston_compositor *comp);
 
diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index b3083d8..dab429d 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "fullscreen-shell-unstable-v1-server-protocol.h"
 #include "shared/helpers.h"
 
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 6226810..d00a25a 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -5357,14 +5357,12 @@ weston_compositor_load_backend(struct weston_compositor 
*compositor,
 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;
+   int (*module_init)(struct weston_compositor *ec);
 
-   module_init = weston_load_module("xwayland.so", "module_init");
+   module_init = weston_load_module("xwayland.so", "weston_module_init");
if (!module_init)
return -1;
-   if (module_init(compositor, &argc, NULL) < 0)
+   if (module_init(compositor) < 0)
return -1;
return 0;
 }
diff --git a/libweston/compositor.h b/libweston/compositor.h
index faeea3a..d59e622 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1754,8 +1754,7 @@ int
 weston_backend_init(struct weston_compositor *c,
struct weston_backend_config *config_base);
 int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[]);
+weston_module_init(struct weston_compositor *compositor);
 
 void
 weston_transformed_coord(int width, int height,
diff --git a/tests/plugin-registry-test.c b/tests/plugin-registry-test.c
index 7fc88f3..2a32e01 100644
--- a/tests/plugin-registry-test.c
+++ b/tests/plugin-registry-test.c
@@ -28,6 +28,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "plugin-registry.h"
 
 static void
diff --git a/tests/surface-global-test.c b/tests/surface-global-test.c
index 55899c6..20d99ce 100644
--- a/tests/surface-global-test.c
+++ b/tests/surface-global-test.c
@@ -29,6 +29,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 
 static void
 surface_to_from_global(void *data)
diff --git a/tests/surface-screenshot.c b/tests/surface-screenshot.c
index 332b5e3..6ff2bfc 100644
--- a/tests/surface-screenshot.c
+++ b/tests/surface-screenshot.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 #include "file-util.h"
 
 static char *
diff --git a/tests/surface-test.c b/tests/surface-test.c
index 243f8dc..4463061 100644
--- a/tests/surface-test.c
+++ b/tests/surface-test.c
@@ -29,6 +29,7 @@
 #include 
 
 #include "compositor.h"
+#include "compositor/weston.h"
 
 static void
 surface_transform(void *data)
diff -

[PATCH weston v2 3/5] weston: Properly namespace modules entrypoint

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 compositor/cms-colord.c |  4 ++--
 compositor/cms-static.c |  4 ++--
 compositor/main.c   | 27 +++
 compositor/screen-share.c   |  4 ++--
 compositor/systemd-notify.c |  4 ++--
 compositor/weston.h |  9 -
 desktop-shell/shell.c   |  4 ++--
 fullscreen-shell/fullscreen-shell.c |  4 ++--
 ivi-shell/ivi-layout.c  |  4 +++-
 ivi-shell/ivi-shell.c   |  4 ++--
 tests/plugin-registry-test.c|  3 ++-
 tests/surface-global-test.c |  3 ++-
 tests/surface-screenshot.c  |  4 ++--
 tests/surface-test.c|  3 ++-
 tests/weston-test.c |  4 ++--
 15 files changed, 54 insertions(+), 31 deletions(-)

diff --git a/compositor/cms-colord.c b/compositor/cms-colord.c
index ae3ef25..0daa2a7 100644
--- a/compositor/cms-colord.c
+++ b/compositor/cms-colord.c
@@ -496,8 +496,8 @@ colord_cms_output_destroy(gpointer data)
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *ec,
+   int *argc, char *argv[])
 {
gboolean ret;
GError *error = NULL;
diff --git a/compositor/cms-static.c b/compositor/cms-static.c
index a6bbfd4..e24501b 100644
--- a/compositor/cms-static.c
+++ b/compositor/cms-static.c
@@ -91,8 +91,8 @@ cms_notifier_destroy(struct wl_listener *listener, void *data)
 
 
 WL_EXPORT int
-module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *ec,
+   int *argc, char *argv[])
 {
struct cms_static *cms;
struct weston_output *output;
diff --git a/compositor/main.c b/compositor/main.c
index 2aa4936..af093f1 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -761,7 +761,7 @@ weston_create_listening_socket(struct wl_display *display, 
const char *socket_na
 }
 
 WL_EXPORT void *
-wet_load_module(const char *name, const char *entrypoint)
+wet_load_module_entrypoint(const char *name, const char *entrypoint)
 {
const char *builddir = getenv("WESTON_BUILD_DIR");
char path[PATH_MAX];
@@ -812,14 +812,28 @@ wet_load_module(const char *name, const char *entrypoint)
return init;
 }
 
+
+WL_EXPORT int
+wet_load_module(struct weston_compositor *compositor,
+   const char *name, int *argc, char *argv[])
+{
+   int (*module_init)(struct weston_compositor *ec,
+  int *argc, char *argv[]);
+
+   module_init = wet_load_module_entrypoint(name, "wet_module_init");
+   if (!module_init)
+   return -1;
+   if (module_init(compositor, argc, argv) < 0)
+   return -1;
+   return 0;
+}
+
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
 int *argc, char *argv[])
 {
const char *p, *end;
char buffer[256];
-   int (*module_init)(struct weston_compositor *ec,
-  int *argc, char *argv[]);
 
if (modules == NULL)
return 0;
@@ -833,16 +847,13 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
if (wet_load_xwayland(ec) < 0)
return -1;
} else {
-   module_init = wet_load_module(buffer, "module_init");
-   if (!module_init)
-   return -1;
-   if (module_init(ec, argc, argv) < 0)
+   if (wet_load_module(ec, buffer, argc, argv) < 0)
return -1;
}
+
p = end;
while (*p == ',')
p++;
-
}
 
return 0;
diff --git a/compositor/screen-share.c b/compositor/screen-share.c
index 0db0203..bcb9def 100644
--- a/compositor/screen-share.c
+++ b/compositor/screen-share.c
@@ -1106,8 +1106,8 @@ share_output_binding(struct weston_keyboard *keyboard, 
uint32_t time, uint32_t k
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[])
 {
struct screen_share *ss;
struct weston_config *config = wet_get_config(compositor);
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index ce18ede..50f03cb 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -115,8 +115,8 @@ weston_compositor_destroy_listener(struct wl_listener 
*listener, void *data)
 }
 
 WL_EXPORT int
-module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_module_init(struct weston_compositor *compositor,
+   int *argc, char *argv[])
 {
char *w

[PATCH weston v2 1/5] libweston: Properly namespace backends entrypoint

[Quentin Glidic]

From: Quentin Glidic 

This prevents loading a backend as a simple module. This will avoid
messing up with backends when we will introduce libweston common
modules.

Signed-off-by: Quentin Glidic 
---
 libweston/compositor-drm.c  | 4 ++--
 libweston/compositor-fbdev.c| 4 ++--
 libweston/compositor-headless.c | 4 ++--
 libweston/compositor-rdp.c  | 4 ++--
 libweston/compositor-wayland.c  | 4 ++--
 libweston/compositor-x11.c  | 4 ++--
 libweston/compositor.c  | 2 +-
 libweston/compositor.h  | 4 ++--
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 70514ea..529662f 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3300,8 +3300,8 @@ config_init_to_defaults(struct weston_drm_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct drm_backend *b;
struct weston_drm_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
index 0c45e98..44f0cf5 100644
--- a/libweston/compositor-fbdev.c
+++ b/libweston/compositor-fbdev.c
@@ -782,8 +782,8 @@ config_init_to_defaults(struct weston_fbdev_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct fbdev_backend *b;
struct weston_fbdev_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-headless.c b/libweston/compositor-headless.c
index e7fc397..a1aec6d 100644
--- a/libweston/compositor-headless.c
+++ b/libweston/compositor-headless.c
@@ -316,8 +316,8 @@ config_init_to_defaults(struct 
weston_headless_backend_config *config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct headless_backend *b;
struct weston_headless_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-rdp.c b/libweston/compositor-rdp.c
index 223382c..4674612 100644
--- a/libweston/compositor-rdp.c
+++ b/libweston/compositor-rdp.c
@@ -1370,8 +1370,8 @@ config_init_to_defaults(struct weston_rdp_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct rdp_backend *b;
struct weston_rdp_backend_config config = {{ 0, }};
diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index d1e387d..b53b170 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -2557,8 +2557,8 @@ config_init_to_defaults(struct 
weston_wayland_backend_config *config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct wayland_backend *b;
struct wayland_parent_output *poutput;
diff --git a/libweston/compositor-x11.c b/libweston/compositor-x11.c
index 34ef854..92033be 100644
--- a/libweston/compositor-x11.c
+++ b/libweston/compositor-x11.c
@@ -1761,8 +1761,8 @@ config_init_to_defaults(struct weston_x11_backend_config 
*config)
 }
 
 WL_EXPORT int
-backend_init(struct weston_compositor *compositor,
-struct weston_backend_config *config_base)
+weston_backend_init(struct weston_compositor *compositor,
+   struct weston_backend_config *config_base)
 {
struct x11_backend *b;
struct weston_x11_backend_config config = {{ 0, }};
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 508c2a9..6226810 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -5347,7 +5347,7 @@ weston_compositor_load_backend(struct weston_compositor 
*compositor,
if (backend >= ARRAY_LENGTH(backend_map))
return -1;
 
-   backend_init = weston_load_module(backend_map[backend], "backend_init");
+   backend_init = weston_load_module(backend_map[backend], 
"weston_backend_init");
if (!backend_init)
return -1;
 
diff --git a/libweston/compositor.h b/libweston/compositor.h
index ce3d9ab..faeea3a 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1751,8 +1751,8 @@ int
 weston_input_init(s

[PATCH weston v2 5/5] weston: Add a specific option to load XWayland

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 compositor/main.c  | 22 +-
 man/weston.ini.man |  7 +--
 man/weston.man |  7 +--
 tests/weston-tests-env |  7 ---
 weston.ini.in  |  3 ++-
 5 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index 74b404b..f9614f5 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -845,7 +845,7 @@ wet_load_shell(struct weston_compositor *compositor,
 
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
-int *argc, char *argv[])
+int *argc, char *argv[], int32_t *xwayland)
 {
const char *p, *end;
char buffer[256];
@@ -859,8 +859,10 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
 
if (strstr(buffer, "xwayland.so")) {
-   if (wet_load_xwayland(ec) < 0)
-   return -1;
+   weston_log("Old Xwayland module loading detected:"
+  "Please use --xwayland command line option"
+  "or weston.ini xwayland=true\n");
+   *xwayland = 1;
} else {
if (wet_load_module(ec, buffer, argc, argv) < 0)
return -1;
@@ -1756,6 +1758,7 @@ int main(int argc, char *argv[])
int i, fd;
char *backend = NULL;
char *shell = NULL;
+   int32_t xwayland = 0;
char *modules = NULL;
char *option_modules = NULL;
char *log = NULL;
@@ -1780,6 +1783,7 @@ int main(int argc, char *argv[])
{ WESTON_OPTION_STRING, "shell", 0, &shell },
{ WESTON_OPTION_STRING, "socket", 'S', &socket_name },
{ WESTON_OPTION_INTEGER, "idle-time", 'i', &idle_time },
+   { WESTON_OPTION_BOOLEAN, "xwayland", 0, &xwayland },
{ WESTON_OPTION_STRING, "modules", 0, &option_modules },
{ WESTON_OPTION_STRING, "log", 0, &log },
{ WESTON_OPTION_BOOLEAN, "help", 'h', &help },
@@ -1914,12 +1918,20 @@ int main(int argc, char *argv[])
goto out;
 
weston_config_section_get_string(section, "modules", &modules, "");
-   if (load_modules(ec, modules, &argc, argv) < 0)
+   if (load_modules(ec, modules, &argc, argv, &xwayland) < 0)
goto out;
 
-   if (load_modules(ec, option_modules, &argc, argv) < 0)
+   if (load_modules(ec, option_modules, &argc, argv, &xwayland) < 0)
goto out;
 
+   if (!xwayland)
+   weston_config_section_get_bool(section, "xwayland", &xwayland,
+  false);
+   if (xwayland) {
+   if (wet_load_xwayland(ec) < 0)
+   goto out;
+   }
+
section = weston_config_get_section(config, "keyboard", NULL, NULL);
weston_config_section_get_bool(section, "numlock-on", &numlock_on, 0);
if (numlock_on) {
diff --git a/man/weston.ini.man b/man/weston.ini.man
index 2eac098..429dcdd 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -106,14 +106,17 @@ directory are:
 .fi
 .RE
 .TP 7
-.BI "modules=" xwayland.so,cms-colord.so
+.BI "xwayland=" true
+ask Weston to load the XWayland module (boolean).
+.RE
+.TP 7
+.BI "modules=" cms-colord.so,screen-share.so
 specifies the modules to load (string). Available modules in the
 .IR "__weston_modules_dir__"
 directory are:
 .PP
 .RS 10
 .nf
-.BR xwayland.so
 .BR cms-colord.so
 .BR screen-share.so
 .fi
diff --git a/man/weston.man b/man/weston.man
index 0c3e8dc..face229 100644
--- a/man/weston.man
+++ b/man/weston.man
@@ -83,7 +83,7 @@ the X server. XWayland provides backwards compatibility to X 
applications in a
 Wayland stack.
 
 XWayland is activated by instructing
-.BR weston " to load " xwayland.so " module, see " EXAMPLES .
+.BR weston " to load the XWayland module, see " EXAMPLES .
 Weston starts listening on a new X display socket, and exports it in the
 environment variable
 .BR DISPLAY .
@@ -143,6 +143,9 @@ Append log messages to the file
 .I file.log
 instead of writing them to stderr.
 .TP
+\fB\-\-xwayland\fR
+Ask Weston to load the XWayland module.
+.TP
 \fB\-\-modules\fR=\fImodule1.so,module2.so\fR
 Load the comma-separated list of modules. Only used by the test
 suite. The file is searched for in
@@ -326,7 +329,7 @@ http://wayland.freedesktop.org/
 .IP "Launch Weston with the DRM bac

[PATCH weston v2 4/5] weston: Make the shell entrypoint specific

[Quentin Glidic]

From: Quentin Glidic 

This avoids loading a shell as a module, so we are sure to have only one
shell loaded at a time.

Signed-off-by: Quentin Glidic 
---
 compositor/main.c   | 17 -
 compositor/weston.h |  3 +++
 desktop-shell/shell.c   |  4 ++--
 fullscreen-shell/fullscreen-shell.c |  4 ++--
 ivi-shell/ivi-shell.c   |  4 ++--
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index af093f1..74b404b 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -828,6 +828,21 @@ wet_load_module(struct weston_compositor *compositor,
return 0;
 }
 
+static int
+wet_load_shell(struct weston_compositor *compositor,
+  const char *name, int *argc, char *argv[])
+{
+   int (*shell_init)(struct weston_compositor *ec,
+ int *argc, char *argv[]);
+
+   shell_init = wet_load_module_entrypoint(name, "wet_shell_init");
+   if (!shell_init)
+   return -1;
+   if (shell_init(compositor, argc, argv) < 0)
+   return -1;
+   return 0;
+}
+
 static int
 load_modules(struct weston_compositor *ec, const char *modules,
 int *argc, char *argv[])
@@ -1895,7 +1910,7 @@ int main(int argc, char *argv[])
weston_config_section_get_string(section, "shell", &shell,
 "desktop-shell.so");
 
-   if (load_modules(ec, shell, &argc, argv) < 0)
+   if (wet_load_shell(ec, shell, &argc, argv) < 0)
goto out;
 
weston_config_section_get_string(section, "modules", &modules, "");
diff --git a/compositor/weston.h b/compositor/weston.h
index 6229b34..5708aca 100644
--- a/compositor/weston.h
+++ b/compositor/weston.h
@@ -64,6 +64,9 @@ void *
 wet_load_module_entrypoint(const char *name, const char *entrypoint);
 
 int
+wet_shell_init(struct weston_compositor *ec,
+  int *argc, char *argv[]);
+int
 wet_module_init(struct weston_compositor *ec,
int *argc, char *argv[]);
 int
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 47f7e16..a5e32bd 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4875,8 +4875,8 @@ handle_seat_created(struct wl_listener *listener, void 
*data)
 }
 
 WL_EXPORT int
-wet_module_init(struct weston_compositor *ec,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *ec,
+  int *argc, char *argv[])
 {
struct weston_seat *seat;
struct desktop_shell *shell;
diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index fcecc8d..2037f1c 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -897,8 +897,8 @@ bind_fullscreen_shell(struct wl_client *client, void *data, 
uint32_t version,
 }
 
 WL_EXPORT int
-wet_module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *compositor,
+  int *argc, char *argv[])
 {
struct fullscreen_shell *shell;
struct weston_seat *seat;
diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
index 6095ff7..efaa889 100644
--- a/ivi-shell/ivi-shell.c
+++ b/ivi-shell/ivi-shell.c
@@ -492,8 +492,8 @@ shell_add_bindings(struct weston_compositor *compositor,
  * Initialization of ivi-shell.
  */
 WL_EXPORT int
-wet_module_init(struct weston_compositor *compositor,
-   int *argc, char *argv[])
+wet_shell_init(struct weston_compositor *compositor,
+  int *argc, char *argv[])
 {
struct ivi_shell *shell;
struct ivi_shell_setting setting = { };
-- 
2.10.0

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

[PATCH weston v2 0/5] libweston common modules preparation

[Quentin Glidic]

From: Quentin Glidic 

Here is the second take at my common modules series, or rather at the 
preparation
work.

The real feature will come in a second series, with README update, and a 
complete
design explanation.

This series is merely a split and reordering of the first two patches frome the
old series, with two modifications, as asked by reviewers:
- dropped the old "module_init" entirely
- maitained "modules=xwayland.so" compatibility

Thanks,


Quentin Glidic (5):
  libweston: Properly namespace backends entrypoint
  libweston: Properly namespace modules entrypoint
  weston: Properly namespace modules entrypoint
  weston: Make the shell entrypoint specific
  weston: Add a specific option to load XWayland

 compositor/cms-colord.c |  5 +--
 compositor/cms-static.c |  4 +--
 compositor/main.c   | 66 +
 compositor/screen-share.c   |  4 +--
 compositor/systemd-notify.c |  5 +--
 compositor/weston.h | 16 -
 desktop-shell/shell.c   |  4 +--
 fullscreen-shell/fullscreen-shell.c |  5 +--
 ivi-shell/ivi-layout.c  |  4 ++-
 ivi-shell/ivi-shell.c   |  4 +--
 libweston/compositor-drm.c  |  4 +--
 libweston/compositor-fbdev.c|  4 +--
 libweston/compositor-headless.c |  4 +--
 libweston/compositor-rdp.c  |  4 +--
 libweston/compositor-wayland.c  |  4 +--
 libweston/compositor-x11.c  |  4 +--
 libweston/compositor.c  | 10 +++---
 libweston/compositor.h  |  7 ++--
 man/weston.ini.man  |  7 ++--
 man/weston.man  |  7 ++--
 tests/plugin-registry-test.c|  4 ++-
 tests/surface-global-test.c |  4 ++-
 tests/surface-screenshot.c  |  5 +--
 tests/surface-test.c|  4 ++-
 tests/weston-test.c |  4 +--
 tests/weston-tests-env  |  7 ++--
 weston.ini.in   |  3 +-
 xwayland/launcher.c |  3 +-
 28 files changed, 137 insertions(+), 69 deletions(-)

-- 
2.10.0

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

[PATCH weston] man/weston.ini: Fix panel-position

[Quentin Glidic]

From: Quentin Glidic 

It was renamed from panel-location in
55d5701ddf018887a30d9ddede38550967da61bc, and gained a few possible
values.

Signed-off-by: Quentin Glidic 
---
 man/weston.ini.man | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/man/weston.ini.man b/man/weston.ini.man
index 54668485..39a06fb1 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -236,9 +236,12 @@ digit pairs are in order transparency, red, green, and 
blue. Examples:
 .fi
 .RE
 .TP 7
-.BI "panel-location=" top
-sets the location of the panel (string). Can be
+.BI "panel-position=" top
+sets the position of the panel (string). Can be
 .B top,
+.B bottom,
+.B left,
+.B right,
 .B none.
 .TP 7
 .BI "locking=" true
-- 
2.11.0

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

[PATCH weston v2] editor: Drop g_type_init() call

[Quentin Glidic]

From: Quentin Glidic 

HAVE_PANGO is not in any AC_DEFINE(), so the check is just wrong.
g_type_init() was never called, which is fine since GLib 2.36 anyway.
It is better not to have a wrong usage of HAVE_PANGO here.
Just check for GLib 2.36 in configure.ac instead.

Signed-off-by: Quentin Glidic 
---
 clients/editor.c   | 4 
 clients/stacking.c | 4 
 configure.ac   | 2 +-
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/clients/editor.c b/clients/editor.c
index 42c7f52d..f1dffe1f 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -1604,10 +1604,6 @@ main(int argc, char *argv[])
 
memset(&editor, 0, sizeof editor);
 
-#ifdef HAVE_PANGO
-   g_type_init();
-#endif
-
editor.display = display_create(&argc, argv);
if (editor.display == NULL) {
fprintf(stderr, "failed to create display: %m\n");
diff --git a/clients/stacking.c b/clients/stacking.c
index 0682e60a..b034cf2a 100644
--- a/clients/stacking.c
+++ b/clients/stacking.c
@@ -288,10 +288,6 @@ main(int argc, char *argv[])
 
memset(&stacking, 0, sizeof stacking);
 
-#ifdef HAVE_PANGO
-   g_type_init();
-#endif
-
stacking.display = display_create(&argc, argv);
if (stacking.display == NULL) {
fprintf(stderr, "Failed to create display: %m\n");
diff --git a/configure.ac b/configure.ac
index 247aa696..aca3b862 100644
--- a/configure.ac
+++ b/configure.ac
@@ -436,7 +436,7 @@ if test x$enable_clients = xyes; then
  [AC_ERROR([cairo-egl not used because $CAIRO_EGL_PKG_ERRORS])])],
   [have_cairo_egl=no])
 
-  PKG_CHECK_MODULES(PANGO, [pangocairo], [have_pango=yes], [have_pango=no])
+  PKG_CHECK_MODULES(PANGO, [pangocairo pango glib >= 2.36], [have_pango=yes], 
[have_pango=no])
 fi
 
 AC_ARG_ENABLE(resize-optimization,
-- 
2.11.0

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

Re: [PATCH] configure: search for lib with clock_getres()

[Quentin Glidic]

On 19/12/2016 18:20, Yann E. MORIN wrote:

Like clock_gettime(), clock_getres() is in -lrt for glibc < 2.17.
Add a check for it, like is done for clock_gettime().

Fixes:
 
http://autobuild.buildroot.net/results/bce/bcecdbbce4a99eb1e9bfbf519857bf94d8952037/

Signed-off-by: "Yann E. MORIN" 


Reviewed-by: Quentin Glidic 
And pushed:
226408d2..b51e6ed7  master -> master

Thanks,



---
  Makefile.am  | 1 +
  configure.ac | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 2219e3d..53f8f51 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -190,6 +190,7 @@ weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) 
$(LIBUNWIND_CFLAGS)
  weston_LDADD = libshared.la libweston-@LIBWESTON_MAJOR@.la \
$(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
$(DLOPEN_LIBS) $(LIBINPUT_BACKEND_LIBS) \
+   $(CLOCK_GETRES_LIBS) \
-lm
  
  weston_SOURCES = 	\

diff --git a/configure.ac b/configure.ac
index 1e251bf..604f51b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -88,8 +88,9 @@ PKG_PROG_PKG_CONFIG()
  
  WESTON_SEARCH_LIBS([DLOPEN], [dl], [dlopen])
  
-# In old glibc versions (< 2.17) clock_gettime() is in librt

+# In old glibc versions (< 2.17) clock_gettime() and clock_getres() are in 
librt
  WESTON_SEARCH_LIBS([CLOCK_GETTIME], [rt], [clock_gettime])
+WESTON_SEARCH_LIBS([CLOCK_GETRES], [rt], [clock_getres])
  
  AC_CHECK_DECL(SFD_CLOEXEC,[],

  [AC_MSG_ERROR("SFD_CLOEXEC is needed to compile weston")],




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH RFC weston 3/4] compositor-x11: Remove support for ancient XCB

[Quentin Glidic]

On 29/11/2016 18:00, Daniel Stone wrote:

We had two non-pkg-config check paths in the configure script, to
support XCB functionality used before XCB had had an accompanying
release: xcb_poll_for_queued_event (released in 1.8, 2012), and a
usable XKB event mechanism (released in 1.9, 2013).

Convert the former to a version-based hard dependency, and the latter to
a version-based soft dependency. This avoids two compiler checks.

Signed-off-by: Daniel Stone 


A good thing to do even without Meson:
Reviewed-by: Quentin Glidic 

Cheers,



---
  configure.ac   | 21 ++---
  libweston/compositor-x11.c |  9 ++---
  2 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1e251bf..7d5eaa1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -183,29 +183,12 @@ AC_ARG_ENABLE(x11-compositor, [  
--enable-x11-compositor],,
  enable_x11_compositor=yes)
  AM_CONDITIONAL(ENABLE_X11_COMPOSITOR, test x$enable_x11_compositor = xyes)
  if test x$enable_x11_compositor = xyes; then
-  PKG_CHECK_MODULES([XCB], xcb)
-  xcb_save_LIBS=$LIBS
-  xcb_save_CFLAGS=$CFLAGS
-  CFLAGS=$XCB_CFLAGS
-  LIBS=$XCB_LIBS
-  AC_CHECK_FUNCS([xcb_poll_for_queued_event])
-  LIBS=$xcb_save_LIBS
-  CFLAGS=$xcb_save_CFLAGS
-
+  PKG_CHECK_MODULES([XCB], xcb >= 1.8)
X11_COMPOSITOR_MODULES="x11 x11-xcb xcb-shm"
  
-  PKG_CHECK_MODULES(X11_COMPOSITOR_XKB, [xcb-xkb],

+  PKG_CHECK_MODULES(X11_COMPOSITOR_XKB, [xcb-xkb >= 1.9],
[have_xcb_xkb="yes"], [have_xcb_xkb="no"])
if test "x$have_xcb_xkb" = xyes; then
-   # Most versions of XCB have totally broken XKB bindings, where the
-   # events don't work.  Make sure we can actually use them.
-   xcb_xkb_save_CFLAGS=$CFLAGS
-   CFLAGS=$X11_COMPOSITOR_XKB_CFLAGS
-   AC_CHECK_MEMBER([struct xcb_xkb_state_notify_event_t.xkbType],
-   [], [have_xcb_xkb=no], [[#include ]])
-   CFLAGS=$xcb_xkb_save_CFLAGS
-  fi
-  if test "x$have_xcb_xkb" = xyes; then
X11_COMPOSITOR_MODULES="$X11_COMPOSITOR_MODULES xcb-xkb"
AC_DEFINE([HAVE_XCB_XKB], [1], [libxcb supports XKB protocol])
fi
diff --git a/libweston/compositor-x11.c b/libweston/compositor-x11.c
index 34ef854..627bb15 100644
--- a/libweston/compositor-x11.c
+++ b/libweston/compositor-x11.c
@@ -1298,15 +1298,10 @@ static int
  x11_backend_next_event(struct x11_backend *b,
   xcb_generic_event_t **event, uint32_t mask)
  {
-   if (mask & WL_EVENT_READABLE) {
+   if (mask & WL_EVENT_READABLE)
*event = xcb_poll_for_event(b->conn);
-   } else {
-#ifdef HAVE_XCB_POLL_FOR_QUEUED_EVENT
+   else
*event = xcb_poll_for_queued_event(b->conn);
-#else
-   *event = xcb_poll_for_event(b->conn);
-#endif
-   }
  
  	return *event != NULL;

  }




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 01/24] xwayland: WM debug prints

[Quentin Glidic]

On 21/12/2016 15:39, Pekka Paalanen wrote:

From: Pekka Paalanen 

Add WM debug prints on map, decoration drawing and geometry setting.
These help see the sequence and timing of operations, when debugging
Xwayland window management glitches.

Signed-off-by: Pekka Paalanen 


Seems good:
Reviewed-by: Quentin Glidic 



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

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 56d65af..1830864 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1050,6 +1050,8 @@ weston_wm_window_draw_decoration(void *data)
uint32_t flags = 0;
struct weston_view *view;
  
+	wm_log("XWM: start draw decoration, win %d\n", window->id);

+
weston_wm_window_read_properties(window);
  
  	window->repaint_source = NULL;

@@ -1108,6 +1110,9 @@ weston_wm_window_draw_decoration(void *data)
pixman_region32_init_rect(&window->surface->pending.input,
  input_x, input_y, input_w, input_h);
  
+		wm_log("XWM: draw decoration, win %d geometry: %d,%d %dx%d\n",

+  window->id, input_x, input_y, input_w, input_h);
+
xwayland_interface->set_window_geometry(window->shsurf,
input_x, input_y, 
input_w, input_h);
}
@@ -1139,6 +1144,8 @@ weston_wm_window_schedule_repaint(struct weston_wm_window 
*window)
if (window->repaint_source)
return;
  
+	wm_log("XWM: schedule repaint, win %d\n", window->id);

+
window->repaint_source =
wl_event_loop_add_idle(wm->server->loop,
   weston_wm_window_draw_decoration,
@@ -2547,6 +2554,9 @@ xserver_map_shell_surface(struct weston_wm_window *window,
   window->surface,
   &shell_client);
  
+	wm_log("XWM: map shell surface, win %d, xwayland surface %p\n",

+  window->id, window->shsurf);
+
if (window->name)
xwayland_interface->set_title(window->shsurf, window->name);
if (window->pid > 0)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 08/24] libweston-desktop/xwayland: debug commits

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Helps tracking what happens with XWM.

Use the same debugging guard as XWM.

Signed-off-by: Pekka Paalanen 


Seems good:
Reviewed-by: Quentin Glidic 



---
  libweston-desktop/xwayland.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
index 449675a..d1566bd 100644
--- a/libweston-desktop/xwayland.c
+++ b/libweston-desktop/xwayland.c
@@ -135,6 +135,10 @@ weston_desktop_xwayland_surface_committed(struct 
weston_desktop_surface *dsurfac
  
  	assert(dsurface == surface->surface);
  
+#ifdef WM_DEBUG

+   weston_log("%s: xwayland surface %p\n", __func__, surface);
+#endif
+
if (surface->has_next_geometry) {
oldgeom = weston_desktop_surface_get_geometry(surface->surface);
sx -= surface->next_geometry.x - oldgeom.x;




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 07/24] desktop-shell: debug set_position_from_xwayland

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Helps tracking what happens with XWM.

Use the same debugging guard as XWM.

Signed-off-by: Pekka Paalanen 


Seems good:
Reviewed-by: Quentin Glidic 



---
  desktop-shell/shell.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 9b39933..f53a49c 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2404,6 +2404,12 @@ set_position_from_xwayland(struct shell_surface *shsurf)
y = shsurf->xwayland.y - geometry.y;
  
  	weston_view_set_position(shsurf->view, x, y);

+
+#ifdef WM_DEBUG
+   weston_log("%s: XWM %d, %d; geometry %d, %d; view %f, %f\n",
+  __func__, shsurf->xwayland.x, shsurf->xwayland.y,
+  geometry.x, geometry.y, x, y);
+#endif
  }
  
  static void





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 09/24] xwm: detect legacy fullscreen on MapRequest

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

The legacy fullscreen state needs to be detected at MapRequest time,
because that is when the X11 client has alredy set up the initial window
state.

Doing it at xserver_map_shell_surface() meant that it would be done as a
response to Xwayland creating the wl_surface and XWM receiving the
WL_SURFACE_ID ClientMessage, whichever came later. At that point the X11
client might still be setting things up in theory, though in practice
most of the X11 communication has already happened when
xserver_map_shell_surface() gets called.

The real reason for this is to clean up xserver_map_shell_surface() from
everything that would affect drawing the decorations. This patch is one
part of that clean-up.

The weston_output_weak_ref logic is not put into compositor.h, because
there are no other users for it at this time. We need to protect against
the output going away.


Just to be clear: “protect” here means “not segfault”, just passing NULL 
if the output got away in the meantime, right?





A side-effect of this patch is that saved_width and saved_height will
now get overwritten also for legacy fullscreen windows. Previously, they
were left to zero as far as I could tell.

NOTE: This stops override-redirect legacy fullscreen windows from being
detected as fullscreen. MapRequest processing does not happen for OR
windows. These windows get detected as type XWAYLAND instead.

Signed-off-by: Pekka Paalanen 


Nice way to do it:
Reviewed-by: Quentin Glidic 



---
  xwayland/window-manager.c | 70 +++
  1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 10bb390..02a44a7 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -36,6 +36,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  
@@ -126,6 +127,11 @@ struct motif_wm_hints {

  #define _NET_WM_MOVERESIZE_MOVE_KEYBOARD10   /* move via keyboard */
  #define _NET_WM_MOVERESIZE_CANCEL   11   /* cancel operation */
  
+struct weston_output_weak_ref {

+   struct weston_output *output;
+   struct wl_listener destroy_listener;
+};
+
  struct weston_wm_window {
struct weston_wm *wm;
xcb_window_t id;
@@ -152,6 +158,7 @@ struct weston_wm_window {
bool pos_dirty;
int map_request_x;
int map_request_y;
+   struct weston_output_weak_ref legacy_fullscreen_output;
int saved_width, saved_height;
int decorate;
int override_redirect;
@@ -174,6 +181,11 @@ weston_wm_set_net_active_window(struct weston_wm *wm, 
xcb_window_t window);
  static void
  weston_wm_window_schedule_repaint(struct weston_wm_window *window);
  
+static int

+legacy_fullscreen(struct weston_wm *wm,
+ struct weston_wm_window *window,
+ struct weston_output **output_ret);
+
  static void
  xserver_map_shell_surface(struct weston_wm_window *window,
  struct weston_surface *surface);
@@ -212,6 +224,47 @@ wm_log_continue(const char *fmt, ...)
  #endif
  }
  
+static void

+weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
+{
+   ref->output = NULL;
+}
+
+static void
+weston_output_weak_ref_clear(struct weston_output_weak_ref *ref)
+{
+   if (!ref->output)
+   return;
+
+   wl_list_remove(&ref->destroy_listener.link);
+   ref->output = NULL;
+}
+
+static void
+weston_output_weak_ref_handle_destroy(struct wl_listener *listener, void *data)
+{
+   struct weston_output_weak_ref *ref;
+
+   ref = wl_container_of(listener, ref, destroy_listener);
+   assert(ref->output == data);
+
+   weston_output_weak_ref_clear(ref);
+}
+
+static void
+weston_output_weak_ref_set(struct weston_output_weak_ref *ref,
+  struct weston_output *output)
+{
+   weston_output_weak_ref_clear(ref);
+
+   if (!output)
+   return;
+
+   ref->destroy_listener.notify = weston_output_weak_ref_handle_destroy;
+   wl_signal_add(&output->destroy_signal, &ref->destroy_listener);
+   ref->output = output;
+}
+
  static bool __attribute__ ((warn_unused_result))
  wm_lookup_window(struct weston_wm *wm, xcb_window_t hash,
 struct weston_wm_window **window)
@@ -955,6 +1008,7 @@ weston_wm_handle_map_request(struct weston_wm *wm, 
xcb_generic_event_t *event)
xcb_map_request_event_t *map_request =
(xcb_map_request_event_t *) event;
struct weston_wm_window *window;
+   struct weston_output *output;
  
  	if (our_resource(wm, map_request->window)) {

wm_log("XCB_MAP_REQUEST (window %d, ours)\n",
@@ -982,6 +1036,12 @@ weston_wm_handle_map_request(struct weston_wm *wm, 
xcb_generic_event_t *event)
weston_wm_window_set_net_wm_state(window);
west

Re: [PATCH weston v2 10/24] xwm: clarify props[] in weston_wm_window_read_properties()

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

The props array contained offsets to struct members. It is convenient
for writing static const arrays as you only store a constant offset and
compute the pointer later. However, the array was not static to begin
with, the atoms are not build time constants. We can as well just store
the pointer directly in the array.

Entries that did not use the offset had bogus offsets, producing
pointers to arbitrary fields. They are changed to have a NULL pointer.
If the code unintentionally used the pointer, it will now explode rather
than corrupt memory.

Also explain the use of the #defined constants and #undef them when they
get out of scope. This clearly documents that they are just a convenient
hack to avoid lots of special cases in the function.

Signed-off-by: Pekka Paalanen 


Much cleaner:
Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 38 +++---
  1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 02a44a7..c4220ab 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -443,7 +443,10 @@ read_and_dump_property(struct weston_wm *wm,
free(reply);
  }
  
-/* We reuse some predefined, but otherwise useles atoms */

+/* We reuse some predefined, but otherwise useles atoms
+ * as local type placeholders that never touch the X11 server,
+ * to make weston_wm_window_read_properties() less exceptional.
+ */
  #define TYPE_WM_PROTOCOLS XCB_ATOM_CUT_BUFFER0
  #define TYPE_MOTIF_WM_HINTS   XCB_ATOM_CUT_BUFFER1
  #define TYPE_NET_WM_STATE XCB_ATOM_CUT_BUFFER2
@@ -456,23 +459,23 @@ weston_wm_window_read_properties(struct weston_wm_window 
*window)
const struct weston_desktop_xwayland_interface *xwayland_interface =
wm->server->compositor->xwayland_interface;
  
-#define F(field) offsetof(struct weston_wm_window, field)

+#define F(field) (&window->field)
const struct {
xcb_atom_t atom;
xcb_atom_t type;
-   int offset;
+   void *ptr;
} props[] = {
-   { XCB_ATOM_WM_CLASS, XCB_ATOM_STRING, F(class) },
-   { XCB_ATOM_WM_NAME, XCB_ATOM_STRING, F(name) },
-   { XCB_ATOM_WM_TRANSIENT_FOR, XCB_ATOM_WINDOW, F(transient_for) 
},
-   { wm->atom.wm_protocols, TYPE_WM_PROTOCOLS, F(protocols) },
-   { wm->atom.wm_normal_hints, TYPE_WM_NORMAL_HINTS, F(protocols) 
},
-   { wm->atom.net_wm_state, TYPE_NET_WM_STATE },
-   { wm->atom.net_wm_window_type, XCB_ATOM_ATOM, F(type) },
-   { wm->atom.net_wm_name, XCB_ATOM_STRING, F(name) },
-   { wm->atom.net_wm_pid, XCB_ATOM_CARDINAL, F(pid) },
-   { wm->atom.motif_wm_hints, TYPE_MOTIF_WM_HINTS, 0 },
-   { wm->atom.wm_client_machine, XCB_ATOM_WM_CLIENT_MACHINE, 
F(machine) },
+   { XCB_ATOM_WM_CLASS,   XCB_ATOM_STRING,
F(class) },
+   { XCB_ATOM_WM_NAME,XCB_ATOM_STRING,
F(name) },
+   { XCB_ATOM_WM_TRANSIENT_FOR,   XCB_ATOM_WINDOW,
F(transient_for) },
+   { wm->atom.wm_protocols,   TYPE_WM_PROTOCOLS,  NULL 
},
+   { wm->atom.wm_normal_hints,TYPE_WM_NORMAL_HINTS,   NULL 
},
+   { wm->atom.net_wm_state,   TYPE_NET_WM_STATE,  NULL 
},
+   { wm->atom.net_wm_window_type, XCB_ATOM_ATOM,  
F(type) },
+   { wm->atom.net_wm_name,XCB_ATOM_STRING,
F(name) },
+   { wm->atom.net_wm_pid, XCB_ATOM_CARDINAL,  
F(pid) },
+   { wm->atom.motif_wm_hints, TYPE_MOTIF_WM_HINTS,NULL 
},
+   { wm->atom.wm_client_machine,  XCB_ATOM_WM_CLIENT_MACHINE, 
F(machine) },
};
  #undef F
  
@@ -511,7 +514,7 @@ weston_wm_window_read_properties(struct weston_wm_window *window)

continue;
}
  
-		p = ((char *) window + props[i].offset);

+   p = props[i].ptr;
  
  		switch (props[i].type) {

case XCB_ATOM_WM_CLIENT_MACHINE:
@@ -605,6 +608,11 @@ weston_wm_window_read_properties(struct weston_wm_window 
*window)
xwayland_interface->set_pid(window->shsurf, window->pid);
  }
  
+#undef TYPE_WM_PROTOCOLS

+#undef TYPE_MOTIF_WM_HINTS
+#undef TYPE_NET_WM_STATE
+#undef TYPE_WM_NORMAL_HINTS
+
  static void
  weston_wm_window_get_frame_size(struct weston_wm_window *window,
int *width, int *height)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 11/24] xwm: move frame_set_title() into draw_decoration()

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

The only thing using the frame title is frame_repaint(). Move the call
to frame_set_title() from weston_wm_window_read_properties() into
weston_wm_window_draw_decoration() where the only call to
frame_repaint() is.

Do not check for window->name == NULL, because frame_set_title() handles
NULL just fine. Also, once window->name becomes set, it cannot become
NULL again unless strndup() fails. The name string can be reset to
the empty string in any case.

This change is prompted by future refactoring where at
weston_wm_window_read_properties() time the frame might not have been
created yet.

Signed-off-by: Pekka Paalanen 


Makes sense:
Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c4220ab..e2b1ebf 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -602,8 +602,6 @@ weston_wm_window_read_properties(struct weston_wm_window 
*window)
  
  	if (window->shsurf && window->name)

xwayland_interface->set_title(window->shsurf, window->name);
-   if (window->frame && window->name)
-   frame_set_title(window->frame, window->name);
if (window->shsurf && window->pid > 0)
xwayland_interface->set_pid(window->shsurf, window->pid);
  }
@@ -1144,6 +1142,7 @@ weston_wm_window_draw_decoration(void *data)
if (wm->focus_window == window)
flags |= THEME_FRAME_ACTIVE;
  
+		frame_set_title(window->frame, window->name);

frame_repaint(window->frame, cr);
} else {
cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 12/24] xwm: move set_title and set_pid

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Move the calls to set_title() and set_pid() out of
weston_wm_window_read_properties() and into the three callers, each
slightly different.

xserver_map_shell_surface(): already calls these functions after
creating the shell surface, so need to add calls.


typo: “so need” → “so no need”



weston_wm_handle_map_request(): can be called only on unmapped (in X11)
Windows, so no need to add calls.

weston_wm_window_draw_decoration(): window->shsurf and window->surface
are either both set or both NULL, so the check for window->shsurf is
removed when moving the set_title() and set_pid() calls under a
window->surface check.

Signed-off-by: Pekka Paalanen 


Nice cleanup:
Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 24 +---
  1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index e2b1ebf..fb83795 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -456,8 +456,6 @@ static void
  weston_wm_window_read_properties(struct weston_wm_window *window)
  {
struct weston_wm *wm = window->wm;
-   const struct weston_desktop_xwayland_interface *xwayland_interface =
-   wm->server->compositor->xwayland_interface;
  
  #define F(field) (&window->field)

const struct {
@@ -599,11 +597,6 @@ weston_wm_window_read_properties(struct weston_wm_window 
*window)
if (!window->machine || strcmp(window->machine, name))
window->pid = 0;
}
-
-   if (window->shsurf && window->name)
-   xwayland_interface->set_title(window->shsurf, window->name);
-   if (window->shsurf && window->pid > 0)
-   xwayland_interface->set_pid(window->shsurf, window->pid);
  }
  
  #undef TYPE_WM_PROTOCOLS

@@ -1027,6 +1020,19 @@ weston_wm_handle_map_request(struct weston_wm *wm, 
xcb_generic_event_t *event)
  
  	weston_wm_window_read_properties(window);
  
+	/* For a new Window, MapRequest happens before the Window is realized

+* in Xwayland. We do the real xcb_map_window() here as a response to
+* MapRequest. The Window will get realized (wl_surface created in
+* Wayland and WL_SURFACE_ID sent in X11) when it has been mapped for
+* real.
+*
+* MapRequest only happens for (X11) unmapped Windows. On UnmapNotify,
+* we reset shsurf to NULL, so even if X11 connection races far ahead
+* of the Wayland connection and the X11 client is repeatedly mapping
+* and unmapping, we will never have shsurf set on MapRequest.
+*/
+   assert(!window->shsurf);
+
window->map_request_x = window->x;
window->map_request_y = window->y;
  
@@ -1190,6 +1196,10 @@ weston_wm_window_draw_decoration(void *data)
  
  		xwayland_interface->set_window_geometry(window->shsurf,

input_x, input_y, 
input_w, input_h);
+   if (window->name)
+   xwayland_interface->set_title(window->shsurf, 
window->name);
+   if (window->pid > 0)
+   xwayland_interface->set_pid(window->shsurf, 
window->pid);
}
  }
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 13/24] xwm: debug changes to override-redirect flag

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

For every event we handle and that delivers the override-redirect flag,
print it to debug log.

Add a comment to one code path explaining when it gets hit, because it
is unobvious. It also serves as a reminder that we do not handle changes
to the OR flag after Window creation.

Signed-off-by: Pekka Paalanen 


Documenting stuff is good:
Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index fb83795..0a6417c 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -741,10 +741,11 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, 
xcb_generic_event_t *eve
(xcb_configure_notify_event_t *) event;
struct weston_wm_window *window;
  
-	wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d\n",

+   wm_log("XCB_CONFIGURE_NOTIFY (window %d) %d,%d @ %dx%d%s\n",
   configure_notify->window,
   configure_notify->x, configure_notify->y,
-  configure_notify->width, configure_notify->height);
+  configure_notify->width, configure_notify->height,
+  configure_notify->override_redirect ? ", override" : "");
  
  	if (!wm_lookup_window(wm, configure_notify->window, &window))

return;
@@ -1069,7 +1070,8 @@ weston_wm_handle_map_notify(struct weston_wm *wm, 
xcb_generic_event_t *event)
return;
}
  
-	wm_log("XCB_MAP_NOTIFY (window %d)\n", map_notify->window);

+   wm_log("XCB_MAP_NOTIFY (window %d%s)\n", map_notify->window,
+  map_notify->override_redirect ? ", override" : "");
  }
  
  static void

@@ -1212,6 +1214,11 @@ weston_wm_window_schedule_repaint(struct 
weston_wm_window *window)
  
  	if (window->frame_id == XCB_WINDOW_NONE) {

if (window->surface != NULL) {
+   /* Override-redirect windows go through here, but we
+* cannot assert(window->override_redirect); because
+* we do not deal with changing OR flag yet.
+* XXX: handle OR flag changes in message handlers
+*/
weston_wm_window_get_frame_size(window, &width, 
&height);
pixman_region32_fini(&window->surface->pending.opaque);
if (window->has_alpha) {
@@ -1386,10 +1393,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)\n",

+   wm_log("XCB_REPARENT_NOTIFY (window %d, parent %d, event %d%s)\n",
   reparent_notify->window,
   reparent_notify->parent,
-  reparent_notify->event);
+  reparent_notify->event,
+  reparent_notify->override_redirect ? ", override" : "");
  
  	if (reparent_notify->parent == wm->screen->root) {

weston_wm_window_create(wm, reparent_notify->window, 10, 10,




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 14/24] xwm: debug print deleted property name

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Use wm_log_continue() to avoid printing the timestamp in the middle of a
message.

Print the name of the property that got deleted.

Signed-off-by: Pekka Paalanen 


Nice to have:
Reviewed-by: Quentin Glidic 

Thanks,



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

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 0a6417c..5466bc9 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1258,7 +1258,8 @@ weston_wm_handle_property_notify(struct weston_wm *wm, 
xcb_generic_event_t *even
  
  	wm_log("XCB_PROPERTY_NOTIFY: window %d, ", property_notify->window);

if (property_notify->state == XCB_PROPERTY_DELETE)
-   wm_log("deleted\n");
+   wm_log_continue("deleted %s\n",
+   get_atom_name(wm->conn, property_notify->atom));
else
read_and_dump_property(wm, property_notify->window,
   property_notify->atom);




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 15/24] xwm: postpone geometry dirtying from pending.opaque

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Changing the opaque region has no immediate effect, therefore there is
no need to mark the view geometry dirty.

The view geometry will be invalidated automatically by the next commit
from Xwayland, in weston_surface_commit_state(). The dirtying did not
apply pending state.

Signed-off-by: Pekka Paalanen 


Not sure I fully understand what the code does (or not), but the patch 
looks to fit the commit message:

Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 6 --
  1 file changed, 6 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 5466bc9..021b18a 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1130,7 +1130,6 @@ weston_wm_window_draw_decoration(void *data)
const struct weston_desktop_xwayland_interface *xwayland_interface =
wm->server->compositor->xwayland_interface;
uint32_t flags = 0;
-   struct weston_view *view;
  
  	wm_log("XWM: start draw decoration, win %d\n", window->id);
  
@@ -1175,8 +1174,6 @@ weston_wm_window_draw_decoration(void *data)

  window->width + 2,
  window->height + 2);
}
-   wl_list_for_each(view, &window->surface->views, surface_link)
-   weston_view_geometry_dirty(view);
  
  		pixman_region32_fini(&window->surface->pending.input);
  
@@ -1209,7 +1206,6 @@ static void

  weston_wm_window_schedule_repaint(struct weston_wm_window *window)
  {
struct weston_wm *wm = window->wm;
-   struct weston_view *view;
int width, height;
  
  	if (window->frame_id == XCB_WINDOW_NONE) {

@@ -1227,8 +1223,6 @@ weston_wm_window_schedule_repaint(struct weston_wm_window 
*window)

pixman_region32_init_rect(&window->surface->pending.opaque, 0, 0,
  width, height);
}
-   wl_list_for_each(view, &window->surface->views, 
surface_link)
-   weston_view_geometry_dirty(view);
}
return;
}




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 16/24] xwm: delete dead flags from weston_wm_window_draw_decorations()

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:

From: Pekka Paalanen 

Obviously unused. Looks like weston_wm_window_activate() is doing that
job.

Signed-off-by: Pekka Paalanen 


Trivial:
Reviewed-by: Quentin Glidic 

Thanks,



---
  xwayland/window-manager.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 021b18a..9071598 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1129,7 +1129,6 @@ weston_wm_window_draw_decoration(void *data)
int32_t input_x, input_y, input_w, input_h;
const struct weston_desktop_xwayland_interface *xwayland_interface =
wm->server->compositor->xwayland_interface;
-   uint32_t flags = 0;
  
  	wm_log("XWM: start draw decoration, win %d\n", window->id);
  
@@ -1146,9 +1145,6 @@ weston_wm_window_draw_decoration(void *data)

if (window->fullscreen) {
/* nothing */
} else if (window->decorate) {
-   if (wm->focus_window == window)
-   flags |= THEME_FRAME_ACTIVE;
-
frame_set_title(window->frame, window->name);
frame_repaint(window->frame, cr);
} else {




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] desktop-shell: Stop asking for a RGB565 background

[Quentin Glidic]

On 15/01/2017 18:54, Emmanuel Gil Peyrot wrote:

This makes the background image look much nicer, at the expense of
slightly more memory bandwidth used.

Signed-off-by: Emmanuel Gil Peyrot 


I see no reason not to:
Reviewed-by: Quentin Glidic 

I’ll push it as soon as someone else agree there’s no harm.

Thanks,



---
  clients/desktop-shell.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index a1cf51df..bd0032a7 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -1101,8 +1101,6 @@ background_create(struct desktop *desktop)
window_set_user_data(background->window, background);
widget_set_redraw_handler(background->widget, background_draw);
widget_set_transparent(background->widget, 0);
-   window_set_preferred_format(background->window,
-   WINDOW_PREFERRED_FORMAT_RGB565);
  
  	s = weston_config_get_section(desktop->config, "shell", NULL, NULL);

weston_config_section_get_string(s, "background-image",




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/2] desktop-shell: Initialise panel surface listener

[Quentin Glidic]

On 16/01/2017 14:23, Daniel Stone wrote:

The desktop-shell output destroy code assumes that we always set up a
panel listener. Initialise its list explicitly, so if we don't have a
panel, then we can still unconditionally destroy the listener on output
destroy.

Signed-off-by: Daniel Stone 


Good point:
Reviewed-by: Quentin Glidic 

Thanks,



---
  desktop-shell/shell.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 8388dc3..ce4b870 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -4667,6 +4667,7 @@ create_shell_output(struct desktop_shell *shell,
shell_output->output = output;
shell_output->shell = shell;
shell_output->destroy_listener.notify = handle_output_destroy;
+   wl_list_init(&shell_output->panel_surface_listener.link);
wl_signal_add(&output->destroy_signal,
  &shell_output->destroy_listener);
wl_list_insert(shell->output_list.prev, &shell_output->link);




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] desktop-shell: Support panel-position 'none'

[Quentin Glidic]

On 16/01/2017 14:23, Daniel Stone wrote:

The manpage claims that none is valid, so let's make it so.

Signed-off-by: Daniel Stone 


Hey, didn’t it work already? A tiny warning is no big. ;-)

Anyway:
Reviewed-by: Quentin Glidic 



---
  clients/desktop-shell.c | 25 -
  1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index bd0032a..b133d86 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -1362,25 +1362,24 @@ parse_panel_position(struct desktop *desktop, struct 
weston_config_section *s)
  {
char *position;
  
+	desktop->want_panel = 1;

+
weston_config_section_get_string(s, "panel-position", &position, "top");
-   if (strcmp(position, "top") == 0)
+   if (strcmp(position, "top") == 0) {
desktop->panel_position = 
WESTON_DESKTOP_SHELL_PANEL_POSITION_TOP;
-   else if (strcmp(position, "bottom") == 0)
+   } else if (strcmp(position, "bottom") == 0) {
desktop->panel_position = 
WESTON_DESKTOP_SHELL_PANEL_POSITION_BOTTOM;
-   else if (strcmp(position, "left") == 0)
+   } else if (strcmp(position, "left") == 0) {
desktop->panel_position = 
WESTON_DESKTOP_SHELL_PANEL_POSITION_LEFT;
-   else if (strcmp(position, "right") == 0)
+   } else if (strcmp(position, "right") == 0) {
desktop->panel_position = 
WESTON_DESKTOP_SHELL_PANEL_POSITION_RIGHT;
-   else


Style question: why not just strcmp(position, "none") != 0 as another 
else if?


Thanks,



-   fprintf(stderr, "Wrong panel position: %s\n", position);
-   free(position);
-
-   if (desktop->panel_position == WESTON_DESKTOP_SHELL_PANEL_POSITION_TOP
-   || desktop->panel_position == 
WESTON_DESKTOP_SHELL_PANEL_POSITION_BOTTOM
-   || desktop->panel_position == 
WESTON_DESKTOP_SHELL_PANEL_POSITION_LEFT
-   || desktop->panel_position == 
WESTON_DESKTOP_SHELL_PANEL_POSITION_RIGHT) {
-   desktop->want_panel = 1;
+   } else {
+   /* 'none' is valid here */
+   if (strcmp(position, "none") != 0)
+   fprintf(stderr, "Wrong panel position: %s\n", position);
+   desktop->want_panel = 0;
}
+   free(position);
  }
  
  static void





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] man/weston.ini: Fix panel-position

[Quentin Glidic]

On 16/01/2017 14:24, Daniel Stone wrote:

Hi Quentin,

On 4 January 2017 at 18:15, Quentin Glidic
 wrote:

It was renamed from panel-location in
55d5701ddf018887a30d9ddede38550967da61bc, and gained a few possible
values.


Conditional on the two patches to actually support panel-position
'none' that I just posted being merged, this is:
Reviewed-by: Daniel Stone 


Pushed all three patches:
9cbe1c6a..962609a7  master -> master

Thanks,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2] editor: Drop g_type_init() call

[Quentin Glidic]

On 16/01/2017 14:04, Daniel Stone wrote:

Hi,

On 16 January 2017 at 13:01, Emil Velikov  wrote:

On 15 January 2017 at 12:26, Quentin Glidic
 wrote:

-  PKG_CHECK_MODULES(PANGO, [pangocairo], [have_pango=yes], [have_pango=no])
+  PKG_CHECK_MODULES(PANGO, [pangocairo pango glib >= 2.36], [have_pango=yes], 
[have_pango=no])


Afaict there is no upstream glib - perhaps you meant glib-2.0 ? There
seems to be a bunch of g_* dependencies outside of libglib-2.0.so,
which seems to be missing in configure.


Indeed, glib.pc is for GLib 1.x; my Fedora version has 1.2.10, so this
will never trigger. With that fixed (i.e. the dep being on glib-2.0
instead):
Reviewed-by: Daniel Stone 


Oops, I probably mixed my distcheck with some other patch.
Fixed, distchecked and pushed:
46ffea63..db776dbe  master -> master

Thanks,


--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/2] Makefile.am: Link modules to libweston.la

[Quentin Glidic]

On 17/01/2017 12:54, Daniel Stone wrote:

Hi,

On 21 November 2016 at 11:07, Jan Engelhardt  wrote:

On Monday 2016-11-21 12:00, Daniel Stone wrote:

COMPOSITOR_LIBS is full of -Lfoo -lsyslib. To the best of my
knowledge, these belong in LDADD


Negative. _LDADD is ignored for library type outputs, and _LIBADD
is ignored for program type outputs.

Everything is correct here..


I only said this on IRC rather than the list, but Jan is completely
right. Merge at leisure.



Added Daniel’s R-b, and pushed:
2d66a7d8..b2f32ab4  master -> master

Thanks,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 0/5] libweston common modules preparation

[Quentin Glidic]

On 17/01/2017 13:02, Daniel Stone wrote:

Hi Quentin,

On 18 December 2016 at 13:26, Quentin Glidic
 wrote:

Here is the second take at my common modules series, or rather at the 
preparation
work.

The real feature will come in a second series, with README update, and a 
complete
design explanation.

This series is merely a split and reordering of the first two patches frome the
old series, with two modifications, as asked by reviewers:
- dropped the old "module_init" entirely
- maitained "modules=xwayland.so" compatibility


This all looks good to me, with a couple of minor quibbles. Firstly,
the 'compositor/weston.h' changes don't seem like they belong in 2/5:
why are we touching compositor modules in a libweston patch? Secondly,
it would be nice if we could do wet_shell_init in one stage, rather
than module_init -> wet_module_init -> wet_shell_init, but just typing
that out makes me realise it may be more effort than it's worth.
Either way, with 2/5 properly split (or convince me why it's necessary
to combine them), let's get this merged early:
Reviewed-by: Daniel Stone 


Thanks, pushed:
b2f32ab4..6d3887ba  master -> master


Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/7] tests/shell: get rid of static variables

[Quentin Glidic]

On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:

From: Pekka Paalanen 

Stop using static variables and clean up when we're done.

[Emilio: update to latest weston_layer API]

Signed-off-by: Pekka Paalanen 
Signed-off-by: Emilio Pozuelo Monfort 


Nice:
Reviewed-by: Quentin Glidic 

Thanks,



---
  tests/weston-test-desktop-shell.c | 128 --
  1 file changed, 82 insertions(+), 46 deletions(-)

diff --git a/tests/weston-test-desktop-shell.c 
b/tests/weston-test-desktop-shell.c
index a1ec7da7..2cf2271e 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -45,58 +45,62 @@
  #define static_assert(cond, msg)
  #endif
  
-static struct weston_desktop *desktop = NULL;

-static struct weston_layer background_layer;
-static struct weston_surface *background_surface = NULL;
-static struct weston_view *background_view = NULL;
-static struct weston_layer layer;
-static struct weston_view *view = NULL;
-/*
- * libweston-desktop
- */
+struct desktest_shell {
+   struct wl_listener compositor_destroy_listener;
+   struct weston_desktop *desktop;
+   struct weston_layer background_layer;
+   struct weston_surface *background_surface;
+   struct weston_view *background_view;
+   struct weston_layer layer;
+   struct weston_view *view;
+};
  
  static void

  desktop_surface_added(struct weston_desktop_surface *desktop_surface,
  void *shell)
  {
+   struct desktest_shell *dts = shell;
  
-	assert(!view);

+   assert(!dts->view);
  
-	view = weston_desktop_surface_create_view(desktop_surface);

+   dts->view = weston_desktop_surface_create_view(desktop_surface);
  
-	assert(view);

+   assert(dts->view);
  }
  
  static void

  desktop_surface_removed(struct weston_desktop_surface *desktop_surface,
void *shell)
  {
-   assert(view);
+   struct desktest_shell *dts = shell;
  
-	weston_desktop_surface_unlink_view(view);

-   weston_view_destroy(view);
-   view = NULL;
+   assert(dts->view);
+
+   weston_desktop_surface_unlink_view(dts->view);
+   weston_view_destroy(dts->view);
+   dts->view = NULL;
  }
  
  static void

  desktop_surface_committed(struct weston_desktop_surface *desktop_surface,
- int32_t sx, int32_t sy, void *data)
+ int32_t sx, int32_t sy, void *shell)
  {
+   struct desktest_shell *dts = shell;
struct weston_surface *surface =
weston_desktop_surface_get_surface(desktop_surface);
struct weston_geometry geometry =
weston_desktop_surface_get_geometry(desktop_surface);
  
-	assert(view);

+   assert(dts->view);
  
  	if (weston_surface_is_mapped(surface))

return;
  
  	surface->is_mapped = true;

-   weston_layer_entry_insert(&layer.view_list, &view->layer_link);
-   weston_view_set_position(view, 0 - geometry.x, 0 - geometry.y);
-   weston_view_update_transform(view);
-   view->is_mapped = true;
+   weston_layer_entry_insert(&dts->layer.view_list, 
&dts->view->layer_link);
+   weston_view_set_position(dts->view, 0 - geometry.x, 0 - geometry.y);
+   weston_view_update_transform(dts->view);
+   dts->view->is_mapped = true;
  }
  
  static void

@@ -157,42 +161,74 @@ static const struct weston_desktop_api shell_desktop_api 
= {
.pong = desktop_surface_pong,
  };
  
-/*  *

- * end of libweston-desktop *
- *  */
+static void
+shell_destroy(struct wl_listener *listener, void *data)
+{
+   struct desktest_shell *dts;
+
+   dts = container_of(listener, struct desktest_shell,
+  compositor_destroy_listener);
+
+   weston_desktop_destroy(dts->desktop);
+   weston_view_destroy(dts->background_view);
+   weston_surface_destroy(dts->background_surface);
+   free(dts);
+}
  
  WL_EXPORT int

  wet_shell_init(struct weston_compositor *ec,
   int *argc, char *argv[])
  {
-   weston_layer_init(&layer, ec);
-   weston_layer_init(&background_layer, ec);
+   struct desktest_shell *dts;
+
+   dts = zalloc(sizeof *dts);
+   if (!dts)
+   return -1;
+
+   dts->compositor_destroy_listener.notify = shell_destroy;
+   wl_signal_add(&ec->destroy_signal, &dts->compositor_destroy_listener);
+
+   weston_layer_init(&dts->layer, ec);
+   weston_layer_init(&dts->background_layer, ec);
  
  	weston_layer_set_position(&dts->layer,

  WESTON_LAYER_POSITION_NORMAL);
weston_layer_set_position(&dts->background_layer,
  WESTON_LAYER_POSITION_BACKGROUND);
  
-	background_surface = weston_surface_create(ec);

-   if (background_

Re: [PATCH weston 3/7] tests/shell: change background color

[Quentin Glidic]

On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:

From: Pekka Paalanen 

Pick the color 0xCC336699 as AARRGGBB, as if blended on black. This is
the color used with developing the sub-surface shot tests.

No other big reason than it should not be black to have better chances
of catching blending problems.

Signed-off-by: Pekka Paalanen 


Good idea:
Reviewed-by: Quentin Glidic 

Thanks,



---
  tests/weston-test-desktop-shell.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/weston-test-desktop-shell.c 
b/tests/weston-test-desktop-shell.c
index 2cf2271e..de844251 100644
--- a/tests/weston-test-desktop-shell.c
+++ b/tests/weston-test-desktop-shell.c
@@ -204,7 +204,7 @@ wet_shell_init(struct weston_compositor *ec,
if (dts->background_view == NULL)
goto out_surface;
  
-	weston_surface_set_color(dts->background_surface, 0.0, 0.0, 0.0, 1);

+   weston_surface_set_color(dts->background_surface, 0.16, 0.32, 0.48, 1.);
pixman_region32_fini(&dts->background_surface->opaque);
pixman_region32_init_rect(&dts->background_surface->opaque, 0, 0, 2000, 
2000);
pixman_region32_fini(&dts->background_surface->input);




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 4/7] tests: implement get_test_name()

[Quentin Glidic]

On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:

From: Pekka Paalanen 

Screenshot tests often want to use the test name for writing out images.
This is a helper to get the test name without writing it multiple times
in the source.

Signed-off-by: Pekka Paalanen 
Reviewed-by: Emilio Pozuelo Monfort 
---
  tests/weston-test-runner.c | 21 +++--
  tests/weston-test-runner.h | 12 
  2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index b1e89bc2..f197265d 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -42,6 +42,14 @@ char __attribute__((weak)) *server_parameters="";
  
  extern const struct weston_test __start_test_section, __stop_test_section;
  
+static const char *test_name_;

+
+const char *
+get_test_name(void)
+{
+   return test_name_;
+}
+
  static const struct weston_test *
  find_test(const char *name)
  {
@@ -55,8 +63,17 @@ find_test(const char *name)
  }
  
  static void

-run_test(const struct weston_test *t, void *data)
+run_test(const struct weston_test *t, void *data, int iteration)
  {
+   char str[512];


Maybe a little comment that this will always be around because this 
function never returns? (The "never returns" comment is easily missed 
since it’s in the caller.)




+
+   if (data) {


Isn’t that supposed to be testing "iteration" instead?
With that fixed (or explained):
Reviewed-by: Quentin Glidic 

Thanks,



+   snprintf(str, sizeof(str), "%s[%d]", t->name, iteration);
+   test_name_ = str;
+   } else {
+   test_name_ = t->name;
+   }
+
t->run(data);
exit(EXIT_SUCCESS);
  }
@@ -83,7 +100,7 @@ exec_and_report_test(const struct weston_test *t, void 
*test_data, int iteration
assert(pid >= 0);
  
  	if (pid == 0)

-   run_test(t, test_data); /* never returns */
+   run_test(t, test_data, iteration); /* never returns */
  
  	if (waitid(P_ALL, 0, &info, WEXITED)) {

fprintf(stderr, "waitid failed: %m\n");
diff --git a/tests/weston-test-runner.h b/tests/weston-test-runner.h
index a4436919..21a059d6 100644
--- a/tests/weston-test-runner.h
+++ b/tests/weston-test-runner.h
@@ -80,4 +80,16 @@ struct weston_test {
  #define TEST_P(name, data) ARG_TEST(name, 0, data)
  #define FAIL_TEST_P(name, data) ARG_TEST(name, 1, data)
  
+/**

+ * Get the test name string with counter
+ *
+ * \return The test name. For an iterated test, e.g. defined with TEST_P(),
+ * the name has a '[%d]' suffix to indicate the iteration.
+ *
+ * This is only usable from code paths inside TEST(), TEST_P(), etc.
+ * defined functions.
+ */
+const char *
+get_test_name(void);
+
  #endif




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 5/7] tests: put screenshots to ./logs by default

[Quentin Glidic]

On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:

From: Pekka Paalanen 

Logs is where we write all our custom test logs, let's also put the
screenshots in the same place by default from cluttering the base
directory.

Signed-off-by: Pekka Paalanen 
Reviewed-by: Emilio Pozuelo Monfort 


Not sure I like "logs" as a path for temp screenshot, but still better 
than cluttering the base dir, indeed. Until we change it to a betterer 
place:

Reviewed-by: Quentin Glidic 

Thanks,



---
  tests/weston-test-client-helper.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/weston-test-client-helper.c 
b/tests/weston-test-client-helper.c
index fd6ebaf8..dd411925 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -932,9 +932,10 @@ output_path(void)
char *path = getenv("WESTON_TEST_OUTPUT_PATH");
  
  	if (!path)

-   return ".";
+   return "./logs";
+
return path;
-   }
+}
  
  char*

  screenshot_output_filename(const char *basename, uint32_t seq)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 6/7] tests: add subsurface-shot test

[Quentin Glidic]

fill_color(buf->image, color);
+   wl_surface_attach(surface, buf->proxy, 0, 0);
+   wl_surface_damage(surface, 0, 0, width, height);
+   wl_surface_commit(surface);
+
+   return buf;
+}
+
+FAIL_TEST(subsurface_z_order)
+{
+   const char *test_name = get_test_name();
+   struct client *client;
+   struct wl_subcompositor *subco;
+   struct buffer *bufs[5] = { 0 };
+   struct wl_surface *surf[5] = { 0 };
+   struct wl_subsurface *sub[5] = { 0 };
+   struct rectangle clip = { 40, 40, 280, 200 };
+   int fail = 0;
+   unsigned i;
+   pixman_color_t red;
+   pixman_color_t blue;
+   pixman_color_t cyan;
+   pixman_color_t green;
+
+   color(&red, 255, 0, 0);
+   color(&blue, 0, 0, 255);
+   color(&cyan, 0, 255, 255);
+   color(&green, 0, 255, 0);
+
+   client = create_client_and_test_surface(100, 50, 100, 100);
+   assert(client);
+   subco = get_subcompositor(client);
+
+   /* move the pointer clearly away from our screenshooting area */
+   weston_test_move_pointer(client->test->weston_test, 2, 30);
+
+   /* make the parent surface red */
+   surf[0] = client->surface->wl_surface;
+   bufs[0] = surface_commit_color(client, surf[0], &red, 100, 100);
+   /* sub[0] is not used */
+
+   fail += check_screen(client, test_name, 0, &clip, 0);
+
+   /* create a blue sub-surface above red */
+   surf[1] = wl_compositor_create_surface(client->wl_compositor);
+   sub[1] = wl_subcompositor_get_subsurface(subco, surf[1], surf[0]);
+   bufs[1] = surface_commit_color(client, surf[1], &blue, 100, 100);
+
+   wl_subsurface_set_position(sub[1], 20, 20);
+   wl_surface_commit(surf[0]);
+
+   fail += check_screen(client, test_name, 1, &clip, 1);
+
+   /* create a cyan sub-surface above blue */
+   surf[2] = wl_compositor_create_surface(client->wl_compositor);
+   sub[2] = wl_subcompositor_get_subsurface(subco, surf[2], surf[1]);
+   bufs[2] = surface_commit_color(client, surf[2], &cyan, 100, 100);
+
+   wl_subsurface_set_position(sub[2], 20, 20);
+   wl_surface_commit(surf[1]);
+   wl_surface_commit(surf[0]);
+
+   fail += check_screen(client, test_name, 2, &clip, 2);
+
+   /* create a green sub-surface above blue, sibling to cyan */
+   surf[3] = wl_compositor_create_surface(client->wl_compositor);
+   sub[3] = wl_subcompositor_get_subsurface(subco, surf[3], surf[1]);
+   bufs[3] = surface_commit_color(client, surf[3], &green, 100, 100);
+
+   wl_subsurface_set_position(sub[3], -40, 10);
+   wl_surface_commit(surf[1]);
+   wl_surface_commit(surf[0]);
+
+   fail += check_screen(client, test_name, 3, &clip, 3);
+
+   /* stack blue below red, which brings also cyan and green below red */
+   wl_subsurface_place_below(sub[1], surf[0]);
+   wl_surface_commit(surf[0]);
+
+   fail += check_screen(client, test_name, 4, &clip, 4);
+
+   assert(fail == 0);


Nit: move it after the destroys maybe?



+
+   for (i = 0; i < ARRAY_LENGTH(sub); i++)
+   if (sub[i])
+   wl_subsurface_destroy(sub[i]);
+
+   for (i = 0; i < ARRAY_LENGTH(surf); i++)
+       if (surf[i])
+   wl_surface_destroy(surf[i]);
+
+   for (i = 0; i < ARRAY_LENGTH(bufs); i++)
+   if (bufs[i])
+   buffer_destroy(bufs[i]);
+}



I’m not knowledgeable enough on subsurface commit order to check the 
“real” stuff, but the rest looks pretty good. I can at least give:

Acked-by: Quentin Glidic 

Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 4/7] tests: implement get_test_name()

[Quentin Glidic]

On 07/02/2017 12:34, Pekka Paalanen wrote:

On Tue, 7 Feb 2017 11:39:38 +0100
Quentin Glidic  wrote:


On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:

From: Pekka Paalanen 

Screenshot tests often want to use the test name for writing out images.
This is a helper to get the test name without writing it multiple times
in the source.

Signed-off-by: Pekka Paalanen 
Reviewed-by: Emilio Pozuelo Monfort 
---
   tests/weston-test-runner.c | 21 +++--
   tests/weston-test-runner.h | 12 
   2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/tests/weston-test-runner.c b/tests/weston-test-runner.c
index b1e89bc2..f197265d 100644
--- a/tests/weston-test-runner.c
+++ b/tests/weston-test-runner.c
@@ -42,6 +42,14 @@ char __attribute__((weak)) *server_parameters="";
   
   extern const struct weston_test __start_test_section, __stop_test_section;
   
+static const char *test_name_;

+
+const char *
+get_test_name(void)
+{
+   return test_name_;
+}
+
   static const struct weston_test *
   find_test(const char *name)
   {
@@ -55,8 +63,17 @@ find_test(const char *name)
   }
   
   static void

-run_test(const struct weston_test *t, void *data)
+run_test(const struct weston_test *t, void *data, int iteration)
   {
+   char str[512];


Maybe a little comment that this will always be around because this
function never returns? (The "never returns" comment is easily missed
since it’s in the caller.)



Mm, the only way out from the function is exit() and it's a small
function.




+
+   if (data) {


Isn’t that supposed to be testing "iteration" instead?
With that fixed (or explained):
Reviewed-by: Quentin Glidic 


The caller exec_and_report_test() already uses 'data' as the condition
for an iterated test. 'iteration' OTOH will always be a valid index so
it cannot be used to detect a (non-)iterated test.

Do you want some comments added in the code or commit message?


Yeah, the less obscure the test code is, the better, I think. :-)

Thanks,


+   snprintf(str, sizeof(str), "%s[%d]", t->name, iteration);
+   test_name_ = str;
+   } else {
+   test_name_ = t->name;
+   }
+
t->run(data);
exit(EXIT_SUCCESS);
   }
@@ -83,7 +100,7 @@ exec_and_report_test(const struct weston_test *t, void 
*test_data, int iteration
assert(pid >= 0);
   
   	if (pid == 0)

-   run_test(t, test_data); /* never returns */
+   run_test(t, test_data, iteration); /* never returns */
   
   	if (waitid(P_ALL, 0, &info, WEXITED)) {

fprintf(stderr, "waitid failed: %m\n");
diff --git a/tests/weston-test-runner.h b/tests/weston-test-runner.h
index a4436919..21a059d6 100644
--- a/tests/weston-test-runner.h
+++ b/tests/weston-test-runner.h
@@ -80,4 +80,16 @@ struct weston_test {
   #define TEST_P(name, data) ARG_TEST(name, 0, data)
   #define FAIL_TEST_P(name, data) ARG_TEST(name, 1, data)
   
+/**

+ * Get the test name string with counter
+ *
+ * \return The test name. For an iterated test, e.g. defined with TEST_P(),
+ * the name has a '[%d]' suffix to indicate the iteration.
+ *
+ * This is only usable from code paths inside TEST(), TEST_P(), etc.
+ * defined functions.
+ */
+const char *
+get_test_name(void);
+
   #endif
   





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




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 1/3] compositor-drm: Remove crtc_allocator

[Quentin Glidic]

On 09/02/2017 17:44, Daniel Stone wrote:

crtc_allocator was used as a bitmask of CRTC IDs, so we didn't try to
use the same CRTC for multiple outputs. Unfortunately, this only works
to the extent that CRTC object IDs fit within the bitmask; though they
were previously, they are not guaranteed to be under 32 or even 64.

Replace the only use of crtc_allocator with a list walk across outputs.

Signed-off-by: Daniel Stone 
Reported-by: Peter Senna Tschudin 


This one’s easy:
Reviewed-by: Quentin Glidic 

Cheers,



---
  libweston/compositor-drm.c | 32 ++--
  1 file changed, 26 insertions(+), 6 deletions(-)

v2: Split drm_output_find_by_crtc into a separate function.

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 7f1eeda9a..5f1ca9592 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -94,7 +94,6 @@ struct drm_backend {
char *filename;
} drm;
struct gbm_device *gbm;
-   uint32_t crtc_allocator;
uint32_t connector_allocator;
struct wl_listener session_listener;
uint32_t gbm_format;
@@ -239,6 +238,25 @@ drm_sprite_crtc_supported(struct drm_output *output, 
struct drm_sprite *sprite)
return !!(sprite->possible_crtcs & (1 << output->pipe));
  }
  
+static struct drm_output *

+drm_output_find_by_crtc(struct drm_backend *b, uint32_t crtc_id)
+{
+   struct drm_output *output;
+
+   wl_list_for_each(output, &b->compositor->output_list, base.link) {
+   if (output->crtc_id == crtc_id)
+   return output;
+   }
+
+   wl_list_for_each(output, &b->compositor->pending_output_list,
+base.link) {
+   if (output->crtc_id == crtc_id)
+   return output;
+   }
+
+   return NULL;
+}
+
  static void
  drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
  {
@@ -1819,9 +1837,13 @@ find_crtc_for_connector(struct drm_backend *b,
drmModeFreeEncoder(encoder);
  
  		for (i = 0; i < resources->count_crtcs; i++) {

-   if (possible_crtcs & (1 << i) &&
-   !(b->crtc_allocator & (1 << resources->crtcs[i])))
-   return i;
+   if (!(possible_crtcs & (1 << i)))
+   continue;
+
+   if (drm_output_find_by_crtc(b, resources->crtcs[i]))
+   continue;
+
+   return i;
}
}
  
@@ -2507,7 +2529,6 @@ drm_output_destroy(struct weston_output *base)

if (output->backlight)
backlight_destroy(output->backlight);
  
-	b->crtc_allocator &= ~(1 << output->crtc_id);

b->connector_allocator &= ~(1 << output->connector_id);
  
  	free(output);

@@ -2585,7 +2606,6 @@ create_output_for_connector(struct drm_backend *b,
output->disable_pending = 0;
output->original_crtc = NULL;
  
-	b->crtc_allocator |= (1 << output->crtc_id);

b->connector_allocator |= (1 << output->connector_id);
  
  	weston_output_init(&output->base, b->compositor);





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 2/3] compositor-drm: Avoid connector_allocator for hotplugs

[Quentin Glidic]

On 09/02/2017 17:44, Daniel Stone wrote:

Rather than using connector_allocator to determine whether an output is
newly connected or not, use a list walk across all outputs instead.

Signed-off-by: Daniel Stone 
Reported-by: Peter Senna Tschudin 


Seems good:
Reviewed-by: Quentin Glidic 

Cheers,



---
  libweston/compositor-drm.c | 31 +--
  1 file changed, 25 insertions(+), 6 deletions(-)

v2: Split drm_output_find_by_connector into a separate function.

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 5f1ca9592..99699d767 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -257,6 +257,25 @@ drm_output_find_by_crtc(struct drm_backend *b, uint32_t 
crtc_id)
return NULL;
  }
  
+static struct drm_output *

+drm_output_find_by_connector(struct drm_backend *b, uint32_t connector_id)
+{
+   struct drm_output *output;
+
+   wl_list_for_each(output, &b->compositor->output_list, base.link) {
+   if (output->connector_id == connector_id)
+   return output;
+   }
+
+   wl_list_for_each(output, &b->compositor->pending_output_list,
+base.link) {
+   if (output->connector_id == connector_id)
+   return output;
+   }
+
+   return NULL;
+}
+
  static void
  drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
  {
@@ -2764,14 +2783,14 @@ update_outputs(struct drm_backend *b, struct 
udev_device *drm_device)
  
  		connected |= (1 << connector_id);
  
-		if (!(b->connector_allocator & (1 << connector_id))) {

-   create_output_for_connector(b, resources,
-   connector, drm_device);
-   weston_log("connector %d connected\n", connector_id);
-
-   } else {
+   if (drm_output_find_by_connector(b, connector_id)) {
drmModeFreeConnector(connector);
+   continue;
}
+
+   create_output_for_connector(b, resources,
+   connector, drm_device);
+   weston_log("connector %d connected\n", connector_id);
}
drmModeFreeResources(resources);
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2 3/3] compositor-drm: Remove connector_allocator

[Quentin Glidic]

On 09/02/2017 17:44, Daniel Stone wrote:

Remove the last usage of connector_allocator, which was to check for
displays which have been hot-unplugged, and replace it with an array
which doesn't rely on the connector IDs remaining below 32 (or 64).

Signed-off-by: Daniel Stone 
Reported-by: Peter Senna Tschudin 


Looks good:
Reviewed-by: Quentin Glidic 

Cheers,



---
  libweston/compositor-drm.c | 64 +-
  1 file changed, 40 insertions(+), 24 deletions(-)

v2: No changes.

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 99699d767..2a80c6d79 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -94,7 +94,6 @@ struct drm_backend {
char *filename;
} drm;
struct gbm_device *gbm;
-   uint32_t connector_allocator;
struct wl_listener session_listener;
uint32_t gbm_format;
  
@@ -2548,8 +2547,6 @@ drm_output_destroy(struct weston_output *base)

if (output->backlight)
backlight_destroy(output->backlight);
  
-	b->connector_allocator &= ~(1 << output->connector_id);

-
free(output);
  }
  
@@ -2625,8 +2622,6 @@ create_output_for_connector(struct drm_backend *b,

output->disable_pending = 0;
output->original_crtc = NULL;
  
-	b->connector_allocator |= (1 << output->connector_id);

-
weston_output_init(&output->base, b->compositor);
weston_compositor_add_pending_output(&output->base, b->compositor);
  
@@ -2754,7 +2749,7 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)

drmModeConnector *connector;
drmModeRes *resources;
struct drm_output *output, *next;
-   uint32_t connected = 0, disconnects = 0;
+   uint32_t *connected;
int i;
  
  	resources = drmModeGetResources(b->drm.fd);

@@ -2763,6 +2758,12 @@ update_outputs(struct drm_backend *b, struct udev_device 
*drm_device)
return;
}
  
+	connected = calloc(resources->count_connectors, sizeof(uint32_t));

+   if (!connected) {
+   drmModeFreeResources(resources);
+   return;
+   }
+
/* collect new connects */
for (i = 0; i < resources->count_connectors; i++) {
uint32_t connector_id = resources->connectors[i];
@@ -2781,7 +2782,7 @@ update_outputs(struct drm_backend *b, struct udev_device 
*drm_device)
continue;
}
  
-		connected |= (1 << connector_id);

+   connected[i] = connector_id;
  
  		if (drm_output_find_by_connector(b, connector_id)) {

drmModeFreeConnector(connector);
@@ -2792,30 +2793,45 @@ update_outputs(struct drm_backend *b, struct 
udev_device *drm_device)
connector, drm_device);
weston_log("connector %d connected\n", connector_id);
}
-   drmModeFreeResources(resources);
  
-	disconnects = b->connector_allocator & ~connected;

-   if (disconnects) {
-   wl_list_for_each_safe(output, next, &b->compositor->output_list,
- base.link) {
-   if (disconnects & (1 << output->connector_id)) {
-   disconnects &= ~(1 << output->connector_id);
-   weston_log("connector %d disconnected\n",
-  output->connector_id);
-   drm_output_destroy(&output->base);
+   wl_list_for_each_safe(output, next, &b->compositor->output_list,
+ base.link) {
+   bool disconnected = true;
+
+   for (i = 0; i < resources->count_connectors; i++) {
+   if (connected[i] == output->connector_id) {
+   disconnected = false;
+   break;
}
}
  
-		wl_list_for_each_safe(output, next, &b->compositor->pending_output_list,

- base.link) {
-   if (disconnects & (1 << output->connector_id)) {
-   disconnects &= ~(1 << output->connector_id);
-   weston_log("connector %d disconnected\n",
-  output->connector_id);
-   drm_output_destroy(&output->base);
+   if (!disconnected)
+   continue;
+
+   weston_log("connector %d disconnected\n", output->connector_id);
+   drm_output_destroy(&output->base);
+   }
+
+   wl_list_for_each_safe(output, next, &a

Re: [PATCH weston] compositor: Improve xwayland warning message

[Quentin Glidic]

On 09/02/2017 21:28, Armin Krezović wrote:

And fix formatting.

Signed-off-by: Armin Krezović 


Nice:
Reviewed-by: Quentin Glidic 
And pushed:
efc2b1d4..e6b71366  master -> master

Thanks,



---
  compositor/main.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index 4bd6e681..72c3cd10 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -860,9 +860,10 @@ load_modules(struct weston_compositor *ec, const char 
*modules,
snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
  
  		if (strstr(buffer, "xwayland.so")) {

-   weston_log("Old Xwayland module loading detected:"
-  "Please use --xwayland command line option"
-  "or weston.ini xwayland=true\n");
+   weston_log("Old Xwayland module loading detected: "
+  "Please use --xwayland command line option "
+  "or set xwayland=true in the [core] section "
+  "in weston.ini\n");
*xwayland = 1;
} else {
if (wet_load_module(ec, buffer, argc, argv) < 0)




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC wayland, weston] Expose an output's connector name

[Quentin Glidic]

On 07/02/2017 20:51, Matthias Treydte wrote:
I already brought this up on IRC before and finally started to work 
on it. The goal is to give an Wayland client some identifier which 
allows it to recognize the physical connector which corresponds to 
some Wayland output.


The rationale why such an API is useful is that a client might want 
to ensure that it's content is available on some well-known 
connector. Imagine, for example, a video player which "knows" that 
the projector is connected to the second HDMI port.


Hi,

(Reminder: with wl_shell and xdg_shell, an app can only decide which 
output to go *fullscreen* on, and that is the only case where 
identifying a wl_output would be on any use.)


I think the video player use case is ill-defined. Users don’t want the
video player to forces itself on a specific output. Instead, they want
the compositor to properly put the video player on the output that will
be the best at said time.
If I’m working on my primary monitor, I want the video player on the
secondary one, not to disturb the work area.

Another use case was for an app to go fullscreen on the last output it 
was fullscreen on. (So app fullscren on A, user un-fullscreens, moves to 
B, user fullscreens again, app asks to go to A.)


My compositor is the only one knowing enough to make such a decision, 
and I have yet to see a use case where an application should be able to 
decide.
wl_shell and xdg_shell already have the “this is a hint” restriction, 
and we cannot change wl_shell now, but I think this wl_output choice 
should go, eventually (in xdg_shell at least).



Our use case is like that: We have an Wayland application which, 
running under the fullscreen shell, presents one surface per output.


This is a very important information: you’re using the fullscreen shell. 
It is not designed for “normal” applications.

And Emre was talking about IVI, which is very different from desktop too.


[snip]
The first patch extends the "wl_output" interface by a new 
"connector" event, providing a string to the client which can be

used as a stable identifier for that output. I'm unsure if this
version bump to the core Wayland protocol is acceptable, but to me
this seems the obvious place where this information should be
published. A lot of monitor related information (resolution(s),
physical size, maker, model, ...) is already published there, so it
seems natural.
[snip]


As a result, I strongly feel like wl_output is not the right place.
It would bring no value to the desktop shells, and even mislead people 
to think it would be useful. (I even remember hearing that wl_output 
already has too much information not useful to the average app that 
should have been hidden.)


Another advantage of not using wl_output is that each shell can define 
its own value range. IVI could use uints because it’s supposed to run on 
known hardware with MAC-like identifiers (wild guess) while 
fullscreen-shell would use strings.



Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] libweston-desktop/xdg_shell_v6: Send error on wrongly-sized buffer

[Quentin Glidic]

From: Quentin Glidic 

Signed-off-by: Quentin Glidic 
---
 libweston-desktop/xdg-shell-v6.c | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
index 7d0bd8e4..600723eb 100644
--- a/libweston-desktop/xdg-shell-v6.c
+++ b/libweston-desktop/xdg-shell-v6.c
@@ -625,7 +625,6 @@ weston_desktop_xdg_toplevel_committed(struct 
weston_desktop_xdg_toplevel *toplev
 {
struct weston_surface *wsurface =

weston_desktop_surface_get_surface(toplevel->base.desktop_surface);
-   bool reconfigure = false;
 
if (!wsurface->buffer_ref.buffer && !toplevel->added) {
weston_desktop_xdg_toplevel_ensure_added(toplevel);
@@ -634,22 +633,27 @@ weston_desktop_xdg_toplevel_committed(struct 
weston_desktop_xdg_toplevel *toplev
if (!wsurface->buffer_ref.buffer)
return;
 
-   if (toplevel->next_state.maximized || toplevel->next_state.fullscreen)
-   reconfigure =
-   ( ( toplevel->requested_size.width != wsurface->width ) 
||
- ( toplevel->requested_size.height != wsurface->height 
) );
-
-   if (reconfigure) {
-   weston_desktop_xdg_surface_schedule_configure(&toplevel->base);
-   } else {
-   toplevel->state = toplevel->next_state;
-   toplevel->min_size = toplevel->next_min_size;
-   toplevel->max_size = toplevel->next_max_size;
-
-   weston_desktop_api_committed(toplevel->base.desktop,
-toplevel->base.desktop_surface,
-sx, sy);
+   if ((toplevel->next_state.maximized || toplevel->next_state.fullscreen) 
&&
+   (toplevel->requested_size.width != wsurface->width ||
+toplevel->requested_size.height != wsurface->height)) {
+   struct weston_desktop_client *client =
+   
weston_desktop_surface_get_client(toplevel->base.desktop_surface);
+   struct wl_resource *client_resource =
+   weston_desktop_client_get_resource(client);
+
+   wl_resource_post_error(client_resource,
+  
ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE,
+  "xdg_surface buffer does not match the 
configured state");
+   return;
}
+
+   toplevel->state = toplevel->next_state;
+   toplevel->min_size = toplevel->next_min_size;
+   toplevel->max_size = toplevel->next_max_size;
+
+   weston_desktop_api_committed(toplevel->base.desktop,
+toplevel->base.desktop_surface,
+sx, sy);
 }
 
 static void
-- 
2.11.1

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

[RFC weston] libweston-desktop: Add size_requested API

[Quentin Glidic]

From: Quentin Glidic 

Some shells (wl_shell) does not let the compositor control the surface
state and instead force one. Therefore, they cannot call
{maximized,fullscreen}_requested as these imply the compositor can still
opt-out.
This new callback is called whenever a state change requests a new size,
be it triggered by the client or the compositor.

Signed-off-by: Quentin Glidic 
---
This patch works correctly as-is, but is RFC because I have yet to add
some API to support output choice in wl_shell.

 libweston-desktop/internal.h  | 12 +++
 libweston-desktop/libweston-desktop.c |  8 +
 libweston-desktop/libweston-desktop.h | 11 ++
 libweston-desktop/surface.c   | 36 
 libweston-desktop/wl-shell.c  | 14 +---
 libweston-desktop/xdg-shell-v5.c  | 64 +--
 libweston-desktop/xdg-shell-v6.c  | 64 +--
 libweston-desktop/xwayland.c  | 11 --
 8 files changed, 194 insertions(+), 26 deletions(-)

diff --git a/libweston-desktop/internal.h b/libweston-desktop/internal.h
index 763355bf..38386916 100644
--- a/libweston-desktop/internal.h
+++ b/libweston-desktop/internal.h
@@ -80,6 +80,9 @@ weston_desktop_api_maximized_requested(struct weston_desktop 
*desktop,
 void
 weston_desktop_api_minimized_requested(struct weston_desktop *desktop,
   struct weston_desktop_surface *surface);
+void
+weston_desktop_api_size_requested(struct weston_desktop *desktop,
+ struct weston_desktop_surface *surface);
 
 void
 weston_desktop_api_set_xwayland_position(struct weston_desktop *desktop,
@@ -123,6 +126,15 @@ struct weston_desktop_surface_implementation {
(*get_min_size)(struct weston_desktop_surface *surface,
void *user_data);
 
+   bool (*get_pending_activated)(struct weston_desktop_surface *surface,
+   void *user_data);
+   bool (*get_pending_fullscreen)(struct weston_desktop_surface *surface,
+void *user_data);
+   bool (*get_pending_maximized)(struct weston_desktop_surface *surface,
+   void *user_data);
+   bool (*get_pending_resizing)(struct weston_desktop_surface *surface,
+  void *user_data);
+
void (*destroy)(struct weston_desktop_surface *surface,
void *user_data);
 };
diff --git a/libweston-desktop/libweston-desktop.c 
b/libweston-desktop/libweston-desktop.c
index 48e90009..a5ae5bd9 100644
--- a/libweston-desktop/libweston-desktop.c
+++ b/libweston-desktop/libweston-desktop.c
@@ -243,6 +243,14 @@ weston_desktop_api_minimized_requested(struct 
weston_desktop *desktop,
desktop->api.minimized_requested(surface, desktop->user_data);
 }
 
+void
+weston_desktop_api_size_requested(struct weston_desktop *desktop,
+ struct weston_desktop_surface *surface)
+{
+   if (desktop->api.size_requested != NULL)
+   desktop->api.size_requested(surface, desktop->user_data);
+}
+
 void
 weston_desktop_api_set_xwayland_position(struct weston_desktop *desktop,
 struct weston_desktop_surface *surface,
diff --git a/libweston-desktop/libweston-desktop.h 
b/libweston-desktop/libweston-desktop.h
index 03b04c7b..8da10a49 100644
--- a/libweston-desktop/libweston-desktop.h
+++ b/libweston-desktop/libweston-desktop.h
@@ -80,6 +80,8 @@ struct weston_desktop_api {
bool maximized, void *user_data);
void (*minimized_requested)(struct weston_desktop_surface *surface,
void *user_data);
+   void (*size_requested)(struct weston_desktop_surface *surface,
+  void *user_data);
 
/** Position suggestion for an Xwayland window
 *
@@ -192,6 +194,15 @@ weston_desktop_surface_get_max_size(struct 
weston_desktop_surface *surface);
 struct weston_size
 weston_desktop_surface_get_min_size(struct weston_desktop_surface *surface);
 
+bool
+weston_desktop_surface_get_pending_activated(struct weston_desktop_surface 
*surface);
+bool
+weston_desktop_surface_get_pending_maximized(struct weston_desktop_surface 
*surface);
+bool
+weston_desktop_surface_get_pending_fullscreen(struct weston_desktop_surface 
*surface);
+bool
+weston_desktop_surface_get_pending_resizing(struct weston_desktop_surface 
*surface);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libweston-desktop/surface.c b/libweston-desktop/surface.c
index d3be9364..50aa4f46 100644
--- a/libweston-desktop/surface.c
+++ b/libweston-desktop/surface.c
@@ -675,6 +675,42 @@ weston_desktop_surface_get_min_size(struct 
weston_desktop_surface *surface)
 
surface->implementation_data);
 }

[PATCH weston] input: Do not override keyboard focus on restore

[Quentin Glidic]

From: Quentin Glidic 

If we start a special (grabbing) client when Weston is unfocused, it
would lose focus when coming back to Weston.

Signed-off-by: Quentin Glidic 
---
 libweston/input.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libweston/input.c b/libweston/input.c
index 4fedc558..6ebb4f97 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2070,7 +2070,8 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct 
wl_array *keys,
 
if (surface) {
wl_list_remove(&seat->saved_kbd_focus_listener.link);
-   weston_keyboard_set_focus(keyboard, surface);
+   if (!keyboard->focus)
+   weston_keyboard_set_focus(keyboard, surface);
seat->saved_kbd_focus = NULL;
}
 }
-- 
2.11.1

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

Re: [PATCH] Fix uninitialized msec_to_next in output_repaint_timer_arm

[Quentin Glidic]

On 18/03/2017 13:01, Sergi Granell wrote:

Signed-off-by: Sergi Granell 


Good:
Reviewed-by: Quentin Glidic 

And pushed:
65e57c93..b4c08863  master -> master

Thanks,


---
  libweston/compositor.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index fb647daa..048b195c 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2388,7 +2388,7 @@ output_repaint_timer_arm(struct weston_compositor 
*compositor)
struct weston_output *output;
bool any_should_repaint = false;
struct timespec now;
-   int64_t msec_to_next;
+   int64_t msec_to_next = INT64_MAX;
  
  	weston_compositor_read_presentation_clock(compositor, &now);
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/2] Add option to disable unconfigured outputs

[Quentin Glidic]

On 08/03/2017 16:43, Ucan, Emre (ADITG/SW1) wrote:

In current implementation, there is no configuration
to disable unconfigured outputs.

One can create an output section for a known output
in weston.ini file and set its mode to "off" to disable
a known output. But there is no configuration to disable
unknown outputs.

This might be usefull for example, if someone wants to
enable just one output and disable all others. Without
this option, we have to right down an output section for
every output known to system and disable all outputs,
which we do not want to enable.

It might be usefull also for startup time optimization,
because some display types (e.g. LVDS and VGA) are always
up. Therefore, weston would modeset every one of them.
Even there are no attached displays.

This introduces a simple configuration in weston.ini:
   [core]
   require-output-config=false

False is the default, so no behavioral change is introduced.

Signed-off-by: Emre Ucan 
---
  compositor/main.c  |8 +++-
  libweston/compositor.h |3 +++
  man/weston.ini.man |4 
  weston.ini.in  |1 +
  4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/compositor/main.c b/compositor/main.c
index e870dd4..92f8741 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1174,7 +1174,8 @@ drm_backend_output_configure(struct wl_listener 
*listener, void *data)
section = weston_config_get_section(wc, "output", "name", output->name);
weston_config_section_get_string(section, "mode", &s, "preferred");
  
-	if (strcmp(s, "off") == 0) {

+   if ((!section && output->compositor->require_output_config) ||
+   (strcmp(s, "off") == 0)) {
weston_output_disable(output);
free(s);
return;
@@ -1785,6 +1786,7 @@ int main(int argc, char *argv[])
struct weston_seat *seat;
struct wet_compositor user_data;
int require_input;
+   int require_output_config;
  
  	const struct weston_option core_options[] = {

{ WESTON_OPTION_STRING, "backend", 'B', &backend },
@@ -1874,6 +1876,10 @@ int main(int argc, char *argv[])
   &require_input, true);
ec->require_input = require_input;
  
+	weston_config_section_get_bool(section, "require-output-config",

+  &require_output_config, false);
+   ec->require_output_config = require_output_config;
+
if (load_backend(ec, backend, &argc, argv, config) < 0) {
weston_log("fatal: failed to create compositor backend\n");
goto out;
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 08e728a..a7abd35 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -891,6 +891,9 @@ struct weston_compositor {
/* Whether to let the compositor run without any input device. */
bool require_input;
  
+	/* Whether to disable unconfigured outputs */

+   bool require_output_config;
+


This is a setting used by Weston, in Weston code. If it were used in 
libweston, it would be the good place to put. Right now it’s not, and we 
don’t want it in libweston.

Put in in wet_compositor instead.

With that fixed:
Reviewed-by: Quentin Glidic 

Thanks,



  };
  
  struct weston_buffer {

diff --git a/man/weston.ini.man b/man/weston.ini.man
index 5ec0e1d..90e1c55 100644
--- a/man/weston.ini.man
+++ b/man/weston.ini.man
@@ -181,6 +181,10 @@ set to 300 seconds.
  .TP 7
  .BI "require-input=" true
  require an input device for launch
+.TP 7
+.BI "require-output-config=" false
+require an output section for every created output. If there is no section
+for an output, compositor disables the output.
  
  .SH "LIBINPUT SECTION"

  The
diff --git a/weston.ini.in b/weston.ini.in
index 257c4ec..fba893d 100644
--- a/weston.ini.in
+++ b/weston.ini.in
@@ -4,6 +4,7 @@
  #shell=desktop-shell.so
  #gbm-format=xrgb2101010
  #require-input=true
+#require-output-config=false
  
  [shell]

  background-image=/usr/share/backgrounds/gnome/Aqua.jpg




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/2] compositor-wayland: Refactor struct wayland_output::name usage

[Quentin Glidic]

On 3/24/17 7:53 PM, Sergi Granell wrote:

struct wayland_output::name was used but never initialized.
Also zxdg_toplevel_v6_set_title was only called for windowed outputs,
and some compositors let you see the client's name even when it is
fullscreen (GNOME Shell's Activities menu for example).

So rename struct wayland_output::name to struct wayland_output::title and
precompute it on wayland_output_create_common(), so it can be later used
on xdg's set_title and frame_create.

Signed-off-by: Sergi Granell 


One thing to fix (see inline), then I’ll push it.


---
  libweston/compositor-wayland.c | 67 +++---
  1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index ebdbd13b..41834692 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -111,7 +111,7 @@ struct wayland_output {
  
  	int keyboard_count;
  
-	char *name;

+   char *title;
struct frame *frame;
  
  	struct {

@@ -708,6 +708,7 @@ wayland_output_destroy(struct weston_output *base)
if (output->frame_cb)
wl_callback_destroy(output->frame_cb);
  
+	free(output->title);

free(output);
  }
  
@@ -835,36 +836,17 @@ wayland_output_set_windowed(struct wayland_output *output)

  {
struct wayland_backend *b =
to_wayland_backend(output->base.compositor);
-   int tlen;
-   char *title;
  
  	if (output->frame)

return 0;
  
-	if (output->name) {

-   tlen = strlen(output->name) + strlen(WINDOW_TITLE " - ");
-   title = malloc(tlen + 1);
-   if (!title)
-   return -1;
-
-   snprintf(title, tlen + 1, WINDOW_TITLE " - %s", output->name);
-   } else {
-   title = strdup(WINDOW_TITLE);
-   }
-
-   if (output->parent.xdg_toplevel)
-   zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, title);
-
if (!b->theme) {
b->theme = theme_create();
-   if (!b->theme) {
-   free(title);
+   if (!b->theme)
return -1;
-   }
}
output->frame = frame_create(b->theme, 100, 100,
-FRAME_BUTTON_CLOSE, title);
-   free(title);
+FRAME_BUTTON_CLOSE, output->title);
if (!output->frame)
return -1;
  
@@ -1139,6 +1121,8 @@ wayland_backend_create_output_surface(struct wayland_output *output)

while (output->parent.wait_for_configure)
wl_display_dispatch(b->parent.wl_display);
  
+		zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, output->title);

+


Move it *before* the first wl_surface_commit(). If you don’t, the parent 
compositor has no info to match against for the first configure.

Also, you could send a follow-up patch to set the app_id too. :-)

Thanks,


weston_log("wayland-backend: Using xdg_shell_v6\n");
}
else if (b->parent.shell) {
@@ -1239,9 +1223,13 @@ err_output:
  }
  
  static struct wayland_output *

-wayland_output_create_common(void)
+wayland_output_create_common(const char *name)
  {
struct wayland_output *output;
+   size_t len;
+
+   /* name can't be NULL. */
+   assert(name);
  
  	output = zalloc(sizeof *output);

if (output == NULL) {
@@ -1252,6 +1240,18 @@ wayland_output_create_common(void)
output->base.destroy = wayland_output_destroy;
output->base.disable = wayland_output_disable;
output->base.enable = wayland_output_enable;
+   output->base.name = strdup(name);
+
+   /* setup output name/title. */
+   len = strlen(WINDOW_TITLE " - ") + strlen(name) + 1;
+   output->title = zalloc(len);
+   if (!output->title) {
+   free(output->base.name);
+   free(output);
+   return NULL;
+   }
+
+   snprintf(output->title, len, WINDOW_TITLE " - %s", name);
  
  	return output;

  }
@@ -1259,12 +1259,7 @@ wayland_output_create_common(void)
  static int
  wayland_output_create(struct weston_compositor *compositor, const char *name)
  {
-   struct wayland_output *output = wayland_output_create_common();
-
-   /* name can't be NULL. */
-   assert(name);
-
-   output->base.name = strdup(name);
+   struct wayland_output *output = wayland_output_create_common(name);
  
  	weston_output_init(&output->base, compositor);

weston_compositor_add_pending_output(&output->base, compositor);
@@ -1324,11 +1319,9 @@ static int
  wayland_output_create_for_parent_output(struct wayland_backend *b,
struct wayland_parent_output *poutput)
  {
-   struct wayland_output *output = wayland_output_create_common();
+   struct wayland_output *output = 
wayland_output_create_common("wl

Re: [PATCH weston v2 1/2] compositor-wayland: Refactor struct wayland_output::name usage

[Quentin Glidic]

On 3/24/17 8:48 PM, Sergi Granell wrote:

struct wayland_output::name was used but never initialized.
Also zxdg_toplevel_v6_set_title was only called for windowed outputs,
and some compositors let you see the client's name even when it is
fullscreen (GNOME Shell's Activities menu for example).

So rename struct wayland_output::name to struct wayland_output::title and
precompute it on wayland_output_create_common(), so it can be later used
on xdg's set_title and frame_create.

v2: Move zxdg_toplevel_v6_set_title() before the wl_surface_commit()
as per Quentin Glidic's suggestion.

Signed-off-by: Sergi Granell 


Nice. Added Armin’s and my Rb and pushed:
b4c08863..7fecb437  master -> master

Thanks,



---
  libweston/compositor-wayland.c | 67 +++---
  1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index ebdbd13b..2bbf4cf0 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -111,7 +111,7 @@ struct wayland_output {
  
  	int keyboard_count;
  
-	char *name;

+   char *title;
struct frame *frame;
  
  	struct {

@@ -708,6 +708,7 @@ wayland_output_destroy(struct weston_output *base)
if (output->frame_cb)
wl_callback_destroy(output->frame_cb);
  
+	free(output->title);

free(output);
  }
  
@@ -835,36 +836,17 @@ wayland_output_set_windowed(struct wayland_output *output)

  {
struct wayland_backend *b =
to_wayland_backend(output->base.compositor);
-   int tlen;
-   char *title;
  
  	if (output->frame)

return 0;
  
-	if (output->name) {

-   tlen = strlen(output->name) + strlen(WINDOW_TITLE " - ");
-   title = malloc(tlen + 1);
-   if (!title)
-   return -1;
-
-   snprintf(title, tlen + 1, WINDOW_TITLE " - %s", output->name);
-   } else {
-   title = strdup(WINDOW_TITLE);
-   }
-
-   if (output->parent.xdg_toplevel)
-   zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, title);
-
if (!b->theme) {
b->theme = theme_create();
-   if (!b->theme) {
-   free(title);
+   if (!b->theme)
return -1;
-   }
}
output->frame = frame_create(b->theme, 100, 100,
-FRAME_BUTTON_CLOSE, title);
-   free(title);
+FRAME_BUTTON_CLOSE, output->title);
if (!output->frame)
return -1;
  
@@ -1132,6 +1114,8 @@ wayland_backend_create_output_surface(struct wayland_output *output)

zxdg_toplevel_v6_add_listener(output->parent.xdg_toplevel,
  &xdg_toplevel_listener, output);
  
+		zxdg_toplevel_v6_set_title(output->parent.xdg_toplevel, output->title);

+
wl_surface_commit(output->parent.surface);
  
  		output->parent.wait_for_configure = true;

@@ -1239,9 +1223,13 @@ err_output:
  }
  
  static struct wayland_output *

-wayland_output_create_common(void)
+wayland_output_create_common(const char *name)
  {
struct wayland_output *output;
+   size_t len;
+
+   /* name can't be NULL. */
+   assert(name);
  
  	output = zalloc(sizeof *output);

if (output == NULL) {
@@ -1252,6 +1240,18 @@ wayland_output_create_common(void)
output->base.destroy = wayland_output_destroy;
output->base.disable = wayland_output_disable;
output->base.enable = wayland_output_enable;
+   output->base.name = strdup(name);
+
+   /* setup output name/title. */
+   len = strlen(WINDOW_TITLE " - ") + strlen(name) + 1;
+   output->title = zalloc(len);
+   if (!output->title) {
+   free(output->base.name);
+   free(output);
+   return NULL;
+   }
+
+   snprintf(output->title, len, WINDOW_TITLE " - %s", name);
  
  	return output;

  }
@@ -1259,12 +1259,7 @@ wayland_output_create_common(void)
  static int
  wayland_output_create(struct weston_compositor *compositor, const char *name)
  {
-   struct wayland_output *output = wayland_output_create_common();
-
-   /* name can't be NULL. */
-   assert(name);
-
-   output->base.name = strdup(name);
+   struct wayland_output *output = wayland_output_create_common(name);
  
  	weston_output_init(&output->base, compositor);

weston_compositor_add_pending_output(&output->base, compositor);
@@ -1324,11 +1319,9 @@ static int
  wayland_output_create_for_parent_output(struct wayland_backend *b,
struct wayland_parent_output *poutput)
  {
-   struct wayland_output *output = wayland_output_create_common();
+   struct wayland_output *output = 
wayland_output_create_common("wlparent");
struct weston_mode *mode;
  
-	ou

Re: [PATCH weston 1/2] os: Check for EINTR on posix_fallocate()

[Quentin Glidic]

On 3/23/17 6:42 PM, Eric Engestrom wrote:

On Thursday, 2017-03-23 11:59:22 -0500, Derek Foreman wrote:

posix_fallocate() can return EINTR and need to be restarted - I've hit
this when running weston-terminal under gdb.

Signed-off-by: Derek Foreman 


Both patches are:
Reviewed-by: Eric Engestrom 


Added mine and pushed:
7fecb437..5ef6bd7e  master -> master

Thanks,



...and now I'm looking for these in $DAYJOB codebase ^^


---
  shared/os-compatibility.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/shared/os-compatibility.c b/shared/os-compatibility.c
index 551f2a99..6b2f3770 100644
--- a/shared/os-compatibility.c
+++ b/shared/os-compatibility.c
@@ -178,7 +178,9 @@ os_create_anonymous_file(off_t size)
return -1;
  
  #ifdef HAVE_POSIX_FALLOCATE

-   ret = posix_fallocate(fd, 0, size);
+   do {
+   ret = posix_fallocate(fd, 0, size);
+   } while (ret == EINTR);
if (ret != 0) {
close(fd);
errno = ret;
--
2.11.0


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




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 3/3] weston-terminal: Fix race at startup

[Quentin Glidic]

On 3/24/17 3:41 PM, Derek Foreman wrote:

If anything is printed for the terminal window to display before the
window has been initially sized we end up with a segfault.

This defers the exec() of the shell child process until after the
window is sized so this can't happen anymore.

Signed-off-by: Derek Foreman 
---
  clients/terminal.c | 32 
  1 file changed, 32 insertions(+)

diff --git a/clients/terminal.c b/clients/terminal.c
index c5531790..1060c58b 100644
--- a/clients/terminal.c
+++ b/clients/terminal.c
@@ -38,6 +38,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -481,6 +482,7 @@ struct terminal {

int selection_start_row, selection_start_col;
int selection_end_row, selection_end_col;
struct wl_list link;
+   int pace_pipe;
  };
  
  /* Create default tab stops, every 8 characters */

@@ -860,6 +862,10 @@ resize_handler(struct widget *widget,
struct terminal *terminal = data;
int32_t columns, rows, m;
  
+	if (terminal->pace_pipe >= 0) {

+   close(terminal->pace_pipe);
+   terminal->pace_pipe = -1;
+   }
m = 2 * terminal->margin;
columns = (width - m) / (int32_t) terminal->average_width;
rows = (height - m) / (int32_t) terminal->extents.height;
@@ -3027,9 +3033,33 @@ terminal_run(struct terminal *terminal, const char *path)
  {
int master;
pid_t pid;
+   int pipes[2];
+
+   /* Awkwardness: There's a sticky race condition here.  If
+* anything prints after the forkpty() but before the window has
+* a size then we'll segfault.  So we make a pipe and wait on
+* it before actually exec()ing the terminal.  The resize
+* handler closes it in the parent process and the child continues
+* on to launch a shell.
+*
+* The reason we don't just do terminal_run() after the window
+* has a size is that we'd prefer to perform the fork() before
+* the process opens a wayland connection.
+*/
+   if (pipe(pipes) == -1) {
+   fprintf(stderr, "Can't create pipe for pacing.\n");
+   exit(EXIT_FAILURE);
+   }
  
  	pid = forkpty(&master, NULL, NULL, NULL);

if (pid == 0) {
+   int ret;
+
+   close(pipes[1]);
+   do {
+   char tmp;
+   ret = read(pipes[0], &tmp, 1);
+   } while (ret == -1 && errno == EINTR);


Shouldn’t the child close(pipes[0]); here?
With that fixed or answered:
Reviewed-by: Quentin Glidic 

Patches 1 and 2 reviewed and pushed:
5ef6bd7e..091c8017  master -> master

Thanks,



setenv("TERM", option_term, 1);
setenv("COLORTERM", option_term, 1);
if (execl(path, path, NULL)) {
@@ -3041,7 +3071,9 @@ terminal_run(struct terminal *terminal, const char *path)
return -1;
}
  
+	close(pipes[0]);

terminal->master = master;
+   terminal->pace_pipe = pipes[1];
fcntl(master, F_SETFL, O_NONBLOCK);
terminal->io_task.run = io_handler;
display_watch_fd(terminal->display, terminal->master,




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v2] weston-terminal: Fix race at startup

[Quentin Glidic]

On 3/24/17 10:29 PM, Derek Foreman wrote:

If anything is printed for the terminal window to display before the
window has been initially sized we end up with a segfault.

This defers the exec() of the shell child process until after the
window is sized so this can't happen anymore.

Signed-off-by: Derek Foreman 
Reviewed-by: Quentin Glidic 

---
Changes from revision 1:
  - don't forget to close the other half of the pipe in the child


Nice, pushed:
091c8017..88353dda  master -> master

Thanks,



  clients/terminal.c | 33 +
  1 file changed, 33 insertions(+)

diff --git a/clients/terminal.c b/clients/terminal.c
index c5531790..274ced09 100644
--- a/clients/terminal.c
+++ b/clients/terminal.c
@@ -38,6 +38,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -481,6 +482,7 @@ struct terminal {

int selection_start_row, selection_start_col;
int selection_end_row, selection_end_col;
struct wl_list link;
+   int pace_pipe;
  };
  
  /* Create default tab stops, every 8 characters */

@@ -860,6 +862,10 @@ resize_handler(struct widget *widget,
struct terminal *terminal = data;
int32_t columns, rows, m;
  
+	if (terminal->pace_pipe >= 0) {

+   close(terminal->pace_pipe);
+   terminal->pace_pipe = -1;
+   }
m = 2 * terminal->margin;
columns = (width - m) / (int32_t) terminal->average_width;
rows = (height - m) / (int32_t) terminal->extents.height;
@@ -3027,9 +3033,34 @@ terminal_run(struct terminal *terminal, const char *path)
  {
int master;
pid_t pid;
+   int pipes[2];
+
+   /* Awkwardness: There's a sticky race condition here.  If
+* anything prints after the forkpty() but before the window has
+* a size then we'll segfault.  So we make a pipe and wait on
+* it before actually exec()ing the terminal.  The resize
+* handler closes it in the parent process and the child continues
+* on to launch a shell.
+*
+* The reason we don't just do terminal_run() after the window
+* has a size is that we'd prefer to perform the fork() before
+* the process opens a wayland connection.
+*/
+   if (pipe(pipes) == -1) {
+   fprintf(stderr, "Can't create pipe for pacing.\n");
+   exit(EXIT_FAILURE);
+   }
  
  	pid = forkpty(&master, NULL, NULL, NULL);

if (pid == 0) {
+   int ret;
+
+   close(pipes[1]);
+   do {
+   char tmp;
+   ret = read(pipes[0], &tmp, 1);
+   } while (ret == -1 && errno == EINTR);
+   close(pipes[0]);
setenv("TERM", option_term, 1);
setenv("COLORTERM", option_term, 1);
if (execl(path, path, NULL)) {
@@ -3041,7 +3072,9 @@ terminal_run(struct terminal *terminal, const char *path)
return -1;
}
  
+	close(pipes[0]);

terminal->master = master;
+   terminal->pace_pipe = pipes[1];
fcntl(master, F_SETFL, O_NONBLOCK);
terminal->io_task.run = io_handler;
display_watch_fd(terminal->display, terminal->master,




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] compositor-wayland: Call set_window_geometry when using zxdg_shell_v6

[Quentin Glidic]

On 3/24/17 11:45 PM, Sergi Granell wrote:

This way Wayland compositors will be aware of Weston's
"visible bounds" (and ignore its shadows).

Signed-off-by: Sergi Granell 


Looks good, added my Rb and pushed:
88353dda..ed016bff  master -> master

Thanks,


---
  libweston/compositor-wayland.c | 16 
  1 file changed, 16 insertions(+)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 27beff62..1900ab08 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -785,6 +785,14 @@ wayland_output_resize_surface(struct wayland_output 
*output)
wl_surface_set_opaque_region(output->parent.surface, region);
wl_region_destroy(region);
  
+		if (output->parent.xdg_surface) {

+   
zxdg_surface_v6_set_window_geometry(output->parent.xdg_surface,
+   ix,
+   iy,
+   iwidth,
+   iheight);
+   }
+
width = frame_width(output->frame);
height = frame_height(output->frame);
} else {
@@ -797,6 +805,14 @@ wayland_output_resize_surface(struct wayland_output 
*output)
wl_region_add(region, 0, 0, width, height);
wl_surface_set_opaque_region(output->parent.surface, region);
wl_region_destroy(region);
+
+   if (output->parent.xdg_surface) {
+   
zxdg_surface_v6_set_window_geometry(output->parent.xdg_surface,
+   0,
+   0,
+   width,
+   height);
+   }
}
  
  #ifdef ENABLE_EGL





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] desktop-shell: Remove unused variable in panel_create

[Quentin Glidic]

On 3/25/17 4:42 PM, Raúl Peñacoba wrote:

Signed-off-by: Raúl Peñacoba 


Good catch:
Reviewed-by: Quentin Glidic 

And pushed:
ed016bff..07a2b99f  master -> master

Thanks,



---
  clients/desktop-shell.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
index 2667e9bb..599295ee 100644
--- a/clients/desktop-shell.c
+++ b/clients/desktop-shell.c
@@ -591,7 +591,6 @@ panel_create(struct desktop *desktop)
  {
struct panel *panel;
struct weston_config_section *s;
-   char *clock_format_option = NULL;
  
  	panel = xzalloc(sizeof *panel);
  
@@ -611,8 +610,6 @@ panel_create(struct desktop *desktop)

if (panel->clock_format != CLOCK_FORMAT_NONE)
panel_add_clock(panel);
  
-	free (clock_format_option);

-
s = weston_config_get_section(desktop->config, "shell", NULL, NULL);
weston_config_section_get_color(s, "panel-color",
&panel->color, 0xaa00);




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] compositor-wayland: Call weston_compositor_exit when receiving an xdg toplevel close event

[Quentin Glidic]

On 3/25/17 5:19 PM, Sergi Granell wrote:

Signed-off-by: Sergi Granell 


Seems pretty sensible:
Reviewed-by: Quentin Glidic 

And pushed:
07a2b99f..eaa73584  master -> master

Thanks,



---
  libweston/compositor-wayland.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index 1900ab08..a76dd08e 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -1097,6 +1097,9 @@ handle_xdg_toplevel_configure(void *data, struct 
zxdg_toplevel_v6 *toplevel,
  static void
  handle_xdg_toplevel_close(void *data, struct zxdg_toplevel_v6 *xdg_toplevel)
  {
+   struct wayland_output *output = data;
+
+   weston_compositor_exit(output->base.compositor);
  }
  
  static const struct zxdg_toplevel_v6_listener xdg_toplevel_listener = {





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] compositor-drm: Add missing drmModeFreeResources in drm_device_is_kms

[Quentin Glidic]

On 3/28/17 12:44 PM, Sergi Granell wrote:

Signed-off-by: Sergi Granell 


Nice:
Reviewed-by: Quentin Glidic 

And pushed:
eaa73584..ceb5981a  master -> master

Thanks,


---
  libweston/compositor-drm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 4cb27b1d..3f7e97e6 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -3055,6 +3055,8 @@ drm_device_is_kms(struct drm_backend *b, struct 
udev_device *device)
b->drm.id = id;
b->drm.filename = strdup(filename);
  
+	drmModeFreeResources(res);

+
return true;
  
  out_res:





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] gl-renderer: Change 'data' type to 'uint8_t *', since 'void *' arithmetic is undefined

[Quentin Glidic]

On 3/28/17 6:17 PM, Raúl Peñacoba wrote:

Signed-off-by: Raúl Peñacoba 

Good:
Reviewed-by: Quentin Glidic 

And pushed:
ceb5981a..5fc8d5eb  master -> master

Thanks,



---
  libweston/gl-renderer.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index c6091af0..0de2803f 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -1237,7 +1237,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
struct weston_view *view;
bool texture_used;
pixman_box32_t *rectangles;
-   void *data;
+   uint8_t *data;
int i, j, n;
  
  	pixman_region32_union(&gs->texture_damage,





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] ivi-layout: Add missing free() in ivi_view_create

[Quentin Glidic]

On 3/29/17 6:13 PM, Raúl Peñacoba wrote:

Signed-off-by: Raúl Peñacoba 

Nice catch:
Reviewed-by: Quentin Glidic 

And pushed:
5fc8d5eb..bd8dc0a2  master -> master

Thanks,



---
  ivi-shell/ivi-layout.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 64e4ead8..298e18ea 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -173,6 +173,7 @@ ivi_view_create(struct ivi_layout_layer *ivilayer,
ivi_view->view = weston_view_create(ivisurf->surface);
if (ivi_view->view == NULL) {
weston_log("fails to allocate memory\n");
+   free(ivi_view);
return NULL;
}
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] editor: Add missing free() and display_destroy() in main

[Quentin Glidic]

On 3/29/17 6:16 PM, Raúl Peñacoba wrote:

Signed-off-by: Raúl Peñacoba 


Good:
Reviewed-by: Quentin Glidic 

And pushed:
bd8dc0a2..745e647f  master -> master

Thanks,


---
  clients/editor.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/clients/editor.c b/clients/editor.c
index a0cc97af..b63c5628 100644
--- a/clients/editor.c
+++ b/clients/editor.c
@@ -1607,6 +1607,7 @@ main(int argc, char *argv[])
editor.display = display_create(&argc, argv);
if (editor.display == NULL) {
fprintf(stderr, "failed to create display: %m\n");
+   free(text_buffer);
return -1;
}
  
@@ -1615,6 +1616,8 @@ main(int argc, char *argv[])
  
  	if (editor.text_input_manager == NULL) {

fprintf(stderr, "No text input manager global\n");
+   display_destroy(editor.display);
+   free(text_buffer);
return -1;
}
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] compositor-wayland: Properly dealloc mmap data using munmap

[Quentin Glidic]

On 3/29/17 10:23 PM, Raúl Peñacoba wrote:

Signed-off-by: Raúl Peñacoba 


Really nice catch:
Reviewed-by: Quentin Glidic 

And pushed:
745e647f..fec723ef  master -> master

Thanks,



---
  libweston/compositor-wayland.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index a76dd08e..14f2c8db 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -301,7 +301,7 @@ wayland_output_get_shm_buffer(struct wayland_output *output)
if (sb == NULL) {
weston_log("could not zalloc %zu memory for sb: %m\n", sizeof 
*sb);
close(fd);
-   free(data);
+   munmap(data, height * stride);
return NULL;
}
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] wcap: Prevent fd leak in wcap_decoder_create() fail path

[Quentin Glidic]

On 3/29/17 10:41 PM, Sergi Granell wrote:

Signed-off-by: Sergi Granell 


Well, good: ;-)
Reviewed-by: Quentin Glidic 

And pushed:
fec723ef..597dde5c  master -> master

Thanks,



---
  wcap/wcap-decode.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/wcap/wcap-decode.c b/wcap/wcap-decode.c
index e3b8985f..7e8c8477 100644
--- a/wcap/wcap-decode.c
+++ b/wcap/wcap-decode.c
@@ -131,6 +131,7 @@ wcap_decoder_create(const char *filename)
PROT_READ, MAP_PRIVATE, decoder->fd, 0);
if (decoder->map == MAP_FAILED) {
fprintf(stderr, "mmap failed\n");
+   close(decoder->fd);
free(decoder);
return NULL;
}
@@ -146,6 +147,7 @@ wcap_decoder_create(const char *filename)
frame_size = header->width * header->height * 4;
decoder->frame = malloc(frame_size);
if (decoder->frame == NULL) {
+   close(decoder->fd);
free(decoder);
return NULL;
}




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 1/2] Introduce keyboard grabbing protocol for Xwayland

[Quentin Glidic]

On 4/2/17 2:57 AM, Peter Hutterer wrote:

woohoo, grabs. My favourite topic! ;)
On Wed, Mar 22, 2017 at 05:27:22PM +0100, Olivier Fourdan wrote:

>> [snip]

+  
+   This protocol specifies a way for Xwayland to request all keyboard
+   events to be forwarded to a surface even when the surface does not
+   have keyboard focus.
+
+   Unlike the X11 protocol, Wayland doesn't have the notion of
+   active grab on the keyboard.
+
+   When an X11 client acquires an active grab on the keyboard, all
+   key events are reported only to the grabbing X11 client.
+   When running in Xwayland, X11 applications may acquire an active
+   grab but that cannot be translated to the Wayland compositor who
+   may set the input focus to some other surface, thus breaking the
+   X11 client assumption that all key events are reported.
+
+   When an X11 client requests a keyboard grab, the Wayland compositor
+   may either inform or ask the user for the right to forward all key
+   events to the given client surface.


this is confusing :) paragraph 3 talks aobut what's not working and
paragraph 4 about what this protocol should fix. How about something like:

 This protocol is application-specific to meet the needs of the X11
 protocol through Xwayland. It provides a way for XWayland to request
 all keyboard events to be forwarded to a surface even when the
 surface does not have keyboard focus.

 In the X11 protocol, a client may request an "active grab" on the
 keyboard. On success, all key events are reported only to the
 grabbing X11 client. For details, see XGrabKeyboard(3).

 The core Wayland protocol does not have a notion of an active
 keyboard grab. When running in Xwayland, X11 applications may
 acquire an active grab inside XWayland but that cannot be translated
 to the Wayland compositor who may set the input focus to some other
 surface. In doing so, it breaks the X11 client assumption that all
 key events are reported to the grabbing client.

This protocol specifies a way for Xwayland to request all keyboard
 be directed to the given surface. The protocol does not guarantee
 that the compositor will honor this request and it does not
 prescribe user interfaces on how to handle the respond. For example,
 a compositor may inform the user that all key events are now
 forwarded to the given client surface, or it may ask the user for
 permission to do so.

 Warning! Things may go boom... etc. etc.


I think it may be worth it to add a little safety net:
“Compositors are required to restrict this interface to Xwayland alone, 
and raise a protocol error for native Wayland clients.”


Cheers,

--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 1/2] Introduce keyboard grabbing protocol for Xwayland

[Quentin Glidic]

On 4/3/17 3:47 PM, Olivier Fourdan wrote:

Hi Quentin,


I think it may be worth it to add a little safety net:
“Compositors are required to restrict this interface to Xwayland alone,
and raise a protocol error for native Wayland clients.”


Yes, agreed, good point! But do we really need to kill a client that would dare 
to try to bind to the grab interface?

Right now my implementation simply ignores the bind request...



I was thinking about your “hide global”[1] API, which does kill the 
client. It seems like a perfect use of it to me.


[1] 




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/3] configure: replace HAVE_LIBDRM with BUILD_DRM_COMPOSITOR

[Quentin Glidic]

On 4/6/17 2:09 PM, Pekka Paalanen wrote:

From: Pekka Paalanen 

HAVE_LIBDRM was used as a condition for the launcher infrastructure to
call libdrm.so functions. It was set by an independent test for libdrm,
which would silently continue if libdrm was not found. It was assumed
that if you enabled a feature that used libdrm at runtime, the test for
that feature would imply that HAVE_LIBDRM is also set. This was quite
subtle.

The only feature that actually uses libdrm.so at runtime is the DRM
backend. No other backend needs the libdrm calls in the launcher
infrastructure.

Therefore to simplify things, stop using HAVE_LIBDRM and use
BUILD_DRM_COMPOSITOR instead. If you enable the DRM compositor, you
automatically also get libdrm support in the launchers.

There are still things depending on LIBDRM_CFLAGS and LIBDRM_LIBS, so
the test cannot be removed completely.

Signed-off-by: Pekka Paalanen 


It makes clear these are DRM-specific:
Reviewed-by: Quentin Glidic 

Thanks,


---
  configure.ac   | 3 +--
  libweston/launcher-direct.c| 2 +-
  libweston/launcher-weston-launch.c | 2 +-
  libweston/weston-launch.c  | 2 +-
  4 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6cc9f26..39c0531 100644
--- a/configure.ac
+++ b/configure.ac
@@ -177,8 +177,7 @@ if test x$enable_xwayland = xyes; then
fi
  fi
  
-PKG_CHECK_MODULES(LIBDRM, [libdrm],

-  [AC_DEFINE(HAVE_LIBDRM, 1, [Define if libdrm is available]) 
have_libdrm=yes], have_libdrm=no)
+PKG_CHECK_MODULES(LIBDRM, [libdrm], have_libdrm=yes, have_libdrm=no)
  
  AC_ARG_ENABLE(x11-compositor, [  --enable-x11-compositor],,

  enable_x11_compositor=yes)
diff --git a/libweston/launcher-direct.c b/libweston/launcher-direct.c
index 3d8f5f6..a5d3ee5 100644
--- a/libweston/launcher-direct.c
+++ b/libweston/launcher-direct.c
@@ -47,7 +47,7 @@
  #define KDSKBMUTE 0x4B51
  #endif
  
-#ifdef HAVE_LIBDRM

+#ifdef BUILD_DRM_COMPOSITOR
  
  #include 
  
diff --git a/libweston/launcher-weston-launch.c b/libweston/launcher-weston-launch.c

index a7535ce..97da18c 100644
--- a/libweston/launcher-weston-launch.c
+++ b/libweston/launcher-weston-launch.c
@@ -55,7 +55,7 @@
  #define KDSKBMUTE 0x4B51
  #endif
  
-#ifdef HAVE_LIBDRM

+#ifdef BUILD_DRM_COMPOSITOR
  
  #include 
  
diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c

index eecb911..aa7e071 100644
--- a/libweston/weston-launch.c
+++ b/libweston/weston-launch.c
@@ -73,7 +73,7 @@
  
  #define MAX_ARGV_SIZE 256
  
-#ifdef HAVE_LIBDRM

+#ifdef BUILD_DRM_COMPOSITOR
  
  #include 
  




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel