Re: [PATCH] wayland-server: update the client fd when it failed to flush with EAGAIN

[Derek Foreman]

On 11/22/18 11:08 PM, Jeonghyun Kang wrote:
> When a wayland compositor gets an EAGAIN error whenever
> sending or receiving event(s) to a client in the
> wl_closure_send() or the wl_closure_queue(), the error
> variable of the wl_client for the client will be set to
> true and the client is going to be killed by the
> compositor later soon. This patch fixes the problem by
> updating the socket fd's event mask as we're doing
> in wl_display_flush_clients() for having another chance
> to do it again.
> 
> Actually, this kind of problem can be watched in an
> environment in which massive input events are coming
> from multi-touch screen or multi-pen devices. In this
> kind of environment, a client receiving the massive
> input events are trying to drawing something very hard
> but it's being killed sooner or later. With the given
> patch, the client receiving the input events is not being
> destroyed and is working well even though it doesn't get
> the whole input events from the compositor.

This is actually intentional behavior to avoid infinitely growing send
queues for clients that process events too slowly.

I don't think dropping events randomly (or at all) because the client
can't keep up is something we should do in libwayland. It'll just make
for nearly impossible to reproduce and debug problems when client and
compositor state fall out of sync.

What if it's a frame event that gets dropped and not an input event?  Or
a resource destroy?

The disconnect is a clear indication that there's a problem that needs
to be fixed somewhere else. The client needs to process events in a
timely fashion, or the compositor needs to send less events.

Thanks,
Derek

> Signed-off-by: jeon  >
> 
> ---
>  src/wayland-server.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index eae8d2e..5afaa28 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -222,8 +222,15 @@ handle_array(struct wl_resource *resource, uint32_t
> opcode,
>  
>     log_closure(resource, closure, true);
>  
> -   if (send_func(closure, resource->client->connection))
> -   resource->client->error = 1;
> +   if (send_func(closure, resource->client->connection)) {
> +   if (errno == EAGAIN) {
> +   wl_event_source_fd_update(resource->client->source,
> + WL_EVENT_WRITABLE |
> + WL_EVENT_READABLE);
> +   } else {
> +   resource->client->error = 1;
> +   }
> +   }
>  
>     wl_closure_destroy(closure);
>  }
> 
> -- 
> 2.7.4
> 
> 
> ___
> 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-protocols v7] Add zwp_linux_explicit_synchronization_v1

[Tomek Bury]

Hi Pekka,

Yes, sorry, I was writing specifically about Weston implementation. In the
merge request from Alexandros the actual compatibility check is in the main
compositor, while compositor doesn't have enough information to decide
whether the selected renderer can handle buffer+fence combination or not.

As for the opaque wl_buffer, that's an internal implementation detail of
Vulkan WSI or EGL so different drivers can choose to do different things
here. It's the 3D driver, and *NOT* the client application, that creates
those buffers and sends attach/damage/commit sequence to the compositor.
The 3D driver makes a decision what type of buffer to use, an EGL or Vulkan
client application doesn't have any means of accessing wl_buffer objects,
it's all hidden away.

On the compositor side buffers are received through one of the protocol
extensions you've mentioned. The compositor has a choice to make. It can
either probe the wl_buffer for known buffer types or leave that task to the
EGL implementation. Let's say a client-side driver uses dma-buf buffers for
its swapchain images. If the compositor knows how to handle dma-buf, it can
either directly access those buffers or it can hand them over to the
compositor-side 3D driver through
eglCreateImage(dpy,  EGL_LINUX_DMA_BUF_EXT, ...). If the compositor doesn't
know the particular type of buffer, it can check if EGL can handle it
either through eglCreateImage(dpy,  EGL_WAYLAND_BUFFER_WL, ...) or
eglQueryWaylandBufferWL(...).
If the client and compositor use the same 3D driver (that's the most likely
scenario) this is bound to work. In multiple-GPU configurations your
mileage may vary though.

If the EGL implementation can use the type of the wl_buffer and can import
the fence fd, you're home. Having said that, I can't think of a way of
figuring out ahead of time what type of wl_buffer the client-side driver is
going to use for its swapchain and whether this particular type will be
accepted by the compositor-side driver.

This is how I use it at the moment: I've written a custom Weston backend
because the code runs on top of an embedded middleware. My backend always
uses GL renderer. The GL renderer has to call eglBindWaylandDisplayWL() at
startup, and the implementation of that API in the 3D driver adds a custom
Wayland protocol extension for sharing buffers. Now the scene is set. When
a Wayland client application starts, the EGL or Vulkan WSI implementation
driver goes for that extension and bails out if unavailable. This way
swapchain buffers from EGL and Vulkan client can be used by Weston's GL
renderer without any knowledge of the embedded platform details.

With regards to using fences directly by the client app, I guess it's the
same principle as drawing into the window. Either client app does
everything "by hand" or lets the Vulkan or EGL/GLES do it. If the app is in
charge, the app manages the window swap chain buffers and synchronization,
otherwise the 3D driver does it. You shouldn't allow more than one thing
managing the Wayland window at the same time. Perhaps you could use wording
similar to Vulkan WSI or EGL window surface when describing what is and
what isn't allowed when it comes to Wayland windows.

Cheers,
Tomek



On Mon, 26 Nov 2018 at 09:22, Pekka Paalanen  wrote:

> On Fri, 23 Nov 2018 16:26:19 +
> Tomek Bury  wrote:
>
> > Hi Pekka,
> >
> > > I presume you have a driver stack that relies on the opaque EGL
> buffers
> > and not zwp_linux_dmabuf any time soon?
> > Yes, exactly. I've added a protocol extension for sharing those buffers
> and
> > our eglCreateImage() implementation can import such buffers into the
> driver
> > on the compositor end. The buffers are carried by an fd to the compositor
> > that's the only similarity. They're not dma-buf.
> >
> > > Yeah, support for opaque EGL buffers could be added, just need to
> think
> > of a good wording, since acquire fences do not make sense for all buffer
> > types.
>
> > Isn't that renderer's and/or backend's decision? The GL renderer can
> accept
> > fence with any buffer it can send to the 3D driver, so, effectively,
> > anything backed by available EGL image extensions. Someone may add a
> custom
> > backend and/or renderer using whatever hardware or API they have at
> hand. A
> > Vulkan renderer could potentially use fences with anything a Vulkan
> driver
> > is capable of importing. A renderer that does the CPU wait could be
> useful
> > at least for debugging. So I wouln't block the explicit sync at the
> > compositor level based on the white list.
>
> Hi Tomek,
>
> fences do not make sense to all buffer types to begin with, today. My
> objection is to allowing fencing buffer types that cannot be sent to
> the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
> and friends. We cannot ignore those in the spec language.
>
> A renderer (a compositor really, we're not talking about just Weston)
> decides what buffer types it accepts, yes. This is communicated to
> 

Re: [PATCH wayland-protocols v7] Add zwp_linux_explicit_synchronization_v1

[Pekka Paalanen]

On Fri, 23 Nov 2018 16:26:19 +
Tomek Bury  wrote:

> Hi Pekka,
> 
> > I presume you have a driver stack that relies on the opaque EGL buffers  
> and not zwp_linux_dmabuf any time soon?
> Yes, exactly. I've added a protocol extension for sharing those buffers and
> our eglCreateImage() implementation can import such buffers into the driver
> on the compositor end. The buffers are carried by an fd to the compositor
> that's the only similarity. They're not dma-buf.
> 
> > Yeah, support for opaque EGL buffers could be added, just need to think  
> of a good wording, since acquire fences do not make sense for all buffer
> types.

> Isn't that renderer's and/or backend's decision? The GL renderer can accept
> fence with any buffer it can send to the 3D driver, so, effectively,
> anything backed by available EGL image extensions. Someone may add a custom
> backend and/or renderer using whatever hardware or API they have at hand. A
> Vulkan renderer could potentially use fences with anything a Vulkan driver
> is capable of importing. A renderer that does the CPU wait could be useful
> at least for debugging. So I wouln't block the explicit sync at the
> compositor level based on the white list.

Hi Tomek,

fences do not make sense to all buffer types to begin with, today. My
objection is to allowing fencing buffer types that cannot be sent to
the 3D driver, e.g. wl_shm which is usually copied through glTexImage2D
and friends. We cannot ignore those in the spec language.

A renderer (a compositor really, we're not talking about just Weston)
decides what buffer types it accepts, yes. This is communicated to
clients through which buffer factory interface globals are being
advertised. Each type is a different protocol extension. The fence
extension OTOH is just a single extension, and currently there is no
protocol to negotiate which buffer types are usable with acquire
fences. The first attempt is to define in the spec language what will
always be supported, provided the buffer factory exists.

The opaque EGL buffer type is really just one type in practise:
compositors and clients use it through a well-known, single API: EGL.
It does not matter that there are multiple incompatible EGL
implementations, it all looks like just one opaque buffer type to
compositors. I think this makes it easier to extend the fence spec
wording to require opaque EGL buffers to be supported.

Either the fence protocol spec needs to be clear on what works, or we
need advertisement events to let clients know in advance what the
compositor supports. A client sending a fence that the compositor
cannot use must not be possible; compositor, client, EGL, driver, etc.
bugs notwithstanding.

Btw. I just realized that if client-side EGL uses the fence extension
internally, that means the client app code must not attempt to add or
request fences of its own, because the spec disallows multiple acquire
fences and multiple release notification requests. I suppose that's
fine?

Alf, can you come up with changes to the spec wording and Weston to
require opaque EGL buffers are supported?

On one hand it is actually a little strange to couple opaque EGL
buffers (a private, EGL implementation specific protocol interface)
with a generic fencing extension, but maybe that is necessary because
there is not enough compositor-side GBM and EGL API so that the EGL
implementation could handle it all in an EGL implementation specific
interface?


Thanks,
pq

> On Fri, 23 Nov 2018 at 13:47, Pekka Paalanen  wrote:
> 
> > On Fri, 23 Nov 2018 13:07:37 +
> > Tomek Bury  wrote:
> >  
> > > Hi Alexandros,
> > >
> > > Sorry for a delay. I've finally got an end-to-end system to test it out.  
> > It  
> > > took some time because Weston backend I wrote a while back needed serious
> > > rework to catch up with latest changes.
> > >
> > > There's one thing that didn't work for me. In compositor you reject
> > > anything that isn't a DMA buffer and then in glrenderer you put an extra
> > > assertion. Why? All you do is use an EGL extension in order to import
> > > external fence_fd. There's no dmabuf dependency there. As long as the EGL
> > > implementation exposes EGL_SYNC_NATIVE_FENCE_ANDROID extension this  
> > should  
> > > "just work" (tm) for the GL renderer. It certainly did for me. CPU-based
> > > renderers can poll() to wait.  
> >
> > Hi Tomek,
> >
> > with Weston it was decided not to implement a poll() based wait at
> > first as implementing that properly (not blocking the compositor) would
> > be a big hassle and no-one could see the benefit of it given what
> > clients could actually produce.
> >
> > Therefore the acquire fence can only apply to buffers which can be
> > pipelined to a GPU. Mesa EGL is using zwp_linux_dmabuf, but the support
> > could be extended to opaque EGL buffers very well. We just chose to
> > start small and bring up the infrastructure around fences first.
> >
> > Restrictions on buffer types etc. can certainly be lifted in the