! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Change image loading backend from GdkPixbuf to abydos
Status:
RESOLVED: MOVED
Product:
Ristretto
Component:
Application

Comments

Description Magnus Bergman 2019-04-14 02:56:00 CEST
Created attachment 8406 
The patch

This post would be better suited for some development discussion forum, but I din't find any (please redirect me if there is one).

Anyway, this relates to bug 10719 and possibly bug 10499. Unlike GdkPixbuf, abydos handles multi page images (TIFF, DjVu and PDF) and can render vector graphics (SVG, PDF and WMF) directly with cairo, not just create a bitmap version.

Apart from that it has an easier API, especially for animation (my first attempt at a patch removes more lines than it adds). It handles more modern image formats like HEIF and WebP and a whole bunch of older ones, including those dropped by GdkPixbuf (but some can't be handled out of the box since they're not in the shared MIME database and won't be recognized as images to begin with). It also supports animated PNG (so called APNG) and some other things.
Comment 1 Andre Miranda editbugs 2019-04-14 20:50:12 CEST
Thanks for the patch, but I can't apply on the current git master.
I never heard of this lib abydos, I suppose it's somehow related to cairo (egyption cities), however it seems to be not available in any distribution repository (only in Nix, but it's just a font):
https://repology.org/project/abydos/versions

Searching on Google also didn't yield any relevant result, can you share a link to the project website?
Comment 2 Magnus Bergman 2019-04-14 22:46:38 CEST
I should of course have mentioned that abydos is my attempt to work around the limitations of GdkPixbuf then used for image viewers. It can be found here: http://snisurset.net/code/abydos/
Comment 3 Andre Miranda editbugs 2019-04-16 03:57:21 CEST
I see, looks very interesting.
Although I'm not ristretto's maintainer I can say the main concern is that this library is too recent, so it's not available on distros repositories. Also maintainers may be worried with "what do we do if this lib goes unmaintained?".
I think one good approach is to support to abydos as optional, but I don't how much code complexity this would introduce.
Comment 4 Magnus Bergman 2019-04-16 18:35:43 CEST
Another possible concern is that abydos cannot be used for image editing (mentioned in bug 5796 and bug 14733). If an image is loaded into memory as a bitmap (cairo image surface), then rotated, flipped and/or cropped, abydos cannot write the changes back to the file (well cairo can write PNG but that's it). IMHO it would be opening a can of worms to add image editing to an image viewer. It could only flaky at best. For some formats it would be virtually impossible to support. And there would be a lot of strange corner cases then it isn't trivial to even know what to ideally do. For example if the user crops a PDF file, should just the current page be cropped or all pages? What if other pages have different orientation or even different sizes, what should happen? It is  probable that users only expect it to work on small set of image formats (the ones gdk-pixbuf is able to write), including JPEG. And for a lossy format JPEG it can be suboptimal decode, edit and encode the image again since some changes can be done without re-encoding and suffer further data loss. So I suggest invoking some external editing tools for applying the changes. (But honestly I think users who want to edit images should use a real image editor instead.)

It sounds like a good idea to make it a compile time option to use abydos. I could prepare a patch for that. But there is one thing that severely affects the difficulty and complexity of the code: There are two approaches to
loading images (both with gdk-pixbuf and abydos), you could either provide the complete content and get the complete image back in one synchronous operation. That could be done in a thread so it doesn't need to be blocking if it's slow for some reason (which might be a good idea anyhow since the current asynchronous code might not be asynchronous enough considering bug 11577). That would be quite simple to implement. The other way is to progressively feed data to the loader as it becomes available and (in cases it's supported) get a gradually loading image to display during the process. Currently Ristretto adds all the complexity to almost support progressive loading, but actually doesn't (nothing is displayed until the image is completely loaded). So that's the intention here? I guess the only use case for progressive loading is very slow internet connections. (But letting non-sandboxed code process potentially malicious data from the internet could be considered a security hazard.) In other cases it's probably preferable to wait until the whole image is available before anything is displayed (which I guess is the reason why the code does what is does today). I could fix the code so progressive loading works. Otherwise I think what progressive loading should be axed in favor of the simpler approach (for improved maintainability). Who has the final say on this?
Comment 5 Magnus Bergman 2019-08-24 02:20:03 CEST
I filed a bug (bug 15874) for changing the file loading according to the first approach mentioned above (loading images in a separate thread). I will wait for that bug to be resolved before preparing a new patch for this one.
Comment 6 Git Bot editbugs 2020-05-25 00:32:37 CEST
-- GitLab Migration Automatic Message --

This bug has been migrated to xfce.org'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.xfce.org/apps/ristretto/-/issues/26.

Please create an account or use an existing account on one of our supported OAuth providers. 

If you want to fork to submit patches and merge requests please continue reading here: https://docs.xfce.org/contribute/dev/git/start#gitlab_forks_and_merge_requests

Also feel free to reach out to us on the mailing list https://mail.xfce.org/mailman/listinfo/xfce4-dev

Bug #15288

Reported by:
Magnus Bergman
Reported on: 2019-04-14
Last modified on: 2020-05-25

People

Assignee:
Xfce Bug Triage
CC List:
3 users

Version

Version:
master

Attachments

The patch (20.23 KB, patch)
2019-04-14 02:56 CEST , Magnus Bergman
no flags

Additional information