GNOME Bugzilla – Bug 617547
Set default window size with respect to the screen height
Last modified: 2018-05-22 13:52:45 UTC
By default, Evince is not smart regarding window sizes. If you open your very first document, on a huge 1920x1200 screen, the document will open up as a tiny, square window that only uses 1/4 of the screen. Instead of having the user think about resizing this, evince should be smart and at least fill the height of the screen (just like the default PDF reader on Mac OS does) and calculate a decent width to go with that height. It makes no sense to present the user with a tiny default size if he has a big screen resolution. Note: this has nothing to do with saving settings. This is *before* the [sometimes broken] "remember the window size for each paper size" feature even comes into play. What I want is to not have to even care about resizing in the first place.
Created attachment 160208 [details] screenshot (on a 1280x1024 desktop)
*** Bug 671899 has been marked as a duplicate of this bug. ***
Hints to fix it: A quick look to the code, the default size (600x600) is defined here: http://git.gnome.org/browse/evince/tree/shell/ev-window.c#n7649 To get an idea on how it is handled when there is a document, check http://git.gnome.org/browse/evince/tree/shell/ev-window.c#n1306 From there, you can get the window-ratio from gsettings and set the proper height and width with respect to screen. Given that, it is not necessary full screen as fallback, which IMHO looks very ugly in big displays.
Bug 685553 is related; there I suggest that it should open new documents at 'physical' size, if possible.
Created attachment 242978 [details] [review] tentative patch Evince window is 90% of screen height. It's not clear to me how I should use the "window-ratio" setting as in https://git.gnome.org/browse/evince/tree/shell/ev-window.c#n1219 I will be very happy if you can explain! Thanks for your time!
(In reply to comment #5) > Created an attachment (id=242978) [details] [review] > tentative patch > > Evince window is 90% of screen height. > > It's not clear to me how I should use the "window-ratio" setting as in > https://git.gnome.org/browse/evince/tree/shell/ev-window.c#n1219 If we consider the document height == document width, then you should use something like: width = height_screen / height_ratio * width_ratio; "height_screen / height_ratio" would calculate the "document" height that already contains the ratio. You might want to check the Bug 685553 for the case when you evince with a document.
Review of attachment 242978 [details] [review]: ::: shell/ev-window.c @@ +7449,3 @@ + screen = gtk_window_get_screen (GTK_WINDOW (ev_window)); + height = floor (gdk_screen_get_height (screen) * 9 / 10); As Jeff suggested, height should be equal to gdk_screen_get_height (screen) What you should calculate is a proportional width. See comment #6.
Created attachment 243300 [details] [review] I'm using window-ratio as suggested Sorry for my late answer but i was little busy! Now I'm using height_ratio and width_ratio to create a window with height = width. I still have one question: how we decide that height=width is better lookin than, say, height/width = (1+sqrt(5))/2 ? :D Thanks for your time! Alessandro
Review of attachment 243300 [details] [review]: This looks good to me, but please move it to a helper function ev_window_set_default_size(), ev_window_init() is already big enough :-)
Created attachment 243649 [details] [review] code moved in new function ev_window_set_default_size
Review of attachment 243649 [details] [review]: Just pushed to git master, thanks
@Jean-Francois, I think the patch just pushed solves the bug. Please reopen and comment if it does not solve it for you
Commit dcfa0a20fbf1c doesn’t work. I get: :; evince primality.pdf (evince:12519): Gtk-CRITICAL **: gtk_window_set_default_size: assertion `width >= -1' failed and the window opens: Absolute upper-left X: 892 Absolute upper-left Y: 27 Relative upper-left X: 0 Relative upper-left Y: 0 Width: 37 Height: 90 Gdb tells me that the call to g_settings_get() returns width_ratio==0 and height_ratio==0. Why not use gdk_screen_get_width() instead? That should work fine.
Oh, you are right... the patch does not check for that condition. I revert it and reopen this until we have a better solution.
Let me get this straight: in some cases width_ratio and heigth_ratio could be zero, right? So we just need to check, after calling g_settings_get, that width_ratio and height_ratio are positive, but what if they're not? we set evince window width to screen width? thanks!
(In reply to comment #15) > Let me get this straight: in some cases width_ratio and heigth_ratio could be > zero, right? So we just need to check, after calling g_settings_get, that > width_ratio and height_ratio are positive, but what if they're not? we set > evince window width to screen width? IIUC, it can be zero if there is no setting stored. In such cases, the ratio should be calculated from the document (which should never be negative). In no case the ratio should be negative. If such thing exists, maybe we should make it notice with g_warning, g_return_if_fail, or something like that. If the document ratio is negative, the information for the document is broken. If the screen ratio (width and height), the screen information is broken (something is really bad in the computer). That said, the only plausible way the value were negative, is a broken value in gsettings, in which case, it can be ignored.
*** Bug 660534 has been marked as a duplicate of this bug. ***
Created attachment 257860 [details] [review] When a document is opened for the first time, the height of document is set to screen height and width is adjusted appropriately From the bug report, I feel that document height should be set to screen height when the document is newly opened i.e for the first time. Hence I have made changes appropriately. I am a beginner, looking for a guide to my patch.
Review of attachment 257860 [details] [review]: Please, fix the coding style. See some comments inline. Also, I think it is necessary to check the default mode. It looks a bit odd in dual page mode (from 1-3 pages). The width is scaled down, so the pages look smaller and the Evince window has plenty of empty space. Maybe in dual page mode we should give preference to the screen width. Otherwise, the screen height. ::: shell/ev-window.c @@ +1241,1 @@ There must be a space between the function name/macro and the arguments. e.g. screen = gtk_window_get_screen (GTK_WINDOW (window)); @@ +1257,3 @@ + { + request_height = 600; + } No braces when there is only instruction within the if statement. The start brace should be after the condition, not in a single line. Although, you avoid the conditionals by using MAX. e.g. request_height = MAX (request_height, 600); @@ +1263,1 @@ } Same here, space after function names/macros.
Created attachment 258744 [details] [review] Sets default window size with respect to screen size Made the changes suggested above.
Review of attachment 258744 [details] [review]: Thanks for the patch, I still don't understand what problem the patch is trying to solve though. ::: shell/ev-window.c @@ +1238,3 @@ + if (ev_document_model_get_dual_page (window->priv->model)) + document_width = 2 * document_width; This looks to me like a different issue, we should consider double size in dual mode. @@ -1236,3 @@ - request_width = (gint)(width_ratio * document_width + 0.5); - request_height = (gint)(height_ratio * document_height + 0.5); So, this patch makes the window-ratio useless then. If that's the case, it doesn't make sense to run this code only when we have a window-ratio saved in the metadata, no? @@ -1241,3 @@ - if (screen) { - request_width = MIN (request_width, gdk_screen_get_width (screen)); - request_height = MIN (request_height, gdk_screen_get_height (screen)); The commit message should explain better what it does and what it changes, because according to this, we already already setting the window size with respect to screen size.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/evince/issues/148.