RegenerateRestPositions strange code

[Teolog]

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);
            }
        }

[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.

[Teolog]

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.

So, if I understand you right, this code may work as original implementation but not flow to GC:
public override void RegenerateRestPositions(){
            ObiDistanceConstraintBatch distanceBatch = DistanceConstraints.GetFirstBatch();
            if (distanceBatch.ConstraintCount > 0)
            {
                Vector3 right=Vector3.right;
                // Iterate trough all distance constraints in order:
                float accumulatedDistance = 0;
                restPositions[0] = new Vector4(0, 0, 0, 1);
                float mindistance = Mathf.Min(interParticleDistance, principalRadii[distanceBatch.springIndices[0]][0]);
                for (int i = 1; i < distanceBatch.ConstraintCount; ++i)
                {
                    int particle = distanceBatch.springIndices[i * 2 + 1];
                    accumulatedDistance += Math.Min(principalRadii[particle][0],mindistance);
                    var ditanceoffset = right * accumulatedDistance;
                    restPositions[particle] =new Vector4(ditanceoffset.x,ditanceoffset.y,ditanceoffset.z,1);
                }
                PushDataToSolver(ParticleData.REST_POSITIONS);
            }
        }

[josemendez]

So, if I understand you right, this code may work as original implementation but not flow to GC:
public override void RegenerateRestPositions(){
            ObiDistanceConstraintBatch distanceBatch = DistanceConstraints.GetFirstBatch();
            if (distanceBatch.ConstraintCount > 0)
            {
                Vector3 right=Vector3.right;
                // Iterate trough all distance constraints in order:
                float accumulatedDistance = 0;
                restPositions[0] = new Vector4(0, 0, 0, 1);
                float mindistance = Mathf.Min(interParticleDistance, principalRadii[distanceBatch.springIndices[0]][0]);
                for (int i = 1; i < distanceBatch.ConstraintCount; ++i)
                {
                    int particle = distanceBatch.springIndices[i * 2 + 1];
                    accumulatedDistance += Math.Min(principalRadii[particle][0],mindistance);
                    var ditanceoffset = right * accumulatedDistance;
                    restPositions[particle] =new Vector4(ditanceoffset.x,ditanceoffset.y,ditanceoffset.z,1);
                }
                PushDataToSolver(ParticleData.REST_POSITIONS);
            }
        }

Yes, as long as you don't use the array version of Mathf.Min() garbage will not be generated. However I'm curious as how often you're calling this method, it should only be called one at initialization, or when adding new particles using ObiCursor.