<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi David,<div> thanks a lot for this contribution, it will probably solve a few problems I had with ICP in the past.</div><div>A few suggestions:</div><div><br></div><div>- I'd suggest is to add a UseTooFarThreshold flag with the relative vtkSetMacro, vtkGetMacro and vtkBooleanMacro to activate/deactivate TooFar (as in UseTooFarThresholdOn, UseTooFarThresholdOff, etc) instead of resorting to the -1.0 trick;</div><div><br></div><div>- there is an indentation glitch in the for loop;</div><div><br></div><div>- you don't really need to Delete closestp, just use closestp->Initialize();</div><div><br></div><div>- actually, the code will crash if rerun with TooFar on first and TooFar off afterwards, because in the latter you call closestp->SetPoint without first setting the NumberOfPoints after you Delete and instantiate a new one (or call Initialize).</div><div><br></div><div>- TooFarThreshold is compared to dist2, which is the squared distance; my vote is for specifying it as a distance and square it internally;</div><div><br></div><div>- I personally would handle sourceLandmarks in a bit more readable way: the smart pointer initialized in the else scope and used outside will behave correctly feels weird, but it's just that I'm old fashioned. Read below for a proposal.</div><div><br></div><div>Here's my try:</div><div><br></div><div> double tooFarThreshold2 = this->TooFarThreshold * this->TooFarThreshold;</div><div><div> do </div><div> {</div><div> // Fill points with the closest points to each vertex in input</div><div> if (!this->UseTooFarThreshold)</div><div> {</div><div> closestp->SetNumberOfPoints(nb_points); // It was set above, but at this point it should be removed from there</div><div> for(i = 0; i < nb_points; i++)</div><div> {</div><div> this->Locator->FindClosestPoint(a->GetPoint(i),</div><div> outPoint,</div><div> cell_id,</div><div> sub_id,</div><div> dist2);</div><div> closestp->SetPoint(i, outPoint);</div><div> }</div><div> this->LandmarkTransform->SetSourceLandmarks(a);</div><div> }</div><div> else //use a TooFarThreshold</div><div> {</div><div> vtkPoints* sourceLandmarks = vtkPoints::New();</div><div> closestp->Initialize();</div><div> for(i = 0; i < nb_points; i++)</div><div> {</div><div> this->Locator->FindClosestPoint(a->GetPoint(i),</div><div> outPoint,</div><div> cell_id,</div><div> sub_id,</div><div> dist2);</div><div> if(dist2 < tooFarThreshold2)</div><div> {</div><div> closestp->InsertNextPoint(outPoint);</div><div> sourceLandmarks->InsertNextPoint(a->GetPoint(i));</div><div> }</div><div> }</div><div> this->LandmarkTransform->SetSourceLandmarks(sourceLandmarks);</div><div><div> sourceLandmarks->Delete();</div></div><div> }</div><div> // Build the landmark transform</div><div><br></div><div> this->LandmarkTransform->SetTargetLandmarks(closestp);</div><div> this->LandmarkTransform->Update();</div><div><br></div><div><br></div><div>Cheers</div><div><br></div></div><div><br></div><div>Luca</div><div><br></div><div><br></div><div><br><div><div>On Feb 22, 2010, at 10:42 PM, David Doria wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; border-collapse: collapse; "><div>I added a "TooFarThreshold" to vtkIterativeClosestPointTransform. This is a standard modification to the ICP algorithm where not all points in set A are assumed to have a reasonable/valid corresponding point in set B. This occurs when aligning a partial surface with a complete surface, for example. It is a simple modification - check the distance between each source landmark and its alleged corresponding point before using them in the landmark transform.</div> <div><br></div><div>I created a feature request for this long ago, so I just put the modified files there:</div></span></div><a href="http://public.kitware.com/Bug/view.php?id=8983" target="_blank">http://public.kitware.com/Bug/view.php?id=8983</a><div> <br></div><div>Any volunteers to take a look so we can commit this? There should be no backwards compatibility issues - if the TooFarThreshold is negative, the old code runs (the default is -1.0).</div><div><br clear="all"> Thanks,<br><br>David<br> </div> _______________________________________________<br>Powered by <a href="http://www.kitware.com">www.kitware.com</a><br><br>Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html">http://www.kitware.com/opensource/opensource.html</a><br><br>Follow this link to subscribe/unsubscribe:<br><a href="http://www.vtk.org/mailman/listinfo/vtk-developers">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br><br></blockquote></div><br></div></body></html>