GNOME Bugzilla – Bug 689468
Smarter page size and layout options
Last modified: 2018-05-22 14:54:27 UTC
I think we can do a bit better job picking the default document display size in order to make documents easier to read. There are a couple of cases to consider. Documents in portrait/letter orientation should fit the window width up to a point window becomes wider than the natural size of the document (ie. 100%). Using a zoom factor > 100% actually makes the document harder to read in most cases. Consider a maximized Evince window on a large display - fit to width would be quite hard to use. When the window width becomes wide enough to fit two pages at the natural size I think we should try to automatically put them side by side. Documents in landscape orientation should always fill the window in a way similar to what Best Fit does now. This is because they are typically things like presentations and spreadsheets and things like that. Things you typically don't want to scroll. Of course, I think the above should only be the defaults and manually zooming should still be possible. The following patches accomplish these things by: * Redefining Best Fit to only zoom up to 100% for portrait and always fit landscape * Adding two new page layout types (to supplement the existing two dual modes) for Automatic and Single Page layout.
Created attachment 230426 [details] [review] Use a more readable best fit
Created attachment 230427 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual-even, dual-odd. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
*** Bug 494882 has been marked as a duplicate of this bug. ***
*** Bug 494880 has been marked as a duplicate of this bug. ***
Created attachment 230498 [details] [review] Make best fit the default
Review of attachment 230427 [details] [review]: ::: libview/ev-document-model.h @@ +46,3 @@ + EV_PAGE_LAYOUT_DUAL_ODD, +} EvPageLayout; + Need to add this type's get_type function to libevview.types .
Created attachment 230501 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual-even, dual-odd. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
Created attachment 230503 [details] [review] Don't show dual pages by default for landscape
Review of attachment 230426 [details] [review]: Maybe we could rename the best fit mode as page fit, and consider adding a new best fit mode (I'm not sure we want more modes though). Our current best fit mode matches what other document viewers do, and even what eog does when using best fit zoom. Also the patch breaks Fit links, see page 366 of the PDF spec: "Fit: Display the page designated by page, with its contents magnified just enough to fit the entire page within the window both horizontally and vertically."
Review of attachment 230503 [details] [review]: I like the idea of using a EvPageLayout enum. This patch breaks the API, we should deprecated old methods instead of removing them, I think. Do you know the gnome3-style branch? I think new patches changing the toolbar and menu UI should probably added to that branch, that will hopefully merged into master soon (only blocked by bug #687931). In gnome3-style branch the dual mode with odd pages on the left is not a mode anymore, but an option for the dual mode, as suggested by lapo.
Created attachment 230623 [details] [review] Use a more readable best fit
Created attachment 230624 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual-even, dual-odd. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
Created attachment 230625 [details] [review] Make best fit the default
Created attachment 230626 [details] [review] Use a more readable best fit Updated for gnome3 branch
Created attachment 230627 [details] [review] Make best fit the default
Created attachment 230628 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual-even, dual-odd. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
Created attachment 230712 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual-even, dual-odd. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
Review of attachment 230712 [details] [review]: The patch still breaks the API, old properties should be deprecated instead of removed. I'm also concerned about removing the layout buttons from the toolbar and hide them in the view menu. It makes the options hard to discover, makes the view menu even longer, and you need two clicks to change the layout. I wonder if instead of a new mode, the automatic layout might be an option that changes automatically between dual and non-dual.
(In reply to comment #0) > I think we can do a bit better job picking the default document display size in > order to make documents easier to read. > > There are a couple of cases to consider. > > Documents in portrait/letter orientation should fit the window width up to a > point window becomes wider than the natural size of the document (ie. 100%). > Using a zoom factor > 100% actually makes the document harder to read in most > cases. Consider a maximized Evince window on a large display - fit to width > would be quite hard to use. > > When the window width becomes wide enough to fit two pages at the natural size > I think we should try to automatically put them side by side. > > Documents in landscape orientation should always fill the window in a way > similar to what Best Fit does now. This is because they are typically things > like presentations and spreadsheets and things like that. Things you typically > don't want to scroll. > > Of course, I think the above should only be the defaults and manually zooming > should still be possible. > > The following patches accomplish these things by: > > * Redefining Best Fit to only zoom up to 100% for portrait and always fit > landscape > * Adding two new page layout types (to supplement the existing two dual modes) > for Automatic and Single Page layout. After trying the new best fit, I wonder if we can achieve the same without redefining best fit and adding a new mode. The new best fit is actually a combination of fit width + 100%. So, why not use current modes to set them by default depending on the window size and document size? The new automatic layout might change the zoom mode also to use the new best fit approach. I'm also concerned about the name, I find very difficult to explain what the new best fit is in a few words, and 'best fit' is very confusing, even more considering it's used to be different thing. I would probably avoid to use 'best', for example, for me the best fit is always fit-width and it's what I always use by default for all the documents.
Review of attachment 230712 [details] [review]: Sorry, with rejected here I meant needs-work (I guess I'm too used to webkit review process already), mainly because of the api break.
I also think we shouldn't break API/ABI this cycle, esp. for something as trivial as this. It should be very easy to keep the old functions and make them do the right thing wrt. the new mode. If we were to break API, we should do it for bigger things, and/or when we've accumulated more reasons. (E.g. I'd want to rework the backend module stuff, but don't have time this cycle, and I don't think we should break API, and next cycle again, etc.)...
Created attachment 231341 [details] [review] Add a new default mode that optimizes for readability This "Automatic" zoom mode will use fit width when the window is smaller than 100% of the actual page size and then use the actual page size up to the point the window is large enough to hold two entire pages side by side.
Created attachment 231342 [details] [review] Make page layout a mode Instead of having a few different mutually exclusive booleans it makes sense to have it be a mode with the following options: automatic, single, dual. With a separate option to determine whether a separate cover page should be shown in dual page views. This allows us to have a way to automatically determine if dual page mode should be used when the window is large enough.
Created attachment 231343 [details] [review] Remove dual from toolbar
(In reply to comment #22) > Created an attachment (id=231341) [details] [review] > Add a new default mode that optimizes for readability > > This "Automatic" zoom mode will use fit width when > the window is smaller than 100% of the actual page size > and then use the actual page size up to the point the > window is large enough to hold two entire pages side > by side. I've split this patch and pushed to gnome3-style branch the renames (best fit -> fit page and fit page width -> fit width) as separate patches without breaking the api
Review of attachment 231341 [details] [review]: ::: libview/ev-view.c @@ +1556,3 @@ + ev_link_dest_get_bottom (dest) - top, + allocation.width, + allocation.height); We can't change this, it breaks fitr links. @@ +1647,3 @@ + doc_width, doc_height, + allocation.width, + allocation.height); Same here for fit links. @@ +5477,3 @@ + double scale; + + int target_width, I think it would be easier to understand what automatic means calling the other sizing methods, for example here: fit_width_scale = zoom_for_size_fit_width (doc_width, doc_height, target_width, target_height); instead of (double)target_width / doc_width; @@ +5609,3 @@ + } else if (view->sizing_mode == EV_SIZING_AUTOMATIC) + scale = zoom_for_size_automatic (gtk_widget_get_screen (GTK_WIDGET (view)), + doc_width, doc_height, width, height); You should consider the vertical scrollbar here @@ +5642,3 @@ + } else if (view->sizing_mode == EV_SIZING_AUTOMATIC) + scale = zoom_for_size_automatic (gtk_widget_get_screen (GTK_WIDGET (view)), + doc_width, doc_height, width, height); You should consider the vertical scrollbar here too ::: previewer/ev-previewer-window.c @@ +136,3 @@ static void +ev_previewer_window_zoom_automatic (GtkToggleAction *action, + EvPreviewerWindow *window) Why do you remove the fit page option from the evince previewer? ::: shell/evince-toolbar.xml @@ +17,3 @@ <toolitem name="ViewZoomOut"/> + <toolitem name="ViewSizingAutomatic"/> + <toolitem name="ViewSizingFitWidth"/> This is the old evince toolbar that has been removed in gnome3-style branch, si this doesn't have any effect. I forgot to remove this file.
(In reply to comment #25) > (In reply to comment #22) > > Created an attachment (id=231341) [details] [review] [details] [review] > > Add a new default mode that optimizes for readability > > > > This "Automatic" zoom mode will use fit width when > > the window is smaller than 100% of the actual page size > > and then use the actual page size up to the point the > > window is large enough to hold two entire pages side > > by side. > > I've split this patch and pushed to gnome3-style branch the renames (best fit > -> fit page and fit page width -> fit width) as separate patches without > breaking the api I've split the patch more and pushed the libview part to add the automatic sizing mode, but without changing the defaults, so that you can start using it from documents/sushi
commit 0cad8758e9d96a1cf3b8a07776e64b6ae602b4e7 claims to enable an automatic two page mode but that code wasn't committed. How would you prefer we add this capability if not the above?
Created attachment 233007 [details] [review] Add missing enum nickname for automatic sizing mode
Created attachment 233008 [details] [review] Make page layout a mode This adds the mode without breaking API or changing the UI
Created attachment 233009 [details] [review] Use page layout mode internally
(In reply to comment #29) > Created an attachment (id=233007) [details] [review] > Add missing enum nickname for automatic sizing mode It's not that is missing, settings are only used by the shell and automatic sizing mode hasn't been implemented in the shell yet, only in the library.
Review of attachment 233008 [details] [review]: Pushed with some minor changes and removing the settings changes, that belong to the shell, not libview. Thanks!
-- 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/319.