GNOME Bugzilla – Bug 534228
Drop librsvg dependency
Last modified: 2008-11-21 10:05:03 UTC
I made a patch that drops librsvg dependency. I think the dependency is useless, as Gdk has its own pixbuf loader for SVGs, so it's not useful for us to depend on that directly. While removing the dependency, I saw some related FIXMEs in the nautilus-icon-container.c and nautilus-icon-canvas-item.c about a funky implementation of the icon that appears when previewing a file (which is an SVG file). I refactored the code, moving some parts to FMIconView as suggested in the relevant bug report, so that it will be easy in the future to change the icon used for the preview. Comments are welcome, I saw no regressions in my testing.
Created attachment 111286 [details] [review] patch
Created attachment 111310 [details] [review] patch v2 Only emit preview signals once.
Dear Cosimo, thanks for your efforts. I like how you try to clean up our architecture, however maybe you should provide two patches. Regarding the combined "librsvg dependency removal" and "preview refactoring" patch: a) regarding explicit librsvg dependency removal: Due to lack of experience, I'd like to get some feedback by the librsvg maintainers first, so I'm CCIng them: Is it OK to remove the explicit calls to rsvg_pixbuf_from_file_at_zoom_with_max(), and instead use the GDK pixbuf loader? We call it after gdk_pixbuf_new_from_file returned NULL. b) regarding refactoring of preview: It's working perfectly - why should we change it?
If librsvg is installed sanely, it will have added a GdkPixbuf plugin. So you should be able to get away with just using plain GdkPixbufLoader calls.
Dom: OK, thanks a lot for that uber-fast response. You're elegible for a hug and an olympic chocholate gold medal: http://www.honestfoods.com/chocolate-gold-medal.html Cosimo: Please split out the librsvg dependency removal code, and commit it to trunk. Maybe you could close this bug report afterwards, and - if you insist on the preview refactoring - open a new bug report for it?
(In reply to comment #3) > > b) regarding refactoring of preview: > > It's working perfectly - why should we change it? AFAIR (it has been some time ago and I dived in my SoC project since then ;-) ) I did that mainly because those parts of code had a lot of FIXMEs and was screaming loud "refactor me!!" :) Also, I think the refactored code provides much more flexibility if we want to add some kind of other preview by pixbufs. If committing the two parts separately is the way you want me to do this, I'll go for it, but I've lost my git history for this patch and it will be some work splitting the two patches :(
This is obsolete as well...we don't depend on rsvg directly anymore in trunk.