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 alocal variable in the caller.Or are you talking about primary_active?
Yes, but is filled with use_hardware_cursor which is copied from
surface->mouse_mode which is uint32_t and confusingly in this
case a TRUE/FALSE value is used.
Maybe now that we know that this value is basically a boolean we can
use mouse_mode as a bool.
On the other hand if somebody used this as a bit field the behaviour
can change a bit.
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