Proposed change to curves resampling

Development
1

I noticed some unexpected behavior while using vtkMRMLMarkupsCurveNode::ResamplePoints

Due to small rounding differences when comparing the remaining segment to the sampling distance, the last point is sometimes not placed, as shown in the figure below. The resampled curve (blue) is plotted over the original curve (red).

OriginalCurve
OriginalCurve1217×908 36.7 KB

This issue can be replicated by creating an arbitrary 5 point curve and running:

resampleNumber=10
curve=getNode("C")
currentPoints = curve.GetCurvePointsWorld()
newPoints = vtk.vtkPoints()
sampleDist = curve.GetCurveLengthWorld()/(resampleNumber-1)

closedCurveOption = 0
curve.ResamplePoints(currentPoints,newPoints,sampleDist,closedCurveOption)

vector=vtk.vtkVector3d()
pt=[0,0,0]
resampledCurve = slicer.mrmlScene.AddNewNodeByClass("vtkMRMLMarkupsCurveNode", "resampledCurveOrig")

for controlPoint in range(0,newPoints.GetNumberOfPoints()):
  newPoints.GetPoint(controlPoint,pt)
  vector[0]=pt[0]
  vector[1]=pt[1]
  vector[2]=pt[2]
  resampledCurve.AddControlPoint(vector)

I fixed this for myself by rounding the remaining segment length and sampling distance to 4 decimal places when they are compared. I was wondering if there’s interest in changing this in vtkMRMLMarkupsCurveNode::ResamplePoints too? I can submit a pull request if so. Thanks!

UpdatedCurve
UpdatedCurve1222×922 40.4 KB

2

Yes, please send a pull request.

1 Like
3

Thanks @lassoan. I have one more question on this. In my module, I rounded to 4 decimal places to get rid of the differences on the order of ~1e-14 when comparing two doubles. I would like to use a more precise approach such as:

double a, b;
if(fabs(a - b) <= epsilon * fabs(a))

Is there a convention or recommended approach for setting this epsilon in Slicer?

4

I’ve just found that the resample function already handled this case properly but only for closed curves. I’ve update the code to do the same for open curves, too.

5

Great! Thanks @lassoan!

6

Fix committed to master (Revision: 28508).

7

Thanks @lassoan, this is working well for me in the preview version.

1 Like