List translation failure - am I missing something?

rhino5
rhino6

(Przemysław Doliwa) #1

I have question about translation performed on List od Point3d

Code:

Transform tForm = Transform.Translation(2, 2, 0);
foreach (var pt in xPts)
{
     pt.Transform(tForm);
}

I tried transforming single point and brep and it works but when iterating list all points aren’t translated i even tried to push one point from list like: xPts[0].Transform(tForm); but this also doesn’t work. Any clues whats going on? I tried this on r5 and r6 and both give same result.


#2

Hm, I wonder if this is the typical “modifying a copy” problem that I used to stumble upon when starting to use C#.

What happens if you try this:

Transform tForm = Transform.Translation(2, 2, 0);
var pt_array = xPts.ToArray();
for (var i = 0; i < pt_array.Length; i++)
{
     pt_array[i].Transform(tForm);
}

Now, when making changes to pt_array[i] you guaranteed to have made the changes on the one and only “original” point in that slot, and not on a copy.

// Rolf


(NARUTO) #3

Your code is working properly


#4

If the code worked properly there wouldn’t be a problem. He probably assigns the xPts list to the outport. But the points in that list has not been transformed, which is what I am saying in my post. Only copies (“pt”) are transformed.

However, if collecting these copies as you do (pts.Add(pt);) then the modified copies goes to the output and everything is fine. That is obviously not the case for the OP.

So, if the assumption is that the points in the xPts list are modified then the code doesn’t work. And I have reason to believe that this is the case, otherwise OP would not have the problem he’s describing.

Comparing the two cases:

  private void RunScript(List<Point3d> xPts, ref object A)
  {
    var trx = Transform.Translation(2, 2, 0);
    
    // Doesn't work
    foreach (var pt in xPts)
    {
      pt.Transform(trx);  // <- transforming a *copy*
    }
    A = xPts;   
    
    // This works
    var pt_array = xPts.ToArray();
    for (var i = 0; i < pt_array.Length;i++)
    {
      pt_array[i].Transform(trx); // trx on the "original"
    }    
    A = pt_array;    
  }

// Rolf


(Przemysław Doliwa) #5

@RIL Yep thats exacly the case List casted to array and translated in for loop works but foreach with list doesn’t. But when using list for existing doc objects and transforming with foreach works so i thought it will work also with ‘in buffer’ Rhino.Geometry.

Btw list with for and xPts[i] also don’t work.

Any idea how to get this copy in this case? I know that arrays are cheaper but lists have few pros especially when i will use those pts in very large numbers.

Hmm maybe i should go dirty like for on list and indeed modify XPts[i] = pt after transforming pt?


#6

The problem with the list, is that the point is “copied out of the list” and into your variable. It’s not obvious or intuitive that it is so, and therefore this error tends to creep in.

But in the case of the array you actually transform the “original in it place in the array” (not copied out of the array). The problem has to do with structs (so for example a List of vectors, and other structs, would have the same problem).

Classes generally don’t have this problem, which is why other object types works well (so this doesn’t have primarily with lists or foreach to do, it has to do with structs ).

Last time when I had the need to hold my points (Point3d) in a list - while also having to perform transforms on them - I used the type Point instead (List< Point >). Then you can operate directly on the list items (like in the case of the array) since a Point class can be referenced, whereas structs can’t.

So, what I am saying is that your code would actually work if you first convert your List< Point3d > to a List< Point > and traverse that list instead, like so:

var pointList = new List<Point>();
foreach (var pt in xPts)
{
    pointList.Add( new Point(pt) );
}

Now your old code would work, if replacing the xPts with the pointList (just like I replaced it with an array).

foreach (var pt in pointList)
{
    pt.Transform(tForm);
}
A = pointList; // or whatever outport name you use

The benefit with Point is that they are of the same “kind” (has a common super class) as other Geometry types, being a class, and they inherit from the GeometryBase class, see this link for Point ).

// Rolf


#7

I often use arrays even for hundreds of thousands of points. Arrays are “inherently threadsafe” whereas Lists are not, which is the reason I often use arrays.

Putting the point back into the list would definitely fix the problem. I haven’t tried what syntax can be used, but if XPts[i] = pt works, then that seems like a convenient way.

// Rolf


(Michael Pryor) #8

What about using a Point3dList since it seems all points are moving same amount (2)?

private void RunScript(List xPts, ref object A)
{
Transform tForm = Transform.Translation(2, 2, 0);
Point3dList xPtsList = new Point3dList(xPts);

xPtsList.Transform(tForm);

A = xPtsList;

}


(Przemysław Doliwa) #9

@RIL idea was good but foreach cant to alter collection on which it performs actualy if you do so you’ll get exeption that colloction was altered :wink: but … i found other quicker way to do this

List<Point3d> tempList = new List<Point3d>();
foreach (var pt in xPts)
{
     pt.Transform(tForm);
     tempList.Add(pt);
}

xPts = tempList;

quick local var but doesn’t alter previous code and it’s easy to understand when looking at it :wink:

@Michael_Pryor 2,2,0 are only temp values and one important thing points don’t exist in doc at all but are directly pushed to conduict - its important here cause refing doc object solves foreach problem.


#10

Yes, that was also 603419608’s solution. However, adding large numbers if items to Lists can be slow, which is why I avoid them. One can set Capacity for the list-copy before adding items (tempList.Capacity = xPts.Count or something like that), or, as I did, use an array.

Large number of points may also invite you to process items in parallel (depending on how much work to do with each item) and then arrays have the benefit of avoiding data races.

I don’t know why you insist in using foreach though. :slight_smile: Skip foreach and use for, and then the List probably won’t complain about being altered during the loop (unless you need to use “Replace(i)” or some other function to insert the translated point).

Try for-loop with xPts[i] = pt; and see if that works.

// Rolf


#11

I didn’t know about that one. That will definitely be useful in some situations! +1

// Rolf


(Michael Pryor) #12

Yea, I use Point3dList often for closest index
Much faster than using Point3d, measuring distances and sorting to find the closest.


#13

@Michael_Pryor,

Sorry for deviating from the subject, but I often traverse meshe vertices, but mesh vertices are Point3f which doesn’t directly convert to double. Have you ever tried finding the fastest way to convert a mesh.Vertices list to a Point3dList?

var ptlist = new Point3dList(Mesh.Vertices);  // ¯\_(ツ)_/¯

// Rolf


(Michael Pryor) #14

The way I know is the way you probably already expect :smiley:

var ptlist = new Point3dList(M.Vertices.Count);
for(int i = 0; i < M.Vertices.Count; i++)
{
ptlist.Add(new Point3d(M.Vertices[i]));
}


#15

Yes, that’s how I do it. Except for in the case of meshes, I update the list only if, and only if, the mesh changes. Which in most cases doesn’t even happen… Like so:

    if (Mesh == null)
      return;
    // Copy list only if it changes
    if (m_ptlist.Count != Mesh.Vertices.Count)
    {
      m_ptlist.Clear();
      for (var i = 0; i < Mesh.Vertices.Count; i++)
      {
        m_ptlist.Add(Mesh.Vertices[i]);
      }
    }

    var ix = m_ptlist.ClosestIndex(Pt);
    // aso
  }

  // <Custom additional code> 
  Point3dList m_ptlist = new Point3dList();

// Rolf


(Przemysław Doliwa) #16

Yep i missed this one indeed

for-loop
pt= xPts[i];
xform
pt.transform
xPts[i] = pt

this holds only additional pt struct instead of whole additional list :wink:


(David Rutten) #17

It is. Point3d is a struct. When you get a struct out of a collection using a foreach loop, or oftentimes even when using an indexer, you get a copy of the data inside the collection. Modifying and then promptly forgetting about this copy is not going to change the state of the collection.


#18

I think my brain is a struct. Because I have stumbled on this problem twice. The first correct answer (here on the forum) to my problem must have corrected a copy of my brain, not the original, because I did the same mistake twice. :wink:

// Rolf