[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