implement workarounds for division by zero

[illwieckz]

This now looks ready to me. This uses 0 as the result of the division if it would be 0/0.

See for a similar PR on game side:

• https://github.com/Unvanquished/Unvanquished/pull/2533

---

Original message:

This is a draft and work-in-progress.

I dirtily enabled exceptions for divizion by zeros to catch them, and implemented mindless dirty workarounds for some of them.

This needs more thoughts for better workarounds. For now I divided by 1 any time there was a divizion by zero, but some other cases may require better values, like actually returning 1 for the whole division if we do 0/0, or returning a specific default value for when it cannot be computed.

That's why this PR is marked as work-in-progress.

[illwieckz]

Interestingly if I set minMsec to 0 in src/engine/qcommon/common.cpp when unlimiting framerate, I get crazy fps:

It looks like this code was doing if 0 < 1; sleep 1ms and then waited at least 1ms when framerate was unlimited:

	msec = com_frameTime - lastTime;

	while ( msec < minMsec )
	{
		//give cycles back to the OS
		Sys::SleepFor(std::chrono::milliseconds(std::min(minMsec - msec, 50)));

And with those division by 0 fix patches, things do not glitch:

[slipher]

When I tried this, there were way more places in the renderer with division by zero than just one. I decided against trying to "fix" them as this would just make the code slower and not fix any actual bugs.

[illwieckz]

I found another division by zero by reproducing the bug that totally breaks the renderer while going outside of the metro map in some places. By totally breaking the renderer I mean even loading a new map can't fix it (it looks like vid_restart works, though).

Unfortunately avoiding the division by zero doesn't fix the bug.

1900 -1971 100 -40 9
Warn: div: 0.000000

Thread 1 "daemon" received signal SIGFPE, Arithmetic exception.
0x000055555566be39 in SetViewportAndScissor () at Daemon/src/engine/renderer/tr_backend.cpp:756
756			scale = 2.0f / (DotProduct( c, q ) + c[3] * q[3]);

Thread 1 (Thread 0x7ffff59fba40 (LWP 1785304) "daemon"):
#0  0x000055555566be39 in SetViewportAndScissor () at Daemon/src/engine/renderer/tr_backend.cpp:756
#1  0x0000555555678cd3 in RB_RenderView (depthPass=false) at Daemon/src/engine/renderer/tr_backend.cpp:4683
#2  DrawViewCommand::ExecuteSelf (this=0x7fffd74ccb94) at Daemon/src/engine/renderer/tr_backend.cpp:5585
#3  0x000055555566e1f9 in RB_ExecuteRenderCommands (data=<optimized out>) at Daemon/src/engine/renderer/tr_backend.cpp:5829
#4  0x000055555568b332 in RE_EndFrame (frontEndMsec=0x0, backEndMsec=0x0) at Daemon/src/engine/renderer/tr_cmds.cpp:889
#5  0x00005555556161b0 in SCR_UpdateScreen () at Daemon/src/engine/client/cl_scrn.cpp:327
#6  0x00005555556084ed in CL_Frame (msec=msec@entry=2) at Daemon/src/engine/client/cl_main.cpp:2553
#7  0x00005555555a0c5f in Com_Frame () at Daemon/src/engine/qcommon/common.cpp:1031
#8  0x000055555559a37d in main (argc=<optimized out>, argv=0x7fffffffd938) at Daemon/src/engine/framework/System.cpp:762

This code only runs when there is a portal on screen. The bug occurs when a portal is on screen while being outside of the map in some very rare conditions (I only know one way to reproduce it with a single map).

Edit: I managed to reproduce it with spacetracks map (setviewpos 378 -1906 625 80 1).

[ghost]

@illwieckz something like this ?

[illwieckz]

I removed a workaround in favor of a proper fix:

• https://github.com/DaemonEngine/Daemon/pull/813

@illwieckz something like this ?

yes, well the bug affecting the nintendo-land map.

[illwieckz]

can I get some approval?

[slipher]

The beacon ones seem wrong to me. The clamp_dir field of cbeacon_t is set only if the calculation of the preferred 2d position of the beacon is off-screen and it needs to be moved to be on the screen (this condition is indicated by clamped = true). So it's a bug that this ProjectPointOntoRectangleOutwards is performed for some reason when clamp_dir is not set. If there is no distance from the center of the screen, the operation should have been skipped.

[illwieckz]

The beacon ones seem wrong to me. The clamp_dir field of cbeacon_t is set only if the calculation of the preferred 2d position of the beacon is off-screen and it needs to be moved to be on the screen (this condition is indicated by clamped = true). So it's a bug that this ProjectPointOntoRectangleOutwards is performed for some reason when clamp_dir is not set. If there is no distance from the center of the screen, the operation should have been skipped.

I caught division by zero with the 2 others occurrence just by being unlucky at looking there or being there. I haven't reproduced with beacon but I haven't tried.

[slipher]

2 out of 3 cases are using clamp_dir from the beacon struct. The other one is the minimap. It uses the same kind of logic, where ProjectPointOntoRectangleOutwards is only supposed to be called if the point is outside the rectangle. So it is also a bug. There are no non-buggy cases.

[slipher]

I noticed that the usage of ProjectPointOntoRectangleOutwards in cg_beacon.cpp is using the result of CG_WorldToScreen. So your fix for CG_WorldToScreen in the gamelogic zero division PR likely prevents the bug.

[illwieckz]

Can I at least merge the first commit?

• tr_bsp: implement workaround for division by zero

If I'm right it's about a division by zero occurring when loading data from BSP file, by using data from the BSP file.

[slipher]

Yeah the BSP lighting one LGTM.

[illwieckz]

Yeah the BSP lighting one LGTM.

I merged this commit.

[illwieckz]

So, there is one unmerged commit remaining in this branch, this also means the related division by zero is still alive.

Here is the dedicated thread for this specific commit:

• https://github.com/DaemonEngine/Daemon/pull/812/files#r1186484684

[slipher]

Obsoleted by https://github.com/Unvanquished/Unvanquished/pull/3028