Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

[Nils Chr. Brause]

Hi!

On Fri, Jul 29, 2016 at 10:52 AM, Pekka Paalanen  wrote:
> On Fri, 22 Jul 2016 21:39:20 +0200
> "Nils Chr. Brause"  wrote:
>
>> HI,
>>
>> On Thu, Jun 2, 2016 at 10:30 AM, Pekka Paalanen  wrote:
>> > On Thu, 2 Jun 2016 15:39:47 +0800
>> > Jonas Ådahl  wrote:
>> >
>> >> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
>> >> > On Wed, 1 Jun 2016 13:16:31 -0500
>> >> > Yong Bakos  wrote:
>> >> >
>> >> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  
>> >> > > wrote:
>> >> > > >
>> >> > > > On Sat, 28 May 2016 08:39:59 -0500
>> >> > > > Yong Bakos  wrote:
>> >> > > >
>> >> > > >> Hi Mike,
>> >> > > >> Regarding the combination of type="array" enum="foo"...
>> >> > > >>
>> >> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
>> >> > > >>>  wrote:
>> >> > > >>>
>> >> > > >>> I've inlined some replies below.
>> >> > > >>>
>> >> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos 
>> >> > > >>> > wrote:
>> >> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
>> >> > > >>> > wrote:
>> >> > > 
>> >> > >  this adds a method for compositors to change various draw 
>> >> > >  attributes
>> >> > >  for a surface
>> >> > > 
>> >> > >  Signed-off-by: Mike Blumenkrantz > >> > >  >
>> >> > >  Signed-off-by: Jonas Ådahl > >> > >  >
>> >> > > >>>
>> >> > > >>> Hi Mike & Jonas,
>> >> > > >>> A question about communicating default state, and some
>> >> > > >>> minor nits you can certainly ignore, inline below.
>> >> > > >>>
>> >> > > >>>
>> >> > >  ---
>> >> > >  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 69 
>> >> > >  
>> >> > >  1 file changed, 69 insertions(+)
>> >> > > 
>> >> > >  diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
>> >> > >  b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >> > >  index dfd7e84..0fa76d4 100644
>> >> > >  --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >> > >  +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
>> >> > > >
>> >> > >  +
>> >> > >  +Calling this after an xdg_toplevel's first commit will 
>> >> > >  raise a client error.
>> >> > >  +  
>> >> > >  +  
>> >> > > >>>
>> >> > > >>> Just a sanity check, since I haven't seen it in a protocol spec 
>> >> > > >>> yet. Does scanner handle
>> >> > > >>> this combination of array and enum correctly?
>> >> > > >>>
>> >> > > >>> Good catch. This also affects the event above it.
>> >> > > >>
>> >> > > >> As we discussed via IRC (27 May), the scanner will choke on this. 
>> >> > > >> While we talked about
>> >> > > >> making a change to the scanner to allow this, perhaps such a 
>> >> > > >> change doesn't make sense.
>> >> > > >>
>> >> > > >> Given a type="array", scanner will generate a parameter of type 
>> >> > > >> wl_array.
>> >> > > >>
>> >> > > >> Perhaps the short story here is to just remove the enum from this 
>> >> > > >> arg, and the similar
>> >> > > >> arg in the configure_draw_states event above. What do you think?
>> >> > > >>
>> >> > > >> (I wonder if it's the DTD that can change, so the scanner's 
>> >> > > >> validation step
>> >> > > >> will catch the unsupported combination of type="array" enum="foo". 
>> >> > > >> My gut tells me that
>> >> > > >> DTDs don't support this logic, but I'll dig into this.)
>> >> > > >
>> >> > > > Hi,
>> >> > > >
>> >> > > > here is some background.
>> >> > > >
>> >> > > > A type="array" argument is really just a binary blob of data. The 
>> >> > > > XML
>> >> > > > description, human documentation aside, does not specify anything 
>> >> > > > about
>> >> > > > the blob contents. Therefore adding an XML attribute pointing to an
>> >> > > > enum definition is half-useless. Generators could use it for 
>> >> > > > creating
>> >> > > > automatic links in documentation, but it cannot be used for code
>> >> > > > generation, because you don't know the types contained in the blob.
>> >> > > >
>> >> > > > We also do not want to add blob content type definitions to the XML
>> >> > > > language, because you might want to have everything C is able to
>> >> > > > express, including nested structs. There is also no requirement that
>> >> > > > the "array" is really an array - every "element" could be a 
>> >> > > > different
>> >> > > > thing. It could be bitstream and whatnot. Only the use of
>> >> > > > wl_array_for_each() implies it is an array of similar elements,
>> >> > > > wl_array_add() does not.
>>
>> Then that's not an 'array' than but more like a struct.
>> Therefore the structure definition should be in the XML.
>> 

[PATCH weston v3 5/5] shared: Test cases for safe_strtoint()

[Bryce Harrington]

Loosely derived from an earlier patch by Imran Zaman.

Signed-off-by: Bryce Harrington 
---
 Makefile.am |  7 
 compositor/main.c   |  7 ++--
 compositor/systemd-notify.c |  9 ++---
 libweston/compositor.c  |  9 ++---
 libweston/libbacklight.c| 11 +++---
 shared/config-parser.c  |  7 ++--
 shared/option-parser.c  |  5 ++-
 tests/string-test.c | 87 +
 xwayland/launcher.c |  7 ++--
 9 files changed, 115 insertions(+), 34 deletions(-)
 create mode 100644 tests/string-test.c

diff --git a/Makefile.am b/Makefile.am
index cdf7bdb..2114b5c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1110,6 +1110,7 @@ internal_tests =  \
 
 shared_tests = \
config-parser.test  \
+   string.test \
vertex-clip.test\
zuctest
 
@@ -1208,6 +1209,12 @@ config_parser_test_CFLAGS =  \
$(AM_CFLAGS)\
-I$(top_srcdir)/tools/zunitc/inc
 
+string_test_SOURCES = \
+   tests/string-test.c \
+   shared/string-helpers.h
+string_test_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
+string_test_LDADD =libtest-client.la
+
 vertex_clip_test_SOURCES = \
tests/vertex-clip-test.c\
shared/helpers.h\
diff --git a/compositor/main.c b/compositor/main.c
index 0f7a0c0..4cf8468 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -52,6 +52,7 @@
 #include "compositor.h"
 #include "../shared/os-compatibility.h"
 #include "../shared/helpers.h"
+#include "../shared/string-helpers.h"
 #include "git-version.h"
 #include "version.h"
 #include "weston.h"
@@ -1568,7 +1569,7 @@ int main(int argc, char *argv[])
char *modules = NULL;
char *option_modules = NULL;
char *log = NULL;
-   char *server_socket = NULL, *end;
+   char *server_socket = NULL;
int32_t idle_time = -1;
int32_t help = 0;
char *socket_name = NULL;
@@ -1685,9 +1686,7 @@ int main(int argc, char *argv[])
server_socket = getenv("WAYLAND_SERVER_SOCKET");
if (server_socket) {
weston_log("Running with single client\n");
-   errno = 0;
-   fd = strtol(server_socket, , 10);
-   if (errno != 0 || end == server_socket || *end != '\0')
+   if (!safe_strtoint(server_socket, ))
fd = -1;
} else {
fd = -1;
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index 6104124..49e51f4 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -25,12 +25,13 @@
 
 #include "config.h"
 
-#include 
 #include 
 #include 
 #include 
 #include 
+
 #include "shared/helpers.h"
+#include "shared/string-helpers.h"
 #include "shared/zalloc.h"
 #include "compositor.h"
 
@@ -116,7 +117,6 @@ WL_EXPORT int
 module_init(struct weston_compositor *compositor,
int *argc, char *argv[])
 {
-   char *tail;
char *watchdog_time_env;
struct wl_event_loop *loop;
long watchdog_time_conv;
@@ -140,13 +140,10 @@ module_init(struct weston_compositor *compositor,
 * by systemd to transfer 'WatchdogSec' watchdog timeout
 * setting from service file.*/
watchdog_time_env = getenv("WATCHDOG_USEC");
-
if (!watchdog_time_env)
return 0;
 
-   errno = 0;
-   watchdog_time_conv = strtol(watchdog_time_env, , 10);
-   if (errno != 0 || tail == watchdog_time_env || *tail != '\0')
+   if (!safe_strtoint(watchdog_time_env, _time_conv))
return 0;
 
/* Convert 'WATCHDOG_USEC' to milliseconds and notify
diff --git a/libweston/compositor.c b/libweston/compositor.c
index e9c2a83..b17c76d 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -58,6 +58,7 @@
 #include "presentation-time-server-protocol.h"
 #include "shared/helpers.h"
 #include "shared/os-compatibility.h"
+#include "shared/string-helpers.h"
 #include "shared/timespec-util.h"
 #include "git-version.h"
 #include "version.h"
@@ -4622,15 +4623,11 @@ compositor_bind(struct wl_client *client,
 WL_EXPORT int
 weston_environment_get_fd(const char *env)
 {
-   char *e, *end;
+   char *e;
int fd, flags;
 
e = getenv(env);
-   if (!e)
-   return -1;
-   errno = 0;
-   fd = strtol(e, , 10);
-   if (errno != 0 || end == e || *end != '\0')
+   if (!e || !safe_strtoint(e, ))
return -1;
 
flags = fcntl(fd, F_GETFD);
diff --git a/libweston/libbacklight.c b/libweston/libbacklight.c
index 59f4e44..4bbc6db 100644
--- a/libweston/libbacklight.c
+++ b/libweston/libbacklight.c
@@ -44,13 +44,14 @@
 #include 
 #include 
 
+#include "shared/string-helpers.h"
+
 

[PATCH weston v3 1/5] Standardize error checking for strtol calls

[Bryce Harrington]

This tightens up the strtol() error checking in several places where it
is used for parsing environment variables, and in the backlight
interface that is reading numbers from files under /sys/class/backlight.
All of these uses are expecting strings containing decimal numbers and
nothing else, so the error checking can all be tightened up and made
consistent with other strtol() calls.

This follows the error checking style used in Wayland
(c.f. wayland-client.c and scanner.c) and c.f. commit cbc05378.

Signed-off-by: Bryce Harrington 
---
 compositor/main.c   |  3 ++-
 compositor/systemd-notify.c |  2 +-
 libweston/compositor.c  |  3 ++-
 libweston/libbacklight.c| 10 +-
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index 99cb868..0f7a0c0 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -1685,8 +1685,9 @@ int main(int argc, char *argv[])
server_socket = getenv("WAYLAND_SERVER_SOCKET");
if (server_socket) {
weston_log("Running with single client\n");
+   errno = 0;
fd = strtol(server_socket, , 10);
-   if (*end != '\0')
+   if (errno != 0 || end == server_socket || *end != '\0')
fd = -1;
} else {
fd = -1;
diff --git a/compositor/systemd-notify.c b/compositor/systemd-notify.c
index 9fbd5ee..6104124 100644
--- a/compositor/systemd-notify.c
+++ b/compositor/systemd-notify.c
@@ -146,7 +146,7 @@ module_init(struct weston_compositor *compositor,
 
errno = 0;
watchdog_time_conv = strtol(watchdog_time_env, , 10);
-   if ((errno != 0) || (*tail != '\0'))
+   if (errno != 0 || tail == watchdog_time_env || *tail != '\0')
return 0;
 
/* Convert 'WATCHDOG_USEC' to milliseconds and notify
diff --git a/libweston/compositor.c b/libweston/compositor.c
index b045381..e9c2a83 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -4628,8 +4628,9 @@ weston_environment_get_fd(const char *env)
e = getenv(env);
if (!e)
return -1;
+   errno = 0;
fd = strtol(e, , 10);
-   if (*end != '\0')
+   if (errno != 0 || end == e || *end != '\0')
return -1;
 
flags = fcntl(fd, F_GETFD);
diff --git a/libweston/libbacklight.c b/libweston/libbacklight.c
index 4039575..59f4e44 100644
--- a/libweston/libbacklight.c
+++ b/libweston/libbacklight.c
@@ -48,6 +48,7 @@ static long backlight_get(struct backlight *backlight, char 
*node)
 {
char buffer[100];
char *path;
+   char *end;
int fd;
long value, ret;
 
@@ -65,8 +66,15 @@ static long backlight_get(struct backlight *backlight, char 
*node)
goto out;
}
 
-   value = strtol(buffer, NULL, 10);
+   errno = 0;
+   value = strtol(buffer, , 10);
+   if (errno != 0 || end == buffer || *end != '\0') {
+   ret = -1;
+   goto out;
+   }
+
ret = value;
+
 out:
if (fd >= 0)
close(fd);
-- 
1.9.1

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

[PATCH weston v3 0/5] Add safe_strtoint() helper

[Bryce Harrington]

This rolls together two earlier patchsets.  First, the 3 patches to the
remaining strtol calls look more consistent.  Second, the implementation
for the safe_strtoint() call.  Tacked on are some test cases inspired by
work by Imran Zaman a couple years ago.

These patches could be squashed together for landing, but are left split
out for ease of review.

Bryce Harrington (5):
  Standardize error checking for strtol calls
  xwayland: Improve error checking for strtol call
  option-parser: Improve error checking for strtol call
  Add safe_strtoint() helper
  shared: Test cases for safe_strtoint()

 Makefile.am |  7 
 compositor/main.c   |  6 ++--
 compositor/systemd-notify.c |  9 ++---
 libweston/compositor.c  |  8 ++---
 libweston/libbacklight.c| 13 +--
 shared/config-parser.c  |  7 ++--
 shared/option-parser.c  | 12 +--
 shared/string-helpers.h | 78 
 tests/string-test.c | 87 +
 xwayland/launcher.c |  6 ++--
 10 files changed, 205 insertions(+), 28 deletions(-)
 create mode 100644 shared/string-helpers.h
 create mode 100644 tests/string-test.c

-- 
1.9.1

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

[PATCH weston v3 3/5] option-parser: Improve error checking for strtol call

[Bryce Harrington]

Make the error checking consistent with other strtol() calls.

Note that since strtol(nptr, ) sets endptr == nptr if there were
no digits, this catches the case where the string was blank, so there's
no need to test *value != '\0'.

Signed-off-by: Bryce Harrington 
---
 shared/option-parser.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/shared/option-parser.c b/shared/option-parser.c
index 33355b8..fb4a342 100644
--- a/shared/option-parser.c
+++ b/shared/option-parser.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "config-parser.h"
 
@@ -40,11 +41,17 @@ handle_option(const struct weston_option *option, char 
*value)
 
switch (option->type) {
case WESTON_OPTION_INTEGER:
+   errno = 0;
* (int32_t *) option->data = strtol(value, , 10);
-   return *value && !*p;
+   if (errno != 0 || p == value || *p != '\0')
+   return 0;
+   return 1;
case WESTON_OPTION_UNSIGNED_INTEGER:
+   errno = 0;
* (uint32_t *) option->data = strtoul(value, , 10);
-   return *value && !*p;
+   if (errno != 0 || p == value || *p != '\0')
+   return 0;
+   return 1;
case WESTON_OPTION_STRING:
* (char **) option->data = strdup(value);
return 1;
-- 
1.9.1

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

[PATCH weston v3 2/5] xwayland: Improve error checking for strtol call

[Bryce Harrington]

This updates the error checking for the strtol() call in xwayland's
create_lockfile to match other cases.  C.f. cbc05378 and other recent
patches.

A notable difference here is that the existing error checking was
verifying that exactly 10 digits were being read from the lock file,
but the fact that it's 10 digits is just an implementation detail for
how we're writing it.  The pid could be a shorter number of digits, and
would just be space-padded on the left.

This change allows the file to contain any number of digits, but it
can't be blank, all of the digits must be numeric, and the resulting
number must be within the accepted range.

Signed-off-by: Bryce Harrington 
---
 xwayland/launcher.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index 15443cd..a83784c 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -165,8 +165,9 @@ create_lockfile(int display, char *lockfile, size_t lsize)
return -1;
}
 
+   errno = 0;
other = strtol(pid, , 10);
-   if (end != pid + 10) {
+   if (errno != 0 || end == pid || *end != '\0') {
weston_log("can't parse lock file %s\n",
lockfile);
close(fd);
-- 
1.9.1

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

[PATCH weston v3 4/5] Add safe_strtoint() helper

[Bryce Harrington]

Adds a safe strtol helper function, modeled loosely after Wayland
scanner's strtouint.  This encapsulates the various quirks of strtol
behavior, and streamlines the interface to just handling base-10 numbers
with a simple true/false error indicator and a uint32_t return by
reference.

Signed-off-by: Bryce Harrington 
Reviewed-by: Thiago Macieira 
---
 shared/string-helpers.h | 78 +
 1 file changed, 78 insertions(+)
 create mode 100644 shared/string-helpers.h

diff --git a/shared/string-helpers.h b/shared/string-helpers.h
new file mode 100644
index 000..0617229
--- /dev/null
+++ b/shared/string-helpers.h
@@ -0,0 +1,78 @@
+/*
+ * Copyright © 2016 Samsung Electronics Co., Ltd
+ *
+ * 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.
+ */
+
+#ifndef WESTON_STRING_HELPERS_H
+#define WESTON_STRING_HELPERS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+/* Convert string to integer
+ *
+ * Parses a base-10 number from the given string.  Checks that the
+ * string is not blank, contains only numerical characters, and is
+ * within the range of -INT_MAX to INT_MAX.  If the validation is
+ * successful the result is stored in *value; otherwise *value is
+ * unchanged and errno is set appropriately.
+ *
+ * \return true if number parsed successfully, false on error
+ */
+static inline bool
+safe_strtoint(const char *str, int32_t *value)
+{
+   long ret;
+   char *end;
+
+   assert(str != NULL);
+
+   errno = 0;
+   ret = strtol(str, , 10);
+   if (errno != 0) {
+   return false;
+   } else if (end == str || *end != '\0') {
+   errno = EINVAL;
+   return false;
+   }
+
+   *value = (int32_t)ret;
+   if ((long)*value != ret) {
+   errno = ERANGE;
+   return false;
+   }
+
+   return true;
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* WESTON_STRING_HELPERS_H */
-- 
1.9.1

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

Re: Two wayland connection in single process context

[Pekka Paalanen]

On Fri, 29 Jul 2016 17:53:38 +0530
Vikas Patil  wrote:

> On Fri, Jul 29, 2016 at 4:53 PM, Pekka Paalanen  wrote:
> > On Fri, 29 Jul 2016 15:03:39 +0530
> > Vikas Patil  wrote:
> >  
> >> On Fri, Jul 29, 2016 at 1:17 PM, Pekka Paalanen  
> >> wrote:
> >>  
> >> > On Wed, 27 Jul 2016 17:06:04 +0530
> >> > Vikas Patil  wrote:
> >> >  
> >> >> Dear All,
> >> >>
> >> >> Could you comment/suggest on following approach of using two different
> >> >> wayland connection in single process?
> >> >>
> >> >> First wayland connection for creating wl_shm backed wayland surface
> >> >> for receiving touch inputs in main thread or as separate thread. Like
> >> >> simple-touch.c example but without painting and transparent.
> >> >>
> >> >> Second wayland connection for another egl wayland surface for
> >> >> rendering in shared library which will be loaded by above application.
> >> >> Like simple-egl.c example.  
> >> >
> >> > Hi,
> >> >
> >> > if you never want your first connection to handle input events on the
> >> > surface created on the second connection, I suppose it would work. You
> >> > also will not be able position your surfaces from the different
> >> > connections relative to each other at all.
> >> >
> >> > The main thing to remember is that *everything* is private to a
> >> > connection. If you create a wl_surface on one connection, it is as if
> >> > it does not exist at all for any other connection. It does not matter
> >> > if the connections come from the same thread, process, or not.
> >> >
> >> > If you expect any sort of interoperability, you *must* use the same
> >> > connection for everything. Opening a second connection from the same
> >> > program to the same server is practically always a design mistake.
> >> >
> >> > Of course, one could make Wayland extensions to work around this, but
> >> > don't go there.
> >> >
> >> > So, in short: don't do it.
> >> >  
> >> >> Will this work? Is it this the right way to do it or is there any
> >> >> other mechanism available for such requirement?  
> >> >
> >> > What do you want to achieve?
> >> > You didn't explain what you really want to do.
> >> >  
> >>
> >> I have a video rendering library (which handles all media related
> >> functions too) which is used by application, and that library make use
> >> of first wayland connection and don't handle the touch events as there
> >> is no way to pass that information to application so thinking to make
> >> transparent wayland surface on top of wayland surface to which the
> >> video is rendered by library to handle the touch events by creating
> >> second wayland connection in application.  
> >
> > There is no quick solution AFAIK.
> >
> > You have to fix both the application and the media library to become
> > Wayland-friendly. The minimal starting point is:
> >
> > - the application (or its toolkit) opens the one Wayland connection
> > - the application passes the open Wayland connection to the library
> > - the library does not open new Wayland connections
> >  
> 
> Thanks a lot for putting your detail thought here.
> 
> Do you know if clients/nested.c, clients/nested-client.c mentioned by
> "Armin Krezović " is helpful here in anyways? When I tried to run it
> it just displayed black window? What functionality it demos?

Those two demo the sub-compositor approach, where you have another
process providing part of the content. It is about embedding where you
really want a separate process for whatever reason, e.g. security.

nested.c is a normal Wayland client, but it is also a Wayland
mini-server. nested-client.c is a Wayland mini-client. The mini-client
draws with GL and commits its buffer to the mini-server, which then
composes its own window and commits that to the proper Wayland server.

If it displays just black, something is broken. Unfortunately the demo
depends on cairo-egl, so I believe it receives very little testing. It
is also possible that proprietary graphics stacks just don't implement
enough.

> What do you mean by passing "wayland connection" or passing
> wl_surface? Does it mean passing the object handle with some kind of
> IPC mechanism (possibly UNIX domain socket)?

No. I just mean 'struct wl_display *' as an argument or return value
to/from a function. Or a 'struct wl_surface *'.

E.g. display_get_display():
https://cgit.freedesktop.org/wayland/weston/tree/clients/window.c?id=1.11.0#n5822

> Do you know any such implementation available for reference? I don't
> have the code for our media library so I could try to create simple
> POC for this first?

Uhh, I'm not sure what to give you. It's really just about designing a
library API so that instead of the library opening its own connection,
it exports a function that takes 'struct wl_display *' as an argument
and uses that.

If you cannot modify the media library, the only remaining option is to
follow the nested.c 

Re: Two wayland connection in single process context

[Vikas Patil]

On Fri, Jul 29, 2016 at 4:53 PM, Pekka Paalanen  wrote:
> On Fri, 29 Jul 2016 15:03:39 +0530
> Vikas Patil  wrote:
>
>> On Fri, Jul 29, 2016 at 1:17 PM, Pekka Paalanen  wrote:
>>
>> > On Wed, 27 Jul 2016 17:06:04 +0530
>> > Vikas Patil  wrote:
>> >
>> >> Dear All,
>> >>
>> >> Could you comment/suggest on following approach of using two different
>> >> wayland connection in single process?
>> >>
>> >> First wayland connection for creating wl_shm backed wayland surface
>> >> for receiving touch inputs in main thread or as separate thread. Like
>> >> simple-touch.c example but without painting and transparent.
>> >>
>> >> Second wayland connection for another egl wayland surface for
>> >> rendering in shared library which will be loaded by above application.
>> >> Like simple-egl.c example.
>> >
>> > Hi,
>> >
>> > if you never want your first connection to handle input events on the
>> > surface created on the second connection, I suppose it would work. You
>> > also will not be able position your surfaces from the different
>> > connections relative to each other at all.
>> >
>> > The main thing to remember is that *everything* is private to a
>> > connection. If you create a wl_surface on one connection, it is as if
>> > it does not exist at all for any other connection. It does not matter
>> > if the connections come from the same thread, process, or not.
>> >
>> > If you expect any sort of interoperability, you *must* use the same
>> > connection for everything. Opening a second connection from the same
>> > program to the same server is practically always a design mistake.
>> >
>> > Of course, one could make Wayland extensions to work around this, but
>> > don't go there.
>> >
>> > So, in short: don't do it.
>> >
>> >> Will this work? Is it this the right way to do it or is there any
>> >> other mechanism available for such requirement?
>> >
>> > What do you want to achieve?
>> > You didn't explain what you really want to do.
>> >
>>
>> I have a video rendering library (which handles all media related
>> functions too) which is used by application, and that library make use
>> of first wayland connection and don't handle the touch events as there
>> is no way to pass that information to application so thinking to make
>> transparent wayland surface on top of wayland surface to which the
>> video is rendered by library to handle the touch events by creating
>> second wayland connection in application.
>
> There is no quick solution AFAIK.
>
> You have to fix both the application and the media library to become
> Wayland-friendly. The minimal starting point is:
>
> - the application (or its toolkit) opens the one Wayland connection
> - the application passes the open Wayland connection to the library
> - the library does not open new Wayland connections
>

Thanks a lot for putting your detail thought here.

Do you know if clients/nested.c, clients/nested-client.c mentioned by
"Armin Krezović " is helpful here in anyways? When I tried to run it
it just displayed black window? What functionality it demos?

What do you mean by passing "wayland connection" or passing
wl_surface? Does it mean passing the object handle with some kind of
IPC mechanism (possibly UNIX domain socket)?

Do you know any such implementation available for reference? I don't
have the code for our media library so I could try to create simple
POC for this first?


> Once you have done that, you have several options to choose from. I can
> think of at least these:
>
> - The application creates the wl_surface and tells the media library to
>   use it, or
> - the media library creates the wl_surface, and tells the app about it.
> - You handle input on the wl_surface that is being used by media
>   library either in the application, or
> - add media library API to handle or export input events.
> - Or you have two surfaces, one being the sub-surface for the other,
>   the media library uses one and the application uses the other for
>   input.
>
> If the media library can draw a complete window, it would be best to
> deal with just one wl_surface.
>
> If you have to have different wl_surfaces for input and output, you
> would use sub-surface protocol to tie them together. If the media
> library only draws video, you can set the input region there to empty,
> in which case input falls through to the next surface. That next
> surface could be your input surface, not transparent but painted with
> the background pattern and perhaps with decorations.
>
>> I tried creating and handling touch event (similar to simple-touch.c)
>> with second wayland connection after the application loads the library
>> but it gave me following errors. So I thought it might not be
>> possible. Also I think you hinted same here
>> https://lists.freedesktop.org/archives/wayland-devel/2015-September/024211.html
>>
>> Error communicating with wayland: Protocol error
>> Error 

[PATCH 6/7] fullscreen-shell: Remap surfaces when attaching a primary output

[Armin Krezović]

When no outputs are present, and no output resource was given,
a fullscreen surface won't get configured because the default
behavior when there is no output resource given is to create
the same surface on all fullscreen outputs.

This patch makes the shell reconfigure the surface as soon
as an output gets plugged in and configured, so it can properly
be displayed if it was started when no outputs were present.

Signed-off-by: Armin Krezović 
---
 fullscreen-shell/fullscreen-shell.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index 3dbd0d9..71d609c 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -299,10 +299,15 @@ pending_surface_destroyed(struct wl_listener *listener, 
void *data)
fsout->pending.surface = NULL;
 }
 
+static void
+configure_presented_surface(struct weston_surface *surface, int32_t sx,
+   int32_t sy);
+
 static struct fs_output *
 fs_output_create(struct fullscreen_shell *shell, struct weston_output *output)
 {
struct fs_output *fsout;
+   struct fs_client_surface *surf, *next;
 
fsout = zalloc(sizeof *fsout);
if (!fsout)
@@ -325,6 +330,13 @@ fs_output_create(struct fullscreen_shell *shell, struct 
weston_output *output)
weston_layer_entry_insert(>layer.view_list,
   >black_view->layer_link);
wl_list_init(>transform.link);
+
+   wl_list_for_each_safe(surf, next, >unmapped_surfaces, link) {
+   fs_output_set_surface(fsout, surf->surface, surf->method, 0, 0);
+   configure_presented_surface(surf->surface, 0, 0);
+   remove_unmapped_surface(surf);
+   }
+
return fsout;
 }
 
@@ -750,6 +762,8 @@ fullscreen_shell_present_surface(struct wl_client *client,
output = wl_resource_get_user_data(output_res);
fsout = fs_output_for_output(output);
fs_output_set_surface(fsout, surface, method, 0, 0);
+   } else if (wl_list_empty(>output_list)) {
+   add_unmapped_surface(shell, surface, method);
} else {
wl_list_for_each(fsout, >output_list, link)
fs_output_set_surface(fsout, surface, method, 0, 0);
-- 
2.9.2

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

[PATCH 4/7] compositor: Mark all views as dirty when all outputs are gone

[Armin Krezović]

When all outputs are gone and views were created before they
were gone, all views would have an output that points to a
destroyed object.

Instead, mark the view as dirty and set the view output to
NULL so a view gets an output assigned as soon as it gets
plugged in.

Signed-off-by: Armin Krezović 
---
 libweston/compositor.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 96eeb17..4467555 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1212,10 +1212,14 @@ get_view_layer(struct weston_view *view)
 WL_EXPORT void
 weston_view_update_transform(struct weston_view *view)
 {
+   struct weston_compositor *ec = view->surface->compositor;;
struct weston_view *parent = view->geometry.parent;
struct weston_layer *layer;
pixman_region32_t mask;
 
+   if (wl_list_empty(>output_list))
+   return;
+
if (!view->transform.dirty)
return;
 
@@ -4085,11 +4089,6 @@ weston_output_destroy(struct weston_output *output)
 
output->destroying = 1;
 
-   wl_list_for_each(view, >compositor->view_list, link) {
-   if (view->output_mask & (1u << output->id))
-   weston_view_assign_output(view);
-   }
-
wl_event_source_remove(output->repaint_timer);
 
weston_presentation_feedback_discard_list(>feedback_list);
@@ -4097,6 +4096,16 @@ weston_output_destroy(struct weston_output *output)
weston_compositor_reflow_outputs(output->compositor, output, 
output->width);
wl_list_remove(>link);
 
+   wl_list_for_each(view, >compositor->view_list, link) {
+   if (wl_list_empty(>compositor->output_list)) {
+   weston_view_geometry_dirty(view);
+   view->output = NULL;
+   }
+   else if (view->output_mask & (1u << output->id)) {
+   weston_view_assign_output(view);
+   }
+   }
+
wl_signal_emit(>compositor->output_destroyed_signal, output);
wl_signal_emit(>destroy_signal, output);
 
-- 
2.9.2

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

[PATCH 7/7] fullscreen-shell: Keep fullscreen surface after all outputs are gone

[Armin Krezović]

Currently, when all outputs are gone (all views are gone), a surface
is unmapped and destroyed, even though the client is still running.

This utilizes the previously added code to remap the surface when a
new output gets attached.

Signed-off-by: Armin Krezović 
---
 fullscreen-shell/fullscreen-shell.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index 71d609c..42b1662 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -262,11 +262,21 @@ fs_output_clear_pending(struct fs_output *fsout);
 static void
 fs_output_destroy(struct fs_output *fsout)
 {
-   fs_output_set_surface(fsout, NULL, 0, 0, 0);
fs_output_clear_pending(fsout);
 
wl_list_remove(>link);
 
+   if (fsout->view) {
+   weston_view_destroy(fsout->view);
+   fsout->view = NULL;
+   }
+
+   if (fsout->surface && wl_list_empty(>shell->output_list)) {
+   add_unmapped_surface(fsout->shell, fsout->surface,
+fsout->method);
+   fsout->surface = NULL;
+   }
+
if (fsout->output)
wl_list_remove(>output_destroyed.link);
 }
-- 
2.9.2

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

[PATCH 3/7] gl-renderer: Make dummy surface current after all outputs are gone

[Armin Krezović]

When all outputs are gone, there are no current read/write
surfaces associated with a context. This makes the previously
created dummy surface current until an output gets attached
to avoid any potential crashes.

Signed-off-by: Armin Krezović 
---
 libweston/gl-renderer.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index be6b11e..56eb344 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -167,6 +167,7 @@ struct gl_surface_state {
 
 struct gl_renderer {
struct weston_renderer base;
+   struct weston_compositor *compositor;
int fragment_shader_debug;
int fan_debug;
struct weston_binding *fragment_binding;
@@ -216,6 +217,9 @@ struct gl_renderer {
struct gl_shader *current_shader;
 
struct wl_signal destroy_signal;
+
+   struct wl_signal output_destroy_signal;
+   struct wl_listener output_destroy_listener;
 };
 
 static PFNEGLGETPLATFORMDISPLAYEXTPROC get_platform_display = NULL;
@@ -2642,6 +2646,8 @@ gl_renderer_destroy(struct weston_compositor *ec)
eglTerminate(gr->egl_display);
eglReleaseThread();
 
+   wl_list_remove(>output_destroy_listener.link);
+
wl_array_release(>vertices);
wl_array_release(>vtxcnt);
 
@@ -2827,6 +2833,21 @@ platform_to_extension(EGLenum platform)
}
 }
 
+static void
+output_handle_destroy(struct wl_listener *listener, void *data)
+{
+   struct gl_renderer *gr;
+   struct weston_compositor *ec;
+
+   gr = container_of(listener, struct gl_renderer,
+ output_destroy_listener);
+   ec = gr->compositor;
+
+   if (wl_list_empty(>output_list))
+   eglMakeCurrent(gr->egl_display, gr->dummy_surface,
+  gr->dummy_surface, gr->egl_context);
+}
+
 static int
 gl_renderer_create_pbuffer_surface(struct gl_renderer *gr) {
EGLConfig pbuffer_config;
@@ -2884,6 +2905,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum 
platform,
if (gr == NULL)
return -1;
 
+   gr->compositor = ec;
gr->base.read_pixels = gl_renderer_read_pixels;
gr->base.repaint_output = gl_renderer_repaint_output;
gr->base.flush_damage = gl_renderer_flush_damage;
@@ -2962,6 +2984,7 @@ gl_renderer_create(struct weston_compositor *ec, EGLenum 
platform,
wl_display_add_shm_format(ec->wl_display, WL_SHM_FORMAT_RGB565);
 
wl_signal_init(>destroy_signal);
+   wl_signal_init(>output_destroy_signal);
 
if (gl_renderer_setup(ec, gr->dummy_surface) < 0) {
if (gr->dummy_surface != EGL_NO_SURFACE)
@@ -3136,6 +3159,10 @@ gl_renderer_setup(struct weston_compositor *ec, 
EGLSurface egl_surface)
fan_debug_repaint_binding,
ec);
 
+   gr->output_destroy_listener.notify = output_handle_destroy;
+   wl_signal_add(>output_destroyed_signal,
+ >output_destroy_listener);
+
weston_log("GL ES 2 renderer features:\n");
weston_log_continue(STAMP_SPACE "read-back format: %s\n",
ec->read_format == PIXMAN_a8r8g8b8 ? "BGRA" : "RGBA");
-- 
2.9.2

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

[PATCH 5/7] fullscreen-shell: Add helpers for managing surfaces on zero outputs

[Armin Krezović]

This adds helper functions for managing fullscreen surfaces outside
of fullscreen outputs.

Signed-off-by: Armin Krezović 
---
 fullscreen-shell/fullscreen-shell.c | 55 +
 1 file changed, 55 insertions(+)

diff --git a/fullscreen-shell/fullscreen-shell.c 
b/fullscreen-shell/fullscreen-shell.c
index 2ec2d02..3dbd0d9 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -46,6 +46,16 @@ struct fullscreen_shell {
struct wl_listener output_created_listener;
 
struct wl_listener seat_created_listener;
+
+   /* List of surfaces that need to be mapped after an output
+* gets created.
+*
+* At the moment, only one surface can be created on an output.
+*
+* This is implemented as a list in case someone fixes the shell
+* implementation to support more than one client.
+*/
+   struct wl_list unmapped_surfaces; /* struct fs_client_surface::link */
 };
 
 struct fs_output {
@@ -83,6 +93,50 @@ struct pointer_focus_listener {
struct wl_listener seat_destroyed;
 };
 
+struct fs_client_surface {
+   struct weston_surface *surface;
+   enum zwp_fullscreen_shell_v1_present_method method;
+   struct wl_list link; /* struct fullscreen_shell::unmapped_surfaces */
+   struct wl_listener surface_destroyed;
+};
+
+static void
+remove_unmapped_surface(struct fs_client_surface *surf)
+{
+   wl_list_remove(>surface_destroyed.link);
+   wl_list_remove(>link);
+   free(surf);
+}
+
+static void
+unmapped_surface_destroy_listener(struct wl_listener *listener, void *data)
+{
+   struct fs_client_surface *surf;
+
+   surf = container_of(listener, struct fs_client_surface, 
surface_destroyed);
+
+   remove_unmapped_surface(surf);
+}
+
+static void
+add_unmapped_surface(struct fullscreen_shell *shell, struct weston_surface 
*surface,
+enum zwp_fullscreen_shell_v1_present_method method)
+{
+   struct fs_client_surface *surf;
+
+   surf = zalloc(sizeof *surf);
+   if (!surf)
+   return;
+
+   surf->surface = surface;
+   surf->method = method;
+
+   wl_list_insert(shell->unmapped_surfaces.prev, >link);
+
+   surf->surface_destroyed.notify = unmapped_surface_destroy_listener;
+   wl_signal_add(>destroy_signal, >surface_destroyed);
+}
+
 static void
 pointer_focus_changed(struct wl_listener *listener, void *data)
 {
@@ -831,6 +885,7 @@ module_init(struct weston_compositor *compositor,
return -1;
 
shell->compositor = compositor;
+   wl_list_init(>unmapped_surfaces);
 
shell->client_destroyed.notify = client_destroyed;
 
-- 
2.9.2

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

[PATCH 2/7] compositor-drm: Do not exit when there are no outputs left

[Armin Krezović]

When there are no outputs left after a hotplug event, weston
will terminate. This isn't desired when trying to get weston
to work with zero outputs.

Signed-off-by: Armin Krezović 
---
 libweston/compositor-drm.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index f089741..de334f1 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2689,10 +2689,6 @@ update_outputs(struct drm_backend *b, struct udev_device 
*drm_device)
}
}
}
-
-   /* FIXME: handle zero outputs, without terminating */
-   if (b->connector_allocator == 0)
-   weston_compositor_exit(b->compositor);
 }
 
 static int
-- 
2.9.2

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

Re: Two wayland connection in single process context

[Pekka Paalanen]

On Fri, 29 Jul 2016 15:03:39 +0530
Vikas Patil  wrote:

> On Fri, Jul 29, 2016 at 1:17 PM, Pekka Paalanen  wrote:
> 
> > On Wed, 27 Jul 2016 17:06:04 +0530
> > Vikas Patil  wrote:
> >  
> >> Dear All,
> >>
> >> Could you comment/suggest on following approach of using two different
> >> wayland connection in single process?
> >>
> >> First wayland connection for creating wl_shm backed wayland surface
> >> for receiving touch inputs in main thread or as separate thread. Like
> >> simple-touch.c example but without painting and transparent.
> >>
> >> Second wayland connection for another egl wayland surface for
> >> rendering in shared library which will be loaded by above application.
> >> Like simple-egl.c example.  
> >
> > Hi,
> >
> > if you never want your first connection to handle input events on the
> > surface created on the second connection, I suppose it would work. You
> > also will not be able position your surfaces from the different
> > connections relative to each other at all.
> >
> > The main thing to remember is that *everything* is private to a
> > connection. If you create a wl_surface on one connection, it is as if
> > it does not exist at all for any other connection. It does not matter
> > if the connections come from the same thread, process, or not.
> >
> > If you expect any sort of interoperability, you *must* use the same
> > connection for everything. Opening a second connection from the same
> > program to the same server is practically always a design mistake.
> >
> > Of course, one could make Wayland extensions to work around this, but
> > don't go there.
> >
> > So, in short: don't do it.
> >  
> >> Will this work? Is it this the right way to do it or is there any
> >> other mechanism available for such requirement?  
> >
> > What do you want to achieve?
> > You didn't explain what you really want to do.
> >  
> 
> I have a video rendering library (which handles all media related
> functions too) which is used by application, and that library make use
> of first wayland connection and don't handle the touch events as there
> is no way to pass that information to application so thinking to make
> transparent wayland surface on top of wayland surface to which the
> video is rendered by library to handle the touch events by creating
> second wayland connection in application.

There is no quick solution AFAIK.

You have to fix both the application and the media library to become
Wayland-friendly. The minimal starting point is:

- the application (or its toolkit) opens the one Wayland connection
- the application passes the open Wayland connection to the library
- the library does not open new Wayland connections

Once you have done that, you have several options to choose from. I can
think of at least these:

- The application creates the wl_surface and tells the media library to
  use it, or
- the media library creates the wl_surface, and tells the app about it.
- You handle input on the wl_surface that is being used by media
  library either in the application, or
- add media library API to handle or export input events.
- Or you have two surfaces, one being the sub-surface for the other,
  the media library uses one and the application uses the other for
  input.

If the media library can draw a complete window, it would be best to
deal with just one wl_surface.

If you have to have different wl_surfaces for input and output, you
would use sub-surface protocol to tie them together. If the media
library only draws video, you can set the input region there to empty,
in which case input falls through to the next surface. That next
surface could be your input surface, not transparent but painted with
the background pattern and perhaps with decorations.

> I tried creating and handling touch event (similar to simple-touch.c)
> with second wayland connection after the application loads the library
> but it gave me following errors. So I thought it might not be
> possible. Also I think you hinted same here
> https://lists.freedesktop.org/archives/wayland-devel/2015-September/024211.html
> 
> Error communicating with wayland: Protocol error
> Error communicating with wayland: Protocol error
> Error communicating with wayland: Protocol error
> Error communicating with wayland: Protocol error
> Error communicating with wayland: Protocol error

Yes, all protocol objects are valid only for the connection where they
were created in.

We lack the sanity checks in libwayland-client to abort() in the cases
where that rule is broken.

Really the very first step is to get everything to use the one and the
same Wayland connection.


Thanks,
pq


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

Re: Two wayland connection in single process context

[Vikas Patil]

On Fri, Jul 29, 2016 at 1:17 PM, Pekka Paalanen  wrote:

> On Wed, 27 Jul 2016 17:06:04 +0530
> Vikas Patil  wrote:
>
>> Dear All,
>>
>> Could you comment/suggest on following approach of using two different
>> wayland connection in single process?
>>
>> First wayland connection for creating wl_shm backed wayland surface
>> for receiving touch inputs in main thread or as separate thread. Like
>> simple-touch.c example but without painting and transparent.
>>
>> Second wayland connection for another egl wayland surface for
>> rendering in shared library which will be loaded by above application.
>> Like simple-egl.c example.
>
> Hi,
>
> if you never want your first connection to handle input events on the
> surface created on the second connection, I suppose it would work. You
> also will not be able position your surfaces from the different
> connections relative to each other at all.
>
> The main thing to remember is that *everything* is private to a
> connection. If you create a wl_surface on one connection, it is as if
> it does not exist at all for any other connection. It does not matter
> if the connections come from the same thread, process, or not.
>
> If you expect any sort of interoperability, you *must* use the same
> connection for everything. Opening a second connection from the same
> program to the same server is practically always a design mistake.
>
> Of course, one could make Wayland extensions to work around this, but
> don't go there.
>
> So, in short: don't do it.
>
>> Will this work? Is it this the right way to do it or is there any
>> other mechanism available for such requirement?
>
> What do you want to achieve?
> You didn't explain what you really want to do.
>

I have a video rendering library (which handles all media related
functions too) which is used by application, and that library make use
of first wayland connection and don't handle the touch events as there
is no way to pass that information to application so thinking to make
transparent wayland surface on top of wayland surface to which the
video is rendered by library to handle the touch events by creating
second wayland connection in application.

I tried creating and handling touch event (similar to simple-touch.c)
with second wayland connection after the application loads the library
but it gave me following errors. So I thought it might not be
possible. Also I think you hinted same here
https://lists.freedesktop.org/archives/wayland-devel/2015-September/024211.html

Error communicating with wayland: Protocol error
Error communicating with wayland: Protocol error
Error communicating with wayland: Protocol error
Error communicating with wayland: Protocol error
Error communicating with wayland: Protocol error


Thanks & Regards,
Vikash
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

How to pick the initial (output) scale for a surface?

[Pekka Paalanen]

On Sat, 23 Jul 2016 18:03:59 +0200
Quentin Glidic  wrote:

> Hi everyone,
> 
> Today I played with a little protocol to supports docks, implementing a 
> simple client and a weston plugin to test it.
> Reusing code from another client of mine, I faced an issue I had 
> encountered already when creating a protocol for notifications:
> There is no way for a client to know the correct scale to render to 
> before pushing a buffer.
> 
> Currently, a client wanting to support output scale will have to use the 
> wl_surface.enter and .leave events, track the different scale factors, 
> and render a buffer that will work for all of them.
> 
> It works great for normal applications, and the toolkit will take care 
> of most of the work, if not all. The surface will (should) magically be 
> rendered at the correct scale when you move it from one output to 
> another, and everything look good.
> 
> However, current implementation in Weston[1] and Mutter[2] do not allow 
> these events to be used for the *initial* buffer.
> While xdg_shell[3] is getting ready to allow clients to provide the 
> “perfect buffer” right from the start, this output scale issue is annoying.
> 
> There are at least three possible solutions:
> A) Changing the code to send wl_surface.enter even if no buffer is attached.
> B) Add a specific event to wl_surface (or an extension like 
> wl_perfect_surface) to solve this specific issue, sending the output the 
> client should optimize its rendering for.
> C) Use per-role-protocol events, as I did in my notification-area 
> protocol[4].
> 

Hi,

without a buffer, the surface size is unknown. Even if the WM was able
to position a surface without knowing its size, the surface might span
more than the initial output if the size is large enough. This would be
a problem if the initial output has a scale less than the additional
output it will end up to. This problem is common to A, B, and some
cases in C.

Option B with its "optimize the surface for this output" event would
solve more than just the issue in question. A compositor could drive it
based on which output has e.g. at least 80% of the surface, or
otherwise send the occupied output with highest scale. But is that a
good thing?

> I see no big downside to either solution. Here are a few thoughts about 
> them:
> A) may break for some clients, but it should have at worst the same 
> effect it has now.
> B) and C) will need explicit client support, while A) could work already 
> for some clients.
> B) avoids the duplication in every role-specific protocols, but wording 
> could be hard to get right to sync the event with role-specific one 
> (where to put “wl_surface.perfect_scale will be sent before 
> xdg_surface.configure”?).
> C) allows a bit more fine-grained behaviour, e.g. in my 
> notification-area protocol, all surfaces are tied to one specific 
> output, thus only one event (before any surface creation) will carry the 
> information.
> 
> 
> Thanks for reading, I hope we can figure out a nice solution to make 
> even the first frame perfect. :-)

I think it might not be reasonably possible for all cases, but if we
ignore the unknown size issue, maybe then.

I don't have good suggestions.

Currently what clients already have is the list of all outputs. As the
initial scale, they could pick the highest scale present when they do
not know which output the window will appear on (e.g. do not start
fullscreened on specific output). It does not remove the possibility
for the glitch, but we can at least pick the direction which way the
scale will change if we guess wrong, and there is a good possibility to
also guess right.

That would be consistent with the heuristic of picking the highest
scale from the outputs a surface is on after it has been mapped. Don't
toolkits do that already?


Thanks,
pq

> [1] 
> 
> [2] 
> 
> [3] 
> 
> [4] 
> 



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

Re: [PATCH wayland-protocols 3/4 v2] xdg-shell: Add resize_x/y constrain adjustment to positioner

[Jonas Ådahl]

On Fri, Jul 29, 2016 at 10:58:47AM +0200, Quentin Glidic wrote:
> On 29/07/2016 06:04, Jonas Ådahl wrote:
> > In order to get feedback of available space where a client can create
> > its popup, let it create requset that its popup rectangle being resized
> > would it not fit the within the work area. This adds two new constrain
> > adjustment values to the adjustment enum, and dimension parameters to
> > the xdg_popup.configure event.
> > 
> > The existing constrain adjustment actions take precedence, and resizing
> > will only be triggered if all other adjustments requested didn't manage
> > to make the popup rectangle fully visible.
> > 
> > Signed-off-by: Jonas Ådahl 
> > Acked-by: Quentin Glidic 
> > ---
> >
> > Chances since v1: none
> 
> Shouldn’t you have squashed with the positioner patch directly?

Could have, but the positioner patch is closer to being landable, so I
let it have only minor changes in this iteration, and continued to have
this as a separate patch.


Jonas

> 
> Cheers,
> 
> 
> > 
> > 
> >  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 40 
> > +---
> >  1 file changed, 30 insertions(+), 10 deletions(-)
> > 
> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> > b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > index 2c8f636..ef45ff4 100644
> > --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> > @@ -249,6 +249,19 @@
> >  
> > 
> >  
> > +  
> > +   The constrain adjustment value define ways the compositor will adjust
> > +   the position of the surface, if the unadjusted position would result
> > +   in the surface being partly constrained.
> > +
> > +   What a constrained surface means is compositor specific, but it could
> > +   for example be partly outside of the work area of a monitor.
> > +
> > +   Multiple constrain adjustment values can be combined, in which case
> > +   they have a defined precedence. The order of adjustments is: flip,
> > +   slide then resize.
> > +  
> > +
> >
> > 
> >   Don't alter the surface position even if it is constrained on some
> > @@ -268,8 +281,6 @@
> >   x axis until either the edge in the direction of the gravity is
> >   unconstrained or the edge in the opposite direction of the gravity is
> >   constrained.
> > -
> > - If 'slide_x' is combined with 'flip_x', 'flip_x' takes precedence.
> > 
> >
> >
> > @@ -285,8 +296,6 @@
> >   y axis until either the edge in the direction of the gravity is
> >   unconstrained or the edge in the opposite direction of the gravity is
> >   constrained.
> > -
> > - If 'slide_y' is combined with 'flip_y', 'flip_y' takes precedence.
> > 
> >
> >
> > @@ -297,9 +306,8 @@
> >   'left', change the gravity to 'right' and the anchor to 'right'.
> > 
> >   If the adjusted position also ends up being constrained, the resulting
> > - position will be the one before the adjustment. If the resulting
> > - position is still constrained, and 'flip_x' is combined with
> > - 'slide_x', the position is adjusted according to 'slide_x'.
> > + position of the flip_x adjustment will be the one before the
> > + adjustment.
> > 
> >
> >
> > @@ -310,9 +318,19 @@
> >   'bottom', change the gravity to 'top' and the anchor to 'top'.
> > 
> >   If the adjusted position also ends up being constrained, the resulting
> > - position will be the one before the adjustment. If the resulting
> > - position is still constrained, and 'flip_y' is combined with
> > - 'slide_y', the position is adjusted according to 'slide_y'.
> > + position of the flip_y adjustment will be the one before the
> > + adjustment.
> > +   
> > +  
> > +  
> > +   
> > + Resize the surface horizontally so that it is completely
> > + unconstrained.
> > +   
> > +  
> > +  
> > +   
> > + Resize the surface vertically so that it is completely unconstrained.
> > 
> >
> >  
> > @@ -1039,6 +1057,8 @@
> >summary="x position relative to parent surface window geometry"/>
> > >summary="y position relative to parent surface window geometry"/>
> > +  
> > +  
> >  
> > 
> >  
> > 
> 
> 
> -- 
> 
> 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 3/4 v2] xdg-shell: Add resize_x/y constrain adjustment to positioner

[Quentin Glidic]

On 29/07/2016 06:04, Jonas Ådahl wrote:

In order to get feedback of available space where a client can create
its popup, let it create requset that its popup rectangle being resized
would it not fit the within the work area. This adds two new constrain
adjustment values to the adjustment enum, and dimension parameters to
the xdg_popup.configure event.

The existing constrain adjustment actions take precedence, and resizing
will only be triggered if all other adjustments requested didn't manage
to make the popup rectangle fully visible.

Signed-off-by: Jonas Ådahl 
Acked-by: Quentin Glidic 
---

>

Chances since v1: none


Shouldn’t you have squashed with the positioner patch directly?

Cheers,





 unstable/xdg-shell/xdg-shell-unstable-v6.xml | 40 +---
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index 2c8f636..ef45ff4 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -249,6 +249,19 @@
 

 
+  
+   The constrain adjustment value define ways the compositor will adjust
+   the position of the surface, if the unadjusted position would result
+   in the surface being partly constrained.
+
+   What a constrained surface means is compositor specific, but it could
+   for example be partly outside of the work area of a monitor.
+
+   Multiple constrain adjustment values can be combined, in which case
+   they have a defined precedence. The order of adjustments is: flip,
+   slide then resize.
+  
+
   

  Don't alter the surface position even if it is constrained on some
@@ -268,8 +281,6 @@
  x axis until either the edge in the direction of the gravity is
  unconstrained or the edge in the opposite direction of the gravity is
  constrained.
-
- If 'slide_x' is combined with 'flip_x', 'flip_x' takes precedence.

   
   
@@ -285,8 +296,6 @@
  y axis until either the edge in the direction of the gravity is
  unconstrained or the edge in the opposite direction of the gravity is
  constrained.
-
- If 'slide_y' is combined with 'flip_y', 'flip_y' takes precedence.

   
   
@@ -297,9 +306,8 @@
  'left', change the gravity to 'right' and the anchor to 'right'.

  If the adjusted position also ends up being constrained, the resulting
- position will be the one before the adjustment. If the resulting
- position is still constrained, and 'flip_x' is combined with
- 'slide_x', the position is adjusted according to 'slide_x'.
+ position of the flip_x adjustment will be the one before the
+ adjustment.

   
   
@@ -310,9 +318,19 @@
  'bottom', change the gravity to 'top' and the anchor to 'top'.

  If the adjusted position also ends up being constrained, the resulting
- position will be the one before the adjustment. If the resulting
- position is still constrained, and 'flip_y' is combined with
- 'slide_y', the position is adjusted according to 'slide_y'.
+ position of the flip_y adjustment will be the one before the
+ adjustment.
+   
+  
+  
+   
+ Resize the surface horizontally so that it is completely
+ unconstrained.
+   
+  
+  
+   
+ Resize the surface vertically so that it is completely unconstrained.

   
 
@@ -1039,6 +1057,8 @@
   summary="x position relative to parent surface window geometry"/>
   
+  
+  
 

 




--

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

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

[Pekka Paalanen]

On Fri, 22 Jul 2016 21:39:20 +0200
"Nils Chr. Brause"  wrote:

> HI,
> 
> On Thu, Jun 2, 2016 at 10:30 AM, Pekka Paalanen  wrote:
> > On Thu, 2 Jun 2016 15:39:47 +0800
> > Jonas Ådahl  wrote:
> >  
> >> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:  
> >> > On Wed, 1 Jun 2016 13:16:31 -0500
> >> > Yong Bakos  wrote:
> >> >  
> >> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  
> >> > > wrote:  
> >> > > >
> >> > > > On Sat, 28 May 2016 08:39:59 -0500
> >> > > > Yong Bakos  wrote:
> >> > > >  
> >> > > >> Hi Mike,
> >> > > >> Regarding the combination of type="array" enum="foo"...
> >> > > >>  
> >> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> >> > > >>>  wrote:
> >> > > >>>
> >> > > >>> I've inlined some replies below.
> >> > > >>>
> >> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  >> > > >>> > wrote:
> >> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
> >> > > >>> > wrote:  
> >> > > 
> >> > >  this adds a method for compositors to change various draw 
> >> > >  attributes
> >> > >  for a surface
> >> > > 
> >> > >  Signed-off-by: Mike Blumenkrantz  >> > >  >
> >> > >  Signed-off-by: Jonas Ådahl  >> > >  >  
> >> > > >>>
> >> > > >>> Hi Mike & Jonas,
> >> > > >>> A question about communicating default state, and some
> >> > > >>> minor nits you can certainly ignore, inline below.
> >> > > >>>
> >> > > >>>  
> >> > >  ---
> >> > >  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 69 
> >> > >  
> >> > >  1 file changed, 69 insertions(+)
> >> > > 
> >> > >  diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
> >> > >  b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> >> > >  index dfd7e84..0fa76d4 100644
> >> > >  --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> >> > >  +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml  
> >> > > >  
> >> > >  +
> >> > >  +Calling this after an xdg_toplevel's first commit will 
> >> > >  raise a client error.
> >> > >  +  
> >> > >  +
> >> > > >>>
> >> > > >>> Just a sanity check, since I haven't seen it in a protocol spec 
> >> > > >>> yet. Does scanner handle
> >> > > >>> this combination of array and enum correctly?
> >> > > >>>
> >> > > >>> Good catch. This also affects the event above it.  
> >> > > >>
> >> > > >> As we discussed via IRC (27 May), the scanner will choke on this. 
> >> > > >> While we talked about
> >> > > >> making a change to the scanner to allow this, perhaps such a change 
> >> > > >> doesn't make sense.
> >> > > >>
> >> > > >> Given a type="array", scanner will generate a parameter of type 
> >> > > >> wl_array.
> >> > > >>
> >> > > >> Perhaps the short story here is to just remove the enum from this 
> >> > > >> arg, and the similar
> >> > > >> arg in the configure_draw_states event above. What do you think?
> >> > > >>
> >> > > >> (I wonder if it's the DTD that can change, so the scanner's 
> >> > > >> validation step
> >> > > >> will catch the unsupported combination of type="array" enum="foo". 
> >> > > >> My gut tells me that
> >> > > >> DTDs don't support this logic, but I'll dig into this.)  
> >> > > >
> >> > > > Hi,
> >> > > >
> >> > > > here is some background.
> >> > > >
> >> > > > A type="array" argument is really just a binary blob of data. The XML
> >> > > > description, human documentation aside, does not specify anything 
> >> > > > about
> >> > > > the blob contents. Therefore adding an XML attribute pointing to an
> >> > > > enum definition is half-useless. Generators could use it for creating
> >> > > > automatic links in documentation, but it cannot be used for code
> >> > > > generation, because you don't know the types contained in the blob.
> >> > > >
> >> > > > We also do not want to add blob content type definitions to the XML
> >> > > > language, because you might want to have everything C is able to
> >> > > > express, including nested structs. There is also no requirement that
> >> > > > the "array" is really an array - every "element" could be a different
> >> > > > thing. It could be bitstream and whatnot. Only the use of
> >> > > > wl_array_for_each() implies it is an array of similar elements,
> >> > > > wl_array_add() does not.  
> 
> Then that's not an 'array' than but more like a struct.
> Therefore the structure definition should be in the XML.
> Leaving it undocumented is a terrible option.

Hi Nils,

yes, things should definitely be documented. I mean human-readable
documentation, not necessarily machine-parseable description.

> >> > > > The 

Re: [PATCH wayland-protocols 4/4] xdg-shell: Drop desktop environment specific state allocations

[Quentin Glidic]

On 29/07/2016 06:04, Jonas Ådahl wrote:

Instead of allocating state ranges that desktop environments can use as
they want, let them introduce their own protocol and their own enums.

If such desktop environment protocols need the configure/ack_configure
semantics, they can design their protocols to extend xdg_surface, and
make their private configure events a latched state tied to
xdg_surface.configure.

Signed-off-by: Jonas Ådahl 
---


Great, with the shared configure event it makes a lot of sense:
Reviewed-by: Quentin Glidic 

Cheers,


 unstable/xdg-shell/xdg-shell-unstable-v6.xml | 12 
 1 file changed, 12 deletions(-)

diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml 
b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
index ef45ff4..72f070c 100644
--- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
+++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
@@ -702,18 +702,6 @@

States set in this way are double-buffered. They will get applied on
the next commit.
-
-   Desktop environments may extend this enum by taking up a range of
-   values and documenting the range they chose in this description.
-   They are not required to document the values for the range that they
-   chose. Ideally, any good extensions from a desktop environment should
-   make its way into standardization into this enum.
-
-   The current reserved ranges are:
-
-   0x - 0x0FFF: xdg-shell core values, documented below.
-   0x1000 - 0x1FFF: GNOME
-   0x2000 - 0x2FFF: EFL
   
   





--

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

Re: Two wayland connection in single process context

[Pekka Paalanen]

On Wed, 27 Jul 2016 17:06:04 +0530
Vikas Patil  wrote:

> Dear All,
> 
> Could you comment/suggest on following approach of using two different
> wayland connection in single process?
> 
> First wayland connection for creating wl_shm backed wayland surface
> for receiving touch inputs in main thread or as separate thread. Like
> simple-touch.c example but without painting and transparent.
> 
> Second wayland connection for another egl wayland surface for
> rendering in shared library which will be loaded by above application.
> Like simple-egl.c example.

Hi,

if you never want your first connection to handle input events on the
surface created on the second connection, I suppose it would work. You
also will not be able position your surfaces from the different
connections relative to each other at all.

The main thing to remember is that *everything* is private to a
connection. If you create a wl_surface on one connection, it is as if
it does not exist at all for any other connection. It does not matter
if the connections come from the same thread, process, or not.

If you expect any sort of interoperability, you *must* use the same
connection for everything. Opening a second connection from the same
program to the same server is practically always a design mistake.

Of course, one could make Wayland extensions to work around this, but
don't go there.

So, in short: don't do it.

> Will this work? Is it this the right way to do it or is there any
> other mechanism available for such requirement?

What do you want to achieve?

You didn't explain what you really want to do.


Thanks,
pq


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