<div dir="ltr">Hi<br><div><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 1, 2017 at 4:16 PM Marc-André Lureau <<a href="mailto:mlureau@redhat.com">mlureau@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
----- Original Message -----<br>
> ><br>
> > From: Marc-André Lureau <<a href="mailto:marcandre.lureau@redhat.com" target="_blank">marcandre.lureau@redhat.com</a>><br>
> ><br>
> > Since 2.8, QEMU now longer creates QXL primary surfaces when using<br>
> > GL. This change broke client-side mouse mode, because Spice server<br>
> > relies on primary surface conditions.<br>
> ><br>
> > When GL is enabled, use GL scanout informations, similar to what<br>
> > red_qxl_get_allow_client_mouse() is doing. The main difference is that<br>
> > the GL surface doesn't have mouse_mode attached (what for?).<br>
> ><br>
> > NB: this code looks like it handles only first QXL instance only, and<br>
> > uses the whole scanout/surface size for the input table size, perhaps<br>
> > monitor-configs would be a better choice.<br>
> ><br>
> > Signed-off-by: Marc-André Lureau <<a href="mailto:marcandre.lureau@redhat.com" target="_blank">marcandre.lureau@redhat.com</a>><br>
> > ---<br>
> >  server/red-qxl.c | 2 ++<br>
> >  server/reds.c    | 8 +++++++-<br>
> >  2 files changed, 9 insertions(+), 1 deletion(-)<br>
> ><br>
> > diff --git a/server/red-qxl.c b/server/red-qxl.c<br>
> > index b6b3770b..ba7b641a 100644<br>
> > --- a/server/red-qxl.c<br>
> > +++ b/server/red-qxl.c<br>
> > @@ -897,6 +897,8 @@ void spice_qxl_gl_scanout(QXLInstance *qxl,<br>
> >      /* FIXME: find a way to coallesce all pending SCANOUTs */<br>
> >      dispatcher_send_message(qxl_state->dispatcher,<br>
> >                              RED_WORKER_MESSAGE_GL_SCANOUT, &payload);<br>
> > +<br>
> > +    reds_update_client_mouse_allowed(qxl_state->reds);<br>
> >  }<br>
> ><br>
> >  SPICE_GNUC_VISIBLE<br>
> > diff --git a/server/reds.c b/server/reds.c<br>
> > index 40c94851..a89c5e90 100644<br>
> > --- a/server/reds.c<br>
> > +++ b/server/reds.c<br>
> > @@ -4256,10 +4256,16 @@ void reds_update_client_mouse_allowed(RedsState<br>
> > *reds)<br>
> >      if (num_active_workers > 0) {<br>
> >          GListIter it;<br>
> >          QXLInstance *qxl;<br>
> > +        SpiceMsgDisplayGlScanoutUnix *gl;<br>
> ><br>
> >          allow_now = TRUE;<br>
> >          FOREACH_QXL_INSTANCE(reds, it, qxl) {<br>
> > -            if (red_qxl_get_primary_active(qxl)) {<br>
> > +            if ((gl = red_qxl_get_gl_scanout(qxl))) {<br>
> > +                x_res = gl->width;<br>
> > +                y_res = gl->height;<br>
> > +                red_qxl_put_gl_scanout(qxl, gl);<br>
> > +                break;<br>
> > +            } else if (red_qxl_get_primary_active(qxl)) {<br>
> >                  allow_now = red_qxl_get_allow_client_mouse(qxl, &x_res,<br>
> >                  &y_res);<br>
> >                  break;<br>
> >              }<br>
><br>
> First though when I first read this patch was:<br>
><br>
> A: Is your cat red?<br>
> R: Yes, it's red. I don't have a cat.<br>
><br>
> Why Qemu don't allocate the primary surface in the first place?<br>
<br>
Because it no longer uses QXL 2d drawing commands.<br>
<br>
> The issue is that multiple code (for instance monitor configs)<br>
> rely on having a primary surface.<br>
<br>
We would need to fix that then, I haven't checked multiple monitors with 2.8, I know it used to work not long ago.<br>
<br>
> Maybe a lazy allocation of the canvas could help?<br>
<br>
What for?<br>
<br>
> This patch looks like a workaround for a bug introduced by Qemu.<br>
<br>
It depends on spice-server behaviour, which is not enforced at this point. Imho, there is no need to create a 2d primary surface if all you do is GL scanouts.<br>
<br>
> I'm sure it solves the client mouse but I'd like to know what<br>
> other people knowing better client and multi monitor code/protocol<br>
> think about not having the primary surface.<br>
><br>
> Reviewed-by: Frediano Ziglio <<a href="mailto:fziglio@redhat.com" target="_blank">fziglio@redhat.com</a>><br></blockquote><div><br></div><div>I still have this patch in my queue, and another user reported it fixed the bug. You proposed an alternative? Could you submit a former patch if you decline this one in favour of your approach?<br><br></div><div>Thanks<br></div></div></div></div><div dir="ltr">-- <br></div><div data-smartmail="gmail_signature"><div dir="ltr">Marc-André Lureau<br></div></div>