Tom94 / tev

High dynamic range (HDR) image viewer for graphics people
BSD 3-Clause "New" or "Revised" License
1.02k stars 86 forks source link

Fix relative path resolution in background loader #138

Closed merlinND closed 2 years ago

merlinND commented 2 years ago

Hi @Tom94,

The worker thread / process used by BackgroundImagesLoader does not seem to use the main thread's current working directory. This leads to relative path resolution when image loading is scheduled by BackgroundImagesLoader::enqueue().

I ran into this issue when using e.g.:

tev -n ./image*.exr

There's now a bit of code duplication between tryLoadImage(path, channelSelector) and BackgroundImagesLoader::enqueue (path resolution is done twice). Let me know if you'd prefer to avoid it, e.g. by finding a way to set the cwd of the worker thread instead (sounds a bit more complicated).

Tom94 commented 2 years ago

Hi Merlin; thanks for the PR!

I have trouble reproducing the problem on my end. My understanding is that the working directory is a process-wide setting, so I am a little confused as to why your solution works in the first place.

Is there anything unusual about your setup that could help me reproduce it? (E.g. images residing in a mapped network drive, a samba share, or similar?) Also: which OS do you test with?

merlinND commented 2 years ago

Hi Thomas,

This happens to me on macOS, both when trying to load images from a local directory and an SMB directory.

Here's how I got suspicious about the CWD: at the top of tryLoadImage(path, channelSelector), I added:

    std::cout << "Current directory: " << path::getcwd() << std::endl;

And here's what comes out:

$ tev -n ./opt*00-00.exr
Current directory: /Users/merlin/Desktop/test-img
16:43:16 SUCCESS  Loaded '/Users/merlin/Desktop/test-img/opt-0000-00.exr' via OpenEXR after 0.046 seconds.
Current directory: /Users/merlin/Desktop/test-img
16:43:16 SUCCESS  Loaded '/Users/merlin/Desktop/test-img/opt-0100-00.exr' via OpenEXR after 0.038 seconds.
Current directory: /path/to/tev.app/Contents/Resources
16:43:16 SUCCESS  Loaded '/Users/merlin/Desktop/test-img/opt-0200-00.exr' via OpenEXR after 0.041 seconds.
Current directory: /path/to/tev.app/Contents/Resources
[etc]

(this this after applying the fix from this PR, otherwise image loading fails since the relative paths cannot be resolved when CWD is /path/to/tev.app/Contents/Resources).

Maybe the worker mechanism is implemented with a separate process on macOS?

Tom94 commented 2 years ago

Addressed in https://github.com/Tom94/tev/pull/141

(Merlin and I troubleshooted this together and ultimately found the better fix above.)