Source Code Project Mantis - FSSCP
View Issue Details
0003127FSSCPmath-relatedpublic2014-10-23 23:592014-11-09 20:56
Assigned Totaylor 
PlatformPCOSWindowsOS VersionWin7
Product Version3.7.2 
Target Version3.7.2Fixed in Version 
Summary0003127: SVN Revision 11126 Causes Major Slowdown
DescriptionSVN Revision 11126 causes major FPS loss, especially in dense missions that were previously playable. I noticed this in BtA, and tracked it to that revision.
Steps To ReproduceThe issue was obvious, but I did not test it on retail. I don't know if any of those missions will display it.
TagsNo tags attached.
Attached Filespatch fvi.cpp.patch (1,543) 2014-10-24 18:45
patch Mantis3127-fvi.cpp.patch (376) 2014-10-25 17:43
patch 3127.patch (372) 2014-10-25 18:18

2014-10-24 15:25   
The FPS loss is dependent on the number of objects within a certain proximity to each other. The first mission I noticed this in resulted in a drop to 40 FPS, the second one I tried had even more objects within a close (around 4 km radius) distance and resulted in a reduction to 6 FPS. To top it off, the problem only occurs when looking at or near the group of objects.
2014-10-24 18:46   
After DahBlount pointed me towards a mission demonstrating the problem, I narrowed the culprit down to the fvi_ray_boundingbox() changes. Reverting just those changes results in the performance drop disappearing.
2014-10-24 22:22   
That makes sense, as those were actual algorithmic changes vs. the other function which contained optimizations but no changes to the algorithm.

This ticket does surprise me though because the new algorithm uses bitwise operations which are usually faster than multiplication operations. It's possible that there's a mistake in the algorithm though. I'll ping Taylor.
2014-10-24 23:16   
I don't see any multiplication operations being changed...?
2014-10-25 17:46   
The multiplication is implicit. Array access is essentially pointer manipulation: multiply the array index by the element size, then add the result to the array base pointer. The new code treats "middle" as a bitfield rather than an array.

Taylor spotted an unintended change which may be the cause of the problem. The if() block that tests "middle" used to check that all three of the axes were still inside the bounding box. The new code merely checks that any of the axes are still inside. I've attached a patch that should restore the original check.
2014-10-25 18:20   
Your patch fails to actually remove the performance drop because it's effectively checking the same thing, in this context. Changing the "&" into an "==", however, gets rid of the performance drop by actually checking that all three bits are set.
2014-10-25 21:41   
Derp, yeah, it *is* checking the same thing, isn't it. :p

Glad to hear that's where the problem is, though. I'll commit the revised fix.
2014-10-25 22:02   
Committed. If the next trunk build fixes the slowdown, I'll resolve the ticket.
2014-10-27 14:19   
Problem solved?
2014-10-28 01:15   
I haven't tested yet.
2014-10-31 00:56   
Latest revision builds are back to normal speed. So I presume that this is fixed.
2014-10-31 01:54   
Let's mark this as resolved, then.
2014-11-09 20:56   
Crediting Taylor because it was he who figured out where the problem was.

Issue History
2014-10-23 23:59MjnMixaelNew Issue
2014-10-24 15:25DahBlountNote Added: 0016348
2014-10-24 16:29Goober5000Changeset attached => fs2open trunk r11126
2014-10-24 16:29Goober5000Assigned To => Goober5000
2014-10-24 16:29Goober5000Statusnew => assigned
2014-10-24 18:45MageKing17File Added: fvi.cpp.patch
2014-10-24 18:46MageKing17Note Added: 0016351
2014-10-24 22:22Goober5000Note Added: 0016352
2014-10-24 23:16MageKing17Note Added: 0016354
2014-10-25 17:43Goober5000File Added: Mantis3127-fvi.cpp.patch
2014-10-25 17:46Goober5000Note Added: 0016355
2014-10-25 18:18MageKing17File Added: 3127.patch
2014-10-25 18:20MageKing17Note Added: 0016356
2014-10-25 21:41Goober5000Note Added: 0016357
2014-10-25 22:02Goober5000Changeset attached => fs2open trunk r11157
2014-10-25 22:02Goober5000Note Added: 0016358
2014-10-27 14:19chief1983Note Added: 0016361
2014-10-28 01:15MjnMixaelNote Added: 0016362
2014-10-31 00:56MjnMixaelNote Added: 0016364
2014-10-31 01:54MageKing17Note Added: 0016365
2014-10-31 01:54MageKing17Statusassigned => resolved
2014-10-31 01:54MageKing17Resolutionopen => fixed
2014-11-09 20:56Goober5000Note Added: 0016381
2014-11-09 20:56Goober5000Assigned ToGoober5000 => taylor