[weston,2/6] libweston: Use the monotonic clock in weston_compositor_get_time

Submitted by Dima Ryazanov on Dec. 5, 2016, 3:36 a.m.

Details

Message ID 20161205033635.20086-3-dima@gmail.com
State New
Headers show
Series "Implement xdg-shell's show_window_menu API" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Dima Ryazanov Dec. 5, 2016, 3:36 a.m.
(This is kind of a workaround, but perhaps the right thing to do anyways.)

The menu implementation in window.c needs to know the time of the event that
triggered the menu - however, the xdg-shell's show_window_menu API does not
give us that info. There doesn't seem to be an easy way to fake it because
different compositors use different times when sending events:
- compositor-drm uses the monotonic clock
- compositor-x11 uses weston_compositor_get_time
- compositor-wayland uses the event times in got from the parent compositor
- GNOME appears to use the monotonic clock

Switching weston_compositor_get_time to use the monotonic clock works around
this issue - though things would break in compositor-wayland if its parent
uses something else.

Signed-off-by: Dima Ryazanov <dima@gmail.com>
---
 libweston/compositor.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 6457858..aa4bf345 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -1693,11 +1693,11 @@  weston_surface_update_size(struct weston_surface *surface)
 WL_EXPORT uint32_t
 weston_compositor_get_time(void)
 {
-       struct timeval tv;
+       struct timespec ts;
 
-       gettimeofday(&tv, NULL);
+       clock_gettime(CLOCK_MONOTONIC, &ts);
 
-       return tv.tv_sec * 1000 + tv.tv_usec / 1000;
+       return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
 }
 
 WL_EXPORT struct weston_view *

Comments

Hi,

On 5 December 2016 at 03:36, Dima Ryazanov <dima@gmail.com> wrote:
> (This is kind of a workaround, but perhaps the right thing to do anyways.)
>
> The menu implementation in window.c needs to know the time of the event that
> triggered the menu - however, the xdg-shell's show_window_menu API does not
> give us that info. There doesn't seem to be an easy way to fake it because
> different compositors use different times when sending events:
> - compositor-drm uses the monotonic clock
> - compositor-x11 uses weston_compositor_get_time
> - compositor-wayland uses the event times in got from the parent compositor
> - GNOME appears to use the monotonic clock
>
> Switching weston_compositor_get_time to use the monotonic clock works around
> this issue - though things would break in compositor-wayland if its parent
> uses something else.
>
> Signed-off-by: Dima Ryazanov <dima@gmail.com>

I think we may want a check that CLOCK_MONOTONIC actually works (i.e.
the first time this function is called, assert that ret == 0). But
yes, it's definitely the right thing to do, so with that:
Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
On Tue, Dec 6, 2016 at 3:32 AM, Daniel Stone <daniel@fooishbar.org> wrote:

> Hi,
>
> On 5 December 2016 at 03:36, Dima Ryazanov <dima@gmail.com> wrote:
> > (This is kind of a workaround, but perhaps the right thing to do
> anyways.)
> >
> > The menu implementation in window.c needs to know the time of the event
> that
> > triggered the menu - however, the xdg-shell's show_window_menu API does
> not
> > give us that info. There doesn't seem to be an easy way to fake it
> because
> > different compositors use different times when sending events:
> > - compositor-drm uses the monotonic clock
> > - compositor-x11 uses weston_compositor_get_time
> > - compositor-wayland uses the event times in got from the parent
> compositor
> > - GNOME appears to use the monotonic clock
> >
> > Switching weston_compositor_get_time to use the monotonic clock works
> around
> > this issue - though things would break in compositor-wayland if its
> parent
> > uses something else.
> >
> > Signed-off-by: Dima Ryazanov <dima@gmail.com>
>
> I think we may want a check that CLOCK_MONOTONIC actually works (i.e.
> the first time this function is called, assert that ret == 0). But
> yes, it's definitely the right thing to do, so with that:
> Reviewed-by: Daniel Stone <daniels@collabora.com>
>

If we're going to assert, do you think it's ok to just assert every time?
That would make the logic simpler - and it doesn't look like it could
succeed once, then fail later.

Though almost all of the existing code just ignores the return value. It
would be good to either handle failures everywhere, or just assert that the
clock works when Weston starts up.

Cheers,
> Daniel
>