TTimo / GtkRadiant

The open source, cross platform level editor for idtech games
http://icculus.org/gtkradiant/
Other
586 stars 155 forks source link

q3map2/light_bounce: prevent infinite loop on obscure bias compute #653

Closed illwieckz closed 4 years ago

illwieckz commented 4 years ago

Original thread there: https://gitlab.com/xonotic/netradiant/-/merge_requests/167


New code produces same result without loop at all, so it cannot fall in infinite loop, and it is faster in use cases requiring more than one loop in previous code.

The Unvanquished vega map is known to trigger the bug: https://github.com/UnvanquishedAssets/map-vega_src.dpkdir

I reproduced it multiple time on various hardware (8 core FX-9590, 12 core/24 thread Ryzen 9 3900X) with commit af40508 and using final compilation profile edited to use -fastbounce instead of -fast option.

The symptom is simple, q3map2 stucks there:

--- Radiosity (bounce 1 of 8) ---
--- RadCreateDiffuseLights ---
0...1...2...3..

Or somewhere else in that progression bar given your hardware and the amount of core your CPU has.

For example, with a 12 core / 24 thread Ryzen 9 3900X CPU, q3map2 stucks there:

--- Radiosity (bounce 1 of 8) ---
--- RadCreateDiffuseLights ---
0...1...2...3...4..

When stuck, all the CPU cores are running 100% but the thread never returns (a strace can reveals it, a gdb backtrace too).

Thanks to @slipher for the precious advices and improving my first attempt to fix it.

For more information on the issue, I asked:

which negative value never can become positive when incremented infinitely?

slipher said:

for a double, any value less than -2^53 would have this property don't know for float off the top of my head

But then, it means that's theoretically verified this loop was able to run forever in some case.

I don't know what this code is doing anyway, but at least we can keep the behaviour without requiring to understand it.

TTimo commented 4 years ago

Your explanations make sense to me :)