>
> > On 7 Sep 2017, at 16:49, Frediano Ziglio <fziglio@xxxxxxxxxx> 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?
>
In this case the existence of such surface.
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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?
>
Yes and not. This strange fields came from a mouse_mode field.
The confusion on this value is that in different cases mouse_mode
means a bit field while in this specific instance the field is used
as a boolean (0/1). For compatibility int was used.
Had recent discussion with Jonathon on this value and turned
out is currently always TRUE (1).
> > {
> > - 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.
>
Specific suggestions?
> > + 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?
>
For a detailed answer see history in the ML.
For a short on: is not valid.
> Actually, looking at the rest of the code, I did not understand why we have a
> field for that.
>
I think old cards (QXL ones) do not support hardware mouse so you can't
get cursor information (for instance position and shape) from the card.
> >
> > 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;
> > }
> > }
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel