Re: [Nautilus-list] Icon directory contents caching

[Darin Adler <darin bentspoon com>]

on 10/5/01 7:49 PM, Alex Larsson at alexl redhat com wrote:

> Here is a first rough cut at removing all the stats that the icon-factory
> does.

I'll have to look closely to be sure, but I think these changes may have
messed up some of the subtle points of themed vs. non-themed icon search.
It's important, when looking for variations of an icon, to find variations
only in the same directory that the master icon is in. I worry that with
these changes we might again mix things up and get a non-themed large
version of a themed icon, so display a mix of different style icons for the
same file. It's important for us to choose what kind of icon is to be used
in a way that's independent of the size of variation of the icon being
loaded. This is made (unnecessarily) complex by the "-aa" and other icon
variation features of the icon factory. The code ought to be structured in a
way that makes this much more clear. In fact, having the cache might make
that possible. The existing code can't afford to do the check for which is
the right icon to use separately from actually loading the icon, but if it
was simply a hash table lookup it might be OK to do it that way.

It does seem to me that a hash table is more of a natural than a ptrarray
for holding a "does this file exist?" data structure. But the real issue is
speed, and an O(n^2) can be faster than O(n lg n) if the n values are small
enough.

> First of all, where it used to use gnome_vfs_icon_path_from_filename() i
> now load it manually. This function looks at the files installed in the
> gnome-prefix. It now uses the hardcoded /usr, but it really should use the
> gnome-libs prefix, or otherwise the nautilus prefix.

It's definitely bad to hardcode "/usr". On the other hand, it would be
slightly bizarre to actually compile in the gnome-libs path at nautilus
configure time. I just want to do this the "normal" way. Too bad there is no
normal way.

What package are these global images coming from? Is it really gnome-libs?
Or gnome-core? Or multiple packages?

> There is a small change in behaviour. When it can't find an icon in the
> current or default theme, it used to look for icons in the global dir
> /usr/share/pixmap and /usr/share/pixmap/$theme_name. Now it only looks in
> /usr/share/pixmap. Is this a problem? I thought the original behaviour was
> sort of strange.

If we are looking for a themed icon and we don't find it, we shouldn't look
anywhere else during the themed icon search pass. If there's no themed icon,
we'll fall back to the non-theme icon search. The non-theme icon search
should include the default theme and the global icon places. I agree that
the global icon places don't really need to allow theme name subdirectories.

> I didn't change the code for the theme_is_in_user_directory case. Partly
> because of laziness, but partly because that code seems kind of broken.
> It always seems to read the default icons from the user directory if
> theme_is_in_user_directory, ignoring default_theme_is_in_user_directory.
> It could be made to work with the cache, simplifying the code, if
> fill_theme_icon_dir_cache() took an theme_is_in_user_directory argument
> and fill_default_icon_dir_cache() took a
> default_theme_is_in_user_directory argument.

While I'm not a fan of the theme_is_in_user_directory feature, I dislike
making it more broken (not cached) without removing it. I'd like to fix
these minor problems with theme_is_in_user_directory or remove the feature.
Ideally we'd do that before putting in this cache. Sorry that I won't have
time to tackle that today.

> -        full_path = nautilus_pixmap_file (partial_path);
> +        if (theme != NULL) {
> +            fill_theme_icon_dir_cache (theme);
> +            cache = themed_icon_dir_cache;
> +        } else {
> +            fill_default_icon_dir_cache ();
> +            cache = default_icon_dir_cache;
> +        }
> +
> +        full_path = NULL;
> +        if (icon_dir_cache_contains (cache, partial_path)) {
> +            full_path = nautilus_make_path (cache->directory, partial_path);
> +        } 
> }
> -    
> +
> +    /* Didn't find the icon in the current or default theme, lets look for a
> global icon */
> if (full_path == NULL) {
> -        full_path = gnome_vfs_icon_path_from_filename (partial_path);
> +        fill_global_icon_dir_caches ();
> +
> +        l = global_icon_dir_caches;
> +        while (l) {
> +            cache = l->data;
> +            if (icon_dir_cache_contains (cache, partial_path)) {
> +                full_path = nautilus_make_path (cache->directory,
partial_path);
> +                break;
> +            } 
> +            l = l->next;
> +        }

It would be nice to use functions here to avoid making this gigantic
function even longer.

    -- Darin