ShowImage doesn't stop at first/last image

[idefix]

When you have a couple of images in a folder, named:

1a
12
b
c

You'll notice that when you cycle through them with ShowImage, it will stop at the last image (c), but it alternates between the first (1a) and second (12).

When you rename the first image to 1, ShowImage will also stop at the first image.

I think this bug is caused by tracker's [changeset:29845 natural sorting algorithm], where one program (ShowImage) thinks that 12 comes before 1a and the other (Tracker) thinks 12 comes after 1a.

[jonas.kirilla]

Anyone opposed to removing ShowImageView::_FindNextImageByDir() ​http://haiku.it.su.se:8180/source/xref/src/apps/showimage/ShowImageView.cpp#2214 and relying only on Tracker scripting?

Essentially offering Next/Previous image functionality (Alt-Up/Down) only when opened directly from Tracker.

[axeld]

Sounds good to me!

[leavengood]

Replying to jonas.kirilla:

Essentially offering Next/Previous image functionality (Alt-Up/Down) only when opened directly from Tracker.

I don't like this idea. I think there may be potentially several different valid ways to open ShowImage that don't involve Tracker. The Terminal, someone launching ShowImage and then opening an image in a folder, another application opening ShowImage (like the browser in the future maybe.) Losing functionality (not being able to move between images in the same folder) because of this seems inconsistent and not user friendly.

What would be better would to provide a way for Tracker's natural sorting and folder iteration code to be used inside ShowImage, instead of badly trying to emulate it. Like maybe a private API exposed from libtracker.so.

I would say this is one area of many where ShowImage could be refactored because it is duplicating code and functionality. Another that comes to mind is the bitmap smooth scaling, which can now be done efficiently in the app_server.

[idefix]

Replying to jonas.kirilla:

Anyone opposed to removing ShowImageView::_FindNextImageByDir() ​http://haiku.it.su.se:8180/source/xref/src/apps/showimage/ShowImageView.cpp#2214 and relying only on Tracker scripting?

Just tested it here and removing that function fixes this bug. It also fixes ticket:4668.

[jonas.kirilla]

From a spatial-Tracker point-of-view, I would think it crucial to have applications like Mail and ShowImage traverse the files in Tracker-order. (Unless the app has got a list of its own, separate from Tracker, like a MediaPlayer playlist.)

Tracker order is not necessarily the files merely sorted (natural or not). Up to two columns can influence the sorting in list view mode, currently. Who knows what other view modes we will come up with. (Wondering in what order icon mode folders are best traversed.. Maybe top-left to bottom-right, depending on one's locale?)

Offline (closed) folder entry traversion should be done perfectly (which as you say, might need some Tracker cooperation), or not at all, IMO. I don't really mind either way, but I oppose the imperfection in-between.

[jonas.kirilla]

That was supposed to be a reply to leavengood. Oh well. :P

[leavengood]

I definitely want the traversal done consistently too. But I think we are dealing with a bit of an edge case here, and I don't want to lose valid functionality over an edge case.

But I think we can solve this in a nice way with a bit of code sharing. I don't know how nice the Tracker folder traversal code is, but if it ugly we can try to refactor it and expose it nicely in a private header for use by ShowImage and whoever else. The fact that most of Tracker's code is already in libtracker.so just makes it that much easier to share.

[axeld]

I don't think there really is a lot of functionality lost if we remove that feature (well, I never used it myself, at least). And why should ShowImage behave differently from Mail in that regard? That is inconsistent, at least.

If you don't open ShowImage from Tracker, you simply cannot know the context the image was shown in, so you should not make any assumptions, IMO.

[axeld]

Furthermore, Haiku promotes a Tracker centric work flow anyway, and that will get much more powerful with future Tracker enhancements, too.

[leavengood]

Honestly I might be grasping at straws trying to find instances where ShowImage was used outside Tracker. Certainly when testing it I would start it myself most of the time. But probably on day to day use it wouldn't be used that way.

I just don't like to see functionality lost.

But if someone wants to make this change, go ahead. If in further testing it does seem like a loss of functionality we can then investigate other options.

[jonas.kirilla]

Replying to leavengood:

instances where ShowImage was used outside Tracker.

The most prominent one must be opening ShowImage indirectly via Deskbar's Recent Documents list.

[leavengood]

OK, so I talked with Rene and have been more convinced about removing the directory iteration stuff. Plus it helps that it fixes a couple bugs :)

Also we talked about the cases where you could still correctly iterate over multiple images when no Tracker is involved, namely:

• If multiple images are opened from the Terminal, treat those as a list to be iterated over in the order given, instead of opening separate windows, which is what happens now.

• Allow the BFilePanel used for opening images to be multi-select, and treat the multiple selections as a list to iterate over.

I think these changes would be nice and would allow for the best of both worlds.

As for Recent Documents, I guess those will just have to be treated as single images.

[jonas.kirilla]

Replying to leavengood:

cases where you could still correctly iterate over multiple images when no Tracker is involved

As ArgvReceived() calls RefsReceived(), one could (with a few changes to ShowImage) propagate a B_REFS_RECIVED message all the way to the window (or its view, iirc), letting it decide for itself how to enable iteration, based on the message content. (Multiple refs, presence of a messenger, ?..) The window could keep the message and iterate over the contained refs, if multiple.

(Watch out for this though: Currently Tracker's primary means to open (plain "Open") sends refs as individual messages though, which I think should be fixed. "Open With" sends entry_refs as a multi-refs BMessage.)

[idefix]

[patch] remove ShowImageView::_FindNextImageByDir

[axeld]

Thanks for the fix, applied in hrev34103.