[Spice-devel] Patch to fix the bug in spice-client
Alon Levy
alevy at redhat.com
Mon Mar 5 04:53:36 PST 2012
On Wed, Feb 29, 2012 at 08:45:02PM +0530, Divya wrote:
> Hello list
Hi Divya,
Could you please use git sendemail to format the patch and send it to
the list? That makes it easier to review then having it as an
attachment.
Apologies for the late reply.
Alon
>
> I find that spice-client (spicec) does not honour the screen value of
> DISPLAY variable in multi head setup. spicec started from any of the
> DISPLAY, opens up the client window on DISPLAY :0.0. I have fixed this
> issue and attaching the patch herewith. Locally it is working fine for me.
> I would request others to test and review the patch and provide your
> inputs on the same. While working on this issue, I figured out following
> which stopped the client to start at the DISPLAY it was started from:
>
> 1] In the Application::init_remainder method, get_screen() was called with
> value 0 which was further taken up id for the screen.
>
> 2] I fixed up this by returning the XDefaultScreen value from
> Platform::init() and using it further for initialization.
>
> 3] I added a method Platform::get_startup_monitor &
> Application::get_startup_monitor_id to get the monitor id for the screen.
>
> 4] Passed the appropriate monitor reference to the RedScreen, which was
> set to NULL earlier and caused the client window to start at screen 0.
>
> Code inside screen.cpp, which checks for monitor and started the window
>
> <snip>
> void RedScreen::show()
> {
> ߽ן߽ן߽ן RecurciveLock lock(_update_lock);
> ߽ן߽ן߽ן _window.show(_monitor ? _monitor->get_screen_id() : 0);
> }
> </snip>
>
> For this part of the code step 3 was required.
>
> 5] I have added a new variable _default_screen_id in the Application class
> to store the value of screen from which spice client starts. Motivation
> for this name was that in x_display variable this appears as
> default_screen. Application::set_default_screen_id &
> get_default_screen_id() are added as getter and setter for the same. Any
> suggestion on the naming conventions would be appreciated if they do not
> look appropriate in the code.
>
> I also find the same bug being reported in the thread
> [1]http://www.mail-archive.com/spice-devel@lists.freedesktop.org/msg05463.htm
> l
> --
> Thanks & regards,߽ן
> Divya
>
> References
>
> Visible links
> 1. http://www.mail-archive.com/spice-devel@lists.freedesktop.org/msg05463.htm
> l
> diff -Naur b/client/application.cpp a/client/application.cpp
> --- b/client/application.cpp 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/application.cpp 2012-02-29 19:55:59.000000000 +0530
> @@ -333,6 +333,7 @@
>
> Application::Application()
> : ProcessLoop (this)
> + , _default_screen_id(0)
> , _client (*this)
> , _con_ciphers ("DEFAULT")
> , _enabled_channels (SPICE_END_CHANNEL, true)
> @@ -658,18 +659,18 @@
> if (_full_screen && mon) {
> size = mon->get_size();
> }
> - screen = _screens[id] = new RedScreen(*this, id, _title, size.x, size.y);
> + screen = _screens[id] = new RedScreen(*this, id, _title, size.x, size.y, mon);
> #ifdef USE_GUI
> create_gui_barrier(*screen, id);
> #endif // USE_GUI
>
> if (id != 0) {
> if (_full_screen) {
> - mon = get_monitor(id);
> - screen->set_monitor(mon);
> - rearrange_monitors(false, true, screen);
> + mon = get_monitor(id);
> + screen->set_monitor(mon);
> + rearrange_monitors(false, true, screen);
> } else {
> - screen->show(false, _main_screen);
> + screen->show(true, NULL);
> }
> }
> } else {
> @@ -1647,6 +1648,18 @@
> _monitors = NULL;
> }
>
> +int Application::get_startup_monitor_id(int default_screen)
> +{
> + int id;
> + id = Platform::get_startup_monitor(default_screen);
> + return id;
> +}
> +
> +void Application::set_default_screen_id(int default_screen)
> +{
> + _default_screen_id = default_screen;
> +}
> +
> void Application::init_monitors()
> {
> exit_full_screen();
> @@ -2543,18 +2556,22 @@
>
> /* seperated from init_globals to allow --help to work
> * faster and not require X on linux. */
> -void Application::init_platform_globals()
> +int Application::init_platform_globals()
> {
> - Platform::init();
> + int def_screen;
> + def_screen = Platform::init();
> RedWindow::init();
> + return def_screen;
> }
>
> -void Application::init_remainder()
> +void Application::init_remainder(int default_screen)
> {
> + int id;
> Platform::set_process_loop(*this);
> init_monitors();
> + id = get_startup_monitor_id(default_screen);
> init_menu();
> - _main_screen = get_screen(0);
> + _main_screen = get_screen(id);
>
> Platform::set_event_listener(this);
> Platform::set_display_mode_listner(this);
> @@ -2581,7 +2598,7 @@
>
> int Application::main(int argc, char** argv, const char* version_str)
> {
> - int ret;
> + int ret, default_screen;
> bool full_screen;
> char *log_level_str;
>
> @@ -2595,12 +2612,13 @@
> std::auto_ptr<Application> app(new Application());
> AutoAbort auto_abort(*app.get());
> if (app->process_cmd_line(argc, argv, full_screen)) {
> - init_platform_globals();
> - app->init_remainder();
> + default_screen = init_platform_globals();\
> + app->set_default_screen_id(default_screen);
> + app->init_remainder(default_screen);
> if (full_screen) {
> app->enter_full_screen();
> } else {
> - app->_main_screen->show(true, NULL);
> + app->_main_screen->show(true, NULL);
> }
> ret = app->run();
> cleanup_platform_globals();
> diff -Naur b/client/application.h a/client/application.h
> --- b/client/application.h 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/application.h 2012-02-27 21:53:21.000000000 +0530
> @@ -171,6 +171,7 @@
> VISIBILITY,
> DISCONECTING,
> };
> +
>
> #ifdef USE_GUI
> enum GuiMode {
> @@ -254,7 +255,8 @@
> void set_menu(Menu* menu);
> void delete_menu();
> void beep();
> -
> + int get_default_screen_id() { return _default_screen_id ; }
> + void set_default_screen_id(int default_screen);
> #ifdef USE_GUI
> bool is_disconnect_allowed();
> void hide_gui();
> @@ -302,6 +304,7 @@
> void restore_screens_size();
> Monitor* find_monitor(int id);
> Monitor* get_monitor(int id);
> + int get_startup_monitor_id(int default_screen);
> void init_monitors();
> void destroy_monitors();
> void assign_monitors();
> @@ -346,10 +349,10 @@
>
> static void init_logger();
> static void init_globals();
> - static void init_platform_globals();
> + static int init_platform_globals();
> static void cleanup_platform_globals();
> static void cleanup_globals();
> - void init_remainder();
> + void init_remainder(int default_screen);
>
> friend class DisconnectedEvent;
> friend class ConnectionErrorEvent;
> @@ -389,6 +392,7 @@
> bool _enable_controller;
> AutoRef<Controller> _controller;
> AppMenuItemMap _app_menu_items;
> + int _default_screen_id ;
> #ifdef USE_GUI
> std::auto_ptr<GUI> _gui;
> AutoRef<GUITimer> _gui_timer;
> diff -Naur b/client/display_channel.cpp a/client/display_channel.cpp
> --- b/client/display_channel.cpp 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/display_channel.cpp 2012-02-27 21:24:34.000000000 +0530
> @@ -1506,7 +1506,7 @@
>
> if (do_create_primary) {
> LOG_INFO("");
> - attach_to_screen(get_client().get_application(), get_id());
> + attach_to_screen(get_client().get_application(), get_client().get_application().get_default_screen_id());
> clear_area();
> AutoRef<CreatePrimarySurfaceEvent> event(new CreatePrimarySurfaceEvent(*this, width, height,
> format));
> diff -Naur b/client/platform.h a/client/platform.h
> --- b/client/platform.h 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/platform.h 2012-02-08 16:33:24.000000000 +0530
> @@ -32,7 +32,7 @@
>
> class Platform {
> public:
> - static void init();
> + static int init();
> static void set_process_loop(ProcessLoop& main_process_loop);
> static void msleep(unsigned int millisec);
> static void yield();
> @@ -50,6 +50,7 @@
> static bool is_monitors_pos_valid();
>
> static void send_quit_request();
> + static int get_startup_monitor(int default_screen);
>
> enum ThreadPriority {
> PRIORITY_INVALID,
> diff -Naur b/client/screen.cpp a/client/screen.cpp
> --- b/client/screen.cpp 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/screen.cpp 2012-02-08 14:01:41.000000000 +0530
> @@ -71,7 +71,7 @@
> _screen->periodic_update();
> }
>
> -RedScreen::RedScreen(Application& owner, int id, const std::string& name, int width, int height)
> +RedScreen::RedScreen(Application& owner, int id, const std::string& name, int width, int height, Monitor *monitor)
> : _owner (owner)
> , _id (id)
> , _refs (1)
> @@ -88,7 +88,7 @@
> , _update_timer (new UpdateTimer(this))
> , _composit_area (NULL)
> , _update_mark (1)
> - , _monitor (NULL)
> + , _monitor (monitor)
> , _default_cursor (NULL)
> , _inactive_cursor (NULL)
> , _pixel_format_index (0)
> diff -Naur b/client/screen.h a/client/screen.h
> --- b/client/screen.h 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/screen.h 2012-02-08 14:00:42.000000000 +0530
> @@ -54,7 +54,7 @@
>
> class RedScreen: public RedWindow::Listener {
> public:
> - RedScreen(Application& owner, int id, const std::string& name, int width, int height);
> + RedScreen(Application& owner, int id, const std::string& name, int width, int height, Monitor *monitor);
>
> RedScreen* ref();
> void unref();
> diff -Naur b/client/x11/platform.cpp a/client/x11/platform.cpp
> --- b/client/x11/platform.cpp 2012-02-08 13:59:02.000000000 +0530
> +++ a/client/x11/platform.cpp 2012-02-08 16:34:24.000000000 +0530
> @@ -2360,6 +2360,18 @@
> return monitors;
> }
>
> +int Platform::get_startup_monitor(int default_screen)
> +{
> + MonitorsList::iterator mon_iter = monitors.begin();
> + for (; mon_iter != monitors.end() ; mon_iter++) {
> + Monitor *mon = *mon_iter;
> + if (mon->get_id() == default_screen) {
> + return mon->get_id();
> + }
> + }
> + return 0;
> +}
> +
> void Platform::destroy_monitors()
> {
> primary_monitor = NULL;
> @@ -3062,7 +3074,7 @@
> return XGetVisualInfo(x_display, VisualIDMask | VisualScreenMask, &vtemplate, &count);
> }
>
> -void Platform::init()
> +int Platform::init()
> {
> #ifdef USE_OGL
> int err, ev;
> @@ -3193,6 +3205,7 @@
> sigaction(SIGFPE, &act, NULL);
>
> atexit(cleanup);
> + return XDefaultScreen(x_display);
> }
>
> void Platform::set_process_loop(ProcessLoop& main_process_loop)
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list