[Spice-devel] [PATCH spice-server v2] gl: fix client mouse mode

[Christophe de Dinechin]

> On 7 Sep 2017, at 16:49, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
> Since 2.8, QEMU now longer creates QXL primary surfaces when using GL.

Typo: “no longer"

> This change broke client-side mouse mode, because Spice server relies on
> primary surface conditions.

I do not understand what you mean by “primary surface conditions”?
Do you mean the state of the primary surface, or specific conditions
testing the primary surface, or something else?

> 
> When GL is enabled, use GL scanout informations.
> Mouse mode is always client when GL surfaces are used.
> 
> This patch and most of the message are based on a patch from
> Marc-André Lureau, just moving responsibility from reds to RedQxl.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> server/red-qxl.c | 31 +++++++++++++++++++++----------
> server/red-qxl.h |  3 +--
> server/reds.c    |  3 +--
> 3 files changed, 23 insertions(+), 14 deletions(-)
> 
> Changes since v1:
> - do not change qxl_state resolution.
> 
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index b556428d2..93e7eb7b8 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -903,6 +903,8 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,
> 
>     qxl_state->gl_draw_async = async_command_alloc(qxl_state, message, cookie);
>     dispatcher_send_message(qxl_state->dispatcher, message, &draw);
> +
> +    reds_update_client_mouse_allowed(qxl_state->reds);
> }
> 
> void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command)
> @@ -1022,20 +1024,29 @@ void red_qxl_clear_pending(QXLState *qxl_state, int pending)
>     clear_bit(pending, &qxl_state->pending);
> }
> 
> -gboolean red_qxl_get_primary_active(QXLInstance *qxl)
> +bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int *x_res, int *y_res, int *allow_now)

While you are at it, why not make allow_now a bool?

> {
> -    return qxl->st->primary_active;
> -}
> +    // try to get resolution from 3D

I would rephrase this comment to be closer to your commit message log,
i.e. that with 3D enabled, qemu does not create a QXL primary surface.

> +    SpiceMsgDisplayGlScanoutUnix *gl;
> +    if ((gl = red_qxl_get_gl_scanout(qxl))) {
> +        *x_res = gl->width;
> +        *y_res = gl->height;
> +        *allow_now = TRUE;
> +        red_qxl_put_gl_scanout(qxl, gl);
> +        return true;
> +    }
> +
> +    // check for 2D
> +    if (!qxl->st->primary_active) {
> +        return false;
> +    }

> 
> -gboolean red_qxl_get_allow_client_mouse(QXLInstance *qxl, gint *x_res, gint *y_res)
> -{
>     if (qxl->st->use_hardware_cursor) {
> -        if (x_res)
> -            *x_res = qxl->st->x_res;
> -        if (y_res)
> -            *y_res = qxl->st->y_res;
> +        *x_res = qxl->st->x_res;
> +        *y_res = qxl->st->y_res;
>     }
> -    return qxl->st->use_hardware_cursor;
> +    *allow_now = qxl->st->use_hardware_cursor;
> +    return true;
> }

Any reason to not set use_hardware_cursor in the 3D case?

Actually, looking at the rest of the code, I did not understand why we have a field for that.

> 
> void red_qxl_on_ic_change(QXLInstance *qxl, SpiceImageCompression ic)
> diff --git a/server/red-qxl.h b/server/red-qxl.h
> index f925f065b..503ba223d 100644
> --- a/server/red-qxl.h
> +++ b/server/red-qxl.h
> @@ -39,8 +39,7 @@ void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command);
> struct Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl);
> gboolean red_qxl_use_client_monitors_config(QXLInstance *qxl);
> gboolean red_qxl_client_monitors_config(QXLInstance *qxl, VDAgentMonitorsConfig *monitors_config);
> -gboolean red_qxl_get_primary_active(QXLInstance *qxl);
> -gboolean red_qxl_get_allow_client_mouse(QXLInstance *qxl, gint *x_res, gint *y_res);
> +bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int *x_res, int *y_res, int *allow_now);
> SpiceMsgDisplayGlScanoutUnix *red_qxl_get_gl_scanout(QXLInstance *qxl);
> void red_qxl_put_gl_scanout(QXLInstance *qxl, SpiceMsgDisplayGlScanoutUnix *scanout);
> void red_qxl_gl_draw_async_complete(QXLInstance *qxl);
> diff --git a/server/reds.c b/server/reds.c
> index b6ecc6c69..2f4f12ab9 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -4374,8 +4374,7 @@ void reds_update_client_mouse_allowed(RedsState *reds)
> 
>         allow_now = TRUE;
>         FOREACH_QXL_INSTANCE(reds, qxl) {
> -            if (red_qxl_get_primary_active(qxl)) {
> -                allow_now = red_qxl_get_allow_client_mouse(qxl, &x_res, &y_res);
> +            if (red_qxl_get_allow_client_mouse(qxl, &x_res, &y_res, &allow_now)) {
>                 break;
>             }
>         }
> -- 
> 2.13.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel