<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><blockquote class=""><div class="">On 3 Oct 2017, at 15:56, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="" target="_blank">fziglio@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote class=""><br class="Apple-interchange-newline">On 7 Sep 2017, at 16:49, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="" target="_blank">fziglio@redhat.com</a>> wrote:<br class=""><br class="">Since 2.8, QEMU now longer creates QXL primary surfaces when using GL.<br class=""></blockquote><br class="">Typo: “no longer"<br class=""><br class=""><blockquote class="">This change broke client-side mouse mode, because Spice server relies on<br class="">primary surface conditions.<br class=""></blockquote><br class="">I do not understand what you mean by “primary surface conditions”?<br class="">Do you mean the state of the primary surface, or specific conditions<br class="">testing the primary surface, or something else?<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">In this case the existence of such surface.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><br class=""></div><div>Oh, then “Spice server relies on having a primary surface"</div><div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote class=""><br class="">When GL is enabled, use GL scanout informations.<br class="">Mouse mode is always client when GL surfaces are used.<br class=""><br class="">This patch and most of the message are based on a patch from<br class="">Marc-André Lureau, just moving responsibility from reds to RedQxl.<br class=""><br class="">Signed-off-by: Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="" target="_blank">fziglio@redhat.com</a>><br class="">---<br class="">server/red-qxl.c | 31 +++++++++++++++++++++----------<br class="">server/red-qxl.h | 3 +--<br class="">server/reds.c | 3 +--<br class="">3 files changed, 23 insertions(+), 14 deletions(-)<br class=""><br class="">Changes since v1:<br class="">- do not change qxl_state resolution.<br class=""><br class="">diff --git a/server/red-qxl.c b/server/red-qxl.c<br class="">index b556428d2..93e7eb7b8 100644<br class="">--- a/server/red-qxl.c<br class="">+++ b/server/red-qxl.c<br class="">@@ -903,6 +903,8 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,<br class=""><br class=""> qxl_state->gl_draw_async = async_command_alloc(qxl_state, message,<br class=""> cookie);<br class=""> dispatcher_send_message(qxl_state->dispatcher, message, &draw);<br class="">+<br class="">+ reds_update_client_mouse_allowed(qxl_state->reds);<br class="">}<br class=""><br class="">void red_qxl_async_complete(QXLInstance *qxl, AsyncCommand *async_command)<br class="">@@ -1022,20 +1024,29 @@ void red_qxl_clear_pending(QXLState *qxl_state, int<br class="">pending)<br class=""> clear_bit(pending, &qxl_state->pending);<br class="">}<br class=""><br class="">-gboolean red_qxl_get_primary_active(QXLInstance *qxl)<br class="">+bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int *x_res, int<br class="">*y_res, int *allow_now)<br class=""></blockquote><br class="">While you are at it, why not make allow_now a bool?<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Yes and not. This strange fields came from a mouse_mode field.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">The confusion on this value is that in different cases mouse_mode</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">means a bit field while in this specific instance the field is used</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">as a boolean (0/1). For compatibility int was used.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>I am confused. In the code I am looking at, “allow_now” is a</div><div>local variable in the caller.</div><div><br class=""></div><div>Or are you talking about primary_active?</div></div></blockquote><div>Yes, but is filled with use_hardware_cursor which is copied from<br></div><div>surface->mouse_mode which is uint32_t and confusingly in this<br></div><div>case a TRUE/FALSE value is used.<br></div><div>Maybe now that we know that this value is basically a boolean we can<br></div><div>use mouse_mode as a bool.<br></div><div>On the other hand if somebody used this as a bit field the behaviour<br></div><div>can change a bit.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Had recent discussion with Jonathon on this value and turned</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">out is currently always TRUE (1).</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote class="">{<br class="">- return qxl->st->primary_active;<br class="">-}<br class="">+ // try to get resolution from 3D<br class=""></blockquote><br class="">I would rephrase this comment to be closer to your commit message log,<br class="">i.e. that with 3D enabled, qemu does not create a QXL primary surface.<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Specific suggestions?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>“try to get resolution when 3D enabled, since qemu did not create QXL primary surface”</div><div><br class=""></div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote class="">+ SpiceMsgDisplayGlScanoutUnix *gl;<br class="">+ if ((gl = red_qxl_get_gl_scanout(qxl))) {<br class="">+ *x_res = gl->width;<br class="">+ *y_res = gl->height;<br class="">+ *allow_now = TRUE;<br class="">+ red_qxl_put_gl_scanout(qxl, gl);<br class="">+ return true;<br class="">+ }<br class="">+<br class="">+ // check for 2D<br class="">+ if (!qxl->st->primary_active) {<br class="">+ return false;<br class="">+ }<br class=""></blockquote><br class=""><blockquote class=""><br class="">-gboolean red_qxl_get_allow_client_mouse(QXLInstance *qxl, gint *x_res,<br class="">gint *y_res)<br class="">-{<br class=""> if (qxl->st->use_hardware_cursor) {<br class="">- if (x_res)<br class="">- *x_res = qxl->st->x_res;<br class="">- if (y_res)<br class="">- *y_res = qxl->st->y_res;<br class="">+ *x_res = qxl->st->x_res;<br class="">+ *y_res = qxl->st->y_res;<br class=""> }<br class="">- return qxl->st->use_hardware_cursor;<br class="">+ *allow_now = qxl->st->use_hardware_cursor;<br class="">+ return true;<br class="">}<br class=""></blockquote><br class="">Any reason to not set use_hardware_cursor in the 3D case?<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">For a detailed answer see history in the ML.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">For a short on: is not valid.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">Actually, looking at the rest of the code, I did not understand why we have a<br class="">field for that.<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I think old cards (QXL ones) do not support hardware mouse so you can't</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">get cursor information (for instance position and shape) from the card.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div>I see. Thanks</div><div><br class=""></div><div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote class=""><br class="">void red_qxl_on_ic_change(QXLInstance *qxl, SpiceImageCompression ic)<br class="">diff --git a/server/red-qxl.h b/server/red-qxl.h<br class="">index f925f065b..503ba223d 100644<br class="">--- a/server/red-qxl.h<br class="">+++ b/server/red-qxl.h<br class="">@@ -39,8 +39,7 @@ void red_qxl_async_complete(QXLInstance *qxl,<br class="">AsyncCommand *async_command);<br class="">struct Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl);<br class="">gboolean red_qxl_use_client_monitors_config(QXLInstance *qxl);<br class="">gboolean red_qxl_client_monitors_config(QXLInstance *qxl,<br class="">VDAgentMonitorsConfig *monitors_config);<br class="">-gboolean red_qxl_get_primary_active(QXLInstance *qxl);<br class="">-gboolean red_qxl_get_allow_client_mouse(QXLInstance *qxl, gint *x_res,<br class="">gint *y_res);<br class="">+bool red_qxl_get_allow_client_mouse(QXLInstance *qxl, int *x_res, int<br class="">*y_res, int *allow_now);<br class="">SpiceMsgDisplayGlScanoutUnix *red_qxl_get_gl_scanout(QXLInstance *qxl);<br class="">void red_qxl_put_gl_scanout(QXLInstance *qxl, SpiceMsgDisplayGlScanoutUnix<br class="">*scanout);<br class="">void red_qxl_gl_draw_async_complete(QXLInstance *qxl);<br class="">diff --git a/server/reds.c b/server/reds.c<br class="">index b6ecc6c69..2f4f12ab9 100644<br class="">--- a/server/reds.c<br class="">+++ b/server/reds.c<br class="">@@ -4374,8 +4374,7 @@ void reds_update_client_mouse_allowed(RedsState<br class="">*reds)<br class=""><br class=""> allow_now = TRUE;<br class=""> FOREACH_QXL_INSTANCE(reds, qxl) {<br class="">- if (red_qxl_get_primary_active(qxl)) {<br class="">- allow_now = red_qxl_get_allow_client_mouse(qxl, &x_res,<br class="">&y_res);<br class="">+ if (red_qxl_get_allow_client_mouse(qxl, &x_res, &y_res,<br class="">&allow_now)) {<br class=""> break;<br class=""> }<br class=""> }<br class=""></blockquote></blockquote></div></div></blockquote></div></blockquote><div>Frediano<br></div><div><br></div></div></body></html>