2018-05-24 01:20 EDT


View Issue Details Jump to Notes ] Related Changesets ]
IDProjectCategoryView StatusLast Update
0003127FSSCPmath-relatedpublic2014-11-09 20:56
ReporterMjnMixael 
Assigned Totaylor 
PriorityurgentSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
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 Files
  • patch file icon fvi.cpp.patch (1,543 bytes) 2014-10-24 18:45 -
    Index: code/math/fvi.cpp
    ===================================================================
    --- code/math/fvi.cpp	(revision 11156)
    +++ code/math/fvi.cpp	(working copy)
    @@ -325,7 +325,8 @@
      */
     int fvi_ray_boundingbox( vec3d *min, vec3d *max, vec3d * p0, vec3d *pdir, vec3d *hitpt )
     {
    -	int middle = ((1<<0) | (1<<1) | (1<<2));
    +	bool inside = true;
    +	bool middle[3] = { true, true, true };
     	int i;
     	int which_plane;
     	float maxt[3];
    @@ -334,15 +335,17 @@
     	for (i = 0; i < 3; i++) {
     		if (p0->a1d[i] < min->a1d[i]) {
     			candidate_plane[i] = min->a1d[i];
    -			middle &= ~(1<<i);
    +			middle[i] = false;
    +			inside = false;
     		} else if (p0->a1d[i] > max->a1d[i]) {
     			candidate_plane[i] = max->a1d[i];
    -			middle &= ~(1<<i);
    +			middle[i] = false;
    +			inside = false;
     		}
     	}
     
     	// ray origin inside bounding box			
    -	if (middle) {
    +	if ( inside ) {
     		*hitpt = *p0;
     		return 1;
     	}
    @@ -349,19 +352,17 @@
     
     	// calculate T distances to candidate plane
     	for (i = 0; i < 3; i++) {
    -		if ( (middle & (1<<i)) || (pdir->a1d[i] == 0.0f) ) {
    +		if ( !middle[i] && (pdir->a1d[i] != 0.0f) )
    +			maxt[i] = (candidate_plane[i] - p0->a1d[i]) / pdir->a1d[i];
    +		else
     			maxt[i] = -1.0f;
    -		} else {
    -			maxt[i] = (candidate_plane[i] - p0->a1d[i]) / pdir->a1d[i];
    -		}
     	}
     
     	// Get largest of the maxt's for final choice of intersection
     	which_plane = 0;
     	for (i = 1; i < 3; i++) {
    -		if (maxt[which_plane] < maxt[i]) {
    +		if (maxt[which_plane] < maxt[i])
     			which_plane = i;
    -		}
     	}
     
     	// check final candidate actually inside box
    
    patch file icon fvi.cpp.patch (1,543 bytes) 2014-10-24 18:45 +
  • patch file icon Mantis3127-fvi.cpp.patch (376 bytes) 2014-10-25 17:43 -
    Index: code/math/fvi.cpp
    ===================================================================
    --- code/math/fvi.cpp	(revision 11156)
    +++ code/math/fvi.cpp	(working copy)
    @@ -341,8 +341,8 @@
     		}
     	}
     
    -	// ray origin inside bounding box			
    -	if (middle) {
    +	// ray origin inside bounding box?
    +	if (middle & ((1<<0) | (1<<1) | (1<<2))) {
     		*hitpt = *p0;
     		return 1;
     	}
    
    patch file icon Mantis3127-fvi.cpp.patch (376 bytes) 2014-10-25 17:43 +
  • patch file icon 3127.patch (372 bytes) 2014-10-25 18:18 -
    Index: code/math/fvi.cpp
    ===================================================================
    --- code/math/fvi.cpp	(revision 11156)
    +++ code/math/fvi.cpp	(working copy)
    @@ -341,8 +341,8 @@
     		}
     	}
     
    -	// ray origin inside bounding box			
    -	if (middle) {
    +	// ray origin inside bounding box?
    +	if (middle == ((1<<0) | (1<<1) | (1<<2))) {
     		*hitpt = *p0;
     		return 1;
     	}
    
    patch file icon 3127.patch (372 bytes) 2014-10-25 18:18 +

-Relationships
+Relationships

-Notes

~0016348

DahBlount (developer)

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.

~0016351

MageKing17 (developer)

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.

~0016352

Goober5000 (administrator)

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.

~0016354

MageKing17 (developer)

I don't see any multiplication operations being changed...?

~0016355

Goober5000 (administrator)

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.

~0016356

MageKing17 (developer)

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.

~0016357

Goober5000 (administrator)

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.

~0016358

Goober5000 (administrator)

Committed. If the next trunk build fixes the slowdown, I'll resolve the ticket.

~0016361

chief1983 (administrator)

Problem solved?

~0016362

MjnMixael (manager)

I haven't tested yet.

~0016364

MjnMixael (manager)

Latest revision builds are back to normal speed. So I presume that this is fixed.

~0016365

MageKing17 (developer)

Let's mark this as resolved, then.

~0016381

Goober5000 (administrator)

Crediting Taylor because it was he who figured out where the problem was.
+Notes

+Related Changesets

-Issue History
Date Modified Username Field Change
2014-10-23 23:59 MjnMixael New Issue
2014-10-24 15:25 DahBlount Note Added: 0016348
2014-10-24 16:29 Goober5000 Changeset attached => fs2open trunk r11126
2014-10-24 16:29 Goober5000 Assigned To => Goober5000
2014-10-24 16:29 Goober5000 Status new => assigned
2014-10-24 18:45 MageKing17 File Added: fvi.cpp.patch
2014-10-24 18:46 MageKing17 Note Added: 0016351
2014-10-24 22:22 Goober5000 Note Added: 0016352
2014-10-24 23:16 MageKing17 Note Added: 0016354
2014-10-25 17:43 Goober5000 File Added: Mantis3127-fvi.cpp.patch
2014-10-25 17:46 Goober5000 Note Added: 0016355
2014-10-25 18:18 MageKing17 File Added: 3127.patch
2014-10-25 18:20 MageKing17 Note Added: 0016356
2014-10-25 21:41 Goober5000 Note Added: 0016357
2014-10-25 22:02 Goober5000 Changeset attached => fs2open trunk r11157
2014-10-25 22:02 Goober5000 Note Added: 0016358
2014-10-27 14:19 chief1983 Note Added: 0016361
2014-10-28 01:15 MjnMixael Note Added: 0016362
2014-10-31 00:56 MjnMixael Note Added: 0016364
2014-10-31 01:54 MageKing17 Note Added: 0016365
2014-10-31 01:54 MageKing17 Status assigned => resolved
2014-10-31 01:54 MageKing17 Resolution open => fixed
2014-11-09 20:56 Goober5000 Note Added: 0016381
2014-11-09 20:56 Goober5000 Assigned To Goober5000 => taylor
+Issue History