Re: [PATCH v2 2/6] shell: register output-destroy_signal handler
On Wed, Oct 23, 2013 at 01:58:32PM +0800, Xiong Zhang wrote:
setup_output_destroy_handler() deal with output created at
drm backend initialize time.
handle_output_create() deal with output created by hot plug handler
output_destroy_handler is removed when output was unplugged or
shell is destroyed.
Signed-off-by: Xiong Zhang xiong.y.zh...@intel.com
---
src/shell.c | 75
+
1 file changed, 75 insertions(+)
I applied this one too, with a couple of renames. I try to avoid
editing patches as I apply to make sure we discuss all changes here on
the list. I this case the changes are mostly renames and I'd like to
get these output unplug fixes in soon so we can do a 1.3.1 release
with them. See below for what I changed.
I'll try to review the remaining patches tomorrow.
Kristian
diff --git a/src/shell.c b/src/shell.c
index f033e8c..2a9c024 100644
--- a/src/shell.c
+++ b/src/shell.c
@@ -85,6 +85,13 @@ struct input_panel_surface {
uint32_t panel;
};
+struct shell_output {
+ struct desktop_shell *shell;
+ struct weston_output *output;
+ struct wl_listenerdestroy_listener;
+ struct wl_listlink;
+};
+
struct desktop_shell {
struct weston_compositor *compositor;
@@ -163,6 +170,9 @@ struct desktop_shell {
uint32_t binding_modifier;
enum animation_type win_animation_type;
+
+ struct wl_listener output_create_listener;
+ struct wl_list output_destroy_listener_list;
Since we renamed output_destroy_listener to just shell_output, I
renamed the output_destroy_listener_list to just output_list.
};
enum shell_surface_type {
@@ -4461,11 +4471,65 @@ workspace_move_surface_down_binding(struct
weston_seat *seat, uint32_t time,
}
static void
+handle_output_destroy(struct wl_listener *listener, void *data)
+{
+ struct shell_output *output_listener =
+ container_of(listener, struct shell_output, destroy_listener);
And here, I renamed output_listener to shell_output.
+ wl_list_remove(output_listener-destroy_listener.link);
+ wl_list_remove(output_listener-link);
+ free(output_listener);
+}
+
+static void
+create_shell_output(struct desktop_shell *shell,
+ struct weston_output *output)
+{
+ struct shell_output *output_listener;
+
+ output_listener = malloc(sizeof(*output_listener));
+ memset(output_listener, 0, sizeof(*output_listener));
Use zalloc instead of malloc+memset. Also we need to handle OOM here.
I changed this to just return if zalloc fails, which means that we
fail to set up the shell_output and can't detect unplug of that
output.
+ output_listener-output = output;
+ output_listener-shell = shell;
+ output_listener-destroy_listener.notify = handle_output_destroy;
+ wl_signal_add(output-destroy_signal,
+ output_listener-destroy_listener);
+ wl_list_insert(shell-output_destroy_listener_list.prev,
+ output_listener-link);
+}
+
+static void
+handle_output_create(struct wl_listener *listener, void *data)
+{
+ struct desktop_shell *shell =
+ container_of(listener, struct desktop_shell,
output_create_listener);
+ struct weston_output *output = (struct weston_output *)data;
+
+ create_shell_output(shell, output);
+}
+
+static void
+setup_output_destroy_handler(struct weston_compositor *ec,
+ struct desktop_shell
*shell)
+{
+ struct weston_output *output;
+
+ wl_list_init(shell-output_destroy_listener_list);
+ wl_list_for_each(output, ec-output_list, link)
+ create_shell_output(shell, output);
+
+ shell-output_create_listener.notify = handle_output_create;
+ wl_signal_add(ec-output_created_signal,
+ shell-output_create_listener);
+}
+
+static void
shell_destroy(struct wl_listener *listener, void *data)
{
struct desktop_shell *shell =
container_of(listener, struct desktop_shell, destroy_listener);
struct workspace **ws;
+ struct shell_output *output_listener, *tmp;
if (shell-child.client)
wl_client_destroy(shell-child.client);
@@ -4475,6 +4539,15 @@ shell_destroy(struct wl_listener *listener, void *data)
wl_list_remove(shell-show_input_panel_listener.link);
wl_list_remove(shell-hide_input_panel_listener.link);
+ wl_list_for_each_safe(output_listener, tmp,
shell-output_destroy_listener_list,
+ link) {
+ wl_list_remove(output_listener-destroy_listener.link);
+ wl_list_remove(output_listener-link);
+ free(output_listener);
+ }
+
+ wl_list_remove(shell-output_create_listener.link);
+
wl_array_for_each(ws, shell-workspaces.array)
Re: Buffer release events (was: Add support for eglSwapInterval)
Hi Thanks for the interesting insights. It seems to me as if the default should always be to just send the event. I think I would vote for leaving the default as it is, ie, queuing the release events. It's really quite a corner case that delaying events has any effect on an application because most applications don't need to know about the release events until they are about to draw something. Usually they would only draw something in response to some event such as a frame callback or an input event. In that case the event will have caused the queue to flush so they will certainly be up-to-date about what buffers are available at the point when they start drawing. If we default to not queuing the event then I'd imagine most applications wouldn't realise they should enable it and would miss out on the optimisation. We can identify buffer release events in weston as coming from one of three sources: 1) wl_surface.commit 2) surface_flush_damage (the gl renderer releases SHM buffers here) 3) random releases from the backdend/renderer Number 2 above happens during the redraw loop so we can just post the event and won't get a double-wakeup. Yes, I guess even if the compositor posts the event it's not going to actually send it to the client until the compositor goes idle again anyway and at that point it will probably have posted a frame complete callback too so the client would wake up anyway. Number 3 is something we can't really control; I'd personally lean towards posting the event here, but it's probably at most one reference per surface so we can probably get away with queueing. (Also, if the backend knows when it would release in the render cycle, it may be able to optimize this better than some compositor-general solution.) For these two, we can add an argument to weston_buffer_reference to set the release event type. I think case number 3 is the main problem. It's useful for most fullscreen apps to have the event queued because most of them will be throttled to the frame callback and don't need the release events immediately. However this is also the use case most likely to want eglSwapInterval(0) which would want them immediately so really for this situation it is an application choice whether they should be queued or not. Number 1 above is the source of the vast majority of out release events. [...] The good news is that we can, from a client perspective, deal with this one easily. The solution is to send a wl_display.sync request immediately after the commit. Yes, I think it makes sense to always sync the rendering to at least a wl_display.sync call and the Mesa patch I sent does already do this. You are right that in practice this effectively solves the problem for most use cases. So really the only case where this matters is when the compositor is directly scanning out from the client's buffer. But on the other hand, that is exactly what a fullscreen game is likely to be doing and that is the most likely candidate for doing eglSwapInterval(0). In any case, dummy sync and frame requests (you may need both) will allow you to achieve glSwapInterval(0) without server-side support. I'm not sure I follow you here. The release event may be queued at any point after the frame complete is sent. In that case sending a sync event to flush the queue is only going to help if Mesa sends it repeatedly, but that amounts to busy-waiting which would be terrible. I still feel like the new request is the right way to go. The difficulty with interface versioning feels like a separate wider problem that we should think about. The crux of the problem is that Mesa probably shouldn't be using proxy objects that are created outside of Mesa because in that case it doesn't have control over what interface version or event queue it is using. Working around the need for the new request would just side-step the issue but it doesn't seem unlikely that Mesa would want to use further new surface interfaces requests later too and the same problem would come back. Maybe we should have a separate object that you can create per surface in order to do new requests. This could be created by a new global object much like the wl_shell interface. In order to make it usable to both Mesa and the application, we would have to allow creating multiple resources of the new interface for a single surface. I'm not sure what to call it though because it would just end up being something like ‘wl_surface_extra_stuff’. Considering that other objects may end up also needing a similar kind of interface, maybe it would make more sense to rethink it a bit and make the compositor allow multiple resources for an object in general. Then you could have something like wl_compositor.rebind_resource(object, version) which would make a new resource for any object and it could have its own interface version. I am just thinking aloud here though, I haven't really thought that through much. I will take a look at how much hassle
Re: Buffer release events (was: Add support for eglSwapInterval)
On Wed, 23 Oct 2013, Jason Ekstrand wrote: There may also be a way that we can sidestep the whole issue. (I suggested this to Axel Davy [mannerov] and it worked for him in his wlglamor DDX.) The solution is to send a wl_display.sync request immediately after the commit. This will force any queued wl_buffer.release events to get sent out immediately. The client can even destroy the returned proxy immediately and completely ignore the resulting event. This seems a bit hackish, but this is exactly the kind of thing the sync request is intended for: flushing the stream. Well I can give my feedback about it: I send a dummy sync request after the commit, and I don't have to care about it. I don't even test if the sync callback was called. I don't need to use dummy frame listeners, and using the dummy sync request solves the issues in all the cases. When the application is fullscreen and the buffers are used as scanout, I don't have any issue too with release events. I'm not sure however this is the best way to solve the issue. As mentionned by Jason, this can be used as a fallback for a compositor not supporting your new wl_surface extension.___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Misc. fixes for the rpi backend
Except for the first one, all are intended to fix the RPi backend after the landing of the views work. Most EGL clients won't work yet because of wl_buffer.release events being always queued, though. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/5] rpi: Link with EGL if ENABLE_EGL
--- src/Makefile.am | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/Makefile.am b/src/Makefile.am index 4224495..9925129 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -224,6 +224,12 @@ rpi_backend_la_SOURCES = \ evdev.c \ evdev.h \ evdev-touchpad.c + +if ENABLE_EGL +rpi_backend_la_LIBADD += $(EGL_LIBS) +rpi_backend_la_CFLAGS += $(EGL_CFLAGS) +endif + endif if ENABLE_HEADLESS_COMPOSITOR -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/5] rpi: Initialize surface's list of views
--- src/rpi-renderer.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c index ea48b08..0b99a60 100644 --- a/src/rpi-renderer.c +++ b/src/rpi-renderer.c @@ -371,6 +371,7 @@ rpir_surface_create(struct rpi_renderer *renderer) if (!surface) return NULL; + wl_list_init(surface-views); surface-visible_views = 0; surface-single_buffer = renderer-single_buffer; rpi_resource_init(surface-resources[0]); -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 4/5] rpi: EGL surfaces need to be swapped always that we have an incoming back buffer
---
src/rpi-renderer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 2db619c..6478838 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -1255,7 +1255,8 @@ rpi_renderer_repaint_output(struct weston_output *base,
if (!wv-surface-touched) {
wv-surface-touched = 1;
- if (view-surface-need_swap)
+ if (view-surface-buffer_type == BUFFER_TYPE_EGL ||
+ view-surface-need_swap)
rpir_surface_swap_pointers(view-surface);
}
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 3/5] rpi: Fix logging of buffer swaps for the EGL case
---
src/rpi-renderer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 0b99a60..2db619c 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -969,13 +969,14 @@ rpir_surface_swap_pointers(struct rpir_surface *surface)
surface-egl_old_front = surface-egl_front;
surface-egl_front = surface-egl_back;
surface-egl_back = NULL;
+ DBG(new front %d\n,
surface-egl_front-resource_handle);
}
} else {
tmp = surface-front;
surface-front = surface-back;
surface-back = tmp;
+ DBG(new back %p, new front %p\n, surface-back,
surface-front);
}
- DBG(new back %p, new front %p\n, surface-back, surface-front);
}
static int
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 5/5] rpi: Only check for prematurely destroyed wl_buffers in the EGL case
---
src/rpi-renderer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c
index 6478838..812e6e7 100644
--- a/src/rpi-renderer.c
+++ b/src/rpi-renderer.c
@@ -1260,8 +1260,9 @@ rpi_renderer_repaint_output(struct weston_output *base,
rpir_surface_swap_pointers(view-surface);
}
- if (view-surface-egl_front-buffer_ref.buffer == NULL) {
- weston_log(warning: client unreffed current front
buffer\n);
+ if (view-surface-buffer_type == BUFFER_TYPE_EGL
+ view-surface-egl_front-buffer_ref.buffer == NULL) {
+ weston_log(warning: client destroyed current front
buffer\n);
wl_list_remove(view-link);
if (view-handle == DISPMANX_NO_HANDLE) {
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] input: Don't leak the initial keymap
weston_xkb_info_create() takes ownership of the xkb_keymap instance so we should drop our reference or we would leak it later if the keymap was changed. --- src/input.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/input.c b/src/input.c index 2fed718..da89b47 100644 --- a/src/input.c +++ b/src/input.c @@ -1706,6 +1706,7 @@ weston_compositor_build_global_keymap(struct weston_compositor *ec) } ec-xkb_info = weston_xkb_info_create(keymap); + xkb_keymap_unref(keymap); if (ec-xkb_info == NULL) return -1; -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC 1/5] Add a fullscreen shell protocol
On Wed, Oct 23, 2013 at 06:08:29PM -0500, Jason Ekstrand wrote: Jonas, Thanks for the review! On Wed, Oct 23, 2013 at 3:56 PM, Jonas Ådahl jad...@gmail.com wrote: Hi, Using this protocol, how would the fullscreen shell client turn on and off outputs? I can see how clone, next-to and modeset by controlling what surface is presented and what fullscreen method is used, but that is not enough for turning an output off, isn't it? I think that would be done by simply calling present_surface with a null surface to remove the surface from the output. I'm not 100% sure how this will interact with KMS surface detection, but hopefully we should be able to make it that simple. I guess this could be added to the protocol clarifing what presenting a null-surface means disabling the output. What disabling means, could be up to the backend. Another question is synchronising. Would a client ever want to change anything atomically? Do we need something like wl_surface.commit? I'm not sure what would get changed atomically. There's only one operation and different outputs will probably have different refresh rates so changes won't be atomic anyway. Or are you thinking about trying to future-proof stuff? Well, yea, atomically is probably not the correct term, and I don't know if the KMS backend support any such functionality. What I imagine is one might want to, for example, disable one output and enable another with minimal time in between, or maybe avoid ending up with no output enabled if the client would crash between calls. Might be overkill though, and if there is no real world use case right now, maybe even more so. Jonas Also, one wording comment below. Thanks Jonas On Tue, Oct 22, 2013 at 08:48:25PM -0500, Jason Ekstrand wrote: --- protocol/fullscreen-shell.xml | 44 +++ 1 file changed, 44 insertions(+) create mode 100644 protocol/fullscreen-shell.xml diff --git a/protocol/fullscreen-shell.xml b/protocol/fullscreen-shell.xml new file mode 100644 index 000..b696828 --- /dev/null +++ b/protocol/fullscreen-shell.xml @@ -0,0 +1,44 @@ +protocol name=fullscreen_shell + interface name=wl_fullscreen_shell version=1 +description summary=Displays a single surface per output + Displays a single surface per output. + + This interface can only be bound to by one client at a time. +/description + +enum name=fullscreen_method + description summary=different method to set the surface fullscreen + Hints to indicate to the compositor how to deal with a conflict + between the dimensions of the surface and the dimensions of the + output. The compositor is free to ignore this parameter. + /description + entry name=default value=0 summary=no preference, apply default policy/ + entry name=scale value=1 summary=scale, preserve the surface's aspect ratio and center on output/ + entry name=driver value=2 summary=switch output mode to the smallest mode that can fit the surface, add black borders to compensate size mismatch/ + entry name=fill value=3 summary=no upscaling, center on output and add black borders to compensate size mismatch/ +/enum + +request name=present_surface + description summary=present surface for display + This requests the system compositor to display surface on output. + Each client of the system compositor can have at most one surface You still use the term 'system compositor' here. Also, it should be display a surface instead of display surface. Thanks. Good catch. I've fixed it locally. + per output at any one time. Subsequent requests with the same + output replace the surface bound to that output. The same surface + may be presented on multiple outputs. + + If the output is null, the compositor will present the surface on + whatever display (or displays) it thinks best. In particular, this + may replace any or all surfaces currently presented so it should + not be used in combination with placing surfaces on specific + outputs. + + The method specifies how the surface is to be persented. These + methods are identical to those in wl_shell_surface.set_fullscreen. + /description + arg name=surface type=object interface=wl_surface/ + arg name=method type=uint/ + arg name=framerate type=uint/ + arg name=output type=object interface=wl_output allow-null=true/ +/request + /interface +/protocol -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___
Re: Misc. fixes for the rpi backend
This whole series looks good to me. Thanks Tomeu. When I rebased on your egl patches I couldn't test due to lack of packages in raspbian. --Jason Ekstrand On Oct 24, 2013 9:22 AM, Tomeu Vizoso to...@tomeuvizoso.net wrote: Except for the first one, all are intended to fix the RPi backend after the landing of the views work. Most EGL clients won't work yet because of wl_buffer.release events being always queued, though. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Buffer release events (was: Add support for eglSwapInterval)
On Thu, Oct 24, 2013 at 11:26:08AM +0100, Neil Roberts wrote: Hi Thanks for the interesting insights. It seems to me as if the default should always be to just send the event. I think I would vote for leaving the default as it is, ie, queuing the release events. It's really quite a corner case that delaying events has any effect on an application because most applications don't need to know about the release events until they are about to draw something. Usually they would only draw something in response to some event such as a frame callback or an input event. In that case the event will have caused the queue to flush so they will certainly be up-to-date about what buffers are available at the point when they start drawing. If we default to not queuing the event then I'd imagine most applications wouldn't realise they should enable it and would miss out on the optimisation. We can identify buffer release events in weston as coming from one of three sources: 1) wl_surface.commit 2) surface_flush_damage (the gl renderer releases SHM buffers here) 3) random releases from the backdend/renderer Number 2 above happens during the redraw loop so we can just post the event and won't get a double-wakeup. Yes, I guess even if the compositor posts the event it's not going to actually send it to the client until the compositor goes idle again anyway and at that point it will probably have posted a frame complete callback too so the client would wake up anyway. Number 3 is something we can't really control; I'd personally lean towards posting the event here, but it's probably at most one reference per surface so we can probably get away with queueing. (Also, if the backend knows when it would release in the render cycle, it may be able to optimize this better than some compositor-general solution.) For these two, we can add an argument to weston_buffer_reference to set the release event type. I think case number 3 is the main problem. It's useful for most fullscreen apps to have the event queued because most of them will be throttled to the frame callback and don't need the release events immediately. However this is also the use case most likely to want eglSwapInterval(0) which would want them immediately so really for this situation it is an application choice whether they should be queued or not. Number 1 above is the source of the vast majority of out release events. [...] The good news is that we can, from a client perspective, deal with this one easily. The solution is to send a wl_display.sync request immediately after the commit. Yes, I think it makes sense to always sync the rendering to at least a wl_display.sync call and the Mesa patch I sent does already do this. You are right that in practice this effectively solves the problem for most use cases. So really the only case where this matters is when the compositor is directly scanning out from the client's buffer. But on the other hand, that is exactly what a fullscreen game is likely to be doing and that is the most likely candidate for doing eglSwapInterval(0). In any case, dummy sync and frame requests (you may need both) will allow you to achieve glSwapInterval(0) without server-side support. I'm not sure I follow you here. The release event may be queued at any point after the frame complete is sent. In that case sending a sync event to flush the queue is only going to help if Mesa sends it repeatedly, but that amounts to busy-waiting which would be terrible. I still feel like the new request is the right way to go. The difficulty with interface versioning feels like a separate wider problem that we should think about. The crux of the problem is that Mesa probably shouldn't be using proxy objects that are created outside of Mesa because in that case it doesn't have control over what interface version or event queue it is using. Working around the need for the new request would just side-step the issue but it doesn't seem unlikely that Mesa would want to use further new surface interfaces requests later too and the same problem would come back. Maybe we should have a separate object that you can create per surface in order to do new requests. This could be created by a new global object much like the wl_shell interface. In order to make it usable to both Mesa and the application, we would have to allow creating multiple resources of the new interface for a single surface. I'm not sure what to call it though because it would just end up being something like ‘wl_surface_extra_stuff’. Considering that other objects may end up also needing a similar kind of interface, maybe it would make more sense to rethink it a bit and make the compositor allow multiple resources for an object in general. Then you could have something like wl_compositor.rebind_resource(object, version) which would make a new resource for any object and it
Re: input handlig in separate thread
Unfortunately the proposal with a separate queue does not work for the use
case.
i added the wl_seat object to is own event queue but dispatching this queue
blocks the egl rendering
currently we are still using wayland 1.05 so my hope is that with the
wayland 1.3 and
wl_display_prepare_read and wl_display_read_events api's i can solve this
issue:
so i maybe just repaid here my understanding how to use those api's in my
input thread:
while(!quit)//main loop
{
while (wl_display_prepare_read(__**display) != 0)
wl_display_dispatch_pending(__**display);
wl_display_flush(display);
poll(fds, nfds, -1); //wait until new input event arrives
wl_display_read_events(__**display);
wl_display_dispatch_pending(__**display);
}
would this sequence be correct?
2013/10/22 Kristian Høgsberg hoegsb...@gmail.com
On Mon, Oct 21, 2013 at 11:32:58PM +0200, Eugen Friedrich wrote:
Hello Kristian,
I'm don't using mesa, we have a Vivante driver stack on imx6 which also
uses a proprietary wayland protocol to send a buffer to the compositor
and it's EGL implementation don't creates a separate event queue it uses
the main wayland display queue and will call wl_display_dispatch
function.
But thanks for the hint, i suppose the current mesa implementation should
answer all my questions above.
That explains it. The separate event queue feature is generally
useful, but it was prompted by the requirement to make EGL threading
work right under Wayland. As such, the Vivante driver really should
use it to make sure it doesn't inadvertently block on the main thread.
In the mean time you can probably work around it by creating a
separate event queue for everything else you do in your application.
Kristian
2013/10/21 Kristian Høgsberg k...@bitplanet.net
On Sat, Oct 19, 2013 at 10:31 AM, Eugen Friedrich fried...@gmail.com
wrote:
Ok! this sounds very promising this should decouple the rendering
thread
and the input handling thread.
so i can set to the wl_seat client object a new queue and then pass
this
queue to wl_display_dispatch_queue call in the input thread.
but one question to this: the documentation of
wl_display_dispatch_queue
says:
For other threads this will block until the main thread queues
events on
the queue passed as argument.
So will pass the events in the queue or how to achieve this it the
second
thread( rendering thread is sleeping)??
In my use case the input thread should wake up when input event
arrives
from
the compositor also when all other thread of the client are idle.
None of this should be necessary. The EGL implementation already uses
its own queue (if your mesa is new enough) and should be able to loop
all by itself without the main event queue being processed. You can
still use a custom queue for your input events, but if you only care
about separating EGL into a separate thread, you shouldn't need to do
anything.
Kristian
2013/10/19 Giulio Camuffo giuliocamu...@gmail.com
You can move a client-side wayland object to a queue using the
function
void wl_proxy_set_queue(struct wl_proxy *proxy, struct
wl_event_queue
*queue).
All the client-side objects are really wl_proxy, so you can safely
cast
them.
If you move to a queue an object which creates new objects and
dispatches them through
an event those objects will be in the same queue too.
2013/10/18 Eugen Friedrich fried...@gmail.com:
Hallo Giulio,
thanks a lot for the fast replay.
if my understanding is correct the frame callbacks and the input
handling
events are coming from the compositor to the main wayland display
queue
on
the client side.
So how i can get the different queues on the client site or is
there a
possibility to get a separate queue for input events?
2013/10/18 Giulio Camuffo giuliocamu...@gmail.com
Oh, I sent my mail before the second one arrived.
Yeah, you need to use different wl_event_queues, and the relative
functions like
wl_display_dispatch_queue.
2013/10/18 Giulio Camuffo giuliocamu...@gmail.com:
And what is it that doesn't work? As a wild bet I'd say you
probably
want to look at wl_event_queue.
See
http://wayland.freedesktop.org/docs/html/chap-Library.html#sect-Library-Client
Giulio
2013/10/18 Eugen Friedrich fried...@gmail.com:
Hallo dear Wayland developer,
I thing i have a very common use case and currently i don't
now
how
to
get
it work in wayland.
We have a wayland client application with 2 threads:
thread1 - rendering thread - the eglSwapbuffer will call
wl_display_dispatch internally:
thread2 - input thread - this should wait for wayland input
events
Re: input handlig in separate thread
Yes, this won't work property unless you have wayland 1.2. If you
have a EGL driver that uses it's own queue, all you need to do is
while (!quit)
wl_display_dispatch(display);
Otherwise, create a queue and move the wl_registry object to it so
that you get all events from all other objects in that queue, then do
while (!quit)
wl_display_dispatch_queue(display, queue);
Kristian
On Thu, Oct 24, 2013 at 1:58 PM, Eugen Friedrich fried...@gmail.com wrote:
Unfortunately the proposal with a separate queue does not work for the use
case.
i added the wl_seat object to is own event queue but dispatching this queue
blocks the egl rendering
currently we are still using wayland 1.05 so my hope is that with the
wayland 1.3 and
wl_display_prepare_read and wl_display_read_events api's i can solve this
issue:
so i maybe just repaid here my understanding how to use those api's in my
input thread:
while(!quit)//main loop
{
while (wl_display_prepare_read(__display) != 0)
wl_display_dispatch_pending(__display);
wl_display_flush(display);
poll(fds, nfds, -1); //wait until new input event arrives
wl_display_read_events(__display);
wl_display_dispatch_pending(__display);
}
would this sequence be correct?
2013/10/22 Kristian Høgsberg hoegsb...@gmail.com
On Mon, Oct 21, 2013 at 11:32:58PM +0200, Eugen Friedrich wrote:
Hello Kristian,
I'm don't using mesa, we have a Vivante driver stack on imx6 which also
uses a proprietary wayland protocol to send a buffer to the compositor
and it's EGL implementation don't creates a separate event queue it uses
the main wayland display queue and will call wl_display_dispatch
function.
But thanks for the hint, i suppose the current mesa implementation
should
answer all my questions above.
That explains it. The separate event queue feature is generally
useful, but it was prompted by the requirement to make EGL threading
work right under Wayland. As such, the Vivante driver really should
use it to make sure it doesn't inadvertently block on the main thread.
In the mean time you can probably work around it by creating a
separate event queue for everything else you do in your application.
Kristian
2013/10/21 Kristian Høgsberg k...@bitplanet.net
On Sat, Oct 19, 2013 at 10:31 AM, Eugen Friedrich fried...@gmail.com
wrote:
Ok! this sounds very promising this should decouple the rendering
thread
and the input handling thread.
so i can set to the wl_seat client object a new queue and then pass
this
queue to wl_display_dispatch_queue call in the input thread.
but one question to this: the documentation of
wl_display_dispatch_queue
says:
For other threads this will block until the main thread queues
events on
the queue passed as argument.
So will pass the events in the queue or how to achieve this it the
second
thread( rendering thread is sleeping)??
In my use case the input thread should wake up when input event
arrives
from
the compositor also when all other thread of the client are idle.
None of this should be necessary. The EGL implementation already uses
its own queue (if your mesa is new enough) and should be able to loop
all by itself without the main event queue being processed. You can
still use a custom queue for your input events, but if you only care
about separating EGL into a separate thread, you shouldn't need to do
anything.
Kristian
2013/10/19 Giulio Camuffo giuliocamu...@gmail.com
You can move a client-side wayland object to a queue using the
function
void wl_proxy_set_queue(struct wl_proxy *proxy, struct
wl_event_queue
*queue).
All the client-side objects are really wl_proxy, so you can safely
cast
them.
If you move to a queue an object which creates new objects and
dispatches them through
an event those objects will be in the same queue too.
2013/10/18 Eugen Friedrich fried...@gmail.com:
Hallo Giulio,
thanks a lot for the fast replay.
if my understanding is correct the frame callbacks and the input
handling
events are coming from the compositor to the main wayland display
queue
on
the client side.
So how i can get the different queues on the client site or is
there a
possibility to get a separate queue for input events?
2013/10/18 Giulio Camuffo giuliocamu...@gmail.com
Oh, I sent my mail before the second one arrived.
Yeah, you need to use different wl_event_queues, and the
relative
functions like
wl_display_dispatch_queue.
2013/10/18 Giulio Camuffo giuliocamu...@gmail.com:
And what is it that doesn't work? As a wild bet I'd say you
probably
want to look at wl_event_queue.
See
[PATCH 0/6] DirectFB integration
Hi, this is an updated patch set. Hopefully in the right shape this time, otherwise please let me know. Please see http://www.directfb.org/docs/DirectFB_Foreseeing_2013-10-07.pdf and http://www.directfb.org/docs/DirectFB_EGL_2013-10-07.pdf for details and plans. With this version you can run Weston on any DirectFB enabled platform including OpenGL ES support via EGL where available. We're also introducing our abstract high-level EGL implementation called EGL United. This aims to provide a complete EGL implementation with all extensions, e.g. WL extensions or DIRECTFB EGLImage support, while integrating different low-level (EGL) implementations in one EGLDisplay. The Wayland extensions in EGL United are implemented in a generic module. No vendor support is needed for the Wayland extensions to work! clients/Makefile.am | 12 clients/dfb_test1.cpp | 290 ++ clients/weston-dfb-adapter.cpp| 1220 +++ configure.ac |3 src/Makefile.am | 25 src/compositor-directfb-input.cpp | 214 src/compositor-directfb-input.h | 91 ++ src/compositor-directfb.cpp | 1614 src/compositor-directfb.h | 225 + src/compositor.c | 13 src/compositor.h |2 src/gl-renderer.h |8 configure.ac| 11 13 files changed, 3727 insertions(+), 1 deletion(-) ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/6] DirectFB integration
gl-renderer: Add protection against multiple includes of
header.
---
src/gl-renderer.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/gl-renderer.h b/src/gl-renderer.h
index 0342134..1f41c9f 100644
--- a/src/gl-renderer.h
+++ b/src/gl-renderer.h
@@ -20,6 +20,9 @@
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
+#ifndef __WESTON_GL_RENDERER_H__
+#define __WESTON_GL_RENDERER_H__
+
#include config.h
#include compositor.h
@@ -64,4 +67,7 @@ struct gl_renderer_interface {
void (*print_egl_error_state)(void);
};
-struct gl_renderer_interface gl_renderer_interface;
+extern struct gl_renderer_interface gl_renderer_interface;
+
+#endif
+
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/6] DirectFB integration
surface: Add compositor_state to weston_surface for the
compositor to keep a per surface context, like renderer does.
---
src/compositor.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/compositor.h b/src/compositor.h
index 73722b5..2f3b7d0 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -865,6 +865,8 @@ struct weston_surface {
*/
struct wl_list subsurface_list; /* weston_subsurface::parent_link */
struct wl_list subsurface_list_pending; /* ...::parent_link_pending */
+
+ void *compositor_state;
};
enum weston_key_state_update {
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 3/6] DirectFB integration
configure: Add C++ compiler support and enable c++0x --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index 80a5d69..30fb4a3 100644 --- a/configure.ac +++ b/configure.ac @@ -27,6 +27,7 @@ AM_SILENT_RULES([yes]) # Check for programs AC_PROG_CC +AC_PROG_CXX AC_PROG_SED # Initialize libtool @@ -474,6 +475,8 @@ if test x$wayland_scanner = x; then AC_MSG_ERROR([wayland-scanner is needed to compile weston]) fi +CXXFLAGS=-std=c++0x $CXXFLAGS + AC_CONFIG_FILES([Makefile shared/Makefile src/Makefile ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 4/6] DirectFB integration
configure: Add DirectFB compositor --- configure.ac | 11 +++ 1 file changed, 11 insertions(+) diff --git a/configure.ac b/configure.ac index 30fb4a3..793da83 100644 --- a/configure.ac +++ b/configure.ac @@ -196,6 +196,17 @@ AS_IF([test x$enable_fbdev_compositor = xyes], [ PKG_CHECK_MODULES([FBDEV_COMPOSITOR], [libudev = 136 mtdev = 1.1.0]) ]) + +AC_ARG_ENABLE([directfb-compositor], [ --enable-directfb-compositor],, + enable_directfb_compositor=yes) +AM_CONDITIONAL([ENABLE_DIRECTFB_COMPOSITOR], + [test x$enable_directfb_compositor = xyes]) +AS_IF([test x$enable_directfb_compositor = xyes], [ + AC_DEFINE([BUILD_DIRECTFB_COMPOSITOR], [1], [Build the directfb compositor]) + PKG_CHECK_MODULES([DIRECTFB_COMPOSITOR], [egl ++dfb = 1.8.0 wayland-dfb = 1.8.0]) +]) + + AC_ARG_ENABLE([rdp-compositor], [ --enable-rdp-compositor],, enable_rdp_compositor=no) AM_CONDITIONAL([ENABLE_RDP_COMPOSITOR], ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC 1/5] Add a fullscreen shell protocol
On Thu, Oct 24, 2013 at 12:33 PM, Jonas Ådahl jad...@gmail.com wrote: On Wed, Oct 23, 2013 at 06:08:29PM -0500, Jason Ekstrand wrote: Jonas, Thanks for the review! On Wed, Oct 23, 2013 at 3:56 PM, Jonas Ådahl jad...@gmail.com wrote: Hi, Using this protocol, how would the fullscreen shell client turn on and off outputs? I can see how clone, next-to and modeset by controlling what surface is presented and what fullscreen method is used, but that is not enough for turning an output off, isn't it? I think that would be done by simply calling present_surface with a null surface to remove the surface from the output. I'm not 100% sure how this will interact with KMS surface detection, but hopefully we should be able to make it that simple. I guess this could be added to the protocol clarifing what presenting a null-surface means disabling the output. What disabling means, could be up to the backend. Yeah, I can make that more clear. I'll send another draft here with things stated better. Another question is synchronising. Would a client ever want to change anything atomically? Do we need something like wl_surface.commit? I'm not sure what would get changed atomically. There's only one operation and different outputs will probably have different refresh rates so changes won't be atomic anyway. Or are you thinking about trying to future-proof stuff? Well, yea, atomically is probably not the correct term, and I don't know if the KMS backend support any such functionality. What I imagine is one might want to, for example, disable one output and enable another with minimal time in between, or maybe avoid ending up with no output enabled if the client would crash between calls. Might be overkill though, and if there is no real world use case right now, maybe even more so. I think I would tend to say that this usually isn't really practical. For KMS, for instance, switching stuff around may make monitors turn on and there's no way to make that atomic. If the client is worried about being on no outputs whatsoever, it can just set the one before removing the other. Then it will get the surface.enter before the surface.leave. Jonas Also, one wording comment below. Thanks Jonas On Tue, Oct 22, 2013 at 08:48:25PM -0500, Jason Ekstrand wrote: --- protocol/fullscreen-shell.xml | 44 +++ 1 file changed, 44 insertions(+) create mode 100644 protocol/fullscreen-shell.xml diff --git a/protocol/fullscreen-shell.xml b/protocol/fullscreen-shell.xml new file mode 100644 index 000..b696828 --- /dev/null +++ b/protocol/fullscreen-shell.xml @@ -0,0 +1,44 @@ +protocol name=fullscreen_shell + interface name=wl_fullscreen_shell version=1 +description summary=Displays a single surface per output + Displays a single surface per output. + + This interface can only be bound to by one client at a time. +/description + +enum name=fullscreen_method + description summary=different method to set the surface fullscreen + Hints to indicate to the compositor how to deal with a conflict + between the dimensions of the surface and the dimensions of the + output. The compositor is free to ignore this parameter. + /description + entry name=default value=0 summary=no preference, apply default policy/ + entry name=scale value=1 summary=scale, preserve the surface's aspect ratio and center on output/ + entry name=driver value=2 summary=switch output mode to the smallest mode that can fit the surface, add black borders to compensate size mismatch/ + entry name=fill value=3 summary=no upscaling, center on output and add black borders to compensate size mismatch/ +/enum + +request name=present_surface + description summary=present surface for display + This requests the system compositor to display surface on output. + Each client of the system compositor can have at most one surface You still use the term 'system compositor' here. Also, it should be display a surface instead of display surface. Thanks. Good catch. I've fixed it locally. + per output at any one time. Subsequent requests with the same + output replace the surface bound to that output. The same surface + may be presented on multiple outputs. + + If the output is null, the compositor will present the surface on + whatever display (or displays) it thinks best. In particular, this + may replace any or all surfaces currently presented so it should + not be used in combination with placing surfaces on specific + outputs. + + The method specifies how the surface is to be
[RFC v3 1/5] Add a fullscreen shell protocol
--- protocol/fullscreen-shell.xml | 61 +++ 1 file changed, 61 insertions(+) create mode 100644 protocol/fullscreen-shell.xml diff --git a/protocol/fullscreen-shell.xml b/protocol/fullscreen-shell.xml new file mode 100644 index 000..9bef555 --- /dev/null +++ b/protocol/fullscreen-shell.xml @@ -0,0 +1,61 @@ +protocol name=fullscreen_shell + interface name=wl_fullscreen_shell version=1 +description summary=Displays a single surface per output + Displays a single surface per output. + + This interface can only be bound to by one client at a time. +/description + +enum name=present_method + description summary=different method to set the surface fullscreen + Hints to indicate to the compositor how to deal with a conflict + between the dimensions of the surface and the dimensions of the + output. The compositor is free to ignore this parameter. + /description + entry name=default value=0 summary=no preference, apply default policy/ + entry name=scale value=1 summary=scale, preserve the surface's aspect ratio and center on output/ + entry name=driver value=2 summary=switch output mode to the smallest mode that can fit the surface, add black borders to compensate size mismatch/ + entry name=fill value=3 summary=no upscaling, center on output and add black borders to compensate size mismatch/ +/enum + +request name=present_surface + description summary=present surface for display + Present a surface on the given output. + + This requests the fullscreen shell to display the given surface on + the given output. Each client of the fullscreen shell can have at + most one surface per output at any one time. Subsequent requests + with the same output replace the surface bound to that output. + Setting a null surface on an output effectively disables that + output for whatever definition of disables applies to the + implementaiton. The same surface may be presented on multiple + outputs. + + If the output is null, the compositor will present the surface on + whatever display (or displays) it thinks best. In particular, this + may replace any or all surfaces currently presented so it should + not be used in combination with placing surfaces on specific + outputs. + + The method specifies how the surface is to be persented. In + particular, this instructs the compositor how to handle a size + mismatch between the presented surface and the output. + + The framerate parameter is used only when the method is set + to driver, to indicate the preferred framerate. A value of 0 + indicates that the app does not care about framerate. The + framerate is specified in mHz, that is framerate of 6 is 60Hz. + + A method of scale or driver implies a scaling operation of + the surface, either via a direct scaling operation or a change of + the output mode. This will override any kind of output scaling, so + that mapping a surface with a buffer size equal to the mode can + fill the screen independent of buffer_scale. + /description + arg name=surface type=object interface=wl_surface/ + arg name=method type=uint/ + arg name=framerate type=uint/ + arg name=output type=object interface=wl_output allow-null=true/ +/request + /interface +/protocol -- 1.8.3.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Buffer release events (was: Add support for eglSwapInterval)
This is mostly a recap of the discussion between Neil and myself on IRC this morning. I think a client can guarantee that it gets all of the buffer releases by simply calling wl_display.sync after the attach/damage/commit. It can even delete the wl_callback object immediately without attaching a listener. What follows is a revised version of the proof of this I gave on IRC this morning. In order for this to work, we need to make the assumption that the backend holds at most N references to the buffer. I will discuss the nature of this number N later. For the DRM backend with one output, N = 2. Built into this assumption is that if the backend already has N references it will not take another one without releasing one of the ones it already has. Seeing that this always works comes down to observing that at any given time we either have enough free buffers or the sync event will flush out at least one release event. Most of the time attach/commit will result in a release on the previously attached buffer. If it does, the release event will get queued and the sync event will flush the queue. The two cases in which it does not release the previously attached buffer directly are if the buffer has already been released in compositor_accumulate_damage or if it has been referenced by the backend. In the first case the release happened before attach/commit, so it will be received before the sync event. In the second case, the backend has either referenced less than N buffers and the client has extra free buffers or the backend had to release a buffer before it could reference a new one. This release must have been queued before the attach/commit so it will get flushed by the sync. Given that we know the number N associated with the backend, a client can continuously render with as few as N+2 buffers (or N+3 for subsurfaces). It needs N for the backend, one to be currently attached and one to render. If it is part of a subsurface that is in synced mode, there may be an additional reference tied up in the commit cache. It's worth noting that using N+2 buffers does require a full-blown roundtrip after attach/commit. However, given that you are probably doing SwapInterval(0) in this case and running the GPU hard, that shouldn't be a bottlekneck. Now a note about this number N. In the DRM backend with a single output, N = 2. For a fullscreen surface, one buffer is used for scanout and the other is tied up in the queued pageflip (which can't be canceled). The DRM backend will only take a reference to the currently attached buffer if it is going to queue it for the next pageflip. However, this implies that a pageflip has happened, freeing the previous scanout buffer. There may be a subtle issue here if the surface is displayed fullscreen on multiple outputs. Mmy knowledge of DRM is a bit lacking here, but it seems like you could end up with 2 buffers tied up for each output. Perhaps we should look into synchronizing pageflips in some way so we use less buffers? Thanks, --Jason Ekstrand On Thu, Oct 24, 2013 at 2:34 PM, Jonas Ådahl jad...@gmail.com wrote: On Thu, Oct 24, 2013 at 11:26:08AM +0100, Neil Roberts wrote: Hi Thanks for the interesting insights. It seems to me as if the default should always be to just send the event. I think I would vote for leaving the default as it is, ie, queuing the release events. It's really quite a corner case that delaying events has any effect on an application because most applications don't need to know about the release events until they are about to draw something. Usually they would only draw something in response to some event such as a frame callback or an input event. In that case the event will have caused the queue to flush so they will certainly be up-to-date about what buffers are available at the point when they start drawing. If we default to not queuing the event then I'd imagine most applications wouldn't realise they should enable it and would miss out on the optimisation. We can identify buffer release events in weston as coming from one of three sources: 1) wl_surface.commit 2) surface_flush_damage (the gl renderer releases SHM buffers here) 3) random releases from the backdend/renderer Number 2 above happens during the redraw loop so we can just post the event and won't get a double-wakeup. Yes, I guess even if the compositor posts the event it's not going to actually send it to the client until the compositor goes idle again anyway and at that point it will probably have posted a frame complete callback too so the client would wake up anyway. Number 3 is something we can't really control; I'd personally lean towards posting the event here, but it's probably at most one reference per surface so we can probably get away with queueing. (Also, if the backend knows when it would release in the render cycle, it may be able to optimize this better
