GNOME Bugzilla – Bug 701883
remove multi-screen support
Last modified: 2014-08-29 17:47:44 UTC
gdk_display_get_n_screens is deprecated in gtk 3.9 and will just return 1 always, so there's no need to be clever in trying to support 'multi-screen'.
Hello, I was looking into this and noticed one of the places gdk_display_get_n_screens is used is in the get_num_monitors function. This function, on the other hand, is not called anywhere else. Is it ok to submit a patch that also removes get_num_monitors? Thank you!
Giselle, That seems to be the point made by Christian. So, please submit a patch. The worst case scenario could only be a review stating a better solution path.
Created attachment 273505 [details] [review] Removing multi-screen support Removing calls to gdk_display_get_n_screens and related functions.
Review of attachment 273505 [details] [review]: ::: shell/ev-application.c @@ -118,3 @@ - * Returns: a #GdkDisplay of the display with the passed name. - */ - * Now Each GdkDisplay has only one GdkScreen attached to it. so, you should remove the use of gdk_screen_get_number... but I don't think you want to remove this whole function. ::: shell/ev-utils.c @@ -281,3 @@ -/** - * get_num_monitors: Get the number of user monitors. if this is not used, then remove it in a different patch that only removes this function, so the patch can be pushed directly
Created attachment 273555 [details] [review] Removes multi-screen support This patch only removes parts of the code related to the deprecated functions from gdk.
Created attachment 273556 [details] [review] Removes ev_display_open_if_needed After removing multi-screen support, the function ev_display_open_if_needed is never called. This patch removes this from the code.
Created attachment 273557 [details] [review] Removes get_num_monitors Removing another function that is not used.
Review of attachment 273555 [details] [review]: I don't think we can remove the display option, we can remove the screen one, and use gdk_display_get_default_screen().
Review of attachment 273556 [details] [review]: Same here, the display option is valid I think, you might want to run evince in a different display.
Review of attachment 273557 [details] [review]: Yes, this can be removed after removing all the callers.
Comment on attachment 273557 [details] [review] Removes get_num_monitors Pushed, thanks!
Created attachment 282465 [details] [review] removes multi-screen support Thank you for reviewing! I modified the patch and left the display (removing only the support for multiple screens). I have two questions: 1. I am working with a laptop and an external screen attached (hdmi). This means that there are two displays, right? In this case, how can I test evince to see if it supports multiple displays? 2. If I open evince on display 1, once I select a document on recent view it opens on display 2. Is this the intended behavior?
Comment on attachment 282465 [details] [review] removes multi-screen support Looks good, thanks!
Pushed to master! Thanks :)