Re: [PATCH weston] libweston-desktop: fix stale ping when a wl_shell_surface is destroyed
Hi
Is this patch for https://bugs.freedesktop.org/show_bug.cgi?id=97892 , or is it
for a different issue that I am getting confused with?
It doesn't seem to fix the issue with wl_shell for qtwayland and EFL (when EFL
is forced to use wl_shell)
It actually seems to make the issue affect all surfaces instead of just the
first one.
Unless if this patch is for a similar sounding, but different issue?
Thanks
On Monday, September 26, 2016 12:37:54 PM EDT Giulio Camuffo wrote:
> When sending a ping event to a surface using the wl_shell interface,
> if that surface is destroyed before we receive the pong we will never
> receive it, even if the client is actually responsive, since the
> interface does not exist anymore. So when the surface if destroyed
> pretend it's a pong and reset the ping state.
> ---
>
> v2: put the logic in the wl_shell specific file instead of the common code
>
> libweston-desktop/client.c | 10 +-
> libweston-desktop/internal.h | 4 +++-
> libweston-desktop/wl-shell.c | 42
> ++--
> libweston-desktop/xdg-shell-v5.c | 2 +-
> libweston-desktop/xdg-shell-v6.c | 2 +-
> libweston-desktop/xwayland.c | 3 ++-
> 6 files changed, 56 insertions(+), 7 deletions(-)
>
> diff --git a/libweston-desktop/client.c b/libweston-desktop/client.c
> index 29c3c98..810b6ba 100644
> --- a/libweston-desktop/client.c
> +++ b/libweston-desktop/client.c
> @@ -39,6 +39,7 @@ struct weston_desktop_client {
> uint32_t ping_serial;
> struct wl_event_source *ping_timer;
> struct wl_signal destroy_signal;
> + void *user_data;
> };
>
> void
> @@ -86,7 +87,7 @@ weston_desktop_client_create(struct weston_desktop *desktop,
>wl_dispatcher_func_t dispatcher,
>const struct wl_interface *interface,
>const void *implementation, uint32_t version,
> - uint32_t id)
> + uint32_t id, void *user_data)
> {
> struct weston_desktop_client *client;
> struct wl_display *display;
> @@ -101,6 +102,7 @@ weston_desktop_client_create(struct weston_desktop
> *desktop,
>
> client->desktop = desktop;
> client->client = wl_client;
> + client->user_data = user_data;
>
> wl_list_init(&client->surface_list);
> wl_signal_init(&client->destroy_signal);
> @@ -210,3 +212,9 @@ weston_desktop_client_pong(struct weston_desktop_client
> *client, uint32_t serial
> wl_event_source_timer_update(client->ping_timer, 0);
> client->ping_serial = 0;
> }
> +
> +void *
> +weston_desktop_client_get_user_data(struct weston_desktop_client *client)
> +{
> + return client->user_data;
> +}
> diff --git a/libweston-desktop/internal.h b/libweston-desktop/internal.h
> index a9c974b..6f8b5aa 100644
> --- a/libweston-desktop/internal.h
> +++ b/libweston-desktop/internal.h
> @@ -128,7 +128,7 @@ weston_desktop_client_create(struct weston_desktop
> *desktop,
>wl_dispatcher_func_t dispatcher,
>const struct wl_interface *interface,
>const void *implementation, uint32_t version,
> - uint32_t id);
> + uint32_t id, void *user_data);
>
> void
> weston_desktop_client_add_destroy_listener(struct weston_desktop_client
> *client,
> @@ -143,6 +143,8 @@ weston_desktop_client_get_surface_list(struct
> weston_desktop_client *client);
> void
> weston_desktop_client_pong(struct weston_desktop_client *client,
> uint32_t serial);
> +void *
> +weston_desktop_client_get_user_data(struct weston_desktop_client *client);
>
> struct weston_desktop_surface *
> weston_desktop_surface_create(struct weston_desktop *desktop,
> diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
> index ded69f7..3a81f42 100644
> --- a/libweston-desktop/wl-shell.c
> +++ b/libweston-desktop/wl-shell.c
> @@ -58,6 +58,11 @@ struct weston_desktop_wl_shell_surface {
> enum weston_desktop_wl_shell_surface_state state;
> };
>
> +struct wl_shell_client {
> + struct weston_desktop_wl_shell_surface *ping_surface;
> + uint32_t ping_serial;
> +};
> +
> static void
> weston_desktop_wl_shell_surface_set_size(struct weston_desktop_surface
> *dsurface,
>void *user_data,
> @@ -109,7 +114,12 @@ weston_desktop_wl_shell_surface_ping(struct
> weston_desktop_surface *dsurface,
>uint32_t serial, void *user_data)
> {
> struct weston_desktop_wl_shell_surface *surface = user_data;
> + struct weston_desktop_client *client =
> + weston_desktop_surface_get_client(dsurface);
> + struct wl_shell_client *wsc =
> weston_desktop_client_get_user_data(client);
>
> + wsc->ping_surface = surface;
> + wsc->ping_serial = serial;
> wl_shell_surface_s
Re: [PATCH] weston-launch: Protect KDGKBMODE K_OFF ioctl by KERNEL_VERSION check
The kernel version used to build Weston isn't necessarily the same as the
version that will be used to run it. Weston should already work fine on
older versions: the second ioctl will return an error - but it's ok as long
as the first one succeeds.
Also, a compile-time check would prevent Weston built on an old kernel from
taking advantage of new features when running on a new kernel.
On Tue, Sep 27, 2016 at 9:37 AM, Krzysztof Konopko wrote:
> From: Tomasz SZKUTKOWSKI
>
> This patch disables unsupported ioctl `KDGKBMODE K_OFF` command if Weston
> is built against kernel older than 2.6.39, as this ioctl has been
> introduced in 2.6.39 kernel version.
>
> No functional changes have been observed by disabling this ioctl.
>
> Signed-off-by: Tomasz SZKUTKOWSKI
> Signed-off-by: Krzysztof Konopko
> ---
> libweston/launcher-direct.c | 5 +
> libweston/weston-launch.c | 5 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/libweston/launcher-direct.c b/libweston/launcher-direct.c
> index 29d9c28..34fe5cd 100644
> --- a/libweston/launcher-direct.c
> +++ b/libweston/launcher-direct.c
> @@ -34,6 +34,7 @@
> #include
> #include
> #include
> +#include
>
> #include "launcher-impl.h"
>
> @@ -157,8 +158,12 @@ setup_tty(struct launcher_direct *launcher, int tty)
> goto err_close;
> }
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
> if (ioctl(launcher->tty, KDSKBMUTE, 1) &&
> ioctl(launcher->tty, KDSKBMODE, K_OFF)) {
> +#else
> + if (ioctl(launcher->tty, KDSKBMUTE, 1)) {
> +#endif
> weston_log("failed to set K_OFF keyboard mode: %m\n");
> goto err_close;
> }
> diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
> index 140fde1..74b80dd 100644
> --- a/libweston/weston-launch.c
> +++ b/libweston/weston-launch.c
> @@ -49,6 +49,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -561,8 +562,12 @@ setup_tty(struct weston_launch *wl, const char *tty)
> if (ioctl(wl->tty, KDGKBMODE, &wl->kb_mode))
> error(1, errno, "failed to get current keyboard mode:
> %m\n");
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
> if (ioctl(wl->tty, KDSKBMUTE, 1) &&
> ioctl(wl->tty, KDSKBMODE, K_OFF))
> +#else
> + if (ioctl(wl->tty, KDSKBMUTE, 1))
> +#endif
> error(1, errno, "failed to set K_OFF keyboard mode: %m\n");
>
> if (ioctl(wl->tty, KDSETMODE, KD_GRAPHICS))
> --
> 2.1.4
>
> This transmission contains information that may be confidential and
> contain personal views which are not necessarily those of YouView TV Ltd.
> YouView TV Ltd (Co No:7308805) is a limited liability company registered in
> England and Wales with its registered address at YouView TV Ltd, 3rd Floor,
> 10 Lower Thames Street, London, EC3R 6YT. For details see our web site at
> http://www.youview.com
> ___
> 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 wayland v2 4/4] array-test: Include wayland-util.h and simplify init test
I think I actually know the point of the test.
It tries to verify that size, alloc, and data were initialized to 0, rather
than left uninitialized - but the difficulty is that uninitialized memory
is often already filled with 0s. So the test repeats the process a whole
bunch of times, hoping to eventually catch non-0 uninitialized memory.
(That doesn't mean it's a good way to test it, though, so I have nothing
against removing it. Something like valgrind is probably better.)
On Tue, Sep 27, 2016 at 8:03 PM, Yong Bakos wrote:
> From: Yong Bakos
>
> Include wayland-util.h in addition to wayland-private.h, to be more
> explicit
> about where wl_array is defined.
>
> Remove the useless repeated testing of wl_array_init, because if it fails
> once
> out of thousands of iterations we're all doomed anyway.
>
> Signed-off-by: Yong Bakos
> Reviewed-by: Eric Engestrom
> Reviewed-by: Pekka Paalanen
> ---
> v2: Add empty line (pq)
>
> tests/array-test.c | 18 ++
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/tests/array-test.c b/tests/array-test.c
> index b0de8e6..ed6fbfb 100644
> --- a/tests/array-test.c
> +++ b/tests/array-test.c
> @@ -25,24 +25,18 @@
>
> #include
> #include
> +#include "wayland-util.h"
> #include "wayland-private.h"
> #include "test-runner.h"
>
> TEST(array_init)
> {
> - const int iterations = 4122; /* this is arbitrary */
> - int i;
> -
> - /* Init array an arbitray amount of times and verify the
> -* defaults are sensible. */
> + struct wl_array array;
>
> - for (i = 0; i < iterations; i++) {
> - struct wl_array array;
> - wl_array_init(&array);
> - assert(array.size == 0);
> - assert(array.alloc == 0);
> - assert(array.data == 0);
> - }
> + wl_array_init(&array);
> + assert(array.size == 0);
> + assert(array.alloc == 0);
> + assert(array.data == 0);
> }
>
> TEST(array_release)
> --
> 2.7.2
>
> ___
> 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] weston-launch: Protect KDGKBMODE K_OFF ioctl by KERNEL_VERSION check
From: Tomasz SZKUTKOWSKI
This patch disables unsupported ioctl `KDGKBMODE K_OFF` command if Weston
is built against kernel older than 2.6.39, as this ioctl has been
introduced in 2.6.39 kernel version.
No functional changes have been observed by disabling this ioctl.
Signed-off-by: Tomasz SZKUTKOWSKI
Signed-off-by: Krzysztof Konopko
---
libweston/launcher-direct.c | 5 +
libweston/weston-launch.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/libweston/launcher-direct.c b/libweston/launcher-direct.c
index 29d9c28..34fe5cd 100644
--- a/libweston/launcher-direct.c
+++ b/libweston/launcher-direct.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include "launcher-impl.h"
@@ -157,8 +158,12 @@ setup_tty(struct launcher_direct *launcher, int tty)
goto err_close;
}
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
if (ioctl(launcher->tty, KDSKBMUTE, 1) &&
ioctl(launcher->tty, KDSKBMODE, K_OFF)) {
+#else
+ if (ioctl(launcher->tty, KDSKBMUTE, 1)) {
+#endif
weston_log("failed to set K_OFF keyboard mode: %m\n");
goto err_close;
}
diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
index 140fde1..74b80dd 100644
--- a/libweston/weston-launch.c
+++ b/libweston/weston-launch.c
@@ -49,6 +49,7 @@
#include
#include
#include
+#include
#include
#include
@@ -561,8 +562,12 @@ setup_tty(struct weston_launch *wl, const char *tty)
if (ioctl(wl->tty, KDGKBMODE, &wl->kb_mode))
error(1, errno, "failed to get current keyboard mode: %m\n");
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
if (ioctl(wl->tty, KDSKBMUTE, 1) &&
ioctl(wl->tty, KDSKBMODE, K_OFF))
+#else
+ if (ioctl(wl->tty, KDSKBMUTE, 1))
+#endif
error(1, errno, "failed to set K_OFF keyboard mode: %m\n");
if (ioctl(wl->tty, KDSETMODE, KD_GRAPHICS))
--
2.1.4
This transmission contains information that may be confidential and contain
personal views which are not necessarily those of YouView TV Ltd. YouView TV
Ltd (Co No:7308805) is a limited liability company registered in England and
Wales with its registered address at YouView TV Ltd, 3rd Floor, 10 Lower Thames
Street, London, EC3R 6YT. For details see our web site at http://www.youview.com
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 3/4] tests: Test wl_array_release
From: Yong Bakos
array-test.c did not cover wl_array_release, so add one test that specifically
tests this method.
Signed-off-by: Yong Bakos
---
v2: Use wl_array_add instead of calloc (pq)
tests/array-test.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tests/array-test.c b/tests/array-test.c
index fe53240..b0de8e6 100644
--- a/tests/array-test.c
+++ b/tests/array-test.c
@@ -45,6 +45,17 @@ TEST(array_init)
}
}
+TEST(array_release)
+{
+ struct wl_array array;
+
+ wl_array_init(&array);
+ array.data = wl_array_add(&array, 1);
+ assert(array.data != NULL);
+ wl_array_release(&array);
+ assert(array.data == WL_ARRAY_POISON_PTR);
+}
+
TEST(array_add)
{
struct mydata {
--
2.7.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 2/4] wl_array: Set data to invalid address after free
From: Yong Bakos
Explicitly set the data member to an invalid memory address during
wl_array_release, such that re-using a freed wl_array without re-initializing
causes a crash. In addition, this pointer assignment makes wl_array_release
testable.
Define a constant for the invalid memory address, and add documentation about
this behavior, starting at libwayland version 1.13.
See
https://lists.freedesktop.org/archives/wayland-devel/2016-September/031116.html
Signed-off-by: Yong Bakos
---
v2: Set data pointer to invalid memory address (pq)
src/wayland-private.h | 3 +++
src/wayland-util.c| 1 +
src/wayland-util.h| 4 +++-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/wayland-private.h b/src/wayland-private.h
index ac712d9..ef58ccf 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -36,6 +36,9 @@
#include "wayland-util.h"
+/* Invalid memory address */
+#define WL_ARRAY_POISON_PTR (void *) 4
+
#define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
#define container_of(ptr, type, member) ({ \
diff --git a/src/wayland-util.c b/src/wayland-util.c
index 639ccf8..077fec7 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -102,6 +102,7 @@ WL_EXPORT void
wl_array_release(struct wl_array *array)
{
free(array->data);
+ array->data = WL_ARRAY_POISON_PTR;
}
WL_EXPORT void *
diff --git a/src/wayland-util.h b/src/wayland-util.h
index 9b7a4b9..42b3027 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -381,7 +381,9 @@ wl_array_init(struct wl_array *array);
/**
* Releases the array data.
*
- * \note Leaves the array in an invalid state.
+ * \note Leaves the wl_array in an invalid state.
+ * Since libwayland version 1.13, using a released wl_array without first
+ * re-initializing it before use will cause an intentional crash.
*
* \param array Array whose data is to be released
*
--
2.7.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 0/4] wl_array: documentation and test
From: Yong Bakos This series adds documentation to wl_array and its methods. There is one implementation change: defining an invalid pointer address, and setting the `data` pointer to the invalid pointer address after free, in wl_array_release. I did not set `size` and `alloc` to 0, in order to keep the change miniminal. (Should I?) Regards, yong Yong Bakos (4): util: Document wl_array wl_array: Set data to invalid address after free tests: Test wl_array_release array-test: Include wayland-util.h and simplify init test src/wayland-private.h | 3 +++ src/wayland-util.c| 1 + src/wayland-util.h| 75 +++ tests/array-test.c| 27 +++ 4 files changed, 90 insertions(+), 16 deletions(-) -- 2.7.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 4/4] array-test: Include wayland-util.h and simplify init test
From: Yong Bakos
Include wayland-util.h in addition to wayland-private.h, to be more explicit
about where wl_array is defined.
Remove the useless repeated testing of wl_array_init, because if it fails once
out of thousands of iterations we're all doomed anyway.
Signed-off-by: Yong Bakos
Reviewed-by: Eric Engestrom
Reviewed-by: Pekka Paalanen
---
v2: Add empty line (pq)
tests/array-test.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/tests/array-test.c b/tests/array-test.c
index b0de8e6..ed6fbfb 100644
--- a/tests/array-test.c
+++ b/tests/array-test.c
@@ -25,24 +25,18 @@
#include
#include
+#include "wayland-util.h"
#include "wayland-private.h"
#include "test-runner.h"
TEST(array_init)
{
- const int iterations = 4122; /* this is arbitrary */
- int i;
-
- /* Init array an arbitray amount of times and verify the
-* defaults are sensible. */
+ struct wl_array array;
- for (i = 0; i < iterations; i++) {
- struct wl_array array;
- wl_array_init(&array);
- assert(array.size == 0);
- assert(array.alloc == 0);
- assert(array.data == 0);
- }
+ wl_array_init(&array);
+ assert(array.size == 0);
+ assert(array.alloc == 0);
+ assert(array.data == 0);
}
TEST(array_release)
--
2.7.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 1/4] util: Document wl_array
From: Yong Bakos
Add doxygen comments for wl_array and its methods.
Signed-off-by: Yong Bakos
Reviewed-by: Eric Engestrom
---
v2: Add description paragraph to wl_array (pq)
Fix s/list/array (pq)
Add note about pos type for wl_array_for_each (pq)
src/wayland-util.h | 73 ++
1 file changed, 68 insertions(+), 5 deletions(-)
diff --git a/src/wayland-util.h b/src/wayland-util.h
index a404390..9b7a4b9 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -350,29 +350,92 @@ wl_list_insert_list(struct wl_list *list, struct wl_list
*other);
pos = tmp, \
tmp = wl_container_of(pos->member.prev, tmp, member))
+/**
+ * \class wl_array
+ *
+ * Dynamic array
+ *
+ * A wl_array is a dynamic array that can only grow until released. It is
+ * intended for relatively small allocations whose size is variable or not
known
+ * in advance. While construction of a wl_array does not require all elements
to
+ * be of the same size, wl_array_for_each() does require all elements to have
+ * the same type and size.
+ *
+ */
struct wl_array {
size_t size;
size_t alloc;
void *data;
};
-#define wl_array_for_each(pos, array) \
- for (pos = (array)->data; \
-(const char *) pos < ((const char *) (array)->data +
(array)->size); \
-(pos)++)
-
+/**
+ * Initializes the array.
+ *
+ * \param array Array to initialize
+ *
+ * \memberof wl_array
+ */
void
wl_array_init(struct wl_array *array);
+/**
+ * Releases the array data.
+ *
+ * \note Leaves the array in an invalid state.
+ *
+ * \param array Array whose data is to be released
+ *
+ * \memberof wl_array
+ */
void
wl_array_release(struct wl_array *array);
+/**
+ * Increases the size of the array by \p size bytes.
+ *
+ * \param array Array whose size is to be increased
+ * \param size Number of bytes to increase the size of the array by
+ *
+ * \return A pointer to the beginning of the newly appended space, or NULL when
+ * resizing fails.
+ *
+ * \memberof wl_array
+ */
void *
wl_array_add(struct wl_array *array, size_t size);
+/**
+ * Copies the contents of \p source to \p array.
+ *
+ * \param array Destination array to copy to
+ * \param source Source array to copy from
+ *
+ * \return 0 on success, or -1 on failure
+ *
+ * \memberof wl_array
+ */
int
wl_array_copy(struct wl_array *array, struct wl_array *source);
+/**
+ * Iterates over an array.
+ *
+ * This macro expresses a for-each iterator for wl_array. It assigns each
+ * element in the array to \p pos, which can then be referenced in a trailing
+ * code block. \p pos must be a pointer to the array element type, and all
+ * array elements must be of the same type and size.
+ *
+ * \param pos Cursor that each array element will be assigned to
+ * \param array Array to iterate over
+ *
+ * \relates wl_array
+ * \sa wl_list_for_each()
+ */
+#define wl_array_for_each(pos, array) \
+ for (pos = (array)->data; \
+(const char *) pos < ((const char *) (array)->data +
(array)->size); \
+(pos)++)
+
typedef int32_t wl_fixed_t;
static inline double
--
2.7.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] clients/stacking: Silence a compiler warning
On Tue, Sep 27, 2016 at 12:35:55PM +0200, Armin Krezović wrote: > This patch fixes a compiler warning when building with > clang, since it doesn't support gnu_printf attribute. > > v2: > > - Switch to WL_PRINTF per suggestion from Eric Engestrom. > > Signed-off-by: Armin Krezović > --- Right now you get the WL_PRINTF macro because clients/stacking.c includes "window.h", which includes "shared/platform.h", which includes , which internally ends up including "wayland-util.h". It would be better to directly include wayland-util.h in this file, making the fragile include-chain irrelevant and making it explicit that you use something in this file. With that #include added, this is: Reviewed-by: Eric Engestrom Cheers! > clients/stacking.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/clients/stacking.c b/clients/stacking.c > index 4285a17..7c711e4 100644 > --- a/clients/stacking.c > +++ b/clients/stacking.c > @@ -184,7 +184,7 @@ fullscreen_handler(struct window *window, void *data) > > static void > draw_string(cairo_t *cr, > -const char *fmt, ...) __attribute__((format (gnu_printf, 2, 3))); > +const char *fmt, ...) WL_PRINTF(2,3); > > static void > draw_string(cairo_t *cr, > -- > 2.10.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 08/14 v3] weston: Port Wayland backend to new output handling API
On Thu, 18 Aug 2016 18:42:36 +0200
Armin Krezović wrote:
> This is a complete port of the Wayland backend that
> uses recently added output handling API for output
> configuration.
>
> - Output can be configured at runtime by passing the
> necessary configuration parameters, which can be
> filled in manually, obtained from the configuration
> file or obtained from the command line using
> previously added functionality. It is required that
> the scale and transform values are set using the
> previously added functionality.
>
> - Output can be created at runtime using the output
> API. The output creation only creates a pending
> output, which needs to be configured the same way as
> mentioned above.
>
> However, the backend can behave both as windowed backend
> and as a backend that issues "hotplug" events, when
> running under fullscreen shell or with --sprawl command
> line option. The first case was covered by reusing
> previously added functionality. The second case required
> another API to be introduced and implemented into both
> the backend and compositor for handling output setup.
>
> After everything has been set, output needs to be
> enabled manually using weston_output_enable().
>
> v2:
>
> - Fix wet_configure_windowed_output_from_config() usage.
> - Call wayland_output_disable() explicitly from
>wayland_output_destroy().
>
> v3:
>
> - Get rid of weston_wayland_output_api and rework output
>creation and configuration in case wayland backend is
>started with --sprawl or on fullscreen-shell.
> - Remove unneeded free().
> - Disallow calling wayland_output_configure more than once.
> - Remove unneeded checks for output->name == NULL as that
>has been disallowed.
> - Use weston_compositor_add_pending_output().
>
> Signed-off-by: Armin Krezović
> ---
> compositor/main.c | 257
> libweston/compositor-wayland.c | 324
> +++--
> libweston/compositor-wayland.h | 8 -
> 3 files changed, 279 insertions(+), 310 deletions(-)
Hi,
I tested this quickly, hosted on weston/x11 with desktop-shell since
fullscreen-shell does not manage to show anything either before or
after these patches.
While --sprawl works fine (tried with parent output-count=2),
--fullscreen is getting a wrong size. More comments on that inline.
> diff --git a/compositor/main.c b/compositor/main.c
> index 7007901..d2df568 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> static int
> -load_wayland_backend_config(struct weston_compositor *compositor, int *argc,
> - char *argv[], struct weston_config *wc,
> - struct weston_wayland_backend_config *config)
> +load_wayland_backend(struct weston_compositor *c,
> + int *argc, char **argv, struct weston_config *wc)
> {
> + struct weston_wayland_backend_config config = {{ 0, }};
> struct weston_config_section *section;
> - struct weston_wayland_backend_output_config *oc;
> - int count, width, height, scale;
> + const struct weston_windowed_output_api *api;
> const char *section_name;
> - char *name;
> + char *output_name = NULL;
> + int count = 1;
> + int ret = 0;
> + int i;
> +
> + struct wet_output_config *parsed_options = wet_init_parsed_options(c);
> + if (!parsed_options)
> + return -1;
> +
> + config.cursor_size = 32;
> + config.cursor_theme = NULL;
> + config.display_name = NULL;
> + config.fullscreen = 0;
> + config.sprawl = 0;
> + config.use_pixman = 0;
>
> const struct weston_option wayland_options[] = {
> - { WESTON_OPTION_INTEGER, "width", 0, &width },
> - { WESTON_OPTION_INTEGER, "height", 0, &height },
> - { WESTON_OPTION_INTEGER, "scale", 0, &scale },
> - { WESTON_OPTION_STRING, "display", 0, &config->display_name },
> - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman },
> + { WESTON_OPTION_INTEGER, "width", 0, &parsed_options->width },
> + { WESTON_OPTION_INTEGER, "height", 0, &parsed_options->height },
> + { WESTON_OPTION_INTEGER, "scale", 0, &parsed_options->scale },
> + { WESTON_OPTION_STRING, "display", 0, &config.display_name },
> + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.use_pixman },
> { WESTON_OPTION_INTEGER, "output-count", 0, &count },
> - { WESTON_OPTION_BOOLEAN, "fullscreen", 0, &config->fullscreen },
> - { WESTON_OPTION_BOOLEAN, "sprawl", 0, &config->sprawl },
> + { WESTON_OPTION_BOOLEAN, "fullscreen", 0, &config.fullscreen },
> + { WESTON_OPTION_BOOLEAN, "sprawl", 0, &config.sprawl },
> };
>
> - width = 0;
> - height = 0;
> - scale = 0;
> - config->display_name = NULL;
> - config->use_pixman = 0;
> - count = 1;
> - conf
Re: [PATCH weston] gl-renderer: Use EGL_KHR_no_config_context
On Tue, Sep 27, 2016 at 12:29:51PM +0200, Armin Krezović wrote:
> This patch makes use of recently implemented
> EGL_KHR_no_config_context extension in Mesa,
> which superseeds EGL_MESA_configless_context.
>
> See also (and the follow-up patch):
>
> https://lists.freedesktop.org/archives/mesa-dev/2016-September/128510.html
>
> Signed-off-by: Armin Krezović
> ---
> libweston/gl-renderer.c| 14 +++---
> libweston/weston-egl-ext.h | 3 +++
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
> index 031576b..7ef7b89 100644
> --- a/libweston/gl-renderer.c
> +++ b/libweston/gl-renderer.c
> @@ -199,6 +199,8 @@ struct gl_renderer {
>
> int has_egl_buffer_age;
>
> + int has_no_config_context;
> +
> int has_configless_context;
You don’t need to keep two different booleans for that, both extensions
are implemented and exposed the same way, only the wording changes a
bit.
>
> int has_surfaceless_context;
> @@ -2564,10 +2566,11 @@ gl_renderer_output_create(struct weston_output
> *output,
> }
>
> if (egl_config != gr->egl_config &&
> + !gr->has_no_config_context &&
> !gr->has_configless_context) {
> weston_log("attempted to use a different EGL config for an "
> -"output but EGL_MESA_configless_context is not "
> -"supported\n");
> +"output but EGL_KHR_no_config_context or "
> +"EGL_MESA_configless_context is not supported\n");
> return -1;
> }
>
> @@ -2726,6 +2729,9 @@ gl_renderer_setup_egl_extensions(struct
> weston_compositor *ec)
> weston_log("warning: EGL_EXT_swap_buffers_with_damage not "
> "supported. Performance could be affected.\n");
>
> + if (weston_check_egl_extension(extensions, "EGL_KHR_no_config_context"))
> + gr->has_no_config_context = 1;
> +
> if (weston_check_egl_extension(extensions,
> "EGL_MESA_configless_context"))
> gr->has_configless_context = 1;
Same here, you can safely set gr->has_no_config_context instead.
>
> @@ -3101,7 +3107,9 @@ gl_renderer_setup(struct weston_compositor *ec,
> EGLSurface egl_surface)
>
> context_config = gr->egl_config;
>
> - if (gr->has_configless_context)
> + if (gr->has_no_config_context)
> + context_config = EGL_NO_CONFIG_KHR;
> + else if (gr->has_configless_context)
> context_config = EGL_NO_CONFIG_MESA;
And same here, the EGL_NO_CONFIG_KHR and EGL_NO_CONFIG_MESA values are
the same, so as long as you have both in the header (you do), it will
not be an issue.
>
> gr->egl_context = eglCreateContext(gr->egl_display, context_config,
> diff --git a/libweston/weston-egl-ext.h b/libweston/weston-egl-ext.h
> index 6e36996..50964a8 100644
> --- a/libweston/weston-egl-ext.h
> +++ b/libweston/weston-egl-ext.h
> @@ -152,5 +152,8 @@ typedef EGLSurface (EGLAPIENTRYP
> PFNEGLCREATEPLATFORMPIXMAPSURFACEEXTPROC) (EGLD
> #define EGL_PLATFORM_X11_KHR 0x31D5
> #endif
>
> +#ifndef EGL_NO_CONFIG_KHR
> +#define EGL_NO_CONFIG_KHR ((EGLConfig)0)
> +#endif
>
> #endif
> --
> 2.10.0
>
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
--
Emmanuel Gil Peyrot
Collabora Ltd.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] clients/stacking: Silence a compiler warning
This patch fixes a compiler warning when building with clang, since it doesn't support gnu_printf attribute. v2: - Switch to WL_PRINTF per suggestion from Eric Engestrom. Signed-off-by: Armin Krezović --- clients/stacking.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/stacking.c b/clients/stacking.c index 4285a17..7c711e4 100644 --- a/clients/stacking.c +++ b/clients/stacking.c @@ -184,7 +184,7 @@ fullscreen_handler(struct window *window, void *data) static void draw_string(cairo_t *cr, -const char *fmt, ...) __attribute__((format (gnu_printf, 2, 3))); +const char *fmt, ...) WL_PRINTF(2,3); static void draw_string(cairo_t *cr, -- 2.10.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] gl-renderer: Use EGL_KHR_no_config_context
This patch makes use of recently implemented
EGL_KHR_no_config_context extension in Mesa,
which superseeds EGL_MESA_configless_context.
See also (and the follow-up patch):
https://lists.freedesktop.org/archives/mesa-dev/2016-September/128510.html
Signed-off-by: Armin Krezović
---
libweston/gl-renderer.c| 14 +++---
libweston/weston-egl-ext.h | 3 +++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index 031576b..7ef7b89 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -199,6 +199,8 @@ struct gl_renderer {
int has_egl_buffer_age;
+ int has_no_config_context;
+
int has_configless_context;
int has_surfaceless_context;
@@ -2564,10 +2566,11 @@ gl_renderer_output_create(struct weston_output *output,
}
if (egl_config != gr->egl_config &&
+ !gr->has_no_config_context &&
!gr->has_configless_context) {
weston_log("attempted to use a different EGL config for an "
- "output but EGL_MESA_configless_context is not "
- "supported\n");
+ "output but EGL_KHR_no_config_context or "
+ "EGL_MESA_configless_context is not supported\n");
return -1;
}
@@ -2726,6 +2729,9 @@ gl_renderer_setup_egl_extensions(struct weston_compositor
*ec)
weston_log("warning: EGL_EXT_swap_buffers_with_damage not "
"supported. Performance could be affected.\n");
+ if (weston_check_egl_extension(extensions, "EGL_KHR_no_config_context"))
+ gr->has_no_config_context = 1;
+
if (weston_check_egl_extension(extensions,
"EGL_MESA_configless_context"))
gr->has_configless_context = 1;
@@ -3101,7 +3107,9 @@ gl_renderer_setup(struct weston_compositor *ec,
EGLSurface egl_surface)
context_config = gr->egl_config;
- if (gr->has_configless_context)
+ if (gr->has_no_config_context)
+ context_config = EGL_NO_CONFIG_KHR;
+ else if (gr->has_configless_context)
context_config = EGL_NO_CONFIG_MESA;
gr->egl_context = eglCreateContext(gr->egl_display, context_config,
diff --git a/libweston/weston-egl-ext.h b/libweston/weston-egl-ext.h
index 6e36996..50964a8 100644
--- a/libweston/weston-egl-ext.h
+++ b/libweston/weston-egl-ext.h
@@ -152,5 +152,8 @@ typedef EGLSurface (EGLAPIENTRYP
PFNEGLCREATEPLATFORMPIXMAPSURFACEEXTPROC) (EGLD
#define EGL_PLATFORM_X11_KHR 0x31D5
#endif
+#ifndef EGL_NO_CONFIG_KHR
+#define EGL_NO_CONFIG_KHR ((EGLConfig)0)
+#endif
#endif
--
2.10.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
