[pdal] filters.icp transformation

[Jim Klassen]

I suspect we could compose it three operations into a single transform matrix, but probably at a loss of precision. (I suspect preserving precision this is why filters.icp bothers with calculating and shifting to the centroid in the first place).

Alternatively, it might be simpler for the user if the centroid value and matrix could be passed directly into filters.transform (instead of needing three filters.transforms in the pipeline).

In my opinion, the current workflow of saving the metadata, manually parsing it, and then constructing a new pipeline with values from the metadata isn't very user friendly in general. If this was something I was doing regularly, I would certainly write a script do those steps for me (probably as part of a larger workflow).  And if a script is generating the second pipeline, it doesn't really matter to me if the second pipeline is one stage or three stages.

I have a first pass at a PR here:
https://github.com/PDAL/PDAL/pull/2962

On Fri, Mar 6, 2020, 08:47 Bradley Chambers <brad.chambers at gmail.com <mailto:brad.chambers at gmail.com>> wrote:

    I’m wondering if it would make more sense to simply compose the three steps into a single output transformation. Making the user do the three steps themselves seems overly complicated. Is there a scenario in which we only want the transformation of the centered clouds (current state of the code)? Your use case is probably more common.

    On Thu, Mar 5, 2020 at 18:53 Jim Klassen <klassen.js at gmail.com <mailto:klassen.js at gmail.com>> wrote:

        Does a strategy of adding the centroid to the filters.icp metadata and updating filters.icp documentation with the pipeline to apply it sound good?

        On 3/5/20 5:41 PM, Andrew Bell wrote:
>         This seems the same as this already reported bug:
>
>         https://github.com/PDAL/PDAL/issues/2939
>
>         If you want to submit a PR, I'll look it over.
>
>         Thanks!
>
>
>         On Thu, Mar 5, 2020 at 6:36 PM Jim Klassen <klassen.js at gmail.com <mailto:klassen.js at gmail.com>> wrote:
>
>             I am trying to do something similar (apply ICP on a subset of a large
>             dataset) and am running into confusion over the transformation matrix in
>             the metadata.
>
>             The point cloud output of the ICP filter on the subset looks good.
>             However when I try to use the resulting "transform" metadata as a input
>             into filters.transformation.matrix over the full dataset, the resulting
>             transformation moves the points wildly out of position.
>
>             Looking at the code, in IterativeClosestPoint.cpp, it looks like the
>             transformation matrix (T) is applied after shifting the point cloud to
>             the centroid of the reference point cloud (C), and the shifted back to
>             regular coordinates.
>
>             So something like Pout = ((P - C) * T) + C
>
>             Where P is a 4 element input point vector (x,y,z,1), Pout is the output
>             point vector, C is the centroid vector (x,y,z,0), T is the 4x4
>             transformation matrix found by ICP.
>
>             T is reported in the metadata for the stage, however, C is not reported
>             in the metadata.
>
>             But the transformation matrix that filters.transformation expects
>             something more like
>
>             Pout = P * T'
>
>             If C is not (0,0,0) then T and T' will be different.
>
>             I modified IterativeClosestPoint.cpp to also export the centroid to the
>             metadata and then created a pipeline with three transformations, first
>             shifting by the negative of the centroid, then applying T, then shifting
>             back by the centroid and that produced the expected results.
>
>
>             On 2/18/20 7:24 PM, Bradley Chambers wrote:
>             > The options are very heavily borrowed from the PCL library, where this
>             > was a squared value. Also, keep in mind that the default values may be
>             > better suited for terrestrial/robotics applications (as opposed to
>             > aerial), and will very likely need some tweaking.
>             >
>             > The reported transformation should also be reported in row major
>             > order, like the transformation filter. If you find it is not, I’d
>             > consider that a bug, so feel free to file a ticket.
>             >
>             > PRs are always welcome, especially for documentation!
>             >
>             > Brad
>             >
>
>             _______________________________________________
>             pdal mailing list
>             pdal at lists.osgeo.org <mailto:pdal at lists.osgeo.org>
>             https://lists.osgeo.org/mailman/listinfo/pdal
>
>
>
>         -- 
>         Andrew Bell
>         andrew.bell.ia at gmail.com <mailto:andrew.bell.ia at gmail.com>
        _______________________________________________
        pdal mailing list
        pdal at lists.osgeo.org <mailto:pdal at lists.osgeo.org>
        https://lists.osgeo.org/mailman/listinfo/pdal

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/pdal/attachments/20200306/27f296ad/attachment-0001.html>