From 33be8633f5712062752efe75adc745130a72c4c2 Mon Sep 17 00:00:00 2001 From: Gal Hammer Date: Mon, 14 Nov 2011 12:51:33 +0200 Subject: [PATCH 01/12] client: handle the redundant right ctrl windows' message send when a alt-gr is pressed bz#709074 Hello, I first updated the translate_key function. It now requires the windows message as parameter (will be used later). It also use the raw wparam and lparam parameters in order to remove the code duplication when calling the function. Gal. --- client/windows/red_window.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/client/windows/red_window.cpp b/client/windows/red_window.cpp index 39960dc7..a4740104 100644 --- a/client/windows/red_window.cpp +++ b/client/windows/red_window.cpp @@ -52,12 +52,13 @@ static inline int to_red_mouse_state(WPARAM wParam) ((wParam & MK_RBUTTON) ? SPICE_MOUSE_BUTTON_MASK_RIGHT : 0); } -static inline RedKey translate_key(int virtual_key, uint32_t scan, bool escape) +static inline RedKey translate_key(UINT message, WPARAM wParam, LPARAM lParam) { + uint32_t scan = HIWORD(lParam) & 0xff; if (scan == 0) { return REDKEY_INVALID; } - switch (virtual_key) { + switch (wParam) { case VK_PAUSE: return REDKEY_PAUSE; case VK_SNAPSHOT: @@ -74,13 +75,16 @@ static inline RedKey translate_key(int virtual_key, uint32_t scan, bool escape) } else if (scan == 0xf2) { return REDKEY_KOREAN_HANGUL; } + break; default: - //todo: always use vitrtual key - if (escape) { - scan += REDKEY_ESCAPE_BASE; - } - return (RedKey)scan; + break; } + // TODO: always use virtual key + bool extended = ((HIWORD (lParam) & KF_EXTENDED) != 0); + if (extended) { + scan += REDKEY_ESCAPE_BASE; + } + return (RedKey)scan; } static inline void send_filtered_keys(RedWindow* window) @@ -214,7 +218,7 @@ LRESULT CALLBACK RedWindow_p::WindowProc(HWND hWnd, UINT message, WPARAM wParam, break; case WM_SYSKEYDOWN: case WM_KEYDOWN: { - RedKey key = translate_key(wParam, HIWORD(lParam) & 0xff, (lParam & (1 << 24)) != 0); + RedKey key = translate_key(message, wParam, lParam); window->get_listener().on_key_press(key); BYTE key_state[256]; @@ -237,7 +241,7 @@ LRESULT CALLBACK RedWindow_p::WindowProc(HWND hWnd, UINT message, WPARAM wParam, } case WM_SYSKEYUP: case WM_KEYUP: { - RedKey key = translate_key(wParam, HIWORD(lParam) & 0xff, (lParam & (1 << 24)) != 0); + RedKey key = translate_key(message, wParam, lParam); window->get_listener().on_key_release(key); break; } @@ -1049,8 +1053,7 @@ static LRESULT CALLBACK MessageFilterProc(int nCode, WPARAM wParam, LPARAM lPara switch (msg->message) { case WM_SYSKEYUP: case WM_KEYUP: { - RedKey key = translate_key(msg->wParam, HIWORD(msg->lParam) & 0xff, - (msg->lParam & (1 << 24)) != 0); + RedKey key = translate_key(msg->message, wParam, lParam); filtered_up_keys.push_back(key); break; } From 9ffa2e9990dc5d5ae61c227d10d5234753c08402 Mon Sep 17 00:00:00 2001 From: Gal Hammer Date: Mon, 14 Nov 2011 12:51:39 +0200 Subject: [PATCH 02/12] client: handle the redundant right ctrl windows' message send when a alt-gr is pressed bz#709074 Hello, The second patch check to see if Windows is sending a fake VK_CONTROL message when the user pressed Alt-Gr when using a non-US keyboard layout (German, Czech, etc...). If the function is_fake_ctrl return true and key event is translated to a REDKEY_INVALID and the event is discarded. Gal. --- client/windows/red_window.cpp | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/client/windows/red_window.cpp b/client/windows/red_window.cpp index a4740104..c86c458e 100644 --- a/client/windows/red_window.cpp +++ b/client/windows/red_window.cpp @@ -52,6 +52,33 @@ static inline int to_red_mouse_state(WPARAM wParam) ((wParam & MK_RBUTTON) ? SPICE_MOUSE_BUTTON_MASK_RIGHT : 0); } +// Return true if VK_RCONTROL is followed by a VK_RMENU with the same timestamp. +static bool is_fake_ctrl(UINT message, WPARAM wParam, LPARAM lParam) +{ + if ((wParam == VK_CONTROL) && ((HIWORD (lParam) & KF_EXTENDED) == 0)) { + UINT next_peek; + if (message == WM_KEYDOWN) { + next_peek = WM_KEYDOWN; + } else if (message == WM_SYSKEYUP) { + next_peek = WM_KEYUP; + } else { + next_peek = WM_NULL; + } + if (next_peek != WM_NULL) { + MSG next_msg; + LONG time = GetMessageTime(); + BOOL msg_exist = PeekMessage(&next_msg, NULL, + next_peek, next_peek, PM_NOREMOVE); + if ((msg_exist == TRUE) && (next_msg.time == time) && + (next_msg.wParam == VK_MENU) && + (HIWORD (next_msg.lParam) & KF_EXTENDED)) { + return true; + } + } + } + return false; +} + static inline RedKey translate_key(UINT message, WPARAM wParam, LPARAM lParam) { uint32_t scan = HIWORD(lParam) & 0xff; @@ -76,6 +103,13 @@ static inline RedKey translate_key(UINT message, WPARAM wParam, LPARAM lParam) return REDKEY_KOREAN_HANGUL; } break; + case VK_CONTROL: + // Ignore the fake right ctrl message which is send when alt-gr is + // pressed when using a non-US keyboard layout. + if (is_fake_ctrl(message, wParam, lParam)) { + return REDKEY_INVALID; + } + break; default: break; } From dedaa9ac39acc2235d1839cdc5ff291c90f158be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrg=20Billeter?= Date: Sun, 27 Nov 2011 16:02:18 +0100 Subject: [PATCH 03/12] server: Move $(Z_LIBS) from INCLUDES to LIBADD in Makefile.am MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes undefined references to deflate* when building tests. Signed-off-by: Jürg Billeter --- server/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/Makefile.am b/server/Makefile.am index 34a6b47b..12847040 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -6,7 +6,6 @@ INCLUDES = \ -I$(top_srcdir) \ -I$(top_srcdir)/common \ -DRED_STATISTICS \ - $(Z_LIBS) \ $(CELT051_CFLAGS) \ $(PIXMAN_CFLAGS) \ $(PROTOCOL_CFLAGS) \ @@ -48,6 +47,7 @@ libspice_server_la_LIBADD = \ $(SASL_LIBS) \ $(SLIRP_LIBS) \ $(SSL_LIBS) \ + $(Z_LIBS) \ $(NULL) libspice_server_la_SOURCES = \ From 5b042031b0bdacde4d5306c860ced6372f8f4104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 14 Dec 2011 00:27:17 +0100 Subject: [PATCH 04/12] spelling: s/cupture/capture --- client/red_window.h | 2 +- client/screen.cpp | 2 +- client/windows/red_window.cpp | 2 +- client/x11/red_window.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/red_window.h b/client/red_window.h index dc5bad79..3dea26bd 100644 --- a/client/red_window.h +++ b/client/red_window.h @@ -66,7 +66,7 @@ public: void set_cursor(LocalCursor* local_cursor); void hide_cursor(); void show_cursor(); - void cupture_mouse(); + void capture_mouse(); void release_mouse(); void start_key_interception(); void stop_key_interception(); diff --git a/client/screen.cpp b/client/screen.cpp index 317618be..94f3bdcc 100644 --- a/client/screen.cpp +++ b/client/screen.cpp @@ -537,7 +537,7 @@ void RedScreen::capture_mouse() _mouse_captured = true; _window.hide_cursor(); reset_mouse_pos(); - _window.cupture_mouse(); + _window.capture_mouse(); } void RedScreen::relase_mouse() diff --git a/client/windows/red_window.cpp b/client/windows/red_window.cpp index c86c458e..1345e918 100644 --- a/client/windows/red_window.cpp +++ b/client/windows/red_window.cpp @@ -695,7 +695,7 @@ bool RedWindow::get_mouse_anchor_point(SpicePoint& pt) return true; } -void RedWindow::cupture_mouse() +void RedWindow::capture_mouse() { RECT client_rect; POINT origin; diff --git a/client/x11/red_window.cpp b/client/x11/red_window.cpp index 2e98ae25..0c959256 100644 --- a/client/x11/red_window.cpp +++ b/client/x11/red_window.cpp @@ -1867,7 +1867,7 @@ void RedWindow::release_mouse() sync(true); } -void RedWindow::cupture_mouse() +void RedWindow::capture_mouse() { int grab_retries = MOUSE_GRAB_RETRIES; XLockDisplay(x_display); From ee315779a0f8b449e0748059a4dd147d68f2be73 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 15 Dec 2011 13:12:31 +0100 Subject: [PATCH 05/12] server/red_parse_qxl.h: License should be LGPLv2+ rather then GPLv2+ Also fixup the header of server/red_parse_qxl.c, which still contained some GPL (program rather then library) text in its header. Signed-off-by: Hans de Goede --- server/red_parse_qxl.c | 6 +++--- server/red_parse_qxl.h | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c index 7737c047..743a82d2 100644 --- a/server/red_parse_qxl.c +++ b/server/red_parse_qxl.c @@ -7,10 +7,10 @@ License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. - This program is distributed in the hope that it will be useful, + This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with this library; if not, see . diff --git a/server/red_parse_qxl.h b/server/red_parse_qxl.h index 93978522..c2edfb92 100644 --- a/server/red_parse_qxl.h +++ b/server/red_parse_qxl.h @@ -2,18 +2,18 @@ /* Copyright (C) 2009,2010 Red Hat, Inc. - This program is free software; you can redistribute it and/or - modify it under the terms of the GNU General Public License as - published by the Free Software Foundation; either version 2 of - the License, or (at your option) any later version. + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. - This program is distributed in the hope that it will be useful, + This library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. - You should have received a copy of the GNU General Public License - along with this program. If not, see . + You should have received a copy of the GNU Lesser General Public + License along with this library; if not, see . */ #ifndef RED_ABI_TRANSLATE_H From cf21af1846939ba9bb2927f63433125ed9c8fbb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 15 Dec 2011 16:51:27 +0100 Subject: [PATCH 06/12] build: remove unused variable --- client/red_client.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/client/red_client.cpp b/client/red_client.cpp index 2f6c6ce7..f3da4c92 100644 --- a/client/red_client.cpp +++ b/client/red_client.cpp @@ -274,7 +274,6 @@ void* Migrate::worker_main(void *data) void Migrate::start(const SpiceMsgMainMigrationBegin* migrate) { - std::string cert_subject; uint32_t peer_major; uint32_t peer_minor; From ffc4de01e6f9ea0676f17b10e45a137d7e15d6ac Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 18 Dec 2011 11:12:56 +0100 Subject: [PATCH 07/12] spicevmc: Fix assert when still connected on session disconnect (fdo#43903) Currently when the main channel disconnects while a spicevmc channel (such as a usbredir channel) is still connected, qemu will abort with the following message: ring_remove: ASSERT item->next != NULL && item->prev != NULL failed This is caused by red_client_destroy() first calling: rcc->channel->client_cbs.disconnect(rcc); And then calling: red_channel_client_destroy(rcc); For each channel. This is fine, but the spicevmc disconnect code does a red_channel_client_destroy(rcc) itself since as usb devices are added / removed, the channels carrying their traffic get connected / disconnected and they get re-used for new devices, which won't work if the old channel is still there when the new connection comes in. This patch fixes the double destroy when there are still spicevmc channels connected by not doing the red_channel_client_destroy from the spicevmc disconnect code when not just the channel, but the entire client is disconnecting. Signed-off-by: Hans de Goede --- server/spicevmc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/spicevmc.c b/server/spicevmc.c index 85809840..b1a7d8dc 100644 --- a/server/spicevmc.c +++ b/server/spicevmc.c @@ -99,7 +99,11 @@ static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc) sin = state->chardev_sin; sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base); - red_channel_client_destroy(rcc); + /* Don't destroy the rcc if the entire client is disconnecting, as then + red_client_destroy will already do this! */ + if (!rcc->client->disconnecting) + red_channel_client_destroy(rcc); + state->rcc = NULL; if (sif->state) { sif->state(sin, 0); From 5d28d1662e6e415367bb283d051e0a690a8ec2f2 Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Tue, 6 Dec 2011 15:36:38 +0200 Subject: [PATCH 08/12] client controller/foreign_menu: use memmove instead of memcpy in readers When src/dst memory areas may overlap, it's safer to use memmove. --- client/controller.cpp | 2 +- client/foreign_menu.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/controller.cpp b/client/controller.cpp index e7c4b955..91c00212 100644 --- a/client/controller.cpp +++ b/client/controller.cpp @@ -216,7 +216,7 @@ bool ControllerConnection::read_msgs() pos += hdr->size; } if (nread > 0 && pos != _read_buf) { - memcpy(_read_buf, pos, nread); + memmove(_read_buf, pos, nread); } _read_pos = _read_buf + nread; return true; diff --git a/client/foreign_menu.cpp b/client/foreign_menu.cpp index 926e266f..00cc57ca 100644 --- a/client/foreign_menu.cpp +++ b/client/foreign_menu.cpp @@ -237,7 +237,7 @@ bool ForeignMenuConnection::read_msgs() pos += hdr->size; } if (nread > 0 && pos != _read_buf) { - memcpy(_read_buf, pos, nread); + memmove(_read_buf, pos, nread); } _read_pos = _read_buf + nread; return true; From 24d5852611c3d5be3ba824af64cd5a3356b82b9c Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Tue, 20 Dec 2011 16:36:13 +0200 Subject: [PATCH 09/12] client: menu: make RedWindow::set_menu() return an error-code (#758260) RedWindow::set_menu() can fail (on Windows when in fullscreen mode). For Windows spice-client, when in fullscreen mode, the system-menu is NULL. Returns 0 upon success, non-0 (currently only -1) upon failure. --- client/red_window.h | 2 +- client/windows/red_window.cpp | 14 +++++++++++--- client/x11/red_window.cpp | 3 ++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/client/red_window.h b/client/red_window.h index 3dea26bd..82353aa1 100644 --- a/client/red_window.h +++ b/client/red_window.h @@ -70,7 +70,7 @@ public: void release_mouse(); void start_key_interception(); void stop_key_interception(); - void set_menu(Menu* menu); + int set_menu(Menu* menu); #ifdef USE_OPENGL void untouch_context(); diff --git a/client/windows/red_window.cpp b/client/windows/red_window.cpp index 1345e918..981fe9a5 100644 --- a/client/windows/red_window.cpp +++ b/client/windows/red_window.cpp @@ -1064,18 +1064,26 @@ void RedWindow_p::release_menu(Menu* menu) } } -void RedWindow::set_menu(Menu* menu) +int RedWindow::set_menu(Menu* menu) { release_menu(_menu); _menu = NULL; if (!menu) { - return; + return 0; } - _menu = menu->ref(); + _sys_menu = GetSystemMenu(_win, FALSE); + if (! _sys_menu) { + return -1; + } + + _menu = menu->ref(); + insert_separator(_sys_menu); insert_menu(_menu, _sys_menu, _commands_map); + + return 0; } static LRESULT CALLBACK MessageFilterProc(int nCode, WPARAM wParam, LPARAM lParam) diff --git a/client/x11/red_window.cpp b/client/x11/red_window.cpp index 0c959256..2d179f89 100644 --- a/client/x11/red_window.cpp +++ b/client/x11/red_window.cpp @@ -2218,8 +2218,9 @@ void RedWindow::on_pointer_leave() } } -void RedWindow::set_menu(Menu* menu) +int RedWindow::set_menu(Menu* menu) { + return 0; } void RedWindow::init() From a91b0b3ff712eb2a7d91a951f2af7842495357c3 Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Tue, 20 Dec 2011 14:46:50 +0200 Subject: [PATCH 10/12] client: update menu if needed when exiting full-screen mode (#758260) --- client/screen.cpp | 7 ++++++- client/screen.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/client/screen.cpp b/client/screen.cpp index 94f3bdcc..0b3ba6f0 100644 --- a/client/screen.cpp +++ b/client/screen.cpp @@ -100,6 +100,7 @@ RedScreen::RedScreen(Application& owner, int id, const std::string& name, int wi , _mouse_captured (false) , _active_layer_change_event (false) , _pointer_on_screen (false) + , _menu_needs_update (false) { region_init(&_dirty_region); set_name(name); @@ -784,6 +785,9 @@ void RedScreen::exit_full_screen() _origin.x = _origin.y = 0; _window.set_origin(0, 0); show(); + if (_menu_needs_update) { + update_menu(); + } _full_screen = false; _out_of_sync = false; _frame_area = false; @@ -875,7 +879,8 @@ void RedScreen::external_show() void RedScreen::update_menu() { AutoRef menu(_owner.get_app_menu()); - _window.set_menu(*menu); + int ret = _window.set_menu(*menu); + _menu_needs_update = (ret != 0); /* try again if menu update failed */ } void RedScreen::on_exposed_rect(const SpiceRect& area) diff --git a/client/screen.h b/client/screen.h index a08415a3..6c470ca1 100644 --- a/client/screen.h +++ b/client/screen.h @@ -178,6 +178,7 @@ private: bool _key_interception; bool _update_by_timer; bool _size_locked; + bool _menu_needs_update; int _forec_update_timer; AutoRef _update_timer; RedDrawable* _composit_area; From fdcef173645e564be71f1b73d476c0716e91663d Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Tue, 13 Dec 2011 17:09:58 +0200 Subject: [PATCH 11/12] client: foreign-menu: pass "active" param when creating a ForeignMenu (#769020) The default stays the same -- false. A race could prevent setting ForeignMenu::_active correctly. That happened when Application::on_app_activated was called before _foriegn_menu was created. When foriegn_menu was created its _active defaults to false, and that has not changed, until focus was taken out and back in spice-client window. This caused usbrdr to sometimes not auto-share devices, unless the user switched focus to a different application and back to spicec. The fix updates ForiegnMenu::_active upon creation. --- client/application.cpp | 2 +- client/foreign_menu.cpp | 4 ++-- client/foreign_menu.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/application.cpp b/client/application.cpp index decf8a19..e120dfe7 100644 --- a/client/application.cpp +++ b/client/application.cpp @@ -599,7 +599,7 @@ int Application::run() void Application::on_start_running() { - _foreign_menu.reset(new ForeignMenu(this)); + _foreign_menu.reset(new ForeignMenu(this, _active)); if (_enable_controller) { _controller.reset(new Controller(this)); return; diff --git a/client/foreign_menu.cpp b/client/foreign_menu.cpp index 00cc57ca..d1df49d0 100644 --- a/client/foreign_menu.cpp +++ b/client/foreign_menu.cpp @@ -36,9 +36,9 @@ #define PIPE_NAME "/tmp/SpiceForeignMenu-%lu.uds" #endif -ForeignMenu::ForeignMenu(ForeignMenuInterface *handler) +ForeignMenu::ForeignMenu(ForeignMenuInterface *handler, bool active) : _handler (handler) - , _active (false) + , _active (active) , _refs (1) { char pipe_name[PIPE_NAME_MAX_LEN]; diff --git a/client/foreign_menu.h b/client/foreign_menu.h index 2fc4e535..6138087a 100644 --- a/client/foreign_menu.h +++ b/client/foreign_menu.h @@ -38,7 +38,7 @@ public: class ForeignMenu : public NamedPipe::ListenerInterface { public: - ForeignMenu(ForeignMenuInterface *handler); + ForeignMenu(ForeignMenuInterface *handler, bool active = false); virtual ~ForeignMenu(); ForeignMenu* ref() { _refs++; return this;} From a3a3b34a46f57ce86da444532e1db292638a74cd Mon Sep 17 00:00:00 2001 From: Uri Lublin Date: Thu, 22 Dec 2011 10:51:19 +0200 Subject: [PATCH 12/12] client: RedScreen::RedScreen: fix initialization order of _menu_needs_update Related to a91b0b3ff712eb2a7d91a951f2af7842495357c3 --- client/screen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/screen.cpp b/client/screen.cpp index 0b3ba6f0..a0dc0dfa 100644 --- a/client/screen.cpp +++ b/client/screen.cpp @@ -87,6 +87,7 @@ RedScreen::RedScreen(Application& owner, int id, const std::string& name, int wi , _key_interception (false) , _update_by_timer (true) , _size_locked (false) + , _menu_needs_update (false) , _forec_update_timer (0) , _update_timer (new UpdateTimer(this)) , _composit_area (NULL) @@ -100,7 +101,6 @@ RedScreen::RedScreen(Application& owner, int id, const std::string& name, int wi , _mouse_captured (false) , _active_layer_change_event (false) , _pointer_on_screen (false) - , _menu_needs_update (false) { region_init(&_dirty_region); set_name(name);