bluescan / tacentview

An image and texture viewer for tga, png, apng, exr, dds, pvr, ktx, ktx2, astc, pkm, qoi, gif, hdr, jpg, tif, ico, webp, and bmp files. Uses Dear ImGui, OpenGL, and Tacent. Useful for game devs as it displays information like the presence of an alpha channel and querying specific pixels for their colour.
ISC License
377 stars 36 forks source link

Natural sorting implementation #173

Closed ClangPan closed 4 months ago

ClangPan commented 4 months ago

Hello,

I noticed some annoying sorting behaviors that were annoying to me:

and so I decided to implement a natural sorting rule, which solved both those problems at the same time for all OSs. I haven't profiled it properly, but I haven't noticed any sort of slowdown, even on a large number of files (the biggest I had was 141, and it was instantaneous).

I only implemented the sorting on the Viewer for now, since it was the part that was bothering me the most, but it might be a good idea to include it in more places, especially in the file explorer (where the filetree is also completely out of order, at least on Linux)

bluescan commented 4 months ago

Hi ClangPan. Thanks for the PR. I haven't had a thorough look yet, but first impression is that your sorting order makes a lot of sense... and I can see how you treat integers differenty than the rest of the string.

I don't have a 'contributing' guideline (I should add one), so my first question is are you ok having your code under the ISC license? (it's pretty similar to MIT). If that's no problem what I'll probably do is accept your PR and then maybe move your 'NaturalSort' function over to Tacent (the base library) as a string compare function so it can be used wherever (including the file dialog).

ClangPan commented 4 months ago

No problem for me! Thanks for accepting the PR ^^

ClangPan commented 4 months ago

Did a quick fix due to an oversight: I called the tAtoi function for the comparison on the remain of the string instead of the singular character, which, on top of being unnecessary, doesn't work in some cases (like when the path contains a number)

I replaced it by simply doing a cast to a long to the singular character, works better and isn't any slower.

I also did a quick profiling on the sort over 141 files:

Around 4x slower, but we're talking about comparison to the microsecond, for something that happens only once when you open a folder

ClangPan commented 4 months ago

Argh, nevermind, I broke it again with this one

bluescan commented 4 months ago

Ha. No worries. Yeah... I think single-character compare will not get numbers sorted properly. And no worries on the perf... it looks pretty reasonable.

ClangPan commented 4 months ago

Alright! I got it now, had to take a different approach, but it seems to behave properly

bluescan commented 4 months ago

So the different approach using strtoul looks good. It's actually highlighted some issues with my implementation of tStrtoiT: a) There are no versions which return the final valid numeric digit position (which I can see now is useful ;) ) b) It doesn't eat whitespace at the start. c) It exits with false/0 if ANY character is not a valid given the input base, instead of just stopping there. It's not that it's 'wrong' per se, but the behaviour not as useful as strtol. In any case, problem for another day. I'll try to get around to testing/merging this PR in the next day or two.

bluescan commented 4 months ago

Couple of refs:

https://blog.codinghorror.com/sorting-for-humans-natural-sort-order/ (old but interesting)

https://github.com/sourcefrog/natsort (a github repo with a zlib licensed c implementation).