[ANNOUNCE] weston 1.99.94

[Bryce Harrington]

This is another release candidate for weston 2.0.  A handful of fixes
have come in since the first release candidate so we're putting out a
second RC just to be safe.  This includes a couple fixes to potential
crashes in screen-share, decouples building with EGL from also requiring
Cairo, and exposes the weston_config_next_section API to permit external
modules to iterate over the configuration file.

Changes since weston 1.12 include EGL_KHR_swap_buffers_with_damage
support, porting backends to new API for output setup, support for
DRM_FORMAT_YUV444 buffers in the gl backend, better panel positioning
support for desktop-shell, improved XWayland warning messages, and
numerous refactoring and bugfixing improvements.

Of particular note, libweston is bumped to 2 due to an ABI breakage
introduced with the new output API.

A couple more days will be allowed for final testing.  So, barring any
other last minute changes, final release for weston 2.0 can be expected
towards the end of this week.


Changes since previous RC:
--

Bryce Harrington (1):
  configure.ac: bump to version 1.99.94 for the RC2 release

Daniel Stone (3):
  screen-share: Avoid NULL dereference
  screen-share: Use wl_list_for_each_safe on destroy
  clients: Fix build without Cairo/GLES2

Emmanuel Gil Peyrot (1):
  config-parser: Export weston_config_next_section

git tag: 1.99.94
http://wayland.freedesktop.org/releases/weston-1.99.94.tar.xz
MD5:  66abe352e37cbbe151cfc15cc891671f  weston-1.99.94.tar.xz
SHA1: a979c305dcdcd8afb498ed434269109c50d58733  weston-1.99.94.tar.xz
SHA256: 953392ab050acaaecc207fc9425a21b921bdcb0816952fe24bc0ce730d3d9ef6  
weston-1.99.94.tar.xz
PGP:  http://wayland.freedesktop.org/releases/weston-1.99.94.tar.xz.sig



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

[ANNOUNCE] wayland 1.13.0

[Bryce Harrington]

This is the official release of Wayland 1.13.  There have been no
changes since beta and RC1.

In addition to a range of bug fixes, changes since wayland 1.12 have
added some API for controlling the visibility of globals and numerous
documentation and other improvements.

Our next major release will be version 1.14, which will be scheduled
tentatively as follows:

  √ Development opens immediately

  - 1.14-alpha in early May

  - 1.14-beta around mid May

  - 1.14-rc1 late May, with rc2 only if necessary

  - 1.14.0 around beginning of June


Changes since RC:
-

Bryce Harrington (1):
  configure.ac: bump to version 1.13.0 for the official release

git tag: 1.13.0
http://wayland.freedesktop.org/releases/wayland-1.13.0.tar.xz
MD5:  cae152ed956da6de53f9727bc1c45039  wayland-1.13.0.tar.xz
SHA1: a8575325ed2885948624043c71629310df928312  wayland-1.13.0.tar.xz
SHA256: 69b052c031a61e89af7cc8780893d0da1e301492352aa449dee9345043e6fe51  
wayland-1.13.0.tar.xz
PGP:  http://wayland.freedesktop.org/releases/wayland-1.13.0.tar.xz.sig



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

Re: [PATCH weston] config-parser: Export weston_config_next_section

[Bryce Harrington]

On Tue, Feb 21, 2017 at 02:35:14PM +0200, Pekka Paalanen wrote:
> On Tue, 21 Feb 2017 11:58:20 +
> Emmanuel Gil Peyrot  wrote:
> 
> > This symbol wasn’t exported from the weston binary, most likely due to
> > an oversight in 6e2c12496bbef3cc913cfe9d5f0aeb4d8b23b368, and because
> > internal modules can link against libshared.la directly it hasn’t been
> > found ever since.
> > 
> > This commit makes it possible for external modules to iterate over the
> > configuration file.
> > 
> > Signed-off-by: Emmanuel Gil Peyrot 
> > ---
> >  shared/config-parser.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/shared/config-parser.c b/shared/config-parser.c
> > index e2b5fa25..2a595b1f 100644
> > --- a/shared/config-parser.c
> > +++ b/shared/config-parser.c
> > @@ -490,6 +490,7 @@ weston_config_get_full_path(struct weston_config 
> > *config)
> > return config == NULL ? NULL : config->path;
> >  }
> >  
> > +WL_EXPORT
> >  int
> >  weston_config_next_section(struct weston_config *config,
> >struct weston_config_section **section,
> 
> Reviewed-by: Pekka Paalanen 
> 
> Bryce, this is a low-risk patch which we cannot land in a stable branch
> after the release because it adds ABI to libweston and that AFAIU would
> require a minor-version bump, but the stable branch only does
> patch-version bumps. OTOH, it is not a release critical bug fix IMHO.
> You should decide if this goes in for Weston 2.0 or waits for a whole
> cycle before being released. I am fine either way, really, but I am
> also in a slightly awkward position to make the call (conflict of
> interests).
> 
> I know Emmanuel has a use case for this function in an out-of-tree
> weston plugin.

I see nothing that concerns me with the patch, and avoiding the
annoyance of an ABI bump seems reasonable enough.  Since we're already
including some other last minute patches and ought to run an RC2 anyway,
I see no reason not to toss this one in as well.

Acked-by: Bryce Harrington 

> The whole libweston thing is a little sad: libshared.la gets built into
> libweston. config-parser.c exports functions, presumably because they
> were necessary to export from weston the executable for plugins to use.
> But because of that, libweston also now exports a bunch of functions it
> shouldn't really. I filed:
> https://phabricator.freedesktop.org/T7713

Kind of makes me idly wonder if an external dependency for config/option
parsing wouldn't be worth considering.

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

Re: [PATCH weston] clients: Fix build without Cairo/GLES2

[Bryce Harrington]

On Mon, Feb 20, 2017 at 03:46:55PM +, Emil Velikov wrote:
> On 16 February 2017 at 21:52, Daniel Stone  wrote:
> > If we're building with EGL support generally, but without Cairo/GLESv2,
> > building the clients fail, because window.c defines the EGL native
> > types, however platform.h also brings these in.
> >
> > Signed-off-by: Daniel Stone 
> > Cc: Emil Velikov 
> > Cc: Bryce Harrington 
> > ---
> >  clients/window.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/clients/window.c b/clients/window.c
> > index 59fc07e..95796d4 100644
> > --- a/clients/window.c
> > +++ b/clients/window.c
> > @@ -55,7 +55,7 @@
> >  #include 
> >
> >  #include 
> > -#else /* HAVE_CAIRO_EGL */
> > +#elif !defined(ENABLE_EGL) /* platform.h defines these if EGL is enabled */
> >  typedef void *EGLDisplay;
> >  typedef void *EGLConfig;
> >  typedef void *EGLContext;
> Not a huge expert on the permutations available here, but this seems correct.
> Reviewed-by: Emil Velikov 
> 
> Fwiw typedef redefinition is a C11 feature. Add -pedantic to GCC to flag it 
> up.
> Although yes, the define has also changed, even if it expands to
> 
> #define EGL_NO_DISPLAY ((EGLDisplay)(0))
> 
> -Emil

Acked-by: Bryce Harrington 

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

Re: [PATCH weston 2/2] screen-share: Use wl_list_for_each_safe on destroy

[Bryce Harrington]

On Thu, Feb 16, 2017 at 07:59:51PM +, Daniel Stone wrote:
> Destroying the shared seat removes the link from so->seat_list.
> 
> Signed-off-by: Daniel Stone 

Acked-by: Bryce Harrington 

> ---
>  compositor/screen-share.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/compositor/screen-share.c b/compositor/screen-share.c
> index 069da1d..a6f82b1 100644
> --- a/compositor/screen-share.c
> +++ b/compositor/screen-share.c
> @@ -884,7 +884,7 @@ shared_output_create(struct weston_output *output, int 
> parent_fd)
>  {
>   struct shared_output *so;
>   struct wl_event_loop *loop;
> - struct ss_seat *seat;
> + struct ss_seat *seat, *tmp;
>   int epoll_fd;
>  
>   so = zalloc(sizeof *so);
> @@ -972,7 +972,7 @@ shared_output_create(struct weston_output *output, int 
> parent_fd)
>   return so;
>  
>  err_display:
> - wl_list_for_each(seat, >seat_list, link)
> + wl_list_for_each_safe(seat, tmp, >seat_list, link)
>   ss_seat_destroy(seat);
>   wl_display_disconnect(so->parent.display);
>  err_alloc:
> -- 
> 2.9.3
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/2] screen-share: Avoid NULL dereference

[Bryce Harrington]

On Thu, Feb 16, 2017 at 07:59:50PM +, Daniel Stone wrote:
> Don't try to dereference the seat if it's NULL.
> 
> Signed-off-by: Daniel Stone 

Yep, obviously correct.

Reviewed-by: Bryce Harrington 

> ---
>  compositor/screen-share.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/compositor/screen-share.c b/compositor/screen-share.c
> index bcb9def..069da1d 100644
> --- a/compositor/screen-share.c
> +++ b/compositor/screen-share.c
> @@ -192,7 +192,7 @@ ss_seat_handle_keymap(void *data, struct wl_keyboard 
> *wl_keyboard,
>   char *map_str;
>  
>   if (!data)
> - goto error;
> + goto error_no_seat;
>  
>   if (format == WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1) {
>   map_str = mmap(NULL, size, PROT_READ, MAP_SHARED, fd, 0);
> @@ -235,6 +235,7 @@ ss_seat_handle_keymap(void *data, struct wl_keyboard 
> *wl_keyboard,
>  
>  error:
>   wl_keyboard_release(seat->parent.keyboard);
> +error_no_seat:
>   close(fd);
>  }
>  
> -- 
> 2.9.3
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v2 4/4] build: remove white space in -uninstalled.pc.in files

[Emil Velikov]

From: Emil Velikov 

v2: Rebase, address wayland-client-uninstalled

Signed-off-by: Emil Velikov 
Reviewed-by: Derek Foreman  (v1)
---
 src/wayland-client-uninstalled.pc.in  | 2 +-
 src/wayland-scanner-uninstalled.pc.in | 2 +-
 src/wayland-server-uninstalled.pc.in  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/wayland-client-uninstalled.pc.in 
b/src/wayland-client-uninstalled.pc.in
index c12a917..fa88d36 100644
--- a/src/wayland-client-uninstalled.pc.in
+++ b/src/wayland-client-uninstalled.pc.in
@@ -1,7 +1,7 @@
 libdir=@abs_builddir@/.libs
 includedir=@abs_srcdir@
 protocoldir=@abs_top_builddir@/protocol
- 
+
 Name: Wayland Client
 Description: Wayland client side library (not installed)
 Version: @PACKAGE_VERSION@
diff --git a/src/wayland-scanner-uninstalled.pc.in 
b/src/wayland-scanner-uninstalled.pc.in
index 132f42d..4559799 100644
--- a/src/wayland-scanner-uninstalled.pc.in
+++ b/src/wayland-scanner-uninstalled.pc.in
@@ -1,6 +1,6 @@
 pkgdatadir=@abs_top_srcdir@
 wayland_scanner=@abs_top_builddir@/wayland-scanner
- 
+
 Name: Wayland Scanner
 Description: Wayland scanner (not installed)
 Version: @PACKAGE_VERSION@
diff --git a/src/wayland-server-uninstalled.pc.in 
b/src/wayland-server-uninstalled.pc.in
index b1985e5..19279c2 100644
--- a/src/wayland-server-uninstalled.pc.in
+++ b/src/wayland-server-uninstalled.pc.in
@@ -1,7 +1,7 @@
 libdir=@abs_builddir@/.libs
 includedir=@abs_srcdir@
 protocoldir=@abs_top_builddir@/protocol
- 
+
 Name: Wayland Server
 Description: Server side implementation of the Wayland protocol (not installed)
 Version: @PACKAGE_VERSION@
-- 
2.11.0

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

[PATCH wayland v2 1/4] wayland-util: do not export the wl_map_* API

[Emil Velikov]

From: Emil Velikov 

Used only internally and explicitly marked as such with commit
cf04b0a18f2 ("Move private definitions and prototypes to new
zwayland-private.h")

Signed-off-by: Emil Velikov 
---
 src/wayland-util.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/wayland-util.c b/src/wayland-util.c
index 077fec7..cab7fc5 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -177,21 +177,21 @@ union map_entry {
 #define map_entry_get_data(entry) ((void *)((entry).next & ~(uintptr_t)0x3))
 #define map_entry_get_flags(entry) (((entry).next >> 1) & 0x1)
 
-WL_EXPORT void
+void
 wl_map_init(struct wl_map *map, uint32_t side)
 {
memset(map, 0, sizeof *map);
map->side = side;
 }
 
-WL_EXPORT void
+void
 wl_map_release(struct wl_map *map)
 {
wl_array_release(>client_entries);
wl_array_release(>server_entries);
 }
 
-WL_EXPORT uint32_t
+uint32_t
 wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data)
 {
union map_entry *start, *entry;
@@ -223,7 +223,7 @@ wl_map_insert_new(struct wl_map *map, uint32_t flags, void 
*data)
return (entry - start) + base;
 }
 
-WL_EXPORT int
+int
 wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data)
 {
union map_entry *start;
@@ -251,7 +251,7 @@ wl_map_insert_at(struct wl_map *map, uint32_t flags, 
uint32_t i, void *data)
return 0;
 }
 
-WL_EXPORT int
+int
 wl_map_reserve_new(struct wl_map *map, uint32_t i)
 {
union map_entry *start;
@@ -290,7 +290,7 @@ wl_map_reserve_new(struct wl_map *map, uint32_t i)
return 0;
 }
 
-WL_EXPORT void
+void
 wl_map_remove(struct wl_map *map, uint32_t i)
 {
union map_entry *start;
@@ -314,7 +314,7 @@ wl_map_remove(struct wl_map *map, uint32_t i)
map->free_list = (i << 1) | 1;
 }
 
-WL_EXPORT void *
+void *
 wl_map_lookup(struct wl_map *map, uint32_t i)
 {
union map_entry *start;
@@ -337,7 +337,7 @@ wl_map_lookup(struct wl_map *map, uint32_t i)
return NULL;
 }
 
-WL_EXPORT uint32_t
+uint32_t
 wl_map_lookup_flags(struct wl_map *map, uint32_t i)
 {
union map_entry *start;
@@ -379,7 +379,7 @@ for_each_helper(struct wl_array *entries, 
wl_iterator_func_t func, void *data)
return ret;
 }
 
-WL_EXPORT void
+void
 wl_map_for_each(struct wl_map *map, wl_iterator_func_t func, void *data)
 {
enum wl_iterator_result ret;
-- 
2.11.0

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

[PATCH wayland v2 3/4] wayland-util: build/ship as separate shared library

[Emil Velikov]

From: Emil Velikov 

Currently both of libwayland-{client,server} export the same util
(amongst other) symbols.

Although not crucial this is something which should be avoided where
possible.

As such let's move the library to a shared one and introduce a static
one for the purposes of wayland-scanner.

Any old (existing) users of the new libwayland-{client,server} will be
safe since the libwayland* libraries will pull the util one and the
symbols will be resolved at runtime.

Any programs building against the new libraries will have the dependency
(both compile and link-wise) resolved automatically by the Requires
field of the .pc file.

Note: it's not possible to 'hide' the wl_list/wl_array API since it's
been part for the ABI (implicitly pulled via the wayland headers) for a
while, plus doing so will break KF5 and mpv, at least.

v2: Rebase

Signed-off-by: Emil Velikov 
---
 Makefile.am  | 18 --
 configure.ac |  2 ++
 src/wayland-client-uninstalled.pc.in |  1 +
 src/wayland-client.pc.in |  1 +
 src/wayland-server-uninstalled.pc.in |  1 +
 src/wayland-server.pc.in |  1 +
 src/wayland-util-uninstalled.pc.in   |  8 
 src/wayland-util.pc.in   | 12 
 8 files changed, 38 insertions(+), 6 deletions(-)
 create mode 100644 src/wayland-util-uninstalled.pc.in
 create mode 100644 src/wayland-util.pc.in

diff --git a/Makefile.am b/Makefile.am
index 7e15465..ad3fdf0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -25,7 +25,7 @@ pkgconfig_DATA =
 bin_PROGRAMS = wayland-scanner
 wayland_scanner_SOURCES = src/scanner.c
 wayland_scanner_CFLAGS = $(EXPAT_CFLAGS) $(LIBXML_CFLAGS) $(AM_CFLAGS)
-wayland_scanner_LDADD = $(EXPAT_LIBS) $(LIBXML_LIBS) libwayland-util.la
+wayland_scanner_LDADD = $(EXPAT_LIBS) $(LIBXML_LIBS) libwayland-util-static.la
 pkgconfig_DATA += src/wayland-scanner.pc
 
 if DTD_VALIDATION
@@ -40,16 +40,17 @@ $(BUILT_SOURCES) : wayland-scanner
 wayland_scanner = $(top_builddir)/wayland-scanner
 endif
 
-libwayland_util_la_CFLAGS = $(AM_CFLAGS)
-libwayland_util_la_SOURCES =   \
+noinst_LTLIBRARIES = libwayland-util-static.la
+
+libwayland_util_static_la_CFLAGS = $(AM_CFLAGS)
+libwayland_util_static_la_SOURCES =\
src/wayland-util.c  \
src/wayland-util.h
 
-noinst_LTLIBRARIES = libwayland-util.la
 
 if ENABLE_LIBRARIES
 noinst_LTLIBRARIES += libwayland-private.la
-lib_LTLIBRARIES = libwayland-server.la libwayland-client.la
+lib_LTLIBRARIES = libwayland-util.la libwayland-server.la libwayland-client.la
 
 libwayland_private_la_CFLAGS = $(FFI_CFLAGS) $(AM_CFLAGS)
 libwayland_private_la_SOURCES =\
@@ -73,6 +74,11 @@ nodist_include_HEADERS = \
protocol/wayland-server-protocol.h  \
protocol/wayland-client-protocol.h
 
+libwayland_util_la_CFLAGS = $(AM_CFLAGS)
+libwayland_util_la_SOURCES =   \
+   src/wayland-util.c  \
+   src/wayland-util.h
+
 libwayland_server_la_CFLAGS = $(FFI_CFLAGS) $(AM_CFLAGS) -pthread
 libwayland_server_la_LIBADD = $(FFI_LIBS) libwayland-private.la 
libwayland-util.la -lrt -lm
 libwayland_server_la_LDFLAGS = -version-info 1:0:1
@@ -95,7 +101,7 @@ nodist_libwayland_client_la_SOURCES =\
protocol/wayland-client-protocol.h  \
protocol/wayland-protocol.c
 
-pkgconfig_DATA += src/wayland-client.pc src/wayland-server.pc
+pkgconfig_DATA += src/wayland-util.pc src/wayland-client.pc 
src/wayland-server.pc
 
 protocol/%-protocol.c : $(top_srcdir)/protocol/%.xml
$(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(wayland_scanner) code < $< > $@
diff --git a/configure.ac b/configure.ac
index c50027b..95ccea0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -196,8 +196,10 @@ AC_CONFIG_FILES([Makefile
 src/wayland-server-uninstalled.pc
 src/wayland-client-uninstalled.pc
 src/wayland-scanner-uninstalled.pc
+src/wayland-util-uninstalled.pc
 src/wayland-server.pc
 src/wayland-client.pc
 src/wayland-scanner.pc
+src/wayland-util.pc
 src/wayland-version.h])
 AC_OUTPUT
diff --git a/src/wayland-client-uninstalled.pc.in 
b/src/wayland-client-uninstalled.pc.in
index 732736e..c12a917 100644
--- a/src/wayland-client-uninstalled.pc.in
+++ b/src/wayland-client-uninstalled.pc.in
@@ -5,5 +5,6 @@ protocoldir=@abs_top_builddir@/protocol
 Name: Wayland Client
 Description: Wayland client side library (not installed)
 Version: @PACKAGE_VERSION@
+Requires.private: wayland-util
 Cflags: -I${includedir} -I${protocoldir}
 Libs: -L${libdir} -lwayland-client
diff --git a/src/wayland-client.pc.in b/src/wayland-client.pc.in
index eef61da..d76f83d 100644
--- a/src/wayland-client.pc.in
+++ b/src/wayland-client.pc.in
@@ -8,5 

[PATCH wayland v2 2/4] wayland-util: split out private functionality to separate file

[Emil Velikov]

From: Emil Velikov 

With next commit we'll make wayland-util a shared library (for reasons
mentioned in the commit). As such we need to make sure that the private
symbols are somewhere that they can be used internally. Otherwise we'll
end up with link errors.

v2: Rebase.

Signed-off-by: Emil Velikov 
---
 Makefile.am|   1 +
 src/wayland-util-private.c | 303 +
 src/wayland-util.c | 272 +---
 3 files changed, 305 insertions(+), 271 deletions(-)
 create mode 100644 src/wayland-util-private.c

diff --git a/Makefile.am b/Makefile.am
index d0c8bd3..7e15465 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -56,6 +56,7 @@ libwayland_private_la_SOURCES =   \
src/connection.c\
src/wayland-os.c\
src/wayland-os.h\
+   src/wayland-util-private.c  \
src/wayland-private.h
 
 include_HEADERS =  \
diff --git a/src/wayland-util-private.c b/src/wayland-util-private.c
new file mode 100644
index 000..9861170
--- /dev/null
+++ b/src/wayland-util-private.c
@@ -0,0 +1,303 @@
+/*
+ * Copyright © 2008-2011 Kristian Høgsberg
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "wayland-private.h"
+
+/** \cond */
+
+struct wl_object global_zombie_object;
+
+int
+wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b)
+{
+   /* In most cases the pointer equality test is sufficient.
+* However, in some cases, depending on how things are split
+* across shared objects, we can end up with multiple
+* instances of the interface metadata constants.  So if the
+* pointers match, the interfaces are equal, if they don't
+* match we have to compare the interface names.
+*/
+   return a == b || strcmp(a->name, b->name) == 0;
+}
+
+union map_entry {
+   uintptr_t next;
+   void *data;
+};
+
+#define map_entry_is_free(entry) ((entry).next & 0x1)
+#define map_entry_get_data(entry) ((void *)((entry).next & ~(uintptr_t)0x3))
+#define map_entry_get_flags(entry) (((entry).next >> 1) & 0x1)
+
+void
+wl_map_init(struct wl_map *map, uint32_t side)
+{
+   memset(map, 0, sizeof *map);
+   map->side = side;
+}
+
+void
+wl_map_release(struct wl_map *map)
+{
+   wl_array_release(>client_entries);
+   wl_array_release(>server_entries);
+}
+
+uint32_t
+wl_map_insert_new(struct wl_map *map, uint32_t flags, void *data)
+{
+   union map_entry *start, *entry;
+   struct wl_array *entries;
+   uint32_t base;
+
+   if (map->side == WL_MAP_CLIENT_SIDE) {
+   entries = >client_entries;
+   base = 0;
+   } else {
+   entries = >server_entries;
+   base = WL_SERVER_ID_START;
+   }
+
+   if (map->free_list) {
+   start = entries->data;
+   entry = [map->free_list >> 1];
+   map->free_list = entry->next;
+   } else {
+   entry = wl_array_add(entries, sizeof *entry);
+   if (!entry)
+   return 0;
+   start = entries->data;
+   }
+
+   entry->data = data;
+   entry->next |= (flags & 0x1) << 1;
+
+   return (entry - start) + base;
+}
+
+int
+wl_map_insert_at(struct wl_map *map, uint32_t flags, uint32_t i, void *data)
+{
+   union map_entry *start;
+   uint32_t count;
+   struct wl_array *entries;
+
+   if (i < WL_SERVER_ID_START) {
+   entries = >client_entries;
+   } else {
+   entries = >server_entries;
+   i -= WL_SERVER_ID_START;
+   

[PATCH wayland v2 0/4] wayland-util to all

[Emil Velikov]

Hi all,

As a continuation of an earlier thread:
While not crucial, it's recommended that different libraries do not 
provide the same [conflicting] global symbol(s).

As such I've fleshed out libwayland-util.so. It is considered internal 
one to wayland, since users implicitly use the API (as pulled by the 
generated wayland headers).

Linking in any direction and permutation is perfectly safe, with the 
only nuisance to package developers - add two/three lines in their build 
recipe.

A simple test project can be found at [1], with anyone welcome to test 
the series with their favourite project.

If that goes well, I'll jump on beating some sense into the scanner to 
produce proper static/shared symbols such that libraries export 
wl.*interface symbols only as needed [for example Mesa's libEGL.so and 
libgbm.so export wl_drm_interface... but only sometimes]

Thanks
Emil

[1] https://github.com/evelikov/wl_link_test

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

Re: [PATCH wayland 1/4] wayland-util: do not export the wl_map_* API

[Emil Velikov]

Hi Daniel, all,

On 21 November 2016 at 17:32, Daniel Stone  wrote:
> Hi Emil,
>
> On 30 August 2016 at 18:24, Emil Velikov  wrote:
>> From: Emil Velikov 
>>
>> Use only internally and explicitly marked as such with commit
>> cf04b0a18f2 ("Move private definitions and prototypes to new
>> zwayland-private.h")
>>
>> Signed-off-by: Emil Velikov 
>> ---
>> I could not find any users of the API and I doubt there was ever one. If
>> people feel nervous about this, we can keep it.
>
> For the actual series, detaching the discussion from the
> wayland-scanner bits in 0/4:
>
> I think this one is fine, but I'd prefer to merge it at the same time
> as the wayland-util split, if and when that happens.
>
> Patch 2/4 (to move to the -private file) no longer applies, because we
> let this series bitrot for so long. Sorry.
>
No worries, it's not something that crazy of an issue to begin with.

> Patch 4/4 removes whitespace from the other -uninstalled.pc.in files,
> but you add the same whitespace into the wayland-util file introduced
> in 3/4 and don't fix it up in 4/4.
I've addressed all the white space bits, plus fixed the -client one ;-)

> As for the actual libwayland-util split, I'm very much on the fence as
> to whether it's a good idea. Broadly speaking I do like the idea and
> sympathise with the aims, but am not sure how happy distro packagers
> would be with an extra binary package to track. What really worries me
> though, is transient symbol dependencies: at least with the pkg-config
> modifications as-is in 3/4, with wayland-util dropping back to
> Requires.private, I believe we'd see the compiler/linker complaining
> that a project directly using symbols from wayland-util does not
> directly link to it, only transiently via libwayland-{client,server}.
> They could fix that by requiring wayland-util, but then they'd need
> versioned fallbacks, and we've just made it a fair bit harder for
> people to properly link to it.
>
> Did you test with something that only has
> wayland-client/wayland-server (and/or -uninstalled variants) in the
> pkg-config file, directly using wl_list/wl_array/etc, and see if they
> generated any warnings?
>
I've tried to answer your questions with the 3/4 commit message,
although I might have failed.

Tl;Dr; everything is file, see the specifics below.

Since I was too lazy to pull/rebuild something crazy big as KF5/the
Gnome equiv./other, I've did a pretty trivial example
https://github.com/evelikov/wl_link_test

- Builds one executable and one shared (DSO) library.
- Both containing the same code - wl_{list,array}_init
- Explicitly link the DSO w/o undefined symbols
- link each binary against libwayland-{client,server}.so
- no warnings at build/link time
- libwayland-utils.so ends up with NEEDED

Tried the above with and w/o the following (the default to Arch) LDFLAGS
LDFLAGS="-Wl,-O1,--sort-common,--as-needed,-z,relro"

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

Re: [PATCH weston 19/68] compositor-drm: Refcount drm_fb

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:34 +
Daniel Stone  wrote:

> Sometimes we need to duplicate an existing drm_fb, e.g. when
> pageflipping to the same buffer to kickstart the repaint loop. To handle
> situations like these, and simplify resource management for dumb and
> cursor buffers, refcount drm_fb.
> 
> Differential Revision: https://phabricator.freedesktop.org/D1491
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor-drm.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index eb735b2..a684ac9 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -140,6 +140,8 @@ enum drm_fb_type {
>  struct drm_fb {
>   enum drm_fb_type type;
>  
> + int refcnt;
> +
>   uint32_t fb_id, stride, handle, size;
>   const struct pixel_format_info *format;
>   int width, height;
> @@ -302,6 +304,8 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int 
> height,
>   if (!fb)
>   return NULL;
>  
> + fb->refcnt = 1;
> +
>   fb->format = pixel_format_get_info(format);
>   if (!fb->format) {
>   weston_log("failed to look up format 0x%lx\n",
> @@ -312,6 +316,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int 
> height,
>   if (!fb->format->depth || !fb->format->bpp) {
>   weston_log("format 0x%lx is not compatible with dumb buffers\n",
>  (unsigned long) format);
> + goto err_fb;

Hi,

this hunk belongs in an earlier patch.

>   }
>  
>   memset(_arg, 0, sizeof create_arg);
> @@ -384,6 +389,13 @@ err_fb:
>  }
>  
>  static struct drm_fb *
> +drm_fb_ref(struct drm_fb *fb)
> +{
> + fb->refcnt++;
> + return fb;
> +}
> +
> +static struct drm_fb *
>  drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend,
>  uint32_t format, enum drm_fb_type type)
>  {
> @@ -392,13 +404,14 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct 
> drm_backend *backend,
>   int ret;
>  
>   if (fb)
> - return fb;
> + return drm_fb_ref(fb);

This path is now different from before, and requires adding an
equivalent drm_fb_unref() call somewhere, but I don't see it.
Previously unref would have released it immediately, now the first call
will be a no-op.

Or, if this is a bug fix, it requires an explanation in the commit
message how in some cases drm_fb_unref() got called twice.

Maybe this hunk belongs in a different patch instead?

>  
>   fb = zalloc(sizeof *fb);
>   if (fb == NULL)
>   return NULL;
>  
>   fb->type = type;
> + fb->refcnt = 1;
>   fb->bo = bo;
>  
>   fb->width = gbm_bo_get_width(bo);
> @@ -472,6 +485,10 @@ drm_fb_unref(struct drm_fb *fb)
>   if (!fb)
>   return;
>  
> + assert(fb->refcnt > 0);
> + if (--fb->refcnt > 0)
> + return;
> +
>   switch (fb->type) {
>   case BUFFER_PIXMAN_DUMB:
>   /* nothing: pixman buffers are destroyed manually */

It took a while to see the paths:

drm_fb_unref -> gbm_bo_destroy -> drm_fb_destroy_gbm

drm_fb_unref -> gbm_surface_release_buffer
gbm_surface_destroy -> drm_fb_destroy_gbm

drm_output_fini_pixman -> drm_fb_destroy_dumb

The goal is to solidify drm_fb_unref() as the main entry point for
destruction, but it's not quite there yet. Very good.

If you move both of the hunks I pointed out into other patches, then
this patch gets:
Reviewed-by: Pekka Paalanen 
If that's not the right thing to do, I'll review the next revision.


Thanks,
pq


pgpXCTX6RiQSF.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 18/68] compositor-drm: Drop output from release_fb

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:33 +
Daniel Stone  wrote:

> We only need it for the GBM surface the FB was originally created
> against; a mismatch here is very bad indeed, so no reason to pass it in
> explictly every time rather than store it.
> 
> Differential Revision: https://phabricator.freedesktop.org/D1490
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor-drm.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7dbfc6b..eb735b2 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -148,6 +148,7 @@ struct drm_fb {
>  
>   /* Used by gbm fbs */
>   struct gbm_bo *bo;
> + struct gbm_surface *gbm_surface;
>  
>   /* Used by dumb fbs */
>   void *map;
> @@ -466,7 +467,7 @@ drm_fb_set_buffer(struct drm_fb *fb, struct weston_buffer 
> *buffer)
>  }
>  
>  static void
> -drm_output_release_fb(struct drm_output *output, struct drm_fb *fb)
> +drm_fb_unref(struct drm_fb *fb)


Hi,

first I was slightly surprised by the renaming, but reading the patch
thoroughly and seeing the following patch explained why it's nice to do
here. It would have saved a bit of effort if it was mentioned in the
commit message.

Reviewed-by: Pekka Paalanen 


Thanks,
pq


pgp4sQpVTTgju.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 17/68] compositor-drm: Refactor destroy drm_fb function

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:32 +
Daniel Stone  wrote:

> From: Tomohito Esaki 
> 
> The drm_fb destroy callback to mostly the same thing regardless of
> whether the buffer is a dumb buffer or gbm buffer. This patch refactors
> the common parts into a new function that can be called for both cases.
> 
> [daniels: Rebased on top of fb->fd changes, cosmetic changes.]
> 
> Differential Revision: https://phabricator.freedesktop.org/D1489
> 
> Signed-off-by: Tomohito Esaki 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor-drm.c | 61 
> +++---
>  1 file changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index af43a15..7dbfc6b 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -251,16 +251,39 @@ drm_sprite_crtc_supported(struct drm_output *output, 
> struct drm_sprite *sprite)
>  }
>  
>  static void
> -drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
> +drm_fb_destroy(struct drm_fb *fb)
>  {
> - struct drm_fb *fb = data;
> + drmModeRmFB(fb->fd, fb->fb_id);

fb_id cannot be zero, even thought earlier there was a check. Ok.

> + weston_buffer_reference(>buffer_ref, NULL);
> + free(fb);
> +}
> +
> +static void
> +drm_fb_destroy_dumb(struct drm_fb *fb)
> +{
> + struct drm_mode_destroy_dumb destroy_arg;
>  
> - if (fb->fb_id)
> - drmModeRmFB(fb->fd, fb->fb_id);
> + if (!fb)
> + return;

Silent acceptance of NULL argument, really?

>  
> - weston_buffer_reference(>buffer_ref, NULL);
> + assert(fb->type == BUFFER_PIXMAN_DUMB);
> +

fb->map cannot be NULL, even thought earlier there was a check. Ok.

> + munmap(fb->map, fb->size);
> +
> + memset(_arg, 0, sizeof(destroy_arg));
> + destroy_arg.handle = fb->handle;
> + drmIoctl(fb->fd, DRM_IOCTL_MODE_DESTROY_DUMB, _arg);
> +
> + drm_fb_destroy(fb);
> +}

With that one nit of silent acceptance fixed:
Reviewed-by: Pekka Paalanen 


Thanks,
pq


pgpy_jQlduUy2.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 16/68] compositor-drm: Store format in drm_fb

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:31 +
Daniel Stone  wrote:

> This uses the new pixel-format helpers, so we can also replace depth/bpp
> with these.
> 
> Signed-off-by: Daniel Stone 
> 
> Differential Revision: https://phabricator.freedesktop.org/D1513
> ---
>  libweston/compositor-drm.c | 43 +++
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 217db32..af43a15 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -54,6 +54,7 @@
>  #include "gl-renderer.h"
>  #include "weston-egl-ext.h"
>  #include "pixman-renderer.h"
> +#include "pixel-formats.h"
>  #include "libbacklight.h"
>  #include "libinput-seat.h"
>  #include "launcher-util.h"
> @@ -140,6 +141,7 @@ struct drm_fb {
>   enum drm_fb_type type;
>  
>   uint32_t fb_id, stride, handle, size;
> + const struct pixel_format_info *format;
>   int width, height;
>   int fd;
>   struct weston_buffer_reference buffer_ref;
> @@ -267,7 +269,6 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int 
> height,
>  {
>   struct drm_fb *fb;
>   int ret;
> - uint32_t bpp, depth;
>  
>   struct drm_mode_create_dumb create_arg;
>   struct drm_mode_destroy_dumb destroy_arg;
> @@ -277,20 +278,20 @@ drm_fb_create_dumb(struct drm_backend *b, int width, 
> int height,
>   if (!fb)
>   return NULL;
>  
> - switch (format) {
> - case GBM_FORMAT_XRGB:
> - bpp = 32;
> - depth = 24;
> - break;
> - case GBM_FORMAT_RGB565:
> - bpp = depth = 16;
> - break;
> - default:
> - return NULL;
> + fb->format = pixel_format_get_info(format);
> + if (!fb->format) {
> + weston_log("failed to look up format 0x%lx\n",
> +(unsigned long) format);
> + goto err_fb;
> + }
> +
> + if (!fb->format->depth || !fb->format->bpp) {
> + weston_log("format 0x%lx is not compatible with dumb buffers\n",
> +(unsigned long) format);

goto err_fb?

Otherwise:
Reviewed-by: Pekka Paalanen 


Thanks,
pq

>   }
>  
>   memset(_arg, 0, sizeof create_arg);
> - create_arg.bpp = bpp;
> + create_arg.bpp = fb->format->bpp;
>   create_arg.width = width;
>   create_arg.height = height;
>  


pgpYyYzAl1R97.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 15/68] compositor-drm: Add explicit type member to drm_fb

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:30 +
Daniel Stone  wrote:

> Rather than magically trying to infer what the buffer is and what we
> should do with it when we go to destroy it, add an explicit type
> instead.
> 
> Differential Revision: https://phabricator.freedesktop.org/D1488
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor-drm.c | 50 
> +-
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 4ef7343..217db32 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -129,11 +129,19 @@ struct drm_mode {
>   drmModeModeInfo mode_info;
>  };
>  
> +enum drm_fb_type {
> + BUFFER_INVALID = 0, /**< never used */
> + BUFFER_CLIENT, /**< directly sourced from client */
> + BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */
> + BUFFER_GBM_SURFACE, /**< internal EGL rendering */
> +};

Hi,

cool.

> +
>  struct drm_fb {
> + enum drm_fb_type type;
> +
>   uint32_t fb_id, stride, handle, size;
>   int width, height;
>   int fd;
> - int is_client_buffer;
>   struct weston_buffer_reference buffer_ref;
>  
>   /* Used by gbm fbs */
> @@ -290,6 +298,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int 
> height,
>   if (ret)
>   goto err_fb;
>  
> + fb->type = BUFFER_PIXMAN_DUMB;
>   fb->handle = create_arg.handle;
>   fb->stride = create_arg.pitch;
>   fb->size = create_arg.size;
> @@ -352,6 +361,8 @@ drm_fb_destroy_dumb(struct drm_fb *fb)
>  {
>   struct drm_mode_destroy_dumb destroy_arg;
>  
> + assert(fb->type == BUFFER_PIXMAN_DUMB);
> +
>   if (!fb->map)
>   return;
>  
> @@ -370,8 +381,8 @@ drm_fb_destroy_dumb(struct drm_fb *fb)
>  }
>  
>  static struct drm_fb *
> -drm_fb_get_from_bo(struct gbm_bo *bo,
> -struct drm_backend *backend, uint32_t format)
> +drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend,
> +uint32_t format, enum drm_fb_type type)
>  {
>   struct drm_fb *fb = gbm_bo_get_user_data(bo);
>   uint32_t handles[4] = { 0 }, pitches[4] = { 0 }, offsets[4] = { 0 };

For the shortcut return:
assert(type == fb->type)?

> @@ -384,6 +395,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo,
>   if (fb == NULL)
>   return NULL;
>  
> + fb->type = type;
>   fb->bo = bo;
>  
>   fb->width = gbm_bo_get_width(bo);
> @@ -440,9 +452,6 @@ static void
>  drm_fb_set_buffer(struct drm_fb *fb, struct weston_buffer *buffer)
>  {
>   assert(fb->buffer_ref.buffer == NULL);
> -
> - fb->is_client_buffer = 1;
> -

assert(fb->type == BUFFER_CLIENT)?

>   weston_buffer_reference(>buffer_ref, buffer);
>  }
>  
> @@ -452,15 +461,19 @@ drm_output_release_fb(struct drm_output *output, struct 
> drm_fb *fb)
>   if (!fb)
>   return;
>  
> - if (fb->map &&
> -(fb != output->dumb[0] && fb != output->dumb[1])) {
> - drm_fb_destroy_dumb(fb);

This piece sent me into a recursive "well, actually..." loop.

It looked like dead code at first hand, as this gets hit when
output->dumb and fb don't match. When would that be? I would guess when
video mode changed.

I think changing resolutions would hit this path, when flipping to a
new dumb buffer of a different size than one coming out of scanout
which cannot be destroyed until pageflip completed.

Except I am wrong in a couple of ways: destroying the buffer is fine,
the kernel will keep it referenced as long as necessary anyway. And,
drm_output_switch_mode() does destroy everything immediately.

So this bit is ok. Unless there is a third well-actually.

> - } else if (fb->bo) {
> - if (fb->is_client_buffer)
> - gbm_bo_destroy(fb->bo);
> - else
> - gbm_surface_release_buffer(output->gbm_surface,
> -fb->bo);
> + switch (fb->type) {
> + case BUFFER_PIXMAN_DUMB:
> + /* nothing: pixman buffers are destroyed manually */
> + break;
> + case BUFFER_CLIENT:
> + gbm_bo_destroy(fb->bo);
> + break;
> + case BUFFER_GBM_SURFACE:
> + gbm_surface_release_buffer(output->gbm_surface, fb->bo);
> + break;
> + default:
> + assert(NULL);
> + break;
>   }
>  }
>  
> @@ -559,7 +572,7 @@ drm_output_prepare_scanout_view(struct drm_output *output,
>   return NULL;
>   }
>  
> - output->next = drm_fb_get_from_bo(bo, b, format);
> + output->next = drm_fb_get_from_bo(bo, b, format, BUFFER_CLIENT);
>   if (!output->next) {
>   gbm_bo_destroy(bo);
>   return NULL;
> @@ -585,7 +598,8 @@ drm_output_render_gl(struct drm_output *output, 
> pixman_region32_t *damage)
>   return;
>   }
>  

Re: [PATCH weston] config-parser: Export weston_config_next_section

[Pekka Paalanen]

On Tue, 21 Feb 2017 11:58:20 +
Emmanuel Gil Peyrot  wrote:

> This symbol wasn’t exported from the weston binary, most likely due to
> an oversight in 6e2c12496bbef3cc913cfe9d5f0aeb4d8b23b368, and because
> internal modules can link against libshared.la directly it hasn’t been
> found ever since.
> 
> This commit makes it possible for external modules to iterate over the
> configuration file.
> 
> Signed-off-by: Emmanuel Gil Peyrot 
> ---
>  shared/config-parser.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/shared/config-parser.c b/shared/config-parser.c
> index e2b5fa25..2a595b1f 100644
> --- a/shared/config-parser.c
> +++ b/shared/config-parser.c
> @@ -490,6 +490,7 @@ weston_config_get_full_path(struct weston_config *config)
>   return config == NULL ? NULL : config->path;
>  }
>  
> +WL_EXPORT
>  int
>  weston_config_next_section(struct weston_config *config,
>  struct weston_config_section **section,

Reviewed-by: Pekka Paalanen 

Bryce, this is a low-risk patch which we cannot land in a stable branch
after the release because it adds ABI to libweston and that AFAIU would
require a minor-version bump, but the stable branch only does
patch-version bumps. OTOH, it is not a release critical bug fix IMHO.
You should decide if this goes in for Weston 2.0 or waits for a whole
cycle before being released. I am fine either way, really, but I am
also in a slightly awkward position to make the call (conflict of
interests).

I know Emmanuel has a use case for this function in an out-of-tree
weston plugin.

The whole libweston thing is a little sad: libshared.la gets built into
libweston. config-parser.c exports functions, presumably because they
were necessary to export from weston the executable for plugins to use.
But because of that, libweston also now exports a bunch of functions it
shouldn't really. I filed:
https://phabricator.freedesktop.org/T7713


Thanks,
pq


pgpWNQwfbjeEg.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 3/3] compositor: don't map surfaces without a buffer

[Pekka Paalanen]

On Fri,  3 Feb 2017 16:10:39 +0100
Emilio Pozuelo Monfort  wrote:

> We were calling weston_surface::committed on surfaces with
> no buffer attached. Stop doing that, since surface::committed
> will map the surfaces and put them in a visible layer. That may
> not be a problem for a single surface as it wouldn't be visible
> anyway because it's got no contents, but it is a problem if the
> surface has subsurfaces.
> 
> This fixes the subsurface_mapped test, so mark it as expected
> to succeed.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=94735
> 
> Signed-off-by: Emilio Pozuelo Monfort 
> ---
>  libweston/compositor.c   | 10 +-
>  tests/subsurface-shot-test.c |  2 +-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 81392063..8a018897 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -1589,6 +1589,12 @@ weston_surface_is_mapped(struct weston_surface 
> *surface)
>   return surface->is_mapped;
>  }
>  
> +static bool
> +weston_surface_has_content(struct weston_surface *surface)
> +{
> + return surface->width > 0 && surface->height > 0;
> +}
> +
>  static void
>  surface_set_size(struct weston_surface *surface, int32_t width, int32_t 
> height)
>  {
> @@ -2928,7 +2934,9 @@ weston_surface_commit_state(struct weston_surface 
> *surface,
>  
>   if (state->newly_attached || state->buffer_viewport.changed) {
>   weston_surface_update_size(surface);
> - if (surface->committed)
> + if (surface->committed &&
> + (state->newly_attached &&
> +  weston_surface_has_content(surface)))
>   surface->committed(surface, state->sx, state->sy);

Hi,

I think this isn't right. The shells need to know when a surface
unmaps or changes dimensions, and they get that via the ->committed
hook (currently less well named, as ->configure was more descriptive,
but I think in the long term we'd like to eliminate the two hooks and
have just one called on every commit regardless of state - maybe have
the hook call weston_surface_apply_state so it can inspect
weston_surface state before and after).

If the viewport changed, it must lead to a call to ->committed, with or
without an attach.

Another thing is that weston_surface_update_size() updates the width,
height you use in weston_surface_has_content(). This means that a
surface first having content, receiving attach NULL + commit, will not
call ->committed.

When I was last thinking about the committed() hook, I came up with:
https://docs.google.com/spreadsheets/d/1b5azX4_gq9ZlYe4Y6UyHnHF8Wsiqnf1uHt9hR4whni4/edit#gid=0
but that just proves how complicated that design is, while not really
saving much in the committed() implementations.

Given my comments to the previous patch (the test), I think the fix
might be completely elsewhere.


Thanks,
pq

>   }
>  
> diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
> index e7da1e0e..275d4726 100644
> --- a/tests/subsurface-shot-test.c
> +++ b/tests/subsurface-shot-test.c
> @@ -261,7 +261,7 @@ TEST(subsurface_z_order)
>   buffer_destroy(bufs[i]);
>  }
>  
> -FAIL_TEST(subsurface_mapped)
> +TEST(subsurface_mapped)
>  {
>   const char *test_name = get_test_name();
>   struct client *client;



pgpvSML_E2MAm.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston] config-parser: Export weston_config_next_section

[Emmanuel Gil Peyrot]

This symbol wasn’t exported from the weston binary, most likely due to
an oversight in 6e2c12496bbef3cc913cfe9d5f0aeb4d8b23b368, and because
internal modules can link against libshared.la directly it hasn’t been
found ever since.

This commit makes it possible for external modules to iterate over the
configuration file.

Signed-off-by: Emmanuel Gil Peyrot 
---
 shared/config-parser.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/shared/config-parser.c b/shared/config-parser.c
index e2b5fa25..2a595b1f 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -490,6 +490,7 @@ weston_config_get_full_path(struct weston_config *config)
return config == NULL ? NULL : config->path;
 }
 
+WL_EXPORT
 int
 weston_config_next_section(struct weston_config *config,
   struct weston_config_section **section,
-- 
2.11.1

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

Re: [PATCH weston 3/3] compositor: don't map surfaces without a buffer

[Pekka Paalanen]

On Fri, 3 Feb 2017 09:27:18 -0600
Derek Foreman  wrote:

> On 03/02/17 09:10 AM, Emilio Pozuelo Monfort wrote:
> > We were calling weston_surface::committed on surfaces with
> > no buffer attached. Stop doing that, since surface::committed
> > will map the surfaces and put them in a visible layer. That may
> > not be a problem for a single surface as it wouldn't be visible
> > anyway because it's got no contents, but it is a problem if the
> > surface has subsurfaces.
> >
> > This fixes the subsurface_mapped test, so mark it as expected
> > to succeed.
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=94735
> >
> > Signed-off-by: Emilio Pozuelo Monfort 
> > ---
> >  libweston/compositor.c   | 10 +-
> >  tests/subsurface-shot-test.c |  2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/libweston/compositor.c b/libweston/compositor.c
> > index 81392063..8a018897 100644
> > --- a/libweston/compositor.c
> > +++ b/libweston/compositor.c
> > @@ -1589,6 +1589,12 @@ weston_surface_is_mapped(struct weston_surface 
> > *surface)
> > return surface->is_mapped;
> >  }
> >
> > +static bool
> > +weston_surface_has_content(struct weston_surface *surface)
> > +{
> > +   return surface->width > 0 && surface->height > 0;  
> 
> Are these going to be 0 if a NULL buffer is attached to an existing surface?
> 
> A quick read has me thinking width_from_buffer and height_from_buffer 
> are reset on a NULL attach, but width and height only get updated on 
> commit - so depending on when this function is called it might not 
> return the expected result.

Whatever happens, an attach without commit must not change those
variables. If it does, it's a bug.

Mind, that weston_surface_attach() is only called from
weston_surface_commit_state().


Thanks,
pq


pgpTCmojF9qNK.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 2/3] shot: add test for sub-surface with unmapped parent

[Pekka Paalanen]

On Fri,  3 Feb 2017 16:10:38 +0100
Emilio Pozuelo Monfort  wrote:

> This reproduces https://bugs.freedesktop.org/show_bug.cgi?id=94735.
> 
> The test currently fails, so mark it as expected to fail.
> 
> Signed-off-by: Emilio Pozuelo Monfort 

Hi,

there is a problem with this test that the conditions are not quite
right for the bug we want to reproduce.

Because of that, you also need to re-evaluate the fix (the following
patch). I suspect the fix needs to happen somewhere else, but we'll see
only after the test conditions are right.

> ---
>  tests/reference/subsurface_mapped-00.png | Bin 0 -> 799 bytes
>  tests/reference/subsurface_mapped-01.png | Bin 0 -> 841 bytes
>  tests/subsurface-shot-test.c |  79 
> +++
>  3 files changed, 79 insertions(+)
>  create mode 100644 tests/reference/subsurface_mapped-00.png
>  create mode 100644 tests/reference/subsurface_mapped-01.png
> 
> diff --git a/tests/reference/subsurface_mapped-00.png 
> b/tests/reference/subsurface_mapped-00.png
> new file mode 100644
> index 
> ..32cf32a0ed11d38b9028eda29109e06c99dce000
> GIT binary patch
> literal 799
> zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V0)  
> zPZ!6KiaBpDJMtb-U^sBV(DcSZzE>X)w43Y u!`PrjQsNK~Pa<2J!KmGIc&-r>L$&*16m=d#Wzp$P!YDfm(V
> 
> literal 0
> HcmV?d1
> 
> diff --git a/tests/reference/subsurface_mapped-01.png 
> b/tests/reference/subsurface_mapped-01.png
> new file mode 100644
> index 
> ..4e7196a2229a2e63adbf49d32a8c47db043f1c1c
> GIT binary patch
> literal 841
> zcmeAS@N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P@Hb9Ck$=lt9;Xep2*t>i(0|V1P  
> zPZ!6KiaBquZsa}Wz`){YUu2kf{Oyc6Wg6{8K0QxPoXkGz!#aIUUe52048Hw0nHzqy
> z@FX&|88AA}Xi)SyAfT4OA#BjXDRF32cth#8hDqmtn^qrc-8g5ndd!V)&);y?A7max
> zA$7y5+Tmoxxtqcd+SvzD9B$b7_V^p#1HN)}PDa0(vIRL#J}^{z1zopr
> E02fd6egFUf
> 
> literal 0
> HcmV?d1
> 
> diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
> index df72ec28..e7da1e0e 100644
> --- a/tests/subsurface-shot-test.c
> +++ b/tests/subsurface-shot-test.c
> @@ -260,3 +260,82 @@ TEST(subsurface_z_order)
>   if (bufs[i])
>   buffer_destroy(bufs[i]);
>  }
> +
> +FAIL_TEST(subsurface_mapped)
> +{
> + const char *test_name = get_test_name();
> + struct client *client;
> + struct wl_surface *surface;
> + struct rectangle clip;
> + int fail = 0;
> + struct wl_subcompositor *subco;
> + struct wl_surface *child;
> + struct wl_subsurface *sub;
> + struct buffer *buf, *child_buf;
> + pixman_color_t red, blue;
> +
> + color(, 255, 0, 0);
> + color(, 0, 0, 255);
> +
> + /* Create the client */
> + client = create_client();
> + assert(client);
> + client->surface = create_test_surface(client);
> + surface = client->surface->wl_surface;
> +
> + /* move the pointer clearly away from our screenshooting area */
> + weston_test_move_pointer(client->test->weston_test, 2, 30);
> +
> + client->surface->x = 100;
> + client->surface->y = 100;
> + weston_test_move_surface(client->test->weston_test, 
> client->surface->wl_surface,
> +  client->surface->x, client->surface->y);

Quite subtle behaviour here, because of of the test plugin works.
weston-test.c:test_surface_committed() marks both the view and the
surface mapped regardless whether there is content.

A contentless surface can become mapped if the
weston_surface::committed hook is triggered by viewport changes or an
attach with a NULL buffer. Whether that is a bug or a feature of the
test plugin, I'm not sure. It certainly allows a surface to become
mapped without content which is kind of not expected.(*)

> +
> + /* we need one of these so that buffer_viewport.changed gets set and 
> the commit
> +  * has an effect */
> + wl_surface_set_buffer_scale(surface, 1);
> + wl_surface_set_buffer_transform(surface, WL_OUTPUT_TRANSFORM_NORMAL);
> +
> + wl_surface_commit(surface);

Here we deliberately trigger that effect, so now we have a mapped
surface without content.

> +
> + clip.x = 100;
> + clip.y = 100;
> + clip.width = 100;
> + clip.height = 100;
> + printf("Clip: %d,%d %d x %d\n", clip.x, clip.y, clip.width, 
> clip.height);
> +
> + /* create a wl_surface */
> + subco = get_subcompositor(client);
> + assert(subco);
> +
> + child = wl_compositor_create_surface(client->wl_compositor);
> + assert(child);
> +
> + /* make it a sub-surface */
> + sub = wl_subcompositor_get_subsurface(subco, child, surface);
> + assert(sub);
> +
> + /* let the subsurface be drawn async from its parent */
> + wl_subsurface_set_desync(sub);
> +
> + /* give it content */
> + child_buf = surface_commit_color(client, child, , 50, 50);
> 

Re: [PATCH weston] clients/simple-egl: add -d option

[Eric Engestrom]

> Subject: [PATCH weston] clients/simple-egl: add -d option

s/-d/delay/ in the commit title?

On Monday, 2017-02-20 16:13:48 +0200, Pekka Paalanen wrote:
> From: Eero Tamminen 
> 
> This emulates extra drawing work by usleep().
> 
> This is an enhancement to reproduce the problem in the bug report.
> 
> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98833
> Signed-off-by: Pekka Paalanen 
> ---
>  clients/simple-egl.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/clients/simple-egl.c b/clients/simple-egl.c
> index 9b6fa1f..c5ee05d 100644
> --- a/clients/simple-egl.c
> +++ b/clients/simple-egl.c
> @@ -100,7 +100,7 @@ struct window {
>   struct ivi_surface *ivi_surface;
>   EGLSurface egl_surface;
>   struct wl_callback *callback;
> - int fullscreen, opaque, buffer_size, frame_sync;
> + int fullscreen, opaque, buffer_size, frame_sync, delay;
>   bool wait_for_configure;
>  };
>  
> @@ -548,6 +548,8 @@ redraw(void *data, struct wl_callback *callback, uint32_t 
> time)
>   glDisableVertexAttribArray(window->gl.pos);
>   glDisableVertexAttribArray(window->gl.col);
>  
> + usleep(window->delay);
> +
>   if (window->opaque || window->fullscreen) {
>   region = 
> wl_compositor_create_region(window->display->compositor);
>   wl_region_add(region, 0, 0,
> @@ -850,6 +852,7 @@ usage(int error_code)
>   "  -o\tCreate an opaque surface\n"
>   "  -s\tUse a 16 bpp EGL config\n"
>   "  -b\tDon't sync to compositor redraw (eglSwapInterval 0)\n"
> + "  -d \tBuffer swap delay in microseconds\n"
>   "  -h\tThis help text\n\n");
>  
>   exit(error_code);
> @@ -870,9 +873,12 @@ main(int argc, char **argv)
>   window.window_size = window.geometry;
>   window.buffer_size = 32;
>   window.frame_sync = 1;
> + window.delay = 0;
>  
>   for (i = 1; i < argc; i++) {
> - if (strcmp("-f", argv[i]) == 0)
> + if (strcmp("-d", argv[i]) == 0 && i+1 < argc)
> + window.delay = atoi(argv[++i]);

Options are displayed (help) in one order and parsed in another...
Kind of a nit-pick, but it looks weird to me.

With the more explicit commit title, patch is:
Reviewed-by: Eric Engestrom 

> + else if (strcmp("-f", argv[i]) == 0)
>   window.fullscreen = 1;
>   else if (strcmp("-o", argv[i]) == 0)
>   window.opaque = 1;
> -- 
> 2.10.2
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston] clients/weston-info: print unknown formats better

[Eric Engestrom]

On Monday, 2017-02-20 15:47:57 +0200, Pekka Paalanen wrote:
> From: Pekka Paalanen 
> 
> Don't just dump the raw 32-bit values, try to interpret it as a DRM
> fourcc too.
> 
> This prints properly the formats YUYV, NV12 and YU12 supported by
> Weston.
> 
> Signed-off-by: Pekka Paalanen 
> ---
>  clients/weston-info.c | 29 -
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/clients/weston-info.c b/clients/weston-info.c
> index 712346a..ab12947 100644
> --- a/clients/weston-info.c
> +++ b/clients/weston-info.c
> @@ -30,6 +30,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -240,9 +242,33 @@ print_output_info(void *data)
>   }
>  }
>  
> +static char
> +bits2graph(uint32_t value, unsigned bitoffset)
> +{
> + int c = (value >> bitoffset) & 0xff;
> +
> + if (isgraph(c) || isspace(c))
> + return c;
> +
> + return '?';
> +}
> +
> +static void
> +fmt2str(uint32_t format, char *str, int len)

`fourcc2str()` ?
This function is specific to 4 char codes, I think its name should
reflect this.

> +{
> + int i;
> +
> + assert(len >= 5);
> +
> + for (i = 0; i < 4; i++)
> + str[i] = bits2graph(format, i * 8);
> + str[i] = '\0';
> +}
> +
>  static void
>  print_shm_info(void *data)
>  {
> + char str[6];

6? not 5?

These are nit-picks, this patch already does what it says on the tin:
Reviewed-by: Eric Engestrom 

>   struct shm_info *shm = data;
>   struct shm_format *format;
>  
> @@ -262,7 +288,8 @@ print_shm_info(void *data)
>   printf(" RGB565");
>   break;
>   default:
> - printf(" unknown(%08x)", format->format);
> + fmt2str(format->format, str, sizeof(str));
> + printf(" '%s'(0x%08x)", str, format->format);
>   break;
>   }
>  
> -- 
> 2.10.2
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel