Re: [PATCH] xwayland: Destroy wl_buffers only after they are released
On Tue, 11 Feb 2014 16:34:13 +0100
Rui Matos wrote:
> Destroying a wl_buffer that is still attached to a wl_surface is
> undefined behavior according to the wayland protocol. We should delay
> the destruction until we get the release event.
> ---
>
> So, I'm not sure why there was this comment saying that it was safe to
> do this, perhaps it was in an old protocol version?
>
> In any case, this has been making xwayland crash under mutter ever
> since this mutter commit
> https://git.gnome.org/browse/mutter/commit/?h=wayland&id=3e98ffaf9958366b584b360ac12bbc03cd070c07
> .
>
> hw/xfree86/xwayland/xwayland-window.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xfree86/xwayland/xwayland-window.c
> b/hw/xfree86/xwayland/xwayland-window.c
> index a2a8206..a005cc6 100644
> --- a/hw/xfree86/xwayland/xwayland-window.c
> +++ b/hw/xfree86/xwayland/xwayland-window.c
> @@ -43,6 +43,16 @@
> static DevPrivateKeyRec xwl_window_private_key;
>
> static void
> +free_buffer(void *data, struct wl_buffer *buffer)
> +{
> +wl_buffer_destroy(buffer);
> +}
> +
> +static const struct wl_buffer_listener buffer_listener = {
> +free_buffer,
The name of the function should say it's the wl_buffer.release handler.
> +};
> +
> +static void
> free_pixmap(void *data, struct wl_callback *callback, uint32_t time)
> {
> PixmapPtr pixmap = data;
> @@ -62,10 +72,8 @@ xwl_window_attach(struct xwl_window *xwl_window, PixmapPtr
> pixmap)
> struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
> struct wl_callback *callback;
>
> -/* We can safely destroy the buffer because we only use one buffer
> - * per surface in xwayland model */
> if (xwl_window->buffer)
> -wl_buffer_destroy(xwl_window->buffer);
> +wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
>
> xwl_screen->driver->create_window_buffer(xwl_window, pixmap);
>
> @@ -185,7 +193,7 @@ xwl_unrealize_window(WindowPtr window)
> return ret;
>
> if (xwl_window->buffer)
> - wl_buffer_destroy(xwl_window->buffer);
> +wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
> wl_surface_destroy(xwl_window->surface);
> xorg_list_del(&xwl_window->link);
> if (RegionNotEmpty(DamageRegion(xwl_window->damage)))
I assume the code never added a wl_buffer listener before, because if
it did, this patch would be a no-op. "wl_buffer_add_listener" is a
misnomer, there can only ever be one listener, and trying to "add"
another will not actually do anything.
Also, you rely on wl_buffer.release not having arrived before you add
the listener. With weston's gl-renderer, the release comes very soon
after each wl_surface.commit for wl_shm buffers. Maybe it works, maybe
it doesn't, but it seems very fragile. Did you check you don't leak
wl_buffers now?
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
On Tue, 11 Feb 2014 12:03:41 -0800 Bill Spitzak wrote: > I think in absolute time you are right, the "P" points do not move. > Instead everything else would move left by .5, resulting in the same > image I described with the vertical arrows always tilting to the right. > > The green decision lines move left by .5 to align with the "P" points > since the decision is strictly whether a "T" is between two "P". > > The "T" points move left by .5 because the client will have to timestamp > everything .5 earlier. > > I still feel it would be less confusing to avoid negative numbers and to > match what I am pretty certain is mainstream usage of integer timestamps > on frames. Ok, so what you are suggesting here is that we should change the whole design to always have presentation come late with a mean delay of half the refresh period (T/2) and the amount of delay being between 0 and T. Just like you say, then the client will have to arbitrarily guess and subtract T/2 always to be able to target the vblank at a P. Also note, that since presentation feedback reports the time P, not P - T/2, the clients really need to do the subtraction instead of just queueing updates with target time t = P + n * T. To avoid is hassle with "subtract T/2 or you will always be late", I deliberately chose to have the mean delay zero, which means that compared to a given target timestamp, presentation may occur between t - T/2 and t + T/2, i.e. half a period early or late. And this was not even my idea, Axel Davy pointed it out to me when I was first going for the "always late" case. So this is not just a technicality in drawing a diagram, no, this would affect how clients need to be programmed. And doing it your way would IMO be more complicated than my way, as you need to account for the T/2 instead of using the presentation feedback timestamps directly and extrapolating with an integer multiple of the refresh period. The timestamps may be integers, but they are in nanoseconds, so dividing the refresh period by 2 is not a problem at all. Besides, in your suggestion, the clients would need to compute T/2 and we would have to document that "always subtract T/2 from your target timestamps so you can on average have zero latency with presentation compared to your actual target times". I just want that if a client estimates that a vblank will happen at time t, and it queues a frame with target time t well in advance, it will get presented at the vblank at time t, not the vblank *after* t. Your suggestion of t,t+1 may make sense if we queued updates with target time given in MSC, but here we use UST-like clock values and can do better, because the values are not limited to "integers" (integer multiples of the refresh period). This is probably the source of your uneasyness: all prior art that I know of uses frame counters, not a clock, so yes, this is a different design. Why have this different design? That question I replied in my original RFCv2 email, the section "3. Why UST all the way?". Thanks, pq > Pekka Paalanen wrote: > > On Mon, 10 Feb 2014 12:20:00 -0800 > > Bill Spitzak wrote: > > > >> Pekka Paalanen wrote: > >> > >>> This algorithm aims to start showing an update between t-T/2 and t+T/2, > >>> which means that presentation may occur a little early or late, with > >>> an "average" of zero. Another option would be to show the update between > >>> t and t+T, which would mean that presentation would be always late with > >>> an "average" of T/2. > >> I think there would be a lot less confusion if this was described using > >> the t,t+1 model. I think in your diagram it would move the 'P' line .5 > >> to the right so they line up with the green lines, and all the red > >> arrows would tilt to the right. It makes no difference to the result > >> (the same frames are selected) but I think makes it a lot easier to > >> describe. > >> > >> Another reason is that media starts at time 0, not time -.5*frame. > > > > Hmm, I'm not sure. The green lines are not frame boundaries, they are > > decision boundaries. In the picture, the points P are the exact time > > when a framebuffer flip happens, which means that the hardware starts > > to scan out a new image. Each image is shown for the interval > > P[n]..P[n+1], not the interval between green lines. > > > > So the green lines only divide the queue axis into intervals, that get > > assigned to a particular P. Both axes are the same time axis, with > > units of nanoseconds which are not marked. The black ticks on both axes > > denote when a new frame begins. > > > > Did we have some confusion here? > > > > - pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] toytoolkit: avoid unnecessary redraws when focus changes
On Mon, Feb 10, 2014 at 04:52:32PM +0100, Emilio Pozuelo Monfort wrote:
> From: Emilio Pozuelo Monfort
>
> Clients that need to be redrawn when the focus changes do that by
> listening to focus_changed and scheduling a redraw.
>
> This was causing unnecessary redraws in the clients, as could be
> easily seen by changing focus on weston-flower.
I verified with both patches applied there are no new warnings in the
build, and all 12 tests continue passing properly.
Running both with and without the patches on my system I didn't notice a
performance difference just to the naked eye, but presumably if it were
instrumented properly it'd be measurable? In any case, I didn't spot
any regressions.
So, for both patches in the set:
Tested-by: Bryce Harrington
Reviewed-by: Bryce Harrington
(For full disclosure - On one test run against master, I noticed the
flower changed shape every time it received or lost focus, however I was
never able to reproduce that behavior even after doing clean rebuilds.)
> Signed-off-by: Emilio Pozuelo Monfort
> ---
> clients/window.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/clients/window.c b/clients/window.c
> index 91c1ea0..b5f0137 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -3863,7 +3863,6 @@ handle_surface_focused_set(void *data, struct
> xdg_surface *xdg_surface)
> {
> struct window *window = data;
> window->focused = 1;
> - window_schedule_redraw(window);
> }
>
> static void
> @@ -3871,7 +3870,6 @@ handle_surface_focused_unset(void *data, struct
> xdg_surface *xdg_surface)
> {
> struct window *window = data;
> window->focused = 0;
> - window_schedule_redraw(window);
> }
>
> static void
> --
> 1.9.rc1
>
> ___
> 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: [PATCH weston.ini.man v3] Improvement of weston.ini.man. Add key:shell and remove tablet-shell
On Mon, Feb 10, 2014 at 12:15:11PM +0900, Nobuhiko Tanibata wrote: > Add description of key:shell to CORE SECTION and move a example of > desktop-shell from key:modules to key:shell. > Add cms-colord.so to key:modules of CORE SECTION. > > Signed-off-by: Nobuhiko Tanibata LGTM Reviewed-by: Bryce Harrington > --- > man/weston.ini.man | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/man/weston.ini.man b/man/weston.ini.man > index ce3f928..dfb00c6 100644 > --- a/man/weston.ini.man > +++ b/man/weston.ini.man > @@ -92,16 +92,29 @@ The > .B core > section is used to select the startup compositor modules. > .TP 7 > -.BI "modules=" desktop-shell.so,xwayland.so > -specifies the modules to load (string). Available modules in the > +.BI "shell=" desktop-shell.so > +specifies a shell to load (string). This can be used to load your own > +implemented shell or one with Weston as default. Available shells > +in the > .IR "__weston_modules_dir__" > directory are: > .PP > .RS 10 > .nf > .BR desktop-shell.so > -.BR tablet-shell.so > +.fi > +.RE > +.TP 7 > +.TP 7 > +.BI "modules=" xwayland.so,cms-colord.so > +specifies the modules to load (string). Available modules in the > +.IR "__weston_modules_dir__" > +directory are: > +.PP > +.RS 10 > +.nf > .BR xwayland.so > +.BR cms-colord.so > .fi > .RE > .TP 7 > -- > 1.8.3.1 > > ___ > 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: [PATCH weston] Popup stacking
I would greatly prefer if the demo Wayland compositor did not enforce "active on top". This assumption is causing endless grief, it makes drag & drop impossible and overlapping windows useless. A lower window that wants a popup or transient should end up with the popup or transient above any higher windows. There certainly should be no need to "activate", and you really should not be using the word "activate" when "raise" is the side-effect you want. Emilio Pozuelo Monfort wrote: From: Emilio Pozuelo Monfort Hi, These two patches fix bug #74831 in two different ways. One of them, originally written by Philip, positions the popup on top of all other surfaces in the layer. That means that you can still end up with the parent surface behind a second parent (as the comment in the commit explains). The other commit activates a surface when you right-click on it. That is what at least gnome-shell does (I don't know about other WMs) and we may want to do the same. That change fixes the pop positioning bug because the parent surface is going to be on top, so placing the popup just above the parent surface is enough to have it on top of everything else. I'm leaning towards the second approach (activating the surface upon right click). Thoughts? Emilio ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Weston : ideas about xdg_sell, and implementation for a taskbar
sardemff7+wayl...@sardemff7.net wrote: I was suggesting that one method of minimizing multiple surfaces would be for the client to arrange them all as children of one of them and then minimize the parent. The primary purpose is so the compositor/taskbar knows all those windows are "related", for instance to produce on a single taskbar entry. All scenarii are supported. You can reparent or simply minimize everyone. I was thinking the compositor would need the parent information. For instance if may show one taskbar entry per minimize request. But there may be other methods of doing this. No, because if the client wants to redraw or raise or show or hide any other surface as a side-effect of the minimize, these changes will not be atomic with the minimize, resulting in unwanted flickering of an intermediate display. There has to be a way for the minimized window to not disappear until the client does some kind of commit. The commit *is* the .set_minimized. You can raise, draw everything you want *then* send the .set_minimized request. It is the last step of the minimization process. In the scenario of multiple .set_minimized requests, the last one must be the “main” one (either the one requested by the compositor or the one triggered by client UI). Okay this is an interesting method for making atomic minimization without having to send a parent tree to the compositor. My main concern is that I think the parent tree is going to be needed anyway so I'm not sure this adds anything. Also I am unsure how a client indicates which minimize is the "main" one, as the compositor can't tell if another minimize is coming. This is a problem even for compositor-generated minimizes as the client may want to respond to a minimize request in a dialog box by minimizing the entire application, not just the dialog box. I mean “unfocus”. There is no reason to treat the minimized with a special event. The client already reparented surfaces and raised everything relevant. As long as the client is in charge of mimimizing itself (which it now sounds like you are proposing and is exactly what I was trying to propose originally) then this is ok. In fact the client could even assume that it loses the focus so no unfocus event is needed (but it appears a Wayland design criteria is to ensure pairing of events so it should be sent, though IMHO this enforced pairing is a waste of time as no sane client will rely on it). A surface without focus can be minimized, it's just that the client will be unable to tell if it worked. It is true that a client may display incorrect graphics if the set_minimized is ignored. If it expects an unfocus it could detect this after a delay but even then the drawing will blink. But I don't think this is a major concern. No, I am describing a .request_set_minimized/.set_minimized use case: 1a. The compositor send the .request_set_minimized event. 1b. The client “minimize” button is pressed. 2. The client reparent, draw, raise, do whatever it needs to keep the UI consistent. 3. The client send the .set_minimized request. 4. The relevant surfaces are hidden by the compositor. In case 1b with a compositor not supporting minimization, the client will just looks a bit weird. But: 1. I believe that a user will not expect anything better when using such a compositor. 2. Using flags in the configure event to inform the client which actions are supported will just remove such a weird case. Okay, that sounds exactly like what I proposed, which is the client is in final control over the minimization. Nothing happens until the client sends the set_minimized request. I definately prefer this but I must have misread your description as it sure sounded like you wanted communication after the compositor minimized the window, not before. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
I think in absolute time you are right, the "P" points do not move. Instead everything else would move left by .5, resulting in the same image I described with the vertical arrows always tilting to the right. The green decision lines move left by .5 to align with the "P" points since the decision is strictly whether a "T" is between two "P". The "T" points move left by .5 because the client will have to timestamp everything .5 earlier. I still feel it would be less confusing to avoid negative numbers and to match what I am pretty certain is mainstream usage of integer timestamps on frames. Pekka Paalanen wrote: On Mon, 10 Feb 2014 12:20:00 -0800 Bill Spitzak wrote: Pekka Paalanen wrote: This algorithm aims to start showing an update between t-T/2 and t+T/2, which means that presentation may occur a little early or late, with an "average" of zero. Another option would be to show the update between t and t+T, which would mean that presentation would be always late with an "average" of T/2. I think there would be a lot less confusion if this was described using the t,t+1 model. I think in your diagram it would move the 'P' line .5 to the right so they line up with the green lines, and all the red arrows would tilt to the right. It makes no difference to the result (the same frames are selected) but I think makes it a lot easier to describe. Another reason is that media starts at time 0, not time -.5*frame. Hmm, I'm not sure. The green lines are not frame boundaries, they are decision boundaries. In the picture, the points P are the exact time when a framebuffer flip happens, which means that the hardware starts to scan out a new image. Each image is shown for the interval P[n]..P[n+1], not the interval between green lines. So the green lines only divide the queue axis into intervals, that get assigned to a particular P. Both axes are the same time axis, with units of nanoseconds which are not marked. The black ticks on both axes denote when a new frame begins. Did we have some confusion here? - pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Destroy wl_buffers only after they are released
On 11 February 2014 16:44, Axel Davy wrote: > I have pending changes to this part to implement XWayland present support. > > Instead a having the wl_buffer attached to a window, I attach it to a > pixmap. > > Could you test with this branch, and tell if you encounter the problems you > have? > > https://github.com/axeldavy/xserver/tree/xwayland-dri3-present Yes, it fixes this problem as well. Thanks, Rui ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Destroy wl_buffers only after they are released
Hi,
I have pending changes to this part to implement XWayland present support.
Instead a having the wl_buffer attached to a window, I attach it to a
pixmap.
Could you test with this branch, and tell if you encounter the problems
you have?
https://github.com/axeldavy/xserver/tree/xwayland-dri3-present
Thanks.
Axel Davy
On 11/02/2014, Rui Matos wrote :
Destroying a wl_buffer that is still attached to a wl_surface is
undefined behavior according to the wayland protocol. We should delay
the destruction until we get the release event.
---
So, I'm not sure why there was this comment saying that it was safe to
do this, perhaps it was in an old protocol version?
In any case, this has been making xwayland crash under mutter ever
since this mutter commit
https://git.gnome.org/browse/mutter/commit/?h=wayland&id=3e98ffaf9958366b584b360ac12bbc03cd070c07
.
hw/xfree86/xwayland/xwayland-window.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/xfree86/xwayland/xwayland-window.c
b/hw/xfree86/xwayland/xwayland-window.c
index a2a8206..a005cc6 100644
--- a/hw/xfree86/xwayland/xwayland-window.c
+++ b/hw/xfree86/xwayland/xwayland-window.c
@@ -43,6 +43,16 @@
static DevPrivateKeyRec xwl_window_private_key;
static void
+free_buffer(void *data, struct wl_buffer *buffer)
+{
+wl_buffer_destroy(buffer);
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+free_buffer,
+};
+
+static void
free_pixmap(void *data, struct wl_callback *callback, uint32_t time)
{
PixmapPtr pixmap = data;
@@ -62,10 +72,8 @@ xwl_window_attach(struct xwl_window *xwl_window, PixmapPtr
pixmap)
struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
struct wl_callback *callback;
-/* We can safely destroy the buffer because we only use one buffer
- * per surface in xwayland model */
if (xwl_window->buffer)
-wl_buffer_destroy(xwl_window->buffer);
+wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
xwl_screen->driver->create_window_buffer(xwl_window, pixmap);
@@ -185,7 +193,7 @@ xwl_unrealize_window(WindowPtr window)
return ret;
if (xwl_window->buffer)
- wl_buffer_destroy(xwl_window->buffer);
+wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
wl_surface_destroy(xwl_window->surface);
xorg_list_del(&xwl_window->link);
if (RegionNotEmpty(DamageRegion(xwl_window->damage)))
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] xwayland: Destroy wl_buffers only after they are released
Destroying a wl_buffer that is still attached to a wl_surface is
undefined behavior according to the wayland protocol. We should delay
the destruction until we get the release event.
---
So, I'm not sure why there was this comment saying that it was safe to
do this, perhaps it was in an old protocol version?
In any case, this has been making xwayland crash under mutter ever
since this mutter commit
https://git.gnome.org/browse/mutter/commit/?h=wayland&id=3e98ffaf9958366b584b360ac12bbc03cd070c07
.
hw/xfree86/xwayland/xwayland-window.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/hw/xfree86/xwayland/xwayland-window.c
b/hw/xfree86/xwayland/xwayland-window.c
index a2a8206..a005cc6 100644
--- a/hw/xfree86/xwayland/xwayland-window.c
+++ b/hw/xfree86/xwayland/xwayland-window.c
@@ -43,6 +43,16 @@
static DevPrivateKeyRec xwl_window_private_key;
static void
+free_buffer(void *data, struct wl_buffer *buffer)
+{
+wl_buffer_destroy(buffer);
+}
+
+static const struct wl_buffer_listener buffer_listener = {
+free_buffer,
+};
+
+static void
free_pixmap(void *data, struct wl_callback *callback, uint32_t time)
{
PixmapPtr pixmap = data;
@@ -62,10 +72,8 @@ xwl_window_attach(struct xwl_window *xwl_window, PixmapPtr
pixmap)
struct xwl_screen *xwl_screen = xwl_window->xwl_screen;
struct wl_callback *callback;
-/* We can safely destroy the buffer because we only use one buffer
- * per surface in xwayland model */
if (xwl_window->buffer)
-wl_buffer_destroy(xwl_window->buffer);
+wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
xwl_screen->driver->create_window_buffer(xwl_window, pixmap);
@@ -185,7 +193,7 @@ xwl_unrealize_window(WindowPtr window)
return ret;
if (xwl_window->buffer)
- wl_buffer_destroy(xwl_window->buffer);
+wl_buffer_add_listener(xwl_window->buffer, &buffer_listener, NULL);
wl_surface_destroy(xwl_window->surface);
xorg_list_del(&xwl_window->link);
if (RegionNotEmpty(DamageRegion(xwl_window->damage)))
--
1.8.3.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Presentation extension RFCv2, feedback implemented
Hi all, I thought I'd share this with you, so you can start experimenting with the presentation feedback extension. The Weston branch at http://cgit.collabora.com/git/user/pq/weston.git/log/?h=presentation-RFCv2-feedback implements the presentation feedback part and should be fully usable. The queueing part is still stubbed out. The protocol is the same as originally proposed in RFCv2 at http://lists.freedesktop.org/archives/wayland-devel/2014-January/012988.html The branch includes a new demo, weston-presentation-shm, which pre-renders 60 frames, and then just cycles those, measuring the timings. The animation speed is intentionally tied to the refresh rate. This explains the print-out: http://cgit.collabora.com/git/user/pq/weston.git/tree/clients/presentation-shm.c?h=presentation-RFCv2-feedback#n345 All timestamps are truncated to milliseconds, because that is what the frame callback carries. The new demo seems to claim, that while weston hits every vblank, the latency from commit to presentation is almost 2 refresh periods. I did not investigate why. I guess this branch could already be merged into Weston, except the exact style of the protocol interfaces is still undecided, and the spec needs some fixes we have been talking about in the RFCv2 thread. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Weston : ideas about xdg_sell, and implementation for a taskbar
On 11/02/2014 02:23, Bill Spitzak wrote: May not have explained it correctly. It sounded like you were not going to allow dialogs to be minimized except as a side-effect of minimizing the parent. I was, but I am not any more. I certainly want to allow this! And I certainly want to support minimizing multiple surfaces. I agree, it is needed. I was suggesting that one method of minimizing multiple surfaces would be for the client to arrange them all as children of one of them and then minimize the parent. The primary purpose is so the compositor/taskbar knows all those windows are "related", for instance to produce on a single taskbar entry. All scenarii are supported. You can reparent or simply minimize everyone. No, because if the client wants to redraw or raise or show or hide any other surface as a side-effect of the minimize, these changes will not be atomic with the minimize, resulting in unwanted flickering of an intermediate display. There has to be a way for the minimized window to not disappear until the client does some kind of commit. The commit *is* the .set_minimized. You can raise, draw everything you want *then* send the .set_minimized request. It is the last step of the minimization process. In the scenario of multiple .set_minimized requests, the last one must be the “main” one (either the one requested by the compositor or the one triggered by client UI). I assume you mean "minimize", not "unfocus"? It must be possible to minimize windows that don't have focus. I mean “unfocus”. There is no reason to treat the minimized with a special event. The client already reparented surfaces and raised everything relevant. It sounds like you are describing the 3-way communication I proposed. I see 3 steps: 1. Client decides it wants to minimize, tells compositor (this step is not done if the compositor chooses to do so). 2. Compositor tells client that the minimize is happening. 3. (the step you are missing) client tells compositor it has corrected all it's surfaces to reflect the result of minimizing and it is ok to perform it. No, I am describing a .request_set_minimized/.set_minimized use case: 1a. The compositor send the .request_set_minimized event. 1b. The client “minimize” button is pressed. 2. The client reparent, draw, raise, do whatever it needs to keep the UI consistent. 3. The client send the .set_minimized request. 4. The relevant surfaces are hidden by the compositor. In case 1b with a compositor not supporting minimization, the client will just looks a bit weird. But: 1. I believe that a user will not expect anything better when using such a compositor. 2. Using flags in the configure event to inform the client which actions are supported will just remove such a weird case. […] The rest of your email should be answered already. Thanks, -- Quentin “Sardem FF7” Glidic ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Popup stacking
From: Emilio Pozuelo Monfort Hi, These two patches fix bug #74831 in two different ways. One of them, originally written by Philip, positions the popup on top of all other surfaces in the layer. That means that you can still end up with the parent surface behind a second parent (as the comment in the commit explains). The other commit activates a surface when you right-click on it. That is what at least gnome-shell does (I don't know about other WMs) and we may want to do the same. That change fixes the pop positioning bug because the parent surface is going to be on top, so placing the popup just above the parent surface is enough to have it on top of everything else. I'm leaning towards the second approach (activating the surface upon right click). Thoughts? Emilio -- 1.9.rc1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] shell: Change stacking order calculation for popup surfaces
From: Philip Withnall
Always put them as the top-most layer in the layer list of their parent.
This ensures that, for example, the popup menu produced by
right-clicking on a surface (which is not currently at the top of the
stacking order in the current workspace) is displayed at the top of the
stacking order.
[ Emilio: handle popups with non-shell-surface parents ]
https://bugs.freedesktop.org/show_bug.cgi?id=74831
Signed-off-by: Philip Withnall
Co-authored-by: Emilio Pozuelo Monfort
---
desktop-shell/shell.c | 47 +--
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index a73e8e0..d362f5f 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2090,15 +2090,49 @@ shell_surface_calculate_layer_link (struct
shell_surface *shsurf)
struct weston_view *parent;
switch (shsurf->type) {
- case SHELL_SURFACE_POPUP:
- case SHELL_SURFACE_TOPLEVEL:
+ case SHELL_SURFACE_POPUP: {
+ /* Popups should go at the front of the workspace of their
+* parent surface, rather than just in front of the parent. This
+* fixes the situation where there are two top-level windows:
+* - Above
+* - Below
+* and a pop-up menu is created for 'Below'. We want:
+* - Popup
+* - Above
+* - Below
+* not:
+* - Above
+* - Popup
+* - Below
+*/
+ struct shell_surface *parent_shsurf;
+
+ parent_shsurf = get_shell_surface(shsurf->parent);
+
+ if (parent_shsurf != NULL)
+ return
shell_surface_calculate_layer_link(parent_shsurf);
+ else if (shsurf->parent) {
+ /* The parent surface may not be a shell surface, e.g.
+* for right clicks on the panel. */
+ parent = get_default_view(shsurf->parent);
+
+ if (parent)
+ return parent->layer_link.prev;
+ }
+
+ break;
+ }
+
+ case SHELL_SURFACE_TOPLEVEL: {
if (shsurf->state.fullscreen) {
return &shsurf->shell->fullscreen_layer.view_list;
} else if (shsurf->parent) {
- /* Move the surface to its parent layer so
-* that surfaces which are transient for
-* fullscreen surfaces don't get hidden by the
-* fullscreen surfaces. */
+ /* Move the surface to its parent layer so that
+* surfaces which are transient for fullscreen surfaces
+* don't get hidden by the fullscreen surfaces.
+* However, unlike popups, transient surfaces are
+* stacked in front of their parent but not in front of
+* other surfaces of the same type. */
/* TODO: Handle a parent with multiple views */
parent = get_default_view(shsurf->parent);
@@ -2106,6 +2140,7 @@ shell_surface_calculate_layer_link (struct shell_surface
*shsurf)
return parent->layer_link.prev;
}
break;
+ }
case SHELL_SURFACE_XWAYLAND:
return &shsurf->shell->fullscreen_layer.view_list;
--
1.9.rc1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] shell: activate windows upon a right click
From: Emilio Pozuelo Monfort This fixes the bug that commit da704d was trying to fix, where a popup would appear on top of its parent but behind other windows. https://bugs.freedesktop.org/show_bug.cgi?id=74831 Signed-off-by: Emilio Pozuelo Monfort --- desktop-shell/shell.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index a73e8e0..be44905 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -5701,6 +5701,9 @@ shell_add_bindings(struct weston_compositor *ec, struct desktop_shell *shell) weston_compositor_add_button_binding(ec, BTN_LEFT, 0, click_to_activate_binding, shell); + weston_compositor_add_button_binding(ec, BTN_RIGHT, 0, +click_to_activate_binding, +shell); weston_compositor_add_touch_binding(ec, 0, touch_to_activate_binding, shell); -- 1.9.rc1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Xserver errors
On 7 February 2014 22:21, Bill Spitzak wrote: > Okay, recompiling the newest xserver and mesa, and removing the "gallium" > driver from mesa, has helped a *little*. > > X windows now show their correct contents initially, and redraw immediately > in response to events. > > However the graphics are still as screwed up as before, with all the window > edges + shadows either missing or drawn incorrectly. It looks exactly like > the previous screenshot I sent. Is anybody else seeing this? > > Another concern is that my fltk2 applications do not see any mouse clicks > until the window is resized. I suspect there is a bug in fltk2 causing it to > ignore them, but the bug must be triggered by an unusual delivery of events > from xwayland compared to other x servers. It may be a good idea to fix > this, I will try to locate exactly what assumption fltk2 is doing. > > I also want to ask again for somebody to send example configurations they > use to compile mesa as I do feel that may be the source of a lot of my > troubles. I often find useful to read the build logs of various distros when I have to build stuff locally, for example: http://kojipkgs.fedoraproject.org//packages/xorg-x11-server/1.15.0/3.fc21/data/logs/x86_64/build.log HTH, Tomeu ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
On Mon, 10 Feb 2014 12:20:00 -0800 Bill Spitzak wrote: > Pekka Paalanen wrote: > > > This algorithm aims to start showing an update between t-T/2 and t+T/2, > > which means that presentation may occur a little early or late, with > > an "average" of zero. Another option would be to show the update between > > t and t+T, which would mean that presentation would be always late with > > an "average" of T/2. > > I think there would be a lot less confusion if this was described using > the t,t+1 model. I think in your diagram it would move the 'P' line .5 > to the right so they line up with the green lines, and all the red > arrows would tilt to the right. It makes no difference to the result > (the same frames are selected) but I think makes it a lot easier to > describe. > > Another reason is that media starts at time 0, not time -.5*frame. Hmm, I'm not sure. The green lines are not frame boundaries, they are decision boundaries. In the picture, the points P are the exact time when a framebuffer flip happens, which means that the hardware starts to scan out a new image. Each image is shown for the interval P[n]..P[n+1], not the interval between green lines. So the green lines only divide the queue axis into intervals, that get assigned to a particular P. Both axes are the same time axis, with units of nanoseconds which are not marked. The black ticks on both axes denote when a new frame begins. Did we have some confusion here? - pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC v2] Wayland presentation extension (video protocol)
On Mon, 10 Feb 2014 09:23:12 -0600 Jason Ekstrand wrote: > On Mon, Feb 10, 2014 at 3:53 AM, Pekka Paalanen wrote: > > > On Sat, 8 Feb 2014 15:23:29 -0600 > > Jason Ekstrand wrote: > > > > > Pekka, > > > First off, I think you've done a great job over-all. I think it will > > both > > > cover most cases and work well I've got a few comments below. > > > > Thank you for the review. :-) > > Replies below. > > > > > On Thu, Jan 30, 2014 at 9:35 AM, Pekka Paalanen > > wrote: > > > > > > > Hi, > > > > > > > > it's time for a take two on the Wayland presentation extension. > > > > > > > > > > > > 1. Introduction > > > > > > > > The v1 proposal is here: > > > > > > > > > > http://lists.freedesktop.org/archives/wayland-devel/2013-October/011496.html > > > > > > > > In v2 the basic idea is the same: you can queue frames with a > > > > target presentation time, and you can get accurate presentation > > > > feedback. All the details are new, though. The re-design started > > > > from the wish to handle resizing better, preferably without > > > > clearing the buffer queue. ... > > > My one latent concern is that I still don't think we're entirely handling > > > the case that QtQuick wants. What they want is to do their rendering a > > few > > > frames in advance in case of CPU/GPU jitter. Technically, this extension > > > handles this by the client simply doing a good job of guessing > > presentation > > > times on a one-per-frame baseis. However, it doesn't allow for any > > damage > > > tracking. In the case of QtQuick they want a linear queue of buffers > > where > > > no buffer ever gets skipped. In this case, you could do damage tracking > > by > > > allowing it to accumulate from one frame to another and you get all of > > the > > > damage-tracking advantages that you had before. I'm not sure how much > > this > > > matters, but it might be worth thinking about it. > > > > Does it really want to display *every* frame regardless of time? It > > doesn't matter that if a deadline is missed, the animation slows down > > rather than jumps to keep up with intended velocity? > > > > That is my understanding of how it works now. I *think* they figure the > compositor isn't the bottle-kneck and that it will git its 60 FPS. That > said, I don't actually work on QtQuick. I'm just trying to make sure they > don't get completely left out in the cold. > > > > > > Axel has a good point, cannot this be just done client side and > > immediate updates based on frame callbacks? > > > > Probably not. They're using GLES and EGL so they can't draw early and > just stash the buffer. Oh yeah, I just realized that. I hope Axel's suggestion works out, or that they actually want the timestamp queue semantics rather than present-every-frame-really-no-skipping. But if they want the every-frame semantics, then I think that needs to be another extension. And then the interactions with immediate commits and timestamp queued updates gets fun. > > If there is a problem in using frame callbacks for that, that is more > > likely a problem in the compositor's frame scheduling than the protocol. > > > > The problem with damage tracking, why I did not take damage as queued > > state, is that it is given in surface coordinates. This will become a > > problem during resizes, where the surface size changes, and wl_viewport > > is used to decouple the content from the surface space. > > > > The separation makes sense. > > > > > > If we queue damage, we basically need to queue also surface resizes. > > Without wl_viewport this is what happens automatically, as surface size > > is taken from the buffer size. > > > > However, in the proposed design, the purpose of wl_viewport is to > > decouple the surface size from buffer size, so that they can change > > independently. The use case is live video: if you resize the window, > > you don't want to redo the video frames, because that would likely > > cause a glitch. Also if the video resolution changes on the fly, by e.g. > > stream quality control, you don't need to do anything extra to keep the > > window at the old size. Damage is a property of the content update, > > yes, but we have it defined in surface coordinates, so when surface and > > buffer sizes change asynchronously, the damage region would be > > incorrect. > > > > The downside is indeed that we lose damage information for queued > > buffers. This is a deliberate design choice, since the extension was > > designed primarily for video where usually the whole surface gets > > damaged. > > > > Yeah, I think you made the right call on this one. Queueing buffers in a > completely serial fassion really does seem to be a special case. Trying to > do damage tracking for an arbitrary queue would very quickly get insane. > Plus all the other problems you mentioned. > > > > > > But, I guess we could add another request, presentation.damage, to give > > the damage region in buffer coordinates. Would it be w
