Open question is what to replace this with.  The attachment rewrite is very lazy about thumbnailing (reuses cached Nautilus thumbnails but doesn't generate any thumbnails itself), mainly because I didn't want to continue using deprecated code.

I see two options:

1) Grow a new dependency on GNOME Desktop Library and use 
   GnomeDesktopThumbnailFactory to write our own internal thumbnailing service
   like in Nautilus (dedicated thread processes thumbnail requests on a LIFO).

   Might have to do this for the short-term, though I don't like it.

2) Wait for a platform solution for requesting thumbnails.  I've weighed in
   on bug #498506 (D-Bus API for Nautilus), and have also suggested adding a
   g_file_request_thumbnail_async() to GIO that would return a path to the
   thumbnail file.

   Philip Van Hoof has proposed a desktop-wide thumbnailing daemon:
   http://live.gnome.org/ThumbnailerSpec

   But to my knowledge there's been no consensus.  Someone has to JFDI.

Hmm. Worth bringing this up on d-d-l, ask if others have dropped gnome-thumbnail.h in their modules already, and/or document the potential solution(s) on the wikipage?

Forwarding comment by mclasen on d-d-l today:

"Retrieving of thumbnails is covered in gio/gvfs with the file attributes
thumbnail::path
thumbnail::failed
preview::icon

What is not covered is creation of thumbnails, for which we probably
have to wait for the outcome of the 'thumbnailing service' ideas that
are being floated..."

This would be as simple as using gnome_desktop_thumbnail_scale_down_pixbuf() instead of gnome_thumbnail_scale_down_pixbuf().
As someone said on d-d-l, it should be fine to depend on gnome-desktop for this, as that doesn't depend on any other obsolete libraries.

why does http://library.gnome.org/devel/gnome-desktop/stable/gnome-desktop-Miscellaneous-Thumbnail-Functions.html#gnome-desktop-thumbnail-scale-down-pixbuf propose
    #include <libgnomeui/gnome-thumbnail.h>
instead of
    #include <libgnomeui/gnome-desktop-thumbnail.h>
?
Because I have also seen some #include <libgnomeui/gnome-desktop-thumbnail.h> around, e.g. in g-c-c.
Also, is that "since 2.2" really true on that page?
Wondering about what to add in Evolution's configure.ac...

(In reply to comment #4)
> This would be as simple as using gnome_desktop_thumbnail_scale_down_pixbuf()
> instead of gnome_thumbnail_scale_down_pixbuf().
> As someone said on d-d-l, it should be fine to depend on gnome-desktop for
> this, as that doesn't depend on any other obsolete libraries.

I do not know why the gdk_pixbuf_scale_simple cannot be extended that way, as it's very slow (as stated in the depending bug). That might make things very simple and add a benefit for all the applications, or not?

Worth filing a GTK+ bug about that.

(In reply to comment #7)
> Worth filing a GTK+ bug about that.

Sure, we've one already, it's the bug 488213

Created attachment 138998 [details] [review]
proposed evo patch

for evolution;

This should do the trick. I saw two crashers with this applied, but maybe not related. One is below, and the other was in time when I kept there also thumbnailing of the mime part, it crashed in time of saving to stream in the main thread and in some other thread simultaneously, if I recall correctly. Note that the thumbnail is created in the proper folder, just for some reason nautilus doesn't pick it, but creates its own. I didn't get why. Note also that the function can be called up to 4 times, as my tests showed. It was there when attaching a file.

Thread 1:
...

+ Trace 216575

Are parts of the patch worth to get mentioned in the gtk+ bug 488213?

I do not know, do you meant the part where we might use as a workaround the gnome_desktop_thumbnail function instead of gnome_thumbnail function? I believe it doesn't change anything on the slowness of gdk scale simple function.

You might need to set the GFile's "thumbnail::path" attribute for Nautilus to pick it up.  Not sure, maybe ask alexl.  That should also avoid thumbnailing the same (unchanged) file multiple times, as your patch correctly generates the thumbnail only after checking for "thumbnail::path".

  g_file_info_set_attribute_byte_string (file, "thumbnail::path", thumbnail);

Also, I think the function would be better placed in e-util as there's nothing really attachment-specific about it.  We may find other uses for thumbnailing in the future.  Dave Richard's "image gallery" composer feature, for example.

Created attachment 139055 [details] [review]
proposed evo patch ][

for evolution;

I didn't find any better place than e-icon-factory, thus moved the working part there.

With respect of Nautilus, I will try to catch Alex some time soon. The thing is that nautilus creates and loads a different thumbnail, it generates different checksum for some reason. Maybe because of the mtime parameter. I will see.

Patch looks pretty good.  One hand-wavy comment and a few nit-picky things.

  - Using gnome-desktop is a stop-gap until GNOME grows a better approach to
    generating thumbnails -- either a thumbnailing daemon or a D-Bus API for
    Nautilus.  So I think the patch is good enough for now, but I'm a little
    concerned about blocking the main thread while generating a thumbnail.

    In lieu of an asynchronous API in GnomeDesktopThumbnailFactory, we really
    should run the factory in a dedicated thread and write our own GIO-style
    asynchronous API around it.  But let's wait and see if it becomes a real
    issue first.

  - In configure.ac, it looks like maybe we can also remove the test for
    <libgnomeui/gnome-icon-lookup.h>.  Would be good to knock that off too.

  - In create_system_thumbnail(), g_file_new_for_path() always succeeds.
    There's no need to test for NULL there.

  - In e_icon_factory_create_thumbnail(), instead of g_stat() use
    "g_file_test (filename, G_FILE_TEST_IS_REGULAR)".

  - In the same function, the "g_strconcat ("file://", filename, NULL)" is
    not portable.  Use g_filename_to_uri() instead.

Apart from that it should be good to go.

(In reply to comment #14)
> Patch looks pretty good.  One hand-wavy comment and a few nit-picky things.
> 
>   - Using gnome-desktop is a stop-gap until GNOME grows a better approach to
>     generating thumbnails -- either a thumbnailing daemon or a D-Bus API for
>     Nautilus.  So I think the patch is good enough for now, but I'm a little
>     concerned about blocking the main thread while generating a thumbnail.
> 
>     In lieu of an asynchronous API in GnomeDesktopThumbnailFactory, we really
>     should run the factory in a dedicated thread and ...

No no no, the factory can be only created in the main thread. Though no such thing had been said about the rest of functions from there. Which one you mean?

>   - In configure.ac, it looks like maybe we can also remove the test for
>     <libgnomeui/gnome-icon-lookup.h>.  Would be good to knock that off too.

Sure, shouldn't be an issue at all.

>   - In create_system_thumbnail(), g_file_new_for_path() always succeeds.
>     There's no need to test for NULL there.

How do you know? It returns pointer, and any pointer can be NULL :)

>   - In e_icon_factory_create_thumbnail(), instead of g_stat() use
>     "g_file_test (filename, G_FILE_TEST_IS_REGULAR)".

Seems you overlooked, slightly later in that function is used file_stat.st_mtime

>   - In the same function, the "g_strconcat ("file://", filename, NULL)" is
>     not portable.  Use g_filename_to_uri() instead.

Oh, yup, will do.

(In reply to comment #15)
> No no no, the factory can be only created in the main thread. Though no
> such thing had been said about the rest of functions from there. Which one 
> you mean?

I'm not sure of the details, but I'm pretty sure Nautilus runs (if not creates) the factory in a dedicated thread.  The saving to disk part could be an issue, but moreso the actual thumbnail creation function, which could take a long time to process if handed a sufficiently large file.

> 
> >   - In create_system_thumbnail(), g_file_new_for_path() always succeeds.
> >     There's no need to test for NULL there.
> 
> How do you know? It returns pointer, and any pointer can be NULL :)

The docs guarantee it.  If it ever returns NULL it's a GIO bug.
http://library.gnome.org/devel/gio/unstable/GFile.html#g-file-new-for-path

How about "g_return_val_if_fail (gf != NULL, FALSE)" as a compromise?


> >   - In e_icon_factory_create_thumbnail(), instead of g_stat() use
> >     "g_file_test (filename, G_FILE_TEST_IS_REGULAR)".
> 
> Seems you overlooked, slightly later in that function is used
> file_stat.st_mtime

You're right, I missed that.  Okay, disregard.

Go ahead and commit your next patch revision.  No need to post a new one.

Created commit 290cd2b in evo master (2.27.6+, slightly modified above patch)