GNOME Bugzilla – Bug 642683
Hardcoded cache size limit restricts maximum zoom to 70% on large (area) documents
Last modified: 2013-06-09 15:48:15 UTC
Forwarded from: http://launchpad.net/bugs/721217 per useful pointer from José on https://bugzilla.gnome.org/show_bug.cgi?id=303365#c48 Evince restricts the maximum zoom level for large PDF documents in order that they can be rendered into a hardcoded cache size. For example, with a document that is nominally: 2743 mm × 914 mm the only zoom levels given to the user are: 50% and 70% this means that the text is impossibly small, and hard to read. Ideally the full allowable zoom range should be presented to the user, regardless of the theoretical paper size; which is completely hypothetical, because it's digital and just bits and bytes. It seems that Evince has a hardcoded cache size limit (in an effort to avoid hanging the user's machine). Ideally tiled rendering, or a more dynamic cache-size selecting algorithm should be used (a 100MB limit on a machine with 3GB makes little sense).
See bug 303365.
We could add a gsettings key to change the cache size if it helps. Although I agree a dynamic cache-size selecting algorithm would be better (maybe based on current free mem rather than total mem)
*** Bug 604723 has been marked as a duplicate of this bug. ***
Any news wrt. this bug? I have recently been reading lots of PDF with building floor plans, and in evince they are even less readable than on a dead-tree medium.
The solution for this particular bug would be to add a Gsetting with the cache size to be read by evince at startup time.
Created attachment 244531 [details] [review] Patch to add new GSetting for page cache size Something like this?
Review of attachment 244531 [details] [review]: Thanks! ::: data/org.gnome.Evince.gschema.xml.in @@ +67,3 @@ </key> + <key name="page-cache-size" type="t"> + <default>52428800</default> Even though it's a "hidden" setting, it's still for users to play with the value, I think it would be easier to use if the value is in MB, using 50 as default one. Then we can convert it before calling ev_view_set_page_cache_size ::: shell/ev-window.c @@ +1297,3 @@ EvDocumentModel *model = ev_window->priv->model; GSettings *settings = ev_window->priv->default_settings; + gsize page_cache_size; You are using "t" as the value type, so this should be guint64, I think gsize is a size_t which is unsigned int. @@ +1319,3 @@ + /* View */ + g_settings_get (settings, "page-cache-size", "t", &page_cache_size); + ev_view_set_page_cache_size (EV_VIEW(ev_window->priv->view), page_cache_size); I think we should also monitor the value and update the view cache when it changes. That way you can play with the value without having to close evince and open it again.
(In reply to comment #7) > Review of attachment 244531 [details] [review]: > > Thanks! > > ::: data/org.gnome.Evince.gschema.xml.in > @@ +67,3 @@ > </key> > + <key name="page-cache-size" type="t"> > + <default>52428800</default> > > Even though it's a "hidden" setting, it's still for users to play with the > value, I think it would be easier to use if the value is in MB, using 50 as > default one. Then we can convert it before calling ev_view_set_page_cache_size > Sure! > ::: shell/ev-window.c > @@ +1297,3 @@ > EvDocumentModel *model = ev_window->priv->model; > GSettings *settings = ev_window->priv->default_settings; > + gsize page_cache_size; > > You are using "t" as the value type, so this should be guint64, I think gsize > is a size_t which is unsigned int. > I checked here before doing that: https://developer.gnome.org/glib/2.30/glib-Basic-Types.html#gsize And that told me it was an unsigned long., but it doesn't matter, if we switch to MB we can use unsigned int. > @@ +1319,3 @@ > + /* View */ > + g_settings_get (settings, "page-cache-size", "t", &page_cache_size); > + ev_view_set_page_cache_size (EV_VIEW(ev_window->priv->view), > page_cache_size); > > I think we should also monitor the value and update the view cache when it > changes. That way you can play with the value without having to close evince > and open it again. Sure! Check patch below.
Created attachment 245027 [details] [review] Updated after review
It's not much code, but alot of it's duplicated. Is a helper function called for?
Created attachment 245109 [details] [review] Add helper Less duplicated code in this one.
Created attachment 245110 [details] [review] Borked last one Forgot commit --amend on last one :/
Created attachment 245111 [details] [review] I hate bugzilla
Created attachment 245112 [details] [review] I hate myself
I look forward for this enhancement to be implemented)
Review of attachment 245112 [details] [review]: Thanks! Please, move the setting to the global schema ::: data/org.gnome.Evince.gschema.xml.in @@ +68,3 @@ + <key name="page-cache-size" type="u"> + <default>50</default> + </key> Sorry, I didn't notice this in the previous patch, but this setting should not be in the default schema (which is for default settings of new documents when there's no metadata). This setting should be like the override-restrictions one. ::: shell/ev-window.c @@ +1332,3 @@ + g_signal_connect (settings, + "changed::page-cache-size", + G_CALLBACK (update_page_cache_size), I prefer something like page_cache_size_changed @@ +1333,3 @@ + "changed::page-cache-size", + G_CALLBACK (update_page_cache_size), + ev_window); This could be connected in ev_window_ensure_settings like the override-restrictions one.
Created attachment 245487 [details] [review] Update after review Something like this? The calling of the function page_cache_size_changed to set the startup size feels a bit awkward, but maybe it's ok?
Review of attachment 245487 [details] [review]: I've just pushed the patch to git master with the minor modifications I suggest below. Thank you very much. ::: data/org.gnome.Evince.gschema.xml.in @@ +28,3 @@ + <key name="page-cache-size" type="u"> + <_summary>Page cache size in Mb</_summary> + <_description>The maximum size that will be used to cache rendered pages, limits maximum zoom level.</_description> This is incorrectly indented, and goes after the default value for consistency with the other keys. ::: shell/ev-window.c @@ +1263,3 @@ + guint page_cache_mb; + + page_cache_mb = g_settings_get_uint (settings, "page-cache-size"); You should use the macro here too GS_PAGE_CACHE_SIZE @@ +1386,3 @@ + + /* Set page cache size on view */ + page_cache_size_changed (priv->settings, NULL, ev_window); We want to set the page cache asap, before anything is load in the view to make sure we don't need to reload, so this should be moved to ev_window_init right after the view is created, that way the pixbuf cache is created with this value
Thanks! For people googling and finding this: You can check the current value of page cache with: $ gsettings get org.gnome.Evince page-cache-size And set a new value with: $ gsettings set org.gnome.Evince page-cache-size 100