[vtk-developers] ICP Enhancement - TooFarThreshold

Luca Antiga luca.antiga at gmail.com
Tue Feb 23 09:16:15 EST 2010


Hi David,

On Feb 23, 2010, at 2:38 PM, David Doria wrote:

> On Tue, Feb 23, 2010 at 3:43 AM, Luca Antiga <luca.antiga at gmail.com>  
> wrote:
> Hi David,
>  thanks a lot for this contribution, it will probably solve a few  
> problems I had with ICP in the past.
> A few suggestions:
>
> - 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;
>
> - there is an indentation glitch in the for loop;
>
> - you don't really need to Delete closestp, just use closestp- 
> >Initialize();
>
> - 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).
>
> - TooFarThreshold is compared to dist2, which is the squared  
> distance; my vote is for specifying it as a distance and square it  
> internally;
>
> - 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.
>
> Here's my try:
>
>   double tooFarThreshold2 = this->TooFarThreshold * this- 
> >TooFarThreshold;
>   do
>     {
>     // Fill points with the closest points to each vertex in input
>     if (!this->UseTooFarThreshold)
>       {
>       closestp->SetNumberOfPoints(nb_points); // It was set above,  
> but at this point it should be removed from there
>       for(i = 0; i < nb_points; i++)
>         {
>         this->Locator->FindClosestPoint(a->GetPoint(i),
>                                         outPoint,
>                                         cell_id,
>                                         sub_id,
>                                         dist2);
>         closestp->SetPoint(i, outPoint);
>         }
>       this->LandmarkTransform->SetSourceLandmarks(a);
>       }
>     else //use a TooFarThreshold
>       {
>       vtkPoints* sourceLandmarks = vtkPoints::New();
>       closestp->Initialize();
>       for(i = 0; i < nb_points; i++)
>         {
>         this->Locator->FindClosestPoint(a->GetPoint(i),
>                                         outPoint,
>                                         cell_id,
>                                         sub_id,
>                                         dist2);
>         if(dist2 < tooFarThreshold2)
>           {
>           closestp->InsertNextPoint(outPoint);
>           sourceLandmarks->InsertNextPoint(a->GetPoint(i));
>           }
>         }
>       this->LandmarkTransform->SetSourceLandmarks(sourceLandmarks);
>       sourceLandmarks->Delete();
>       }
>     // Build the landmark transform
>
>     this->LandmarkTransform->SetTargetLandmarks(closestp);
>     this->LandmarkTransform->Update();
>
>
> Cheers
>
>
> Luca
>
> Luca -
>
> Overall, sounds/looks good to me.
>
> 1) UseTooFarThreshold - Sure, I like it.
>
> 2) I had never used Initialize() - so if it clears the data without  
> having to delete it, that was my goal.
>
> 3) Any reason to compare TooFarThreshold2 to dist2 instead of  
> TooFarThreshold to sqrt(dist2)?

just because sqrt(dist2) is more expensive and squaring won't ever  
give you floating point issues no matter what happens, but it will  
hardly make a difference in this context.

>
> Any other thoughts?
>
> Thanks,
>
> David


Bye


Luca

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100223/8d861db9/attachment.html>


More information about the vtk-developers mailing list