Browse Source

Added Clone function to HeightFieldShape/MutableCompoundShape (#1128)

Added documentation on how to avoid a race condition.
Jorrit Rouwe 1 year ago
parent
commit
5b25fd5df6

+ 30 - 0
Jolt/Physics/Collision/Shape/HeightFieldShape.cpp

@@ -714,6 +714,36 @@ HeightFieldShape::~HeightFieldShape()
 		AlignedFree(mRangeBlocks);
 }
 
+Ref<HeightFieldShape> HeightFieldShape::Clone() const
+{
+	Ref<HeightFieldShape> clone = new HeightFieldShape;
+	clone->SetUserData(GetUserData());
+
+	clone->mOffset = mOffset;
+	clone->mScale = mScale;
+	clone->mSampleCount = mSampleCount;
+	clone->mBlockSize = mBlockSize;
+	clone->mBitsPerSample = mBitsPerSample;
+	clone->mSampleMask = mSampleMask;
+	clone->mMinSample = mMinSample;
+	clone->mMaxSample = mMaxSample;
+
+	clone->AllocateBuffers();
+	memcpy(clone->mRangeBlocks, mRangeBlocks, mRangeBlocksSize * sizeof(RangeBlock) + mHeightSamplesSize + mActiveEdgesSize); // Copy the entire buffer in 1 go
+
+	clone->mMaterials.reserve(mMaterials.capacity()); // Ensure we keep the capacity of the original
+	clone->mMaterials = mMaterials;
+	clone->mMaterialIndices = mMaterialIndices;
+	clone->mNumBitsPerMaterialIndex = mNumBitsPerMaterialIndex;
+
+#ifdef JPH_DEBUG_RENDERER
+	clone->mGeometry = mGeometry;
+	clone->mCachedUseMaterialColors = mCachedUseMaterialColors;
+#endif // JPH_DEBUG_RENDERER
+
+	return clone;
+}
+
 inline void HeightFieldShape::sGetRangeBlockOffsetAndStride(uint inNumBlocks, uint inMaxLevel, uint &outRangeBlockOffset, uint &outRangeBlockStride)
 {
 	outRangeBlockOffset = sGridOffsets[inMaxLevel - 1];

+ 9 - 0
Jolt/Physics/Collision/Shape/HeightFieldShape.h

@@ -108,6 +108,10 @@ public:
 };
 
 /// A height field shape. Cannot be used as a dynamic object.
+///
+/// Note: If you're using HeightFieldShape and are querying data while modifying the shape you'll have a race condition.
+/// In this case it is best to create a new HeightFieldShape using the Clone function. You replace the shape on a body using BodyInterface::SetShape.
+/// If a query is still working on the old shape, it will have taken a reference and keep the old shape alive until the query finishes.
 class JPH_EXPORT HeightFieldShape final : public Shape
 {
 public:
@@ -118,6 +122,9 @@ public:
 									HeightFieldShape(const HeightFieldShapeSettings &inSettings, ShapeResult &outResult);
 	virtual							~HeightFieldShape() override;
 
+	/// Clone this shape. Can be used to avoid race conditions. See the documentation of this class for more information.
+	Ref<HeightFieldShape>			Clone() const;
+
 	// See Shape::MustBeStatic
 	virtual bool					MustBeStatic() const override				{ return true; }
 
@@ -202,6 +209,7 @@ public:
 
 	/// Set the height values of a block of data.
 	/// Note that this requires decompressing and recompressing a border of size mBlockSize in the negative x/y direction so will cause some precision loss.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	/// @param inX Start X position, must be a multiple of mBlockSize and in the range [0, mSampleCount - 1]
 	/// @param inY Start Y position, must be a multiple of mBlockSize and in the range [0, mSampleCount - 1]
 	/// @param inSizeX Number of samples in X direction, must be a multiple of mBlockSize and in the range [0, mSampleCount - inX]
@@ -225,6 +233,7 @@ public:
 	void							GetMaterials(uint inX, uint inY, uint inSizeX, uint inSizeY, uint8 *outMaterials, intptr_t inMaterialsStride) const;
 
 	/// Set the material indices of a block of data.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	/// @param inX Start X position, must in the range [0, mSampleCount - 1]
 	/// @param inY Start Y position, must in the range [0, mSampleCount - 1]
 	/// @param inSizeX Number of samples in X direction

+ 14 - 0
Jolt/Physics/Collision/Shape/MutableCompoundShape.cpp

@@ -55,6 +55,20 @@ MutableCompoundShape::MutableCompoundShape(const MutableCompoundShapeSettings &i
 	outResult.Set(this);
 }
 
+Ref<MutableCompoundShape> MutableCompoundShape::Clone() const
+{
+	Ref<MutableCompoundShape> clone = new MutableCompoundShape();
+	clone->SetUserData(GetUserData());
+
+	clone->mCenterOfMass = mCenterOfMass;
+	clone->mLocalBounds = mLocalBounds;
+	clone->mSubShapes = mSubShapes;
+	clone->mInnerRadius = mInnerRadius;
+	clone->mSubShapeBounds = mSubShapeBounds;
+
+	return clone;
+}
+
 void MutableCompoundShape::AdjustCenterOfMass()
 {
 	// First calculate the delta of the center of mass

+ 16 - 7
Jolt/Physics/Collision/Shape/MutableCompoundShape.h

@@ -24,9 +24,9 @@ public:
 /// This shape is optimized for adding / removing and changing the rotation / translation of sub shapes but is less efficient in querying.
 /// Shifts all child objects so that they're centered around the center of mass (which needs to be kept up to date by calling AdjustCenterOfMass).
 ///
-/// Note: If you're using MutableCompoundShapes and are querying data while modifying the shape you'll have a race condition.
-/// In this case it is best to create a new MutableCompoundShape and set the new shape on the body using BodyInterface::SetShape. If a
-/// query is still working on the old shape, it will have taken a reference and keep the old shape alive until the query finishes.
+/// Note: If you're using MutableCompoundShape and are querying data while modifying the shape you'll have a race condition.
+/// In this case it is best to create a new MutableCompoundShape using the Clone function. You replace the shape on a body using BodyInterface::SetShape.
+/// If a query is still working on the old shape, it will have taken a reference and keep the old shape alive until the query finishes.
 class JPH_EXPORT MutableCompoundShape final : public CompoundShape
 {
 public:
@@ -36,6 +36,9 @@ public:
 									MutableCompoundShape() : CompoundShape(EShapeSubType::MutableCompound) { }
 									MutableCompoundShape(const MutableCompoundShapeSettings &inSettings, ShapeResult &outResult);
 
+	/// Clone this shape. Can be used to avoid race conditions. See the documentation of this class for more information.
+	Ref<MutableCompoundShape>		Clone() const;
+
 	// See Shape::CastRay
 	virtual bool					CastRay(const RayCast &inRay, const SubShapeIDCreator &inSubShapeIDCreator, RayCastResult &ioHit) const override;
 	virtual void					CastRay(const RayCast &inRay, const RayCastSettings &inRayCastSettings, const SubShapeIDCreator &inSubShapeIDCreator, CastRayCollector &ioCollector, const ShapeFilter &inShapeFilter = { }) const override;
@@ -61,20 +64,25 @@ public:
 	///@{
 	/// @name Mutating shapes. Note that this is not thread safe, so you need to ensure that any bodies that use this shape are locked at the time of modification using BodyLockWrite. After modification you need to call BodyInterface::NotifyShapeChanged to update the broadphase and collision caches.
 
-	/// Adding a new shape
+	/// Adding a new shape.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	/// @return The index of the newly added shape
 	uint							AddShape(Vec3Arg inPosition, QuatArg inRotation, const Shape *inShape, uint32 inUserData = 0);
 
-	/// Remove a shape by index
+	/// Remove a shape by index.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	void							RemoveShape(uint inIndex);
 
-	/// Modify the position / orientation of a shape
+	/// Modify the position / orientation of a shape.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	void							ModifyShape(uint inIndex, Vec3Arg inPosition, QuatArg inRotation);
 
-	/// Modify the position / orientation and shape at the same time
+	/// Modify the position / orientation and shape at the same time.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	void							ModifyShape(uint inIndex, Vec3Arg inPosition, QuatArg inRotation, const Shape *inShape);
 
 	/// @brief Batch set positions / orientations, this avoids duplicate work due to bounding box calculation.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	/// @param inStartIndex Index of first shape to update
 	/// @param inNumber Number of shapes to update
 	/// @param inPositions A list of positions with arbitrary stride
@@ -86,6 +94,7 @@ public:
 	/// Recalculate the center of mass and shift all objects so they're centered around it
 	/// (this needs to be done of dynamic bodies and if the center of mass changes significantly due to adding / removing / repositioning sub shapes or else the simulation will look unnatural)
 	/// Note that after adjusting the center of mass of an object you need to call BodyInterface::NotifyShapeChanged and Constraint::NotifyShapeChanged on the relevant bodies / constraints.
+	/// Beware this can create a race condition if you're running collision queries in parallel. See class documentation for more information.
 	void							AdjustCenterOfMass();
 
 	///@}