More work

src/kraft.pas

@@ -1859,6 +1859,12 @@ type TKraftForceMode=(kfmForce,        // The unit of the force parameter is app
 
        procedure CalculateMassData;
 
+       // The actual build steps, wrapped by Build and Finish, which make sure that the FPU is in the state
+       // which these expect even when the convex hull is standalone and thus has no physics instance which
+       // did that already
+       procedure BuildInternal(const AMaximumCountConvexHullPoints:TKraftInt32;const AUserDefinedTolerance:double);
+       procedure FinishInternal;
+
       public
 
        constructor Create(const aPhysics:TKraft=nil);
@@ -2236,6 +2242,10 @@ type TKraftForceMode=(kfmForce,        // The unit of the force parameter is app
 
        procedure IdentifyEdges;
 
+       // The actual build, wrapped by Finish, which makes sure that the FPU is in the state which this
+       // expects even when the mesh is standalone and thus has no physics instance which did that already
+       procedure FinishInternal(const aBVHBuildMode:TKraftMeshBVHBuildMode);
+
       public
 
        constructor Create(const aPhysics:TKraft=nil{$if defined(KraftPasMP)};const aPasMPInstance:TPasMP=nil{$ifend});
@@ -7505,6 +7515,46 @@ begin
 end;
 {$ifend}
 
+// Geometry which is built without a physics instance has nobody who put the FPU and the SIMD unit into the
+// state which the geometry math expects, since TKraft does that in its constructor and in Step. Without it,
+// for example the zero volume of a flat mesh lets the centroid division of CalculateMassData trap on an
+// invalid operation instead of quietly yielding a NaN. Unlike TKraft.Create, which sets the state and keeps
+// it, these restore what they found, so that merely building a mesh does not change the mode of its caller.
+type TKraftFPUState=record
+      PrecisionMode:TFPUPrecisionMode;
+      ExceptionMask:TFPUExceptionMask;
+      SIMDFlags:TKraftUInt32;
+     end;
+
+// The SIMD control word is saved first and restored last on purpose: on the targets where the scalar
+// floating point math goes through SSE, SetExceptionMask writes it as well. Saving it afterwards would
+// capture the already modified word, and restoring it before SetExceptionMask would let that overwrite it
+// again, which would silently undo the masking that TKraft.Create established for the whole thread.
+procedure KraftEnterPhysicsFPUState(out aOldState:TKraftFPUState);
+begin
+ aOldState.SIMDFlags:=SIMDGetFlags;
+ aOldState.PrecisionMode:=GetPrecisionMode;
+ aOldState.ExceptionMask:=GetExceptionMask;
+ if aOldState.PrecisionMode<>PhysicsFPUPrecisionMode then begin
+  SetPrecisionMode(PhysicsFPUPrecisionMode);
+ end;
+ if aOldState.ExceptionMask<>PhysicsFPUExceptionMask then begin
+  SetExceptionMask(PhysicsFPUExceptionMask);
+ end;
+ SIMDSetOurFlags;
+end;
+
+procedure KraftLeavePhysicsFPUState(const aOldState:TKraftFPUState);
+begin
+ if aOldState.ExceptionMask<>PhysicsFPUExceptionMask then begin
+  SetExceptionMask(aOldState.ExceptionMask);
+ end;
+ if aOldState.PrecisionMode<>PhysicsFPUPrecisionMode then begin
+  SetPrecisionMode(aOldState.PrecisionMode);
+ end;
+ SIMDSetFlags(aOldState.SIMDFlags);
+end;
+
 {$if (defined(cpu386) or defined(cpuamd64) or defined(cpux86_64) or defined(cpux64)) and not defined(KraftDelphiOnNonWindowsTarget)}
 type TCPUIDData=record
       case TKraftUInt8 of
@@ -30145,6 +30195,22 @@ begin
 end;
 
 procedure TKraftConvexHull.Build(const AMaximumCountConvexHullPoints:TKraftInt32=-1;const AUserDefinedTolerance:double=-1.0);
+var OldFPUState:TKraftFPUState;
+begin
+ if assigned(fPhysics) then begin
+  // The physics instance has already put this thread into the FPU state which the build expects
+  BuildInternal(AMaximumCountConvexHullPoints,AUserDefinedTolerance);
+ end else begin
+  KraftEnterPhysicsFPUState(OldFPUState);
+  try
+   BuildInternal(AMaximumCountConvexHullPoints,AUserDefinedTolerance);
+  finally
+   KraftLeavePhysicsFPUState(OldFPUState);
+  end;
+ end;
+end;
+
+procedure TKraftConvexHull.BuildInternal(const AMaximumCountConvexHullPoints:TKraftInt32;const AUserDefinedTolerance:double);
 const HashBits=8;
       HashSize=1 shl HashBits;
       HashMask=HashSize-1;
@@ -30942,6 +31008,22 @@ begin
 end;
 
 procedure TKraftConvexHull.Finish;
+var OldFPUState:TKraftFPUState;
+begin
+ if assigned(fPhysics) then begin
+  // The physics instance has already put this thread into the FPU state which the build expects
+  FinishInternal;
+ end else begin
+  KraftEnterPhysicsFPUState(OldFPUState);
+  try
+   FinishInternal;
+  finally
+   KraftLeavePhysicsFPUState(OldFPUState);
+  end;
+ end;
+end;
+
+procedure TKraftConvexHull.FinishInternal;
 const Steps=1024;
       //ModuloThree:array[0..5] of TKraftInt32=(0,1,2,0,1,2);
 var VertexIndex:TKraftInt32;
@@ -34576,6 +34658,22 @@ begin
 end;
 
 procedure TKraftMesh.Finish(const aBVHBuildMode:TKraftMeshBVHBuildMode);
+var OldFPUState:TKraftFPUState;
+begin
+ if assigned(fPhysics) then begin
+  // The physics instance has already put this thread into the FPU state which the build expects
+  FinishInternal(aBVHBuildMode);
+ end else begin
+  KraftEnterPhysicsFPUState(OldFPUState);
+  try
+   FinishInternal(aBVHBuildMode);
+  finally
+   KraftLeavePhysicsFPUState(OldFPUState);
+  end;
+ end;
+end;
+
+procedure TKraftMesh.FinishInternal(const aBVHBuildMode:TKraftMeshBVHBuildMode);
 type TDynamicAABBTreeNode=record
       AABB:TKraftAABB;
       Parent:TKraftSizeInt;

src/tests/sharedgeometrytest.dpr

@@ -127,13 +127,18 @@ begin
  end;
 end;
 
-function CountConvexHullsOf(const aPhysics:TKraft):longint;
+// A world always holds internal convex hulls of its own, for the triangle shapes it builds in its
+// constructor, so only the membership of one particular hull is meaningful here
+function IsConvexHullOf(const aPhysics:TKraft;const aConvexHull:TKraftConvexHull):boolean;
 var ConvexHull:TKraftConvexHull;
 begin
- result:=0;
+ result:=false;
  ConvexHull:=aPhysics.ConvexHullFirst;
  while assigned(ConvexHull) do begin
-  inc(result);
+  if ConvexHull=aConvexHull then begin
+   result:=true;
+   exit;
+  end;
   ConvexHull:=ConvexHull.Next;
  end;
 end;
@@ -166,9 +171,9 @@ begin
    // The standalone objects must stay out of both worlds, otherwise the first world to die would free them
    Check('standalone mesh not in mesh list of world A',CountMeshesOf(PhysicsA)=0);
    Check('standalone mesh not in mesh list of world B',CountMeshesOf(PhysicsB)=0);
-   // Each world builds its own internal hulls for the shapes it needs, the shared one must not be among them
-   Check('standalone hull not in convex hull list of world A',CountConvexHullsOf(PhysicsA)=0);
-   Check('standalone hull not in convex hull list of world B',CountConvexHullsOf(PhysicsB)=0);
+   // Each world builds internal hulls of its own, the shared one must not be among them
+   Check('standalone hull not in convex hull list of world A',not IsConvexHullOf(PhysicsA,Hull));
+   Check('standalone hull not in convex hull list of world B',not IsConvexHullOf(PhysicsB,Hull));
 
    HeightA:=StepAndGetHeight(PhysicsA,BodyA,180);
    HeightB:=StepAndGetHeight(PhysicsB,BodyB,180);
@@ -339,7 +344,7 @@ begin
   Check('owned mesh is linked into the mesh list',Mesh.Physics=Physics);
   Check('mesh list of the world holds exactly the owned mesh',CountMeshesOf(Physics)=1);
   Check('owned hull is linked into the convex hull list',Hull.Physics=Physics);
-  Check('convex hull list of the world holds exactly the owned hull',CountConvexHullsOf(Physics)=1);
+  Check('owned hull is in the convex hull list of the world',IsConvexHullOf(Physics,Hull));
 
   Body:=PopulateWorld(Physics,Mesh,Hull,3.0);
   Height:=StepAndGetHeight(Physics,Body,180);