Share private Point class between Normal and Crop filters

It was desirable to have the Normal filter viewpoint option specified in a
manner consistent with other PDAL filters. The Crop filter already accepts a
point as WKT or GeoJSON. We use the same class to specify the viewpoint now.

filters/CropFilter.cpp

@@ -40,7 +40,7 @@
 #include <pdal/Polygon.hpp>
 #include <pdal/pdal_macros.hpp>
 #include <pdal/util/ProgramArgs.hpp>
-#include <filters/private/crop/Point.hpp>
+#include <filters/private/Point.hpp>
 
 #include <sstream>
 #include <cstdarg>
@@ -251,7 +251,7 @@ void CropFilter::crop(const Polygon& g, PointView& input, PointView& output)
 }
 
 
-bool CropFilter::crop(const PointRef& point, const cropfilter::Point& center)
+bool CropFilter::crop(const PointRef& point, const Point& center)
 {
     double x = point.getFieldAs<double>(Dimension::Id::X);
     double y = point.getFieldAs<double>(Dimension::Id::Y);
@@ -275,7 +275,7 @@ bool CropFilter::crop(const PointRef& point, const cropfilter::Point& center)
 }
 
 
-void CropFilter::crop(const cropfilter::Point& center, PointView& input,
+void CropFilter::crop(const Point& center, PointView& input,
     PointView& output)
 {
     PointRef point = input.point(0);

filters/CropFilter.hpp

@@ -43,12 +43,8 @@ extern "C" PF_ExitFunc CropFilter_InitPlugin();
 
 namespace pdal
 {
-namespace cropfilter
-{
-    class Point;
-};
-
 
+class Point;
 class ProgramArgs;
 
 // removes any points outside of the given range
@@ -69,7 +65,7 @@ class PDAL_DLL CropFilter : public Filter
     SpatialReference m_assignedSrs;
     double m_distance;
     double m_distance2;
-    std::vector<cropfilter::Point> m_centers;
+    std::vector<Point> m_centers;
     std::vector<Polygon> m_geoms;
     std::vector<BOX2D> m_boxes;
 
@@ -83,8 +79,8 @@ class PDAL_DLL CropFilter : public Filter
     void crop(const BOX2D& box, PointView& input, PointView& output);
     bool crop(const PointRef& point, const Polygon& g);
     void crop(const Polygon& g, PointView& input, PointView& output);
-    bool crop(const PointRef& point, const cropfilter::Point& center);
-    void crop(const cropfilter::Point& center, PointView& input,
+    bool crop(const PointRef& point, const Point& center);
+    void crop(const Point& center, PointView& input,
         PointView& output);
     void transform(const SpatialReference& srs);
 
@@ -93,4 +89,3 @@ class PDAL_DLL CropFilter : public Filter
 };
 
 } // namespace pdal
-

filters/NormalFilter.cpp

@@ -39,6 +39,8 @@
 #include <pdal/pdal_macros.hpp>
 #include <pdal/util/ProgramArgs.hpp>
 
+#include "private/Point.hpp"
+
 #include <Eigen/Dense>
 
 #include <string>
@@ -61,11 +63,10 @@ std::string NormalFilter::getName() const
 void NormalFilter::addArgs(ProgramArgs& args)
 {
     args.add("knn", "k-Nearest Neighbors", m_knn, 8);
-    m_vxArg = &args.add("vx", "Viewpoint X coordinate", m_vx);
-    m_vyArg = &args.add("vy", "Viewpoint Y coordinate", m_vy);
-    m_vzArg = &args.add("vz", "Viewpoint Z coordinate", m_vz);
+    m_viewpointArg =
+        &args.add("viewpoint", "Viewpoint as WKT or GeoJSON", m_viewpoint);
     args.add("always_up", "Normals always oriented with positive Z?", m_up,
-             false);
+             true);
 }
 
 void NormalFilter::addDimensions(PointLayoutPtr layout)
@@ -85,11 +86,12 @@ void NormalFilter::doFilter(PointView& view)
 
 void NormalFilter::prepared(PointTableRef table)
 {
-    if (m_up && (m_vxArg->set() || m_vyArg->set() || m_vzArg->set()))
-        throwError("Use either always_up or vx/vy/vz, but not both.");
-    if ((m_vxArg->set() || m_vyArg->set() || m_vzArg->set()) &&
-        !(m_vxArg->set() && m_vyArg->set() && m_vzArg->set()))
-        throwError("When setting viewpoint, must set vx, vy, and vz.");
+    if (m_up && m_viewpointArg->set())
+    {
+        log()->get(LogLevel::Warning)
+            << "Viewpoint provided. Ignoring always_up = TRUE." << std::endl;
+        m_up = false;
+    }
 }
 
 void NormalFilter::filter(PointView& view)
@@ -111,19 +113,19 @@ void NormalFilter::filter(PointView& view)
         auto eval = solver.eigenvalues();
         Eigen::Vector3f normal = solver.eigenvectors().col(0);
 
-        if (m_up)
+        if (m_viewpointArg->set())
         {
-            if (normal[2] < 0)
+            PointRef p = view.point(i);
+            Eigen::Vector3f vp(
+                m_viewpoint.x - p.getFieldAs<double>(Dimension::Id::X),
+                m_viewpoint.y - p.getFieldAs<double>(Dimension::Id::Y),
+                m_viewpoint.z - p.getFieldAs<double>(Dimension::Id::Z));
+            if (vp.dot(normal) < 0)
                 normal *= -1.0;
         }
-        else if (m_vxArg->set() && m_vyArg->set() && m_vzArg->set())
+        else if (m_up)
         {
-            // flip normal towards viewpoint
-            PointRef p = view.point(i);
-            Eigen::Vector3f vp(m_vx - p.getFieldAs<double>(Dimension::Id::X),
-                               m_vy - p.getFieldAs<double>(Dimension::Id::Y),
-                               m_vz - p.getFieldAs<double>(Dimension::Id::Z));
-            if (vp.dot(normal) < 0)
+            if (normal[2] < 0)
                 normal *= -1.0;
         }
 

filters/NormalFilter.hpp

@@ -38,6 +38,8 @@
 #include <pdal/plugin.hpp>
 #include <pdal/util/ProgramArgs.hpp>
 
+#include "private/Point.hpp"
+
 #include <cstdint>
 #include <memory>
 #include <string>
@@ -69,8 +71,8 @@ class PDAL_DLL NormalFilter : public Filter
 
 private:
     int m_knn;
-    double m_vx, m_vy, m_vz;
-    Arg *m_vxArg, *m_vyArg, *m_vzArg;
+    Point m_viewpoint;
+    Arg* m_viewpointArg;
     bool m_up;
 
     virtual void addArgs(ProgramArgs& args);

filters/private/crop/Point.cpp → filters/private/Point.cpp

@@ -45,9 +45,6 @@ const double HIGHEST = (std::numeric_limits<double>::max)();
 
 }
 
-namespace cropfilter
-{
-
 Point::Point()
     : Geometry()
     , x(LOWEST)
@@ -114,7 +111,4 @@ bool Point::is3d() const
     return (z != LOWEST);
 }
 
-} //namespace cropfilter
-
 } //namespace pdal
-

filters/private/crop/Point.hpp → filters/private/Point.hpp

@@ -39,9 +39,6 @@
 namespace pdal
 {
 
-namespace cropfilter
-{
-
 class PDAL_DLL Point : public Geometry
 {
 public:
@@ -58,5 +55,5 @@ class PDAL_DLL Point : public Geometry
     double y;
     double z;
 };
-} // namespace cropfilter
+
 } // namespace pdal