[PATCH xserver v3 23/24] xwayland: Cut off upper 32bit of queued vblank delay

[Michel Dänzer]

On 2018-03-13 04:00 PM, Roman Gilg wrote:
> This ensures the same behavior as in Present's fake counter and xfree86.
> 
> At the moment clients might do put vblanks too far into the future, because
> the fake vblank code in Present and the xfree86 driver tolerate cut off upper
> 32bit due to an 64 to 32bit conversion. Do this therefore here as well to not
> suddenly regress on Xwayland only.
> 
> The sample client, that triggers this behavior, is the Steam client.
> 
> Signed-off-by: Roman Gilg <subdiff at gmail.com>
> ---
>  hw/xwayland/xwayland-present.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index f403ff7..300d96f 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -299,6 +299,7 @@ xwl_present_queue_vblank(WindowPtr present_window,
>  {
>      struct xwl_window *xwl_window = xwl_window_of_top(present_window);
>      struct xwl_present_event *event;
> +    INT32 delay;
>  
>      if (!xwl_window)
>          return BadMatch;
> @@ -317,7 +318,10 @@ xwl_present_queue_vblank(WindowPtr present_window,
>      event->event_id = event_id;
>      event->present_window = present_window;
>      event->xwl_window = xwl_window;
> -    event->target_msc = msc;
> +
> +    /* Cut off upper 32bit, copies present_fake_queue_vblank. */
> +    delay = (int64_t) (msc - xwl_window->present_msc);
> +    event->target_msc = xwl_window->present_msc + delay;

FWIW, I think it would be better to make delay unsigned, and calculate
it something like

    delay = max((int64_t) (msc - xwl_window->present_msc), 0);


However, since I've been unable to reproduce the issue you described
with Steam, despite kind of going out of my way to do so, I'm really
reluctant to add this workaround without getting more information about
the problem from someone who can reproduce it. Without understanding the
problem, the workaround might turn out to be too strict or too lenient,
and clients might accidentally become dependent on the workaround.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer