Comment 10 for bug 255030

Revision history for this message
arQon (arqon) wrote :

> There is no gnome-wide disable-thumbnail setting, and I doubt you'll convince anyone that it is worth having one.

Exactly so Mike: except that piece appears to have been oversnipped from my notes, yay 4am editing...

> gThumb has a disable-thumbnail setting, although it can only be enabled through gconf-editor. Maybe you can convince eog to do the same

Which is exactly what I did, but you'd have had to read the patch to see it since I didn't make that clear. My bad.

The chmod "fix" isn't a fix for the same reason that dev/null isn't. A user may want thumbs for a photo collection in Shotwell/etc or a set of "active" images, but not want them for images sent in email etc where they just use eog to view them once (which now that I think of it is a perfect example of a case where mindless thumb creation is an especially-bad policy).

Here's the rest of the notes:

===

These bugfixes expose an interesting bug in eog where the "Properties" page fails to update properly on Next/Prev if thumbnails are disabled.
This appears to be because it's using eog_thumb_view_select_single() to step through the list rather than the actual images themselves. It correctly advances the MAIN window to the next image, but since it's dereferencing the wrong object for the dialog, not only does that not update, but attempting to get the properties of any image always shows those of the one from the first USE of the properties dialog (not necessarily the first image in the directory).

This is a major design error rather than a simple implementation bug, but the bigger problem is image_thumb_changed_cb() failing to handle null thumbnails correctly. Since that's a valid return from eog_thumbnail_load(), this is a pre-existing bug in eog (##). The best fix for that bug is probably to just use
"image-loading" or "image-missing", and that's very convenient, because correcting the design would be a huge amount of work, but once we have this other fix we can leverage that to fix the original bug as well, like so:

eog_thumbnail_load (EogImage *image, GError **error)
{
 g_object_ref( eog_missing_image );
 return eog_missing_image;
}

and all we have to do to wrap things up is add eog_missing_image to eog-thumbnail.c as a static and initialise it in eog_thumbnail_init.

## This bug may well be what Felix really meant: it's not so much that there are any "genuine" reasons for the thumbnail-dependency, just that there are parts of the implementation that don't cope properly with them being absent.

Given the existing code this is about as non-invasive as you can get; and it fixes the bug. Those are two pretty compelling arguments for it as a solution.

Note that this is the *correct* implementation of a Nautilus-equivalent "Never Create Thumbnails" setting.
It's arguably not the most *desirable* implementation for eog though, because it essentially makes the "image collection" function useless. Preserving support for that is only slightly more work: all you have to do is replace the flag with a policy: "Never; RAM-only; Persistent" and add another half-dozen lines. So I've done that too.
Although I personally don't find "image collection" useful anyway, even that second approach is a significant improvement over the current behavior, because it saves all the IO (and file system pollution, "security" issues, etc). It does come with CPU and RAM costs, but we've already established that the CPU cost is trivial; and although the RAM cost is ?64K+overhead? per image, if you do want thumbnails you have to accept that they don't come for free.

--

Anyway, getting back to the original bug:

> Whether the bug is that eog doesn't have its own setting or it doesn't honor Nautilus's ...

eog has its own gconf schema, so that's where it belongs. Users will continue to *expect* that Nautilus's setting has global effect, but that ship's long since sailed.

For Nautilus, the flag is apps/nautilus/preferences/show_image_thumbnails.
eog has 4 subkeys: full_screen, plugins, ui, view; none of which are really an appropriate place for this.

GNOME says it prefers that apps not use the (app) root node for keys, but since gconf-editor doesn't allow the creation of subnodes and plenty of official GNOME apps ignore that anyway, we will too.

apps/eog/thumbnail_policy (int)
 0 - Never create thumbnails (or use them even if they exist).
 This matches the behavior of the Nautilus setting.
 1 - Create and use thumbnails, but don't write them to disk.
 This preserves all of the current functionality while removing the IO thrash (nice for SSD, important for USB or non-local /home)
 2 - Save thumbnails
 As pre-patch

===

Sebastien: sorry for the lack of clarity last night. This is a "proper" fix for the bug, fully tested, not some random hack; and from what Mike says it's *at worst* consistent with the gThumb (and Nautilus) behavior, and arguably a superset of it.