After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 307885 - Nautilus don't resize vectorial images when preparing thumbnail : use more than 90% of memory for following file
Nautilus don't resize vectorial images when preparing thumbnail : use more th...
Status: RESOLVED FIXED
Product: libgnomeui
Classification: Deprecated
Component: general
2.11.x
Other All
: High critical
: future
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-06-16 07:21 UTC by Victor STINNER
Modified: 2005-12-09 13:43 UTC
See Also:
GNOME target: ---
GNOME version: 2.11/2.12


Attachments
"Big" svg picture which use all my memory (21.62 KB, image/svg+xml)
2005-06-16 07:22 UTC, Victor STINNER
  Details
Another example (26.16 KB, image/svg+xml )
2005-06-19 13:04 UTC, Victor STINNER
  Details
Ok, you win, here is my first patch to fix this *bug*. (2.28 KB, patch)
2005-06-28 12:35 UTC, Victor STINNER
needs-work Details | Review
New patch which use gnome-vfs and keep aspect ratio (3.61 KB, patch)
2005-12-06 15:50 UTC, Victor STINNER
none Details | Review
Patch 55689 + new type SizePrepareContext to be sure that they use same struct (3.87 KB, patch)
2005-12-09 00:17 UTC, Victor STINNER
none Details | Review

Description Victor STINNER 2005-06-16 07:21:09 UTC
Steps to reproduce:
1. Copy attached file in a directory
2. Try to open the directory with Nautilus

Bug always appears.

The problem is that Nautilus don't resize SVG image before trying to do a
thumbnail. The evil picture is very big (if we speak about pixels) :

$ identify cheese_mateya_01.svg 
cheese_mateya_01.svg SVG 10534x16000 DirectClass 22kb 0.010u 0:01

So, Nautilus should do something like :
$ convert \
 -size '128x128' \ # should use right aspect ratio
 image.svg thumbnail.png

Instead of :
$ convert \
 -resize '128x128' \
 image.svg thumbnail.png
(resize after loading picture)

Stack trace:


Other information:
It's the third time than I post my bug. First I thaugh that it comes from
Gnome-session. Then I realised that only one file need all my memory : a SVG
file. So I reported a bug for librsvg. The team answer that it's not a bug. I
understand their answer. So it's a Nautilus bug :)

Bug #307861 on librsvg :
http://bugzilla.gnome.org/show_bug.cgi?id=307861

Bug #307754 on gnome-session :
http://bugzilla.gnome.org/show_bug.cgi?id=307754
Comment 1 Victor STINNER 2005-06-16 07:22:00 UTC
Created attachment 47851 [details]
"Big" svg picture which use all my memory
Comment 2 Sebastien Bacher 2005-06-19 12:31:21 UTC
according the the librsvg bug this is due to a bugged image and NOTABUG
Comment 3 Victor STINNER 2005-06-19 13:04:51 UTC
Created attachment 47991 [details]
Another example
Comment 4 Victor STINNER 2005-06-19 13:08:43 UTC
I think that you didn't understood the problem ! The problem is that Nautilus
algorithm to generate thumbnail from vectorial pictures is buggy. The vectorial
size is relative, it's not like pixels ! So if you would like a thumbnail with a
size of 128x128, you should ask librsvg to use this size, and not original size !

See the second attachment, a normal SVG picture, not a buggy one. I scaled an
old draw to 5000x5000 pixels. For a SVG about 28 Ko, Nautilus eat *100 MB* to
generate a thumbnail and you said that it isn't a bug ???

Is the *bug* more clear with this example ?

Bye, Haypo
Comment 5 Victor STINNER 2005-06-19 13:42:43 UTC
Ok, I downloaded nautilus and libgnomeui source code. Interresting parts are :

(1) In Nautilus : file ./libnautilus-private/nautilus-thumbnails.c

In function thumbnail_thread_start,
  (...)
  pixbuf = gnome_thumbnail_factory_generate_thumbnail (
    thumbnail_factory, info->image_uri, info->mime_type);
  (...) 

(2) In libgnomeui : file libgnomeui/gnome-thumbnail.c

In function gnome_thumbnail_factory_generate_thumbnail :
  (...)
  script = NULL;
  if (factory->priv->scripts_hash != NULL)
    script = g_hash_table_lookup (factory->priv->scripts_hash, mime_type);
  if (script) (...)
  if (pixbuf == NULL)
    {
#ifdef HAVE_LIBJPEG
      if (strcmp (mime_type, "image/jpeg") == 0)
	pixbuf = _gnome_thumbnail_load_scaled_jpeg (uri, size, size);
      else
#endif
	pixbuf = gnome_gdk_pixbuf_new_from_uri (uri);
    }
  width = gdk_pixbuf_get_width (pixbuf);
  height = gdk_pixbuf_get_height (pixbuf);

  if (width > size || height > size)
    {
      scale = (double)size / MAX (width, height);

      scaled = gnome_thumbnail_scale_down_pixbuf (pixbuf,
						  floor (width * scale + 0.5),
						  floor (height * scale + 0.5));

      g_object_unref (pixbuf);
      pixbuf = scaled;
    }
  return pixbuf;

-------

I reassigned to bug to libgnomeui.

So, would it be possible to :
(1) Get picture size before loading it ?
(2) If yes, would it be possible to scale picture when loading it ?

Bye, Haypo
Comment 6 Victor STINNER 2005-06-19 19:30:31 UTC
I got it ! I don't see easy solution, because my patch will broke libgnomeui-2
API. The function gnome_gdk_pixbuf_new_from_uri should accept (optionnal) size
argument.

So :
    local_path = gnome_vfs_get_local_path_from_uri (uri);
    if (local_path != NULL) {
	pixbuf = gdk_pixbuf_new_from_file (local_path, NULL);
becomes
    local_path = gnome_vfs_get_local_path_from_uri (uri);
    if (local_path != NULL) {
        if (1<=width &&1<=height) 
            pixbuf = gdk_pixbuf_new_from_file_at_size (local_path, width,
height, NULL);
        else
            pixbuf = gdk_pixbuf_new_from_file (local_path, NULL);

And later :
    loader = gdk_pixbuf_loader_new ();
    while (1)
becomes
    loader = gdk_pixbuf_loader_new ();
    if (1<=width &&1<=height) gdk_pixbuf_loader_set_size(loader, width, height);
    while (1)

Then Nautilus (more exactly gnome_thumbnail_factory_generate_thumbnail) should
call the function with width=128 and height=128.

I tried this hack on my computer : it's really faster. Works for SVG. I'm not
sure that it works well with PNG files, but looks to work too. => GTK is a great
library :-D

Bye, Haypo
Comment 7 Victor STINNER 2005-06-28 12:35:29 UTC
Created attachment 48444 [details] [review]
Ok, you win, here is my first patch to fix this *bug*.

This patch is small and doesn't broke libgnomeui API ;-)

I wrote a new function : gnome_gdk_pixbuf_new_from_uri_at_size.

This fuctions will be used by gnome_gdk_pixbuf_new_from_uri with size=(-1,-1),
which means "use original size".

The patch works well with SVG, but I think that Gnome can't load PNG at fixed
size (the picture have to be scaled later). But I can understand that, the
algorithm is more complex for PNG than for SVG.
Comment 8 Kjartan Maraas 2005-07-01 09:46:16 UTC
Matthias, does this look ok to you?
Comment 9 Chris Scobell 2005-07-21 15:34:23 UTC
I can quite painfully confirm this bug using nautilus 2.10.1, it'll go off and
happily eat all your ram. Marking it as NEW.
Comment 10 Mike Robinson 2005-07-21 15:44:36 UTC
Bug reproduced with Nautilus 2.10.1 and libgnomeui 2.10.1.

Patch in comment #7 applies cleanly to libgnomeui 2.10.1, but does not appear to
fix the bug.  Patch does not seem to break anything.

Is a patch to Nautilus required also?  Please could somebody with more
familiarity with the code take a look at this.
Comment 11 Victor STINNER 2005-07-21 16:50:03 UTC
Mike: Nautilus don't need the patch. Are you sure that you installed patched
libgnomeui ? You should restart Gnome (or X11/Xorg ?) after patching libgnomeui.

You can also use the env. variable : LD_LIBRARY_PATH.

Haypo
Comment 12 Mike Robinson 2005-07-21 18:18:26 UTC
Sorry, I didn't have the patched libgnomeui installed correctly.  The patch
works correctly.
Comment 13 Kjartan Maraas 2005-08-30 10:28:04 UTC
Would be ok to get this in if it's the right fix. Could someone with knowledge
of this piece of code please review the patch?
Comment 14 Alexander Larsson 2005-08-31 08:20:36 UTC
That patch is not really right, for two reasons:
* it only changes things for local files (and the local special-case is even
removed in HEAD).
* it scales things wrongly for thumbnails. You don't want to scale to size*size.
You want to scale to the smallest size with the same aspect ratio that fits in
size*size.

Also, it adds API, so it breaks the freeze. (Of course, the added api could be
made internal since the user of it for the fix is in the same lib.)
Comment 15 Kjartan Maraas 2005-09-21 10:21:30 UTC
Victor, could you fix up the patch according to Alex' comments?
Comment 16 Kjartan Maraas 2005-11-20 17:22:34 UTC
Victor? Ping?
Comment 17 Victor STINNER 2005-11-22 00:50:24 UTC
Pong. I'm busy. I will try to make a new patch next month (december) ...
Comment 18 Victor STINNER 2005-12-06 15:50:12 UTC
Created attachment 55689 [details] [review]
New patch which use gnome-vfs and keep aspect ratio
Comment 19 Victor STINNER 2005-12-06 16:00:45 UTC
After months, I wrote a new patch (attachment #55689 [details]). It uses gnome-vfs. I used
part of gdk-pixbuf code to set loader size => connect to signal "size-prepared".
So the patch should be correct as Alexander Larsson asked.

I hope that someone can read, test and maybe commit my patch. You have to know
that I worked on this patch since June, and during a lot of hours (20, 30 or
more). So please, take few minutes to test it.

TODO: Maybe (?) fix comment in gnome-vfs-util.h, function
gnome_gdk_pixbuf_new_from_uri_at_scale(). Parameters meaning are like parameters
of function gdk_pixbuf_new_from_file_at_scale in GDK:
http://developer.gnome.org/doc/API/2.0/gdk-pixbuf/gdk-pixbuf-file-loading.html#gdk-pixbuf-new-from-file-at-scale

TODO: Don't use special code in gnome-thumbnail.c for JPEG as my code do it
automaticly (tested with picture of 2,000x20,000 pixels, 316 MB => works well).

Haypo
Comment 20 Kjartan Maraas 2005-12-08 23:33:19 UTC
Can someone look at this new patch please?
Comment 21 Victor STINNER 2005-12-09 00:17:16 UTC
Created attachment 55794 [details] [review]
Patch 55689 + new type SizePrepareContext to be sure that they use same struct
Comment 22 Alexander Larsson 2005-12-09 13:43:26 UTC
That looks good. I commited it with some docs (we need docs for all API
additions these days).