RegenerateRestPositions strange code

[josemendez]

Here original code from ObiRope
public override void RegenerateRestPositions(){

            var distanceBatch = DistanceConstraints.GetFirstBatch();
            if (distanceBatch.ConstraintCount > 0)
            {
                // Iterate trough all distance constraints in order:
                int particle = -1;
                int lastParticle = -1;
                float accumulatedDistance = 0;
                for (int i = 0; i < distanceBatch.ConstraintCount; ++i)
                {
                    if (i == 0)
                    {
                        lastParticle = particle = distanceBatch.springIndices[i * 2];
                        restPositions[particle] = new Vector4(0, 0, 0, 1);
                    }
                    accumulatedDistance += Mathf.Min(interParticleDistance, principalRadii[particle][0],principalRadii[lastParticle][0]);
                    particle = distanceBatch.springIndices[i * 2 + 1];
                    restPositions[particle] = Vector3.right * accumulatedDistance;
                    restPositions[particle][3] = 1; // activate rest position
                }
            }

            PushDataToSolver(ParticleData.REST_POSITIONS);
        }
First: this code generate (distanceBatch.ConstraintCount-1) float[3] arrays while Mathf.Min function, this makes unity garbage collector crazy.
Second: lastParticle  newer updated in cycle, this result in accumulatedDistance sum of distances from rope start to current point, not from previous to current point.
So finally accumulatedDistance  contains not total rope length but (distanceBatch.ConstraintCount-1)*ropeLength/2.
Are you sure that is right, because this is strange.
This is my implementation
public override void RegenerateRestPositions(){
            ObiDistanceConstraintBatch distanceBatch = DistanceConstraints.GetFirstBatch();
            if (distanceBatch.ConstraintCount > 0)
            {
                Vector3 right=Vector3.right;
                // Iterate trough all distance constraints in order:
                int lastParticle = distanceBatch.springIndices[0];
                float accumulatedDistance = 0;
                restPositions[0] = new Vector4(0, 0, 0, 1);
                for (int i = 0; i < distanceBatch.ConstraintCount; ++i)
                {
                    int particle = distanceBatch.springIndices[i * 2 + 1];
                    accumulatedDistance += Mathf.Min(interParticleDistance, Math.Min(principalRadii[particle][0],principalRadii[lastParticle][0]));
                    var ditanceoffset = right * accumulatedDistance;
                    restPositions[particle] =new Vector4(ditanceoffset.x,ditanceoffset.y,ditanceoffset.z,1);
                    lastParticle = particle;
                }
                PushDataToSolver(ParticleData.REST_POSITIONS);
            }
        }

Hi,

The original code is 100% correct.

Rest positions are used internally to ensure particles that initially overlap do not generate inter collisions. So what this code does is :take the first particle in the array as the reference frame (hence lastParticle is only updated if i == 0), then iterate over all other particles calculating the distance from the current particle to the first particle (assuming the rope is tense and completely straight along the X axis): accumulatedDistance += Mathf.Min(interParticleDistance,principalRadii[particle][0],principalRadii[lastParticle][0]);

Your code will generate incorrect rest positions, because it adds to accumulatedDistance the radius of the smallest particle encountered up to the current particle, instead of the minimum between the first one, the current one and the inter-particle distance. This will cause false negatives when dealing with self-collisions, leading to jitter.