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

Issue with Image::to_time_t in Image.cpp (system_exception on Windows 8.1 x64 on VisualStudio 2019) #172

Closed mmmovania closed 2 years ago

mmmovania commented 2 years ago

Hi i noticed an issue in the to_time_t function defined in Image.cpp. I was trying to use the latest version of tev on pbrtv4 on Win8.1 and tev crashed. I managed to dig the reason and its the to_time_t function. Currently its defined as follows:

template <typename T>
time_t to_time_t(T timePoint) {
    // TODO: clean up this mess once all compilers support clock_cast.
#ifdef _WIN32
    return chrono::system_clock::to_time_t(chrono::clock_cast<chrono::system_clock>(timePoint));
#elif __APPLE__
    using namespace chrono;
    return system_clock::to_time_t(time_point_cast<system_clock::duration>(timePoint - T::clock::now() + system_clock::now()));
#else
    return chrono::system_clock::to_time_t(T::clock::to_sys(timePoint));
#endif

which causes a system_exception and crashes on Win8.1 VS 2019. I managed to fix it by changing it to this which basically treats both windows and apple the same way.

template <typename T>
time_t to_time_t(T timePoint) {
    // TODO: clean up this mess once all compilers support clock_cast.
#if defined(_WIN32) || defined(__APPLE__)
    using namespace chrono;
    return system_clock::to_time_t(time_point_cast<system_clock::duration>(timePoint - T::clock::now() + system_clock::now()));
#else
    return chrono::system_clock::to_time_t(T::clock::to_sys(timePoint));
#endif
}
Tom94 commented 2 years ago

Thanks for reporting this! Didn't expect a clock_cast to error out like that (on just some systems on top of that).

I'll implement your fix and push a hotfix release!

In the interim, could you share the contents of the system_exception you're getting? I'd be curious what the underlying error message is to maybe fix the issue in a more elegant way.

Reverting to Apple behavior is a bit unfortunate, because the term T::clock::now() + system_clock::now() is not guaranteed to call now() at the same time for both clocks -- so the resulting time_t will randomly fluctuate. Not that it matters much... this function is only used to display timestamps up to 1s accuracy, but it still irks me a little.

mmmovania commented 2 years ago

The following is printed on console. Caught exception in main loop: The specified module could not be found

The Output windows spits this.

Exception thrown at 0x00007FF95C727AFC in tev.exe: Microsoft C++ exception: std::system_error at memory location 0x0000004DCAEA9620.
Exception thrown at 0x00007FF95C727AFC in tev.exe: Microsoft C++ exception: [rethrow] at memory location 0x0000000000000000.
Exception thrown at 0x00007FF95C727AFC in tev.exe: Microsoft C++ exception: std::system_error at memory location 0x0000004DCAEA9620.
Tom94 commented 2 years ago

Thank you! This unfortunately doesn't shed a lot of light on the issue... oh well. The hacky approach it is. :)

mmmovania commented 2 years ago

Just to let you know the error is thrown at line 3153 chrono header //3153 const auto& _Tzdb = _CHRONO get_tzdb();