* [Qemu-devel] [PATCH] vnc: track LED state separately
@ 2016-12-15 13:16 Pierre Ossman
2016-12-15 16:41 ` Gerd Hoffmann
0 siblings, 1 reply; 7+ messages in thread
From: Pierre Ossman @ 2016-12-15 13:16 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Piggy-backing on the modifier state array made it difficult to send
out updates at the proper times. So keep track of the LED state in
a separate variable.
This also moves the handling up a layer in to the VNC Display object
since the state is global, and also makes sure the state is readily
available directly when a client connects.
Signed-off-by: Pierre Ossman <ossman@cendio.se>
---
ui/vnc.c | 56 +++++++++++++-------------------------------------------
ui/vnc.h | 3 ++-
2 files changed, 15 insertions(+), 44 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 2c28a59..c449a7d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1231,8 +1231,6 @@ void vnc_disconnect_finish(VncState *vs)
vnc_update_server_surface(vs->vd);
}
- if (vs->vd->lock_key_sync)
- qemu_remove_led_event_handler(vs->led);
vnc_unlock_output(vs);
qemu_mutex_destroy(&vs->output_mutex);
@@ -1665,69 +1663,38 @@ static void press_key(VncState *vs, int keysym)
qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
}
-static int current_led_state(VncState *vs)
-{
- int ledstate = 0;
-
- if (vs->modifiers_state[0x46]) {
- ledstate |= QEMU_SCROLL_LOCK_LED;
- }
- if (vs->modifiers_state[0x45]) {
- ledstate |= QEMU_NUM_LOCK_LED;
- }
- if (vs->modifiers_state[0x3a]) {
- ledstate |= QEMU_CAPS_LOCK_LED;
- }
-
- return ledstate;
-}
-
static void vnc_led_state_change(VncState *vs)
{
- int ledstate = 0;
-
if (!vnc_has_feature(vs, VNC_FEATURE_LED_STATE)) {
return;
}
- ledstate = current_led_state(vs);
vnc_lock_output(vs);
vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
vnc_write_u8(vs, 0);
vnc_write_u16(vs, 1);
vnc_framebuffer_update(vs, 0, 0, 1, 1, VNC_ENCODING_LED_STATE);
- vnc_write_u8(vs, ledstate);
+ vnc_write_u8(vs, vs->vd->ledstate);
vnc_unlock_output(vs);
vnc_flush(vs);
}
static void kbd_leds(void *opaque, int ledstate)
{
- VncState *vs = opaque;
- int caps, num, scr;
- bool has_changed = (ledstate != current_led_state(vs));
+ VncDisplay *vd = opaque;
+ VncState *client;
trace_vnc_key_guest_leds((ledstate & QEMU_CAPS_LOCK_LED),
(ledstate & QEMU_NUM_LOCK_LED),
(ledstate & QEMU_SCROLL_LOCK_LED));
- caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
- num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0;
- scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
+ if (ledstate != vd->ledstate)
+ return;
- if (vs->modifiers_state[0x3a] != caps) {
- vs->modifiers_state[0x3a] = caps;
- }
- if (vs->modifiers_state[0x45] != num) {
- vs->modifiers_state[0x45] = num;
- }
- if (vs->modifiers_state[0x46] != scr) {
- vs->modifiers_state[0x46] = scr;
- }
+ vd->ledstate = ledstate;
- /* Sending the current led state message to the client */
- if (has_changed) {
- vnc_led_state_change(vs);
+ QTAILQ_FOREACH(client, &vd->clients, next) {
+ vnc_led_state_change(client);
}
}
@@ -3083,8 +3050,6 @@ void vnc_start_protocol(VncState *vs)
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
vnc_read_when(vs, protocol_version, 12);
- if (vs->vd->lock_key_sync)
- vs->led = qemu_add_led_event_handler(kbd_leds, vs);
vs->mouse_mode_notifier.notify = check_pointer_type_change;
qemu_add_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
@@ -3191,6 +3156,8 @@ static void vnc_display_close(VncDisplay *vd)
}
g_free(vd->tlsaclname);
vd->tlsaclname = NULL;
+ if (vd->lock_key_sync)
+ qemu_remove_led_event_handler(vd->led);
}
int vnc_display_password(const char *id, const char *password)
@@ -3758,6 +3725,9 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
vd->lock_key_sync = lock_key_sync;
+ if (lock_key_sync)
+ vd->led = qemu_add_led_event_handler(kbd_leds, vd);
+ vd->ledstate = 0;
vd->key_delay_ms = key_delay_ms;
device_id = qemu_opt_get(opts, "display");
diff --git a/ui/vnc.h b/ui/vnc.h
index d20b154..d8c9de5 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -154,6 +154,8 @@ struct VncDisplay
DisplayChangeListener dcl;
kbd_layout_t *kbd_layout;
int lock_key_sync;
+ QEMUPutLEDEntry *led;
+ int ledstate;
int key_delay_ms;
QemuMutex mutex;
@@ -304,7 +306,6 @@ struct VncState
size_t read_handler_expect;
/* input */
uint8_t modifiers_state[256];
- QEMUPutLEDEntry *led;
bool abort;
QemuMutex output_mutex;
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vnc: track LED state separately
2016-12-15 13:16 [Qemu-devel] [PATCH] vnc: track LED state separately Pierre Ossman
@ 2016-12-15 16:41 ` Gerd Hoffmann
2016-12-16 9:00 ` Pierre Ossman
0 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2016-12-15 16:41 UTC (permalink / raw)
To: Pierre Ossman; +Cc: qemu-devel
On Do, 2016-12-15 at 14:16 +0100, Pierre Ossman wrote:
> Piggy-backing on the modifier state array made it difficult to send
> out updates at the proper times. So keep track of the LED state in
> a separate variable.
>
> This also moves the handling up a layer in to the VNC Display object
> since the state is global, and also makes sure the state is readily
> available directly when a client connects.
Better, thanks.
> static void kbd_leds(void *opaque, int ledstate)
> {
> - VncState *vs = opaque;
> - int caps, num, scr;
> - bool has_changed = (ledstate != current_led_state(vs));
> + VncDisplay *vd = opaque;
> + VncState *client;
>
> trace_vnc_key_guest_leds((ledstate & QEMU_CAPS_LOCK_LED),
> (ledstate & QEMU_NUM_LOCK_LED),
> (ledstate & QEMU_SCROLL_LOCK_LED));
>
> - caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
> - num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0;
> - scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
> + if (ledstate != vd->ledstate)
> + return;
Hmm? Shouldn't that be (ledstate == vd->ledstate)?
cheers,
Gerd
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vnc: track LED state separately
2016-12-15 16:41 ` Gerd Hoffmann
@ 2016-12-16 9:00 ` Pierre Ossman
2017-01-04 8:57 ` Gerd Hoffmann
0 siblings, 1 reply; 7+ messages in thread
From: Pierre Ossman @ 2016-12-16 9:00 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 15/12/16 17:41, Gerd Hoffmann wrote:
>>
>> - caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
>> - num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0;
>> - scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
>> + if (ledstate != vd->ledstate)
>> + return;
>
> Hmm? Shouldn't that be (ledstate == vd->ledstate)?
>
Right, sorry. Brain fart. :)
Can you fix that up before commit or do you want a new patch?
Regards
--
Pierre Ossman Software Development
Cendio AB https://cendio.com
Teknikringen 8 https://twitter.com/ThinLinc
583 30 Linköping https://facebook.com/ThinLinc
Phone: +46-13-214600 https://plus.google.com/+CendioThinLinc
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vnc: track LED state separately
2016-12-16 9:00 ` Pierre Ossman
@ 2017-01-04 8:57 ` Gerd Hoffmann
2017-01-09 16:14 ` Pierre Ossman
0 siblings, 1 reply; 7+ messages in thread
From: Gerd Hoffmann @ 2017-01-04 8:57 UTC (permalink / raw)
To: Pierre Ossman; +Cc: qemu-devel
On Fr, 2016-12-16 at 10:00 +0100, Pierre Ossman wrote:
> On 15/12/16 17:41, Gerd Hoffmann wrote:
> >>
> >> - caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
> >> - num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0;
> >> - scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
> >> + if (ledstate != vd->ledstate)
> >> + return;
> >
> > Hmm? Shouldn't that be (ledstate == vd->ledstate)?
> >
>
> Right, sorry. Brain fart. :)
/me wonders how you've tested the patch ...
> Can you fix that up before commit or do you want a new patch?
Tried, but checkpatch found some more issues:
=== checkpatch complains ===
ERROR: braces {} are necessary for all arms of this statement
#71: FILE: ui/vnc.c:1691:
+ if (ledstate != vd->ledstate)
[...]
ERROR: braces {} are necessary for all arms of this statement
#106: FILE: ui/vnc.c:3159:
+ if (vd->lock_key_sync)
[...]
ERROR: braces {} are necessary for all arms of this statement
#115: FILE: ui/vnc.c:3728:
+ if (lock_key_sync)
[...]
total: 3 errors, 0 warnings, 125 lines checked
Please fix and resend.
thanks,
Gerd
>
> Regards
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vnc: track LED state separately
2017-01-04 8:57 ` Gerd Hoffmann
@ 2017-01-09 16:14 ` Pierre Ossman
0 siblings, 0 replies; 7+ messages in thread
From: Pierre Ossman @ 2017-01-09 16:14 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 04/01/17 09:57, Gerd Hoffmann wrote:
>
> /me wonders how you've tested the patch ...
>
That would be not at all. Or rather, just a compile. :)
>
> Tried, but checkpatch found some more issues:
>
Fixed. New patch incoming.
Regards
--
Pierre Ossman Software Development
Cendio AB https://cendio.com
Teknikringen 8 https://twitter.com/ThinLinc
583 30 Linköping https://facebook.com/ThinLinc
Phone: +46-13-214600 https://plus.google.com/+CendioThinLinc
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] vnc: track LED state separately
2017-01-09 16:14 Pierre Ossman
@ 2017-01-10 11:40 ` Gerd Hoffmann
0 siblings, 0 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2017-01-10 11:40 UTC (permalink / raw)
To: Pierre Ossman; +Cc: qemu-devel
On Mo, 2017-01-09 at 17:14 +0100, Pierre Ossman wrote:
> Piggy-backing on the modifier state array made it difficult to send
> out updates at the proper times.
>
> Signed-off-by: Pierre Ossman <ossman@cendio.se>
Added to ui queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] vnc: track LED state separately
@ 2017-01-09 16:14 Pierre Ossman
2017-01-10 11:40 ` Gerd Hoffmann
0 siblings, 1 reply; 7+ messages in thread
From: Pierre Ossman @ 2017-01-09 16:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Piggy-backing on the modifier state array made it difficult to send
out updates at the proper times.
Signed-off-by: Pierre Ossman <ossman@cendio.se>
---
ui/vnc.c | 59 ++++++++++++++++-------------------------------------------
ui/vnc.h | 3 ++-
2 files changed, 18 insertions(+), 44 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 2c28a59..99ec382 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1231,8 +1231,6 @@ void vnc_disconnect_finish(VncState *vs)
vnc_update_server_surface(vs->vd);
}
- if (vs->vd->lock_key_sync)
- qemu_remove_led_event_handler(vs->led);
vnc_unlock_output(vs);
qemu_mutex_destroy(&vs->output_mutex);
@@ -1665,69 +1663,39 @@ static void press_key(VncState *vs, int keysym)
qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
}
-static int current_led_state(VncState *vs)
-{
- int ledstate = 0;
-
- if (vs->modifiers_state[0x46]) {
- ledstate |= QEMU_SCROLL_LOCK_LED;
- }
- if (vs->modifiers_state[0x45]) {
- ledstate |= QEMU_NUM_LOCK_LED;
- }
- if (vs->modifiers_state[0x3a]) {
- ledstate |= QEMU_CAPS_LOCK_LED;
- }
-
- return ledstate;
-}
-
static void vnc_led_state_change(VncState *vs)
{
- int ledstate = 0;
-
if (!vnc_has_feature(vs, VNC_FEATURE_LED_STATE)) {
return;
}
- ledstate = current_led_state(vs);
vnc_lock_output(vs);
vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
vnc_write_u8(vs, 0);
vnc_write_u16(vs, 1);
vnc_framebuffer_update(vs, 0, 0, 1, 1, VNC_ENCODING_LED_STATE);
- vnc_write_u8(vs, ledstate);
+ vnc_write_u8(vs, vs->vd->ledstate);
vnc_unlock_output(vs);
vnc_flush(vs);
}
static void kbd_leds(void *opaque, int ledstate)
{
- VncState *vs = opaque;
- int caps, num, scr;
- bool has_changed = (ledstate != current_led_state(vs));
+ VncDisplay *vd = opaque;
+ VncState *client;
trace_vnc_key_guest_leds((ledstate & QEMU_CAPS_LOCK_LED),
(ledstate & QEMU_NUM_LOCK_LED),
(ledstate & QEMU_SCROLL_LOCK_LED));
- caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
- num = ledstate & QEMU_NUM_LOCK_LED ? 1 : 0;
- scr = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
-
- if (vs->modifiers_state[0x3a] != caps) {
- vs->modifiers_state[0x3a] = caps;
- }
- if (vs->modifiers_state[0x45] != num) {
- vs->modifiers_state[0x45] = num;
- }
- if (vs->modifiers_state[0x46] != scr) {
- vs->modifiers_state[0x46] = scr;
+ if (ledstate == vd->ledstate) {
+ return;
}
- /* Sending the current led state message to the client */
- if (has_changed) {
- vnc_led_state_change(vs);
+ vd->ledstate = ledstate;
+
+ QTAILQ_FOREACH(client, &vd->clients, next) {
+ vnc_led_state_change(client);
}
}
@@ -3083,8 +3051,6 @@ void vnc_start_protocol(VncState *vs)
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
vnc_read_when(vs, protocol_version, 12);
- if (vs->vd->lock_key_sync)
- vs->led = qemu_add_led_event_handler(kbd_leds, vs);
vs->mouse_mode_notifier.notify = check_pointer_type_change;
qemu_add_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
@@ -3191,6 +3157,9 @@ static void vnc_display_close(VncDisplay *vd)
}
g_free(vd->tlsaclname);
vd->tlsaclname = NULL;
+ if (vd->lock_key_sync) {
+ qemu_remove_led_event_handler(vd->led);
+ }
}
int vnc_display_password(const char *id, const char *password)
@@ -3758,6 +3727,10 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
vd->lock_key_sync = lock_key_sync;
+ if (lock_key_sync) {
+ vd->led = qemu_add_led_event_handler(kbd_leds, vd);
+ }
+ vd->ledstate = 0;
vd->key_delay_ms = key_delay_ms;
device_id = qemu_opt_get(opts, "display");
diff --git a/ui/vnc.h b/ui/vnc.h
index d20b154..d8c9de5 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -154,6 +154,8 @@ struct VncDisplay
DisplayChangeListener dcl;
kbd_layout_t *kbd_layout;
int lock_key_sync;
+ QEMUPutLEDEntry *led;
+ int ledstate;
int key_delay_ms;
QemuMutex mutex;
@@ -304,7 +306,6 @@ struct VncState
size_t read_handler_expect;
/* input */
uint8_t modifiers_state[256];
- QEMUPutLEDEntry *led;
bool abort;
QemuMutex output_mutex;
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-01-10 11:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-15 13:16 [Qemu-devel] [PATCH] vnc: track LED state separately Pierre Ossman
2016-12-15 16:41 ` Gerd Hoffmann
2016-12-16 9:00 ` Pierre Ossman
2017-01-04 8:57 ` Gerd Hoffmann
2017-01-09 16:14 ` Pierre Ossman
2017-01-09 16:14 Pierre Ossman
2017-01-10 11:40 ` Gerd Hoffmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.