> On 3 Oct 2017, at 15:56, Frediano Ziglio <fzig...@redhat.com> wrote:
>
>>>
>>> On 7 Sep 2017, at 16:49, Frediano Ziglio <fzig...@redhat.com
>>> <mailto:fzig...@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?
>>
>
> In this case the existence of such surface.
Oh, then “Spice server relies on having a primary 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 <fzig...@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?
>>
>
> 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.
I am confused. In the code I am looking at, “allow_now” is a
local variable in the caller.
Or are you talking about primary_active?
>
> 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?
“try to get resolution when 3D enabled, since qemu did not create 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?
>>
>
> 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.
I see. Thanks
>
>>>
>>> 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@lists.freedesktop.org <mailto:Spice-devel@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
