Collaborating on this project

[aferrero2707]

Hi! I just discovered the existence of this VIPS-based image viewer, and I am very much interested in helping you to extend the functionalities of vipsdisp.

At the moment I have just forked the project and added some travis ci configuration that allows to automatically build and generate an AppImage package for the application.

However, I have few suggestions on which I am ready to contribute:

1. add proper color management that takes into account the monitor profile (if I understand correctly, at the moment the image is converted to sRGB)
2. improve zooming speed by pre-computing a bunch of scaled-down copies
3. change the "best fit" zoom mode so that it shows the full image, and center it in the preview area
4. full screen and slideshow modes (but at a second stage)

The idea would be to take advantage of VIPS resizing speed and multi-threading to create a fast and lightweight image viewer with accurate color management, something that seems to be in quite some high demand in the FLOSS community.

What do you think?

[jcupitt]

Hey Andrea, nice to hear from you!

Sure, I've not had time to finish it :( My plan was to get vipsdisp mostly done, then paste it into nip2 to make nip3.

The big issue is flickering: as you zoom in and out, you'll see horrible flashing of the image. I've experimented with a range of solutions and I think it has to be something quite complicated :-(

vipsdisp needs to allocate an off-screen client-side pixel buffer the size of the window (not the image!), and paint the screen from that. When you zoom, instead of discarding the old pipeline and linking the display to the new one, it'll need to make a new pipeline and use that to incrementally update the off-screen buffer.

This means that vipsdisp must handle all scrolling in the window: it'll have to implement the scrollable interface, and handle moves that keep content intelligently. What a pain! The current nip2 image viewer has all this code, but it'll need to be copied over and reworked. I was hoping to use the gtk3 scroll system.

The other big TODO is a proper model-view split for rendering parameters. There needs to be an object that holds things like contrast and scale, and the display painter should be linked to it with a set of change signals. Again, the nip2 image viewer has this (it's called conversion, ie. the thing that manages the pixel -> display conversion), it would need to be copied over.

Anyway, that's the point I got to. On your ideas:

1. Good idea! It should be a simple thing to add. A proper conversion model would make it even simpler.

2. I'd like to be able to exploit things like pyramidal tiffs and openslide images too.

3. Agreed.

4. gtk3 full-screen mode is simple to support (I think).

I like "eog", but vipsdisp could (potentially) be nicer, especially with large images (obviously).

[jcupitt]

I've added you as a collaborator, please push away!

Large changes that need discussion are best done via branches and PRs still, but please feel free to push small stuff to master as you like.

[aferrero2707]

Concerning zooming, I had similar problems to solve for photoflow, but I took a different route... I do not like to use the mouse wheel for zooming, because it means that you will not be able to pan the image with a touchpad. In my experience, using two-fingers horizontal and vertical scrolling is the most intuitive and quick way to look through an image in 1:1 zoom mode.

Moreover, I'm ready to bet that most of the users will basically use two zoom modes, "best fit" to look at the image as a whole, and 1:1 for pixel-peeping and checking the image sharpness. All the rest can be handled with keyboard shortcuts, which IMHO are at least as good (if not better) than mouse wheel zooming.

This means you can use the signals of a standard GTK scrolled window in order to handle the image panning, and simply change the values of the adjustments associated to the horizontal and vertical scroll bars when you change the zoom level.

Do you think this makes sense?

For point 1, I am writing some code that bypasses the slow LCMS2 conversions in some specific (but quite common) cases, like conversions between two matrix-base RGB profiles.

For point 2, I am already using in-memory pyramids in photoflow: whenever a new image is opened, I immediately compute the 1:2, 1:4, 1:8... versions, storing them either in memory or raw disk buffers depending on their size. When computing the final image at a given zoom level, the closest pyramid level is picked as a starting point.

What I do not understand is what you mean by "contrast and scale". Do you have in mind to also allow some edits to the image? I would personally not go along this way, at least not at the beginning... an image viewer is not an editor, and we have already plenty of editors out in the wild ;-)

[jcupitt]

For contrast and scale, I need to be able to put pixels through:

V' = a V + b

with sliders for a and b, and with a on a log scale. I'd like things like false-colour too.

My idea with the mousewheel was just to use the standard bindings, so left drag for pan and wheel for scroll. I usually use keybindings myself, of course.

[jcupitt]

Oh, and contrast/scale just affect the conversion for display, they do not change the image, of course. You need them for medical imaging, and they are useful for photography too, if you want to be able to check pixels in very dark or light areas, especially with 16-bit images.

[aferrero2707]

Now I understand! Given that scaling of values works best in linear gamma, here is the kind of pipeline that I would propose, at least for photographic needs: * open image * promote to floating point and normalize to [0..1] (needed for the icc conversions) * convert to a common linear RGB color space, like Rec2020 or ACEScg * pre-compute the zoom levels For displaying the image, here is what I would do: * pick the closest pyramid level, do the final scaling and insert a tile cache * apply the aV+b scaling * convert to the monitor profile and finally to 8-bit integer Does this make sense? Il 04/dic/2017 19:10, "John Cupitt" <notifications@github.com> ha scritto:

…

Oh, and contrast/scale just affect the conversion for display, they do not change the image, of course. You need them for medical imaging, and they are useful for photography too, if you want to be able to check pixels in very dark or light areas, especially with 16-bit images. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF8M9SYBsK_7nF9QgzZyBWOJS6G2mK6wks5s9DWogaJpZM4Q1Afq> .

[jcupitt]

Yes, you can see the two-stage pipeline it's using here:

https://github.com/jcupitt/vipsdisp/blob/master/imagedisplay.c#L329

That's doing any geometric stuff, like zoom and subsample, but does not alter pixel values. The output of this stage is used to update the status bar, for example, which shows the pixel values stored in the file, not as sent to the screen.

Stage two takes that and does radiometric stuff, ie. conversion to 8-bit sRGB pixels for the screen:

https://github.com/jcupitt/vipsdisp/blob/master/imagedisplay.c#L365

So the aV+b would slot in there somewhere.

I see now I started adding a separate conversion.c object to manage these transforms:

https://github.com/jcupitt/vipsdisp/blob/master/conversion.c

but I think it's unfinished.

[jcupitt]

I thought about this some more overnight, and I think we might have slightly different aims.

My desire is to have a testbed for the image display component for nip3, and as a side-benefit, have a small tool for image display. So, for me, all the features of the nip2 image display window are must-haves, including things like overlays, which are probably not so useful for you.

Perhaps we should fork the project after all? libvips 8.6 is (finally!!) almost done (the end of this week, hopefully), so I plan to attack nip3 next, and this project is the next thing on that list.

Perhaps you could just do small things on it for a few months, and then when I get it to the point where I can copy-paste it into nip3, you could take over and do whatever you like. What do you think?

I have a new job, by the way. In January I start full time work on babies brains! I've been part-time on diseased lungs for 12 years now, so it'll be a nice change. I'm hoping I'll be able to do some nip3 work in my new role.

Did you see the what-new-in-8.6 post?

http://jcupitt.github.io/libvips/2017/11/28/What's-new-in-8.6.html

[aferrero2707]

Congrats for your new job!

I have forked the project already, and will do some experimenting in my own copy for a while.

First things I want to try are image pyramids for faster zooming, better positioning of the displayed image when it does not fill the drawing area, and full color management.

My initial goal would be to have something like eog, but faster (particularly with big images), with accurate color management, multi-threaded and multi-platform.

[aferrero2707]

I started to put my hands on your code, in my forked repo.

The main thing I added was the creation of image pyramids when a file is loaded (see code starting from here). There is a define switch at the beginning of the file that allows to enable/disable the use of pyramids, to compare performances.

I also changed the magnification factor to floating point, to allow for non-integer factors and use vips_resize() for the final scaling.

Another modification I made is to start the visualisation in "best fit" mode, and added a couple of methods to refresh the preview in "best fit" mode whenever the window size is changed.

I also observed a couple of issues, namely:

• when I try to open some TIFF files, the program stops with this message:
  (vipsdisp:4594): VIPS-WARNING **: Unknown field with tag 36867 (0x9003) encountered
  When I open the same with photoflow I also get the message, but then the processing continues... do you have any idea of why this warning is fatal in the vipsdisp case?

• now that the zooming speed is increased by the use of pyramids, I sometimes get crashes due to the use of un-referenced objects. I have no clue why...

Let me know your impressions, if you have some time to test the code...

[jcupitt]

Hiya, on 1) there's a thing in disp.c to enable abort on warning:

https://github.com/aferrero2707/vipsdisp/blob/master/disp.c#L164

it's handy for debugging, but you'll want it off for production.

2. Sorry, some kind of race I guess? They are very annoying to debug :(

If I run the non-pyr version, I can whizz the mousewheel up and down very fast and not see the problem, I don't know if that helps.

Races usually happen when callbacks occur in an order the program is not expecting (eg. a paint after the image has been unreffed), so to debug them you add printf()s to the start of all the relevant functions to you can record execution order, then trigger a crash to a logfile, then read the logfile backwards from the end until you find something happening in the wrong order. It's a really horrible job.

[aferrero2707]

there's a thing in disp.c to enable abort on warning

Thanks, that made the trick!

The race condition seems to be gone after fixing some obvious mistake in the gobject references.

I am now scratching my head to find an elegant way to solve the flickering of the image during zooming/panning. One possibility would be to first draw an approximate image by up-scaling the closest (but smaller) pyramid level, and then refine the result with the accurate vips_resize() method.

Meanwhile I have introduced an ICC transform operation that is adapted to floating-point data, and promoted the input image to floating point for better accuracy.

[jcupitt]

As I said, I think we need our own off-screen buffer and to handle all scrolling and resize events ourselves :( what a pain. I'll have a go next week, maybe.

[aferrero2707]

Another thing that I have noticed is that the image updating by vips_sinkscreen() does not seem to saturate the available CPUs, and the tiles seem to be drawn sequentially more than in parallel.

Is there a way to quantify the threading overhead and the amount of parallelisation?

Thanks!

[jcupitt]

I tried watching top while zooming out of a large jpg, and I hit about 170% CPU peak. A long way off the 12 cores this machine has, you're right.

My guess is that for large jpg image, libvips is unpacking to a temporary file, then allocating a single mmap window, sharing that between threads, and scrolling it up and down. With many threads working over a large area, you'll see horrible thrashing as the window is moved constantly.

The idea was to keep virtual memory use down by sharing input regions, but perhaps that's not a good idea in this case. We should probably experiment with a private window for each thread.