Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scan folder recursively misbehaviour #297

Open
keinerweiss opened this issue Dec 28, 2018 · 7 comments · May be fixed by #1148
Open

Scan folder recursively misbehaviour #297

keinerweiss opened this issue Dec 28, 2018 · 7 comments · May be fixed by #1148

Comments

@keinerweiss
Copy link

Scenario 1:

  1. Activate "File -> Scan folder recursively"
  2. Click "File -> Open a directory"
  3. Chose a directory with one image and at least 2 subfolders with each one image in it.
  4. Click "Open"
  5. The preview tiles show only the image in the selected folder and none of the subfolders

Scenario 2:

  1. Activate "File -> Scan folder recursively"
  2. Click "File -> Open a directory"
  3. Chose a directory with zero images and at least 2 subfolders with each one image in it.
  4. Click "Open"
  5. The preview tiles show only the image in the first subfolder (not from the second subfolder)
@diemmarkus
Copy link
Member

hi,

thanks for reporting. the preview tiles do not properly handle recursive scans. this is because it might become very complex (i.e. if the root folder is selected) and due to design issues in our loader. I will keep this issue open, but it might take a while because we need to redesign the loader to properly fix this behavior.

best,
markus

@ggoebel
Copy link

ggoebel commented Aug 26, 2020

It has been 20 months. Any updates?

Current status on arch linux running nomacs v3.17.0 with "Scan folder recursively" set appears to be that it displays images found in one of the directories that was found recursively.

@blissend
Copy link

blissend commented Jun 29, 2022

Noticing similar behavior with it not including subfolders. Any plans to implement now that it's been a couple of years? This is on Linux using version 3.17.0

@Korb
Copy link

Korb commented Apr 25, 2023

nomacs 3.17.2206 [x64] (Aug 13 2020, Portable), OpenCV 4.3.0, Qt 5.14.2, Microsoft Windows [Version 10.0.19045.2846] — same.

Do I understand correctly that "Scan Folder Recursive no longer works" (#797, Aug 8, 2021) discusses the same problem of not being able to display all images from all subfolders of the current folder, as implemented in IrfanView?

@anarcat
Copy link

anarcat commented Aug 18, 2024

seeing the same issue here, still present in 3.19.

@anarcat
Copy link

anarcat commented Aug 18, 2024

it looks like the code that's supposed to do this never worked. from the original implementation (0da0072) shipped in 1.6.4, this line was already commented:

https://github.com/nomacs/nomacs/blame/ad0c650ae56d14ef4d1fac94ff249e3987915263/ImageLounge/src/DkImage.cpp#L1601

// getFoldersRecursive(dirs.filePath(), subFolders);
// qDebug() << "loop: " << dirs.filePath();

that code is still there, unchanged. i wonder why? maybe recursive scans didn't work so well?

actually, scratch all of that, QDirIterator is supposed to handle recursive parsing on its own, something else is going on here.

the problem is the "recursive" parser is explicitly built (since the beginning) to stop at the first folder with images:

https://github.com/nomacs/nomacs/blob/master/ImageLounge/src/DkCore/DkImageLoader.cpp#L1707-L1715

testing a patch.

@anarcat
Copy link

anarcat commented Aug 18, 2024

Do I understand correctly that "Scan Folder Recursive no longer works" (#797, Aug 8, 2021) discusses the same problem of not being able to display all images from all subfolders of the current folder, as implemented in IrfanView?

i would say so, yes.

anarcat added a commit to anarcat/nomacs that referenced this issue Aug 18, 2024
It seems the original code did everything to do recursion into
subfolders except it explicitly stopped at the first folder with
images. This made browsing folders weird; it would find some
subfolder (any subfolder, not necessarily the first one listed in the
file explorer) and show *that*.

We change that behavior and just bite the bullet: we take *everything*
the QDirIterator gives us and throw that in the QFileInfoList, which
is really a QList. According to the documentation:

"This operation is relatively fast, because QList typically allocates
more memory than necessary, so it can grow without reallocating the
entire list each time."

https://doc.qt.io/qt-6/qlist.html#append

That said, obviously that list can grow to be absolutely huge, but
doing otherwise would need to refactor the loader into something that
returns an iterator (which raises the question of why we're not just
passing along that QDirIterator, probably because we're sorting
things...)

Closes: nomacs#797
Closes: nomacs#297
anarcat added a commit to anarcat/nomacs that referenced this issue Aug 18, 2024
It seems the original code did everything to do recursion into
subfolders except it explicitly stopped at the first folder with
images. This made browsing folders weird; it would find some
subfolder (any subfolder, not necessarily the first one listed in the
file explorer) and show *that*.

We change that behavior and just bite the bullet: we take *everything*
the QDirIterator gives us and throw that in the QFileInfoList, which
is really a QList. According to the documentation:

"This operation is relatively fast, because QList typically allocates
more memory than necessary, so it can grow without reallocating the
entire list each time."

https://doc.qt.io/qt-6/qlist.html#append

That said, obviously that list can grow to be absolutely huge, but
doing otherwise would need to refactor the loader into something that
returns an iterator (which raises the question of why we're not just
passing along that QDirIterator, probably because we're sorting
things...)

This also doesn't work so well for directories that have multiple
layers of subdirectories. From what I can tell from experience, the
images in the final list come out of order, and all mixed up
together. For example, if you have:

Photos/2024/01/01/foo.jpg
Photos/2024/01/01/bar.jpg
Photos/2024/01/02/quux.jpg

quux.jpg might be inserted between foo and bar.

On the other hand, if you have:

Photos/2024-01-01/foo.jpg
Photos/2024-01-01/bar.jpg
Photos/2024-01-02/quux.jpg

... the new code seems to handle this fine.

Either way, this *feels* to me like an improvement to the current
code, but I'll let the maintainers decide of that.

Closes: nomacs#797
Closes: nomacs#297
@anarcat anarcat linked a pull request Aug 18, 2024 that will close this issue
5 tasks
scrubbbbs pushed a commit to scrubbbbs/nomacs that referenced this issue Aug 27, 2024
It seems the original code did everything to do recursion into
subfolders except it explicitly stopped at the first folder with
images. This made browsing folders weird; it would find some
subfolder (any subfolder, not necessarily the first one listed in the
file explorer) and show *that*.

We change that behavior and just bite the bullet: we take *everything*
the QDirIterator gives us and throw that in the QFileInfoList, which
is really a QList. According to the documentation:

"This operation is relatively fast, because QList typically allocates
more memory than necessary, so it can grow without reallocating the
entire list each time."

https://doc.qt.io/qt-6/qlist.html#append

That said, obviously that list can grow to be absolutely huge, but
doing otherwise would need to refactor the loader into something that
returns an iterator (which raises the question of why we're not just
passing along that QDirIterator, probably because we're sorting
things...)

This also doesn't work so well for directories that have multiple
layers of subdirectories. From what I can tell from experience, the
images in the final list come out of order, and all mixed up
together. For example, if you have:

Photos/2024/01/01/foo.jpg
Photos/2024/01/01/bar.jpg
Photos/2024/01/02/quux.jpg

quux.jpg might be inserted between foo and bar.

On the other hand, if you have:

Photos/2024-01-01/foo.jpg
Photos/2024-01-01/bar.jpg
Photos/2024-01-02/quux.jpg

... the new code seems to handle this fine.

Either way, this *feels* to me like an improvement to the current
code, but I'll let the maintainers decide of that.

Closes: nomacs#797
Closes: nomacs#297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants