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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 3 Oct 2017, at 15:56, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:


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.

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 <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.

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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]