Re: [RFC v2] Wayland presentation extension (video protocol)

2014-02-11 Thread Pekka Paalanen
On Mon, 10 Feb 2014 09:23:12 -0600
Jason Ekstrand ja...@jlekstrand.net wrote:

 On Mon, Feb 10, 2014 at 3:53 AM, Pekka Paalanen ppaala...@gmail.com wrote:
 
  On Sat, 8 Feb 2014 15:23:29 -0600
  Jason Ekstrand ja...@jlekstrand.net 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 ppaala...@gmail.com
  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 worth it?

Well, I don't think the damage tracking would get particularly nasty.
You queue damage with the buffers, apply the damage (and convert to
surface space) when 

Re: [RFC v2] Wayland presentation extension (video protocol)

2014-02-11 Thread Pekka Paalanen
On Mon, 10 Feb 2014 12:20:00 -0800
Bill Spitzak spit...@gmail.com 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: Xserver errors

2014-02-11 Thread Tomeu Vizoso
On 7 February 2014 22:21, Bill Spitzak spit...@gmail.com 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


[PATCH weston] shell: Change stacking order calculation for popup surfaces

2014-02-11 Thread Emilio Pozuelo Monfort
From: Philip Withnall philip.withn...@collabora.co.uk

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 philip.withn...@collabora.co.uk
Co-authored-by: Emilio Pozuelo Monfort emilio.pozu...@collabora.co.uk
---
 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

2014-02-11 Thread Emilio Pozuelo Monfort
From: Emilio Pozuelo Monfort emilio.pozu...@collabora.co.uk

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 emilio.pozu...@collabora.co.uk
---
 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


[PATCH weston] Popup stacking

2014-02-11 Thread Emilio Pozuelo Monfort
From: Emilio Pozuelo Monfort emilio.pozu...@collabora.co.uk

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


Re: Weston : ideas about xdg_sell, and implementation for a taskbar

2014-02-11 Thread sardemff7+wayland

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


Presentation extension RFCv2, feedback implemented

2014-02-11 Thread Pekka Paalanen
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


[PATCH] xwayland: Destroy wl_buffers only after they are released

2014-02-11 Thread Rui Matos
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=waylandid=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


Re: [PATCH] xwayland: Destroy wl_buffers only after they are released

2014-02-11 Thread Axel Davy

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=waylandid=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


Re: Weston : ideas about xdg_sell, and implementation for a taskbar

2014-02-11 Thread Bill Spitzak

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: [PATCH weston] Popup stacking

2014-02-11 Thread Bill Spitzak
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 emilio.pozu...@collabora.co.uk

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: [PATCH weston.ini.man v3] Improvement of weston.ini.man. Add key:shell and remove tablet-shell

2014-02-11 Thread Bryce W. Harrington
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 nobuhiko_tanib...@xddp.denso.co.jp
LGTM
Reviewed-by: Bryce Harrington b.harring...@samsung.com
 ---
  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 1/2] toytoolkit: avoid unnecessary redraws when focus changes

2014-02-11 Thread Bryce W. Harrington
On Mon, Feb 10, 2014 at 04:52:32PM +0100, Emilio Pozuelo Monfort wrote:
 From: Emilio Pozuelo Monfort emilio.pozu...@collabora.co.uk
 
 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 b.harring...@samsung.com
Reviewed-by: Bryce Harrington b.harring...@samsung.com


(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 emilio.pozu...@collabora.co.uk
 ---
  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: [RFC v2] Wayland presentation extension (video protocol)

2014-02-11 Thread Pekka Paalanen
On Tue, 11 Feb 2014 12:03:41 -0800
Bill Spitzak spit...@gmail.com 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 spit...@gmail.com 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

2014-02-11 Thread Pekka Paalanen
On Tue, 11 Feb 2014 16:34:13 +0100
Rui Matos tiagoma...@gmail.com 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=waylandid=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