Re: GEP 7 - icons and thumbnails - gnome_icon_loader

[Alexander Larsson <alexl redhat com>]

On 26 Sep 2002, Michael Meeks wrote:

> Hi Alex,
> 
> On Sat, 2002-09-21 at 15:14, Alexander Larsson wrote:
> > So, nobody is commenting on this ...
> 
> 	Ok, I got there in the end; overall it's great. I have a few more
> questions and points though. Also, my suggestion to use a requirements
> GEP seems particularly pointless here; far better to have the actions
> more clearly codified. Would you like me to massage it into a more
> reasonable Action form ?
> 
> 	+ why call it GnomeIconLoader - it does appear to load icons.
> 	  would GnomeIconFinder be a better name ?

Yeah. It's not a perfect name, but I'm not sure GnomeIconFinder is perfect 
either. Maybe GnomeIconTheme?

There is also some code that already uses it (panel, nautilus), so it 
would be some work to rename it. I'm not against renaming it, but i'm not 
sure it's necessary.
 
> 	+ I assume we'd need to include gnome-icon-theme.[ch], is that
> 	  intended as an API for public consumption ? or will it be 
> 	  implementation private ?

You mean gnome-theme-parser.[ch]? It is implementation private.
 
> 	+ I'm not quite clear on (theme_lookup_icon):
> 
>   if (min_dir)
>     {
>       suffix = GPOINTER_TO_INT (g_hash_table_lookup (min_dir->icons,
> icon_name));
>       file = g_strconcat (icon_name, string_from_suffix (suffix), NULL);
> 
> 	What happens if there is no icon of name 'icon_name' in min_dir?
> do we need to detect that before assert failure in string_from_suffix ?
> or is that not possible ?

min_dir is only set in the previous loop if:
 g_hash_table_lookup (dir->icons, icon_name) != ICON_SUFFIX_NONE)
so we shoulc be safe.

> 	+ I assume this rids us of the theme's extra XML description
> 	  file containing the attach points, unifying that into the 
> 	  theme file ? - it seems you bin the per size attach points
> 	  too which is great :-)

Yupp. I designed the icon theme spec with nautilus in mind :)
 
> 	+ I'm slightly concerned about the attach points / text 
> 	  rectangle it looks like you're specifying the points as
> 	  guint16's - why ? is that a fixed point fraction of the
> 	  icon's size ? - or it's ultimate bounding box ? Similarly
> 	  for the embedded text stuff. Of course - quite possibly since
> 	  you're returning pixmaps of a certain size that's no problem,
> 	  but we need some sane way to normalise this on disk so it can
> 	  be scaled [esp. for SVG] - or perhaps I lost it again ? ;-)

It's mostly a memory usage thing, coordinates are in icon coordinates, so 
unless you have 64kx64k icons we're all right. For SVG icons the 
coordinates are in a normalized 1000x1000 coordinate system that maps to 
the size of the rendered svg. (see the icon theme spec)
 
> 	+ I assume that there will be very few GnomeIconData records
> 	  hanging around in memory of the order of ~2, is that so ?

Hmmm. That depends on how many icons use the DisplayName property. There 
certainly won't be many users of attachpoints and text embeding.
 
> 	+ Unsure what theme->display_name, theme->comment are for, is it
> 	  exposed, looks like they are read, stored, destroyed without
>           use ? similarly, what is the GnomeIconData->display_name for ?
> 	  and if neither of these are that useful - do we need the
> 	  localization burden of "Yet another file to localise" :-) at
> 	  least with XML files we have existing tools. So ... in 
> 	  summary:

They are not used in the current code, but a icon theme chooser applet 
would use them.
 
> 	+ is gnome_theme_file_get_locale_string neccessary ? and what
> 	  i18ntools work / autotools burden does this create ?

Not much. See gnome-icon-theme.
 
> 	+ my_g_str_has_suffix: presumably this can be axed in favour of
> 	  the version in glib ?

yes.
 
> 	+ What is the thinking behind 'rescan_if_needed' ? when would
> 	  that be invoked ? idly / timeout - in case someone added new
> 	  icons to a theme, without poking the gconf key ?

Quoting the spec:
-----------------------------------------------------------------------
The algorithms as described in this document works by always looking up 
filenames in directories (a stat in unix terminology). A good 
implementation is expected to read the directories once, and do all 
lookups in memory using that information.

This caching can make it impossible for users to add icons without having 
to restart applications. In order to handle this any implementation that 
does caching is required to look at the mtime of the toplevel icon 
directories when doing a cache lookup, unless it already did so less than 
5 seconds ago. This means that any icon editor or theme installation 
program need only to change the mtime of the the toplevel directory where 
it changed the theme to make sure that the new icons will eventually get 
used.
-----------------------------------------------------------------------

This is not fully implemented yet, as you noticed.

> 	It'd be nice to see an example working icon setup, perhaps I missed one
> in nautilus ?

gnome-icon-theme in gnome cvs.
 
> 	Overall - it's really good it seems to me;

Thanks.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a maverick Jewish card sharp moving from town to town, helping folk in 
trouble. She's a warm-hearted paranoid mechanic with only herself to blame. 
They fight crime!