[RFC] Weston GL shader compilation re-design
Hi, This is in continuation to the discussion in the mail chain here [1]. We are currently working on enabling HDR rendering support in the gl-renderer. HDR support requires color space conversion (709->2020 or other way around) and Tone Mapping, for which, we need to execute the following steps in the shader: * Degamma (to linearize the buffer) * Color space conversion (BT2020 -> BT709...) * Tone mapping (SDR -> HDR, HDR -> SDR, HDR -> HDR conversion) * Gamma: Apply gamma/transfer function for the display (PQ/HLG...) We are currently targeting the DCI-P3, SRGB and 2084 color spaces. One way to implement it would be to create multiple sets of shaders for each degamma/gamma combination. Like the one done by Ville in his POC [2]. This solution was a POC, and isn't scalable when we need to support multiple color spaces. I would like to propose 2 different solutions: Proposal 1: * Each of the shaders (gamma/degamma/main/tone mapping) would be independent strings. * When the view needs to be rendered, renderer will generate a set of requirements like need degamma PQ curve, need tone mapping, need gamma SRGB curve and so on. * These requirements (NEED_GAMMA_PQ...) would be bit fields and will be OR'd to create the total requirements. * This total requirement would be used to stitch the required shaders to compile a program. The total requirements integer will also be used as a key to store that program in a hashmap for re-use. Proposal 2: We could go for multi pass rendering, where each of the steps (gamma, csc, tone mappping, degamma) will be applied in different passes. Each pass would bind an fbo and call draw elements with the corresponding shader. This would not be as efficient but is another approach. Please comment on the approaches and do suggest if there is a better one. Thank you Regards Harish Krupo [1] https://lists.freedesktop.org/archives/wayland-devel/2019-January/039809.html [2] https://github.com/vsyrjala/weston/blob/hdr_poc/libweston/gl-renderer.c#L257 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Fbdev-backend removal from Weston (Re: Upcoming release)
On Tuesday, January 22, 2019 9:55:15 AM EST Pekka Paalanen wrote: > Let's make the title more catchy, so that people who care about fbdev > would notice. > > > On Tue, 22 Jan 2019 10:17:32 +0200 > Pekka Paalanen wrote: > > > On Mon, 21 Jan 2019 20:30:52 -0500 > > nerdopolis wrote: > > > > > On Friday, January 18, 2019 5:20:40 PM EST Derek Foreman wrote: > > > > Also, I'd like to float the idea of removing the fbdev backend for this > > > > release (or the next). The bulk of the interesting features target the > > > > drm backend, and it feels like fbdev can sometimes be a bit of a pain to > > > > update when changes are required elsewhere. Any strong opinions either > > > > way? > > > > > I feel like the fbdev backend is a good fallback still. Maybe not for > > > Virtual > > > Box anymore, since there is now a driver for it in Mainline (I haven't > > > tested > > > it personally though) ...Currently the Framebuffer backend is the only > > > way to > > > get Weston working on some obscure hardware such as UDL/DisplayLink2 > > > devices. > > > > Hi, > > > > do you actually have a use case for that? It's ok if you do, we can > > keep it around for some more, but the simple fact that some hardware > > exists that does not have a working DRM driver is not enough. There > > should be users too. > > > > The fbdev-backend certainly has its shortcomings, and I don't mean just > > the lack of multi-output or monitor hotplug or GPU acceleration or > > tearing or timings based on nothing but timers; I recall VT-switching > > being broken for years now, and it doesn't (cannot?) use logind to open > > the FB devices. Did I forget some? > > > > > > Thanks, > > pq > Hi Well, TBH, It's just my quazi-distribution Live CD that I still maintain... It falls back to the Framebuffer on systems (or seats) that don't have Kernel Mode Setting. The number of users now probably is low... VT switching works great. It works without weston-launch, and I can switch away to a second instance of Weston using the Framebuffer, without them stepping on each other, and I can switch back to the original one, and it continues working I do have to create a udev file that sets SUBSYSTEM=="graphics", KERNEL=="fb*", TAG+="uaccess" though... Thanks ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Fbdev-backend removal from Weston (Re: Upcoming release)
Let's make the title more catchy, so that people who care about fbdev would notice. On Tue, 22 Jan 2019 10:17:32 +0200 Pekka Paalanen wrote: > On Mon, 21 Jan 2019 20:30:52 -0500 > nerdopolis wrote: > > > On Friday, January 18, 2019 5:20:40 PM EST Derek Foreman wrote: > > > Also, I'd like to float the idea of removing the fbdev backend for this > > > release (or the next). The bulk of the interesting features target the > > > drm backend, and it feels like fbdev can sometimes be a bit of a pain to > > > update when changes are required elsewhere. Any strong opinions either > > > way? > > > I feel like the fbdev backend is a good fallback still. Maybe not for > > Virtual > > Box anymore, since there is now a driver for it in Mainline (I haven't > > tested > > it personally though) ...Currently the Framebuffer backend is the only way > > to > > get Weston working on some obscure hardware such as UDL/DisplayLink2 > > devices. > > Hi, > > do you actually have a use case for that? It's ok if you do, we can > keep it around for some more, but the simple fact that some hardware > exists that does not have a working DRM driver is not enough. There > should be users too. > > The fbdev-backend certainly has its shortcomings, and I don't mean just > the lack of multi-output or monitor hotplug or GPU acceleration or > tearing or timings based on nothing but timers; I recall VT-switching > being broken for years now, and it doesn't (cannot?) use logind to open > the FB devices. Did I forget some? > > > Thanks, > pq pgp6Onkuf6qm9.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/3] client: Send errors should be fatal but not abort
On Fri, 7 Sep 2018 18:14:03 -0700
Lloyd Pique wrote:
> If the core-client code encounters an error when doing an internal flush
> because the send buffers are full, the previous behavior is to
> wl_abort(). This is not very friendly if the client is a nested server
> running as a service.
>
> This patch instead sets a fatal display error, which is one step
> better, and causes the send logic to do nothing once a fatal display
> error is set. The client should eventually fall back to its main loop,
> and be able to detect the fatal error there.
>
> The existing display test is extended to exercise the error state.
>
> Signed-off-by: Lloyd Pique
> ---
> COPYING | 1 +
> src/wayland-client.c | 9 +-
> tests/display-test.c | 77
> 3 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/COPYING b/COPYING
> index eb25a4e..bace5d7 100644
> --- a/COPYING
> +++ b/COPYING
> @@ -2,6 +2,7 @@ Copyright © 2008-2012 Kristian Høgsberg
> Copyright © 2010-2012 Intel Corporation
> Copyright © 2011 Benjamin Franzke
> Copyright © 2012 Collabora, Ltd.
> +Copyright © 2018 Google LLC
Hi Lloyd,
would you like to add this in wayland-client.c as well?
The overall logic looks good, but there are details to discuss below.
>
> Permission is hereby granted, free of charge, to any person obtaining a
> copy of this software and associated documentation files (the "Software"),
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 0ccfc66..29ae1e0 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -736,6 +736,9 @@ wl_proxy_marshal_array_constructor_versioned(struct
> wl_proxy *proxy,
> goto err_unlock;
> }
>
> + if (proxy->display->last_error)
> + goto err_unlock;
> +
> closure = wl_closure_marshal(>object, opcode, args, message);
> if (closure == NULL)
> wl_abort("Error marshalling request: %s\n", strerror(errno));
> @@ -744,7 +747,11 @@ wl_proxy_marshal_array_constructor_versioned(struct
> wl_proxy *proxy,
> wl_closure_print(closure, >object, true);
>
> if (wl_closure_send(closure, proxy->display->connection))
> - wl_abort("Error sending request: %s\n", strerror(errno));
> + // Convert EAGAIN into EPIPE, since EAGAIN is not really
> + // supposed to be fatal, especially when returned by
> + // wl_display_flush().
The style is to use /* */ comments.
> + display_fatal_error(proxy->display,
> + errno != EAGAIN ? errno : EPIPE);
>
> wl_closure_destroy(closure);
The logic in wayland-client.c looks good. I'm just wondering if EPIPE is
a good error code to return. EPIPE can result otherwise as well,
particularly when the server disconnects.
Could we use something else like ENOMEM or ENOSPC that would be more
unique? I see ENOMEM already used, so maybe ENOSPC?
I checked, and libwayland-client does not seem to be checking
last_error against EPIPE or really any other specific value, so I think
the concern is about what the users of the library will do. FWIW,
Weston has no occurrences of EPIPE.
>
> diff --git a/tests/display-test.c b/tests/display-test.c
> index 9b49a0e..23f0635 100644
> --- a/tests/display-test.c
> +++ b/tests/display-test.c
> @@ -1315,3 +1315,80 @@ TEST(zombie_fd_errant_consumption)
>
> display_destroy(d);
> }
> +
> +static void
> +terminate_on_bind_attempt(struct wl_client *client, void *data,
> + uint32_t version, uint32_t id)
> +{
> + wl_display_terminate(((struct display *)data)->wl_display);
> +}
> +
> +static void
> +fill_client_send_buffers(struct wl_display *display,
> + void (*send_check)(struct wl_display *, int *), int *send_check_result)
Align this line with "struct" above and split the last argument to a
new line.
> +{
> + int sync_count = 4096;
> + int socket_buf = 1024;
> +
> + /* Use the minimum send buffer size. We want the client to be
> + * able to fill the buffer easily. */
> + assert(setsockopt(wl_display_get_fd(display), SOL_SOCKET, SO_SNDBUF,
> + _buf, sizeof(socket_buf)) == 0);
One more space of alignment to right.
> +
> + /* Fill up the client and server buffers With the default error
> + * handling, this will abort the client. */
> + while(sync_count--) {
Add space between "while" and "(".
> + wl_callback_destroy(wl_display_sync(display));
This should consume 12 bytes per call IIRC, so it should easily fill up
the 4096+1024 bytes of wl_buffer and socket send buffer. Ok.
> + if (send_check != NULL) {
> + send_check(display, send_check_result);
> + }
> + }
> +
> + wl_display_roundtrip(display);
> +}
> +
> +static void
> +send_error_client(void)
> +{
> + struct client *c = client_connect();
> +
> + /* Try and bind a wl_set,
Re: Upcoming release
On Mon, 21 Jan 2019 20:30:52 -0500 nerdopolis wrote: > On Friday, January 18, 2019 5:20:40 PM EST Derek Foreman wrote: > > Also, I'd like to float the idea of removing the fbdev backend for this > > release (or the next). The bulk of the interesting features target the > > drm backend, and it feels like fbdev can sometimes be a bit of a pain to > > update when changes are required elsewhere. Any strong opinions either way? > I feel like the fbdev backend is a good fallback still. Maybe not for Virtual > Box anymore, since there is now a driver for it in Mainline (I haven't tested > it personally though) ...Currently the Framebuffer backend is the only way to > get Weston working on some obscure hardware such as UDL/DisplayLink2 devices. Hi, do you actually have a use case for that? It's ok if you do, we can keep it around for some more, but the simple fact that some hardware exists that does not have a working DRM driver is not enough. There should be users too. The fbdev-backend certainly has its shortcomings, and I don't mean just the lack of multi-output or monitor hotplug or GPU acceleration or tearing or timings based on nothing but timers; I recall VT-switching being broken for years now, and it doesn't (cannot?) use logind to open the FB devices. Did I forget some? Thanks, pq pgpuKn4ox0aP7.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
