GNOME Bugzilla – Bug 65902
Selecting an image from a multiple-image file type (like ICO)
Last modified: 2018-05-22 13:00:44 UTC
Galeon has the feature to download favicon.ico files when bookmarking sites. This file contains small images that are used in bookmark menus. These files may contain more many images of different size. The problem is that galeon always seems to pick the first image, even if that isn't the correct size. Then galeon rescales that image to something like 16x16, which doesn't look good at all. Example: www.freshmeat.net/favicon.ico This file contains three images, as reported by my tool icoutils (http://freshmeat.net/projects/icoutils/): --icon --index=0 --width=48 --height=48 --colors=256 --icon --index=1 --width=32 --height=32 --colors=256 --icon --index=2 --width=16 --height=16 --colors=256 Galeon picks the first image and resizes it to 16x16. However, the third image is already 16x16, and looks much better (no sharp edges). (Use icoutils or ImageMagick to extract the separate image to see for yourself.)
we use gdk-pixbuf for handling the downloaded icon, i am not sure what the best solution for this is probably extending gdk-pixbuf in some nondisturbing manner like adding a loader api for preselecting the image in the composite one
this is something for gdk-pixbuf, not for galeon.
uhm fixed is not really the case, i'll move this to gdk-pixbuf i strongly suspect this will be WONTFIX
Couldn't this be achieved by making the ICO loader do the right thing wrt. to load_at_size, which has been requested already ? (see http://bugzilla.gnome.org/show_bug.cgi?id=53726) On the other hand, there may be compound image types where it would be more appropriate to have a selection API independend from load_at_size.
*** Bug 68721 has been marked as a duplicate of this bug. ***
It may be more appropriate to represent ICO and other compound image files as GtkIconSource.
My current (sort-of) plan for this is to have a new load_collection method which would load all images in a multi-image format and return an ImageCollection which would be similar to an Animation (maybe Animation should be made a subclass of ImageCollection) and also allow to iterate through all images. One problem I still see with this is that for formats like ico, where the multiple images are really just variants of one motif, the app is going to need additional information about the variation to make sensible choices. Some parameters, e.g. size or number of channels are already available from the pixbufs, but others, e.g. number of colors, are not.
I think it is OK as long as the application can find out number of bits per pixel (1,4,8,16,24 or 32 in the case of ICO files). Maybe that selection-possiblity should be incorporated into http://bugzilla.gnome.org/show_bug.cgi?id=53726 as well?
mathias: I have code to to this, I'll wrap up a patch as soon as possible.
Anders: code to implement general image collections, or code to do load_at_size_and_depth ?
To implement image collections. The code probably clashes with your patch to implement registration of pixbuf loaders. I'll attach a patch as soon as I can.
Can't wait to see it...
Created attachment 10589 [details] [review] gdk-pixbuf-collection
Here's the collection code. I need to refactor some of the io-ico code to make it work with both collections and single pixbufs so I'll attach that code later.
Hmm, I had envisioned something a bit more abstract, ie, make the collection class a superclass of animation and have iterators on it - what you have here is just a thinly disguised GdkPixbuf*[]. A more abstract implementation would allow to implement load_collection in terms of load_animation for animation formats. It may also allow to load svgs without converting them to a raster image and represent them as a collection with an unlimited number of images, which would be rendered on demand. But maybe this is over-the-top, and the simple approach covers enough ground. I'll try to come up with an alternative patch, then we can compare the approaches. gdk_pixbuf_collection_get_bits_per_sample is a) buggy: the factor 8 in there is superfluous b) useless: the pixbuf->bits_per_sampple is the number of bits used in the memory representation of the pixbuf and currently hardwired to 8. What we are interested in is the bpp/number of colors of the loaded image. This information is available to the loader, but is lost unless we attach it to the pixbuf in some way, maybe as an option.
Sorry, the comments above were based on superficial reading of the patch. The comment about the uselessness of gdk_pixbuf_collection_get_bits_per_sample doesn't hold any water, since it refers to the non-collection implementation. I still think that it would be nicer if animations were just special collections. With the current animation code, there doesn't even seem to be a simple way to wrap a collection around an animation, since there is no way to step through it frame-by-frame. Looking forward to see io-ico-collection.c and the refactored io-ico.c... if you refactor io-ico.c please also fix the stupidity that it continuously decodes its header...
Hi Anders, is your collection-loading-ICO-loader still forthcoming ? Otherwise I might start working on that. I've just finished adding get_collection methods to the animation classes. Works nicely to display animated gifs or cursors in your testpixbuf-collection. I'll attach a patch as soon as canvas stops claiming that I do not exist.
Here is the promised patch. Also contains the necessary changes for incremental collection loading. I've omitted the testpixbuf-collection stuff from your patch.
Created attachment 10959 [details] [review] animations as collections
It occurred to me that adding a virtual function to GdkPixbufAnimation might be considered an ABI change and thus prevent inclusion of the patch in 2.2. If this is the case, here is an alternative patch which comes without gdk_pixbuf_animation_get_collection and the get_collection virtual function, but still supports loading animations as collections.
Created attachment 10968 [details] [review] animations as collections - ABI unchanged
loading ico animations is almost done, just needs a bit of cleanup and robustification before I attach the patch.
Created attachment 11008 [details] [review] new patch
The last patch includes a refactored ico loader which loads a collection . The three collection implementations (gif, ani and ico) have been moved into their own .h/.c files. I also added a gdk_pixbuf_collection_get_static_image which wraps the corresponding animation functions for gif and ani, and implements the former selection algorithm of the ico loader for ico collections. This ensures that existing users of the ico loader get the same pixbufs as they used to. Feedback on the last patch would be most welcome...
Final version for now; these patches are getting large... This one incorporates some feedback from Owen and gets load_at_size right for collections.
Created attachment 11143 [details] [review] new patch
*** Bug 133760 has been marked as a duplicate of this bug. ***
*** Bug 142246 has been marked as a duplicate of this bug. ***
I finally took the time to look at this. Great work! Being able to see animations as collections is a great idea, and this API will work with multiple-page TIFFs as well. One minor comment. It looks like gdk_pixbuf_ani_collection_get_static_image should really be gdk_pixbuf_ani_collection_load_static_image. I really do think that this should go into 2.7 as soon as we branch.
Has there been any further work on including the patch? There still seems to be no way to easily get multiple images out of an .ico
So... ten years later. I bet the patches don't apply any more... What about that functionality? Is that something still missing? Is it still desired or solved better differently?
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gdk-pixbuf/issues/1.