New writer constructor (#73)

* FEAT-434: Make header scale and offset writable in Writer.

* FEAT-434: New Writer constructor.

* FEAT-434: More python debugging.

* FEAT-434: More python debugging.

* disable scan angle check

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Write WKT as EVLR instead of VLR.

* Update tests to reflect removal of scan angle check.

* Improved messages for ValidateSpatialBounds.

* Improved messages for ValidateSpatialBounds.

* Finish debugging new CopcFileWriter constructor.

* Review comments and more.

* Review comments.

* Change wording.

Co-authored-by: Christopher Lee <csl170000@utdallas.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **\[Python/C++\]** Update `CopcFileWriter` constructor to provide optional arguments to update information when using a `CopcConfig` object from a `Reader`. Optional arguments are `scale`, `offset`, `wkt`, `extra_bytes_vlr`, and `has_extended_stats`.
+- **\[Python/C++\]** Make `LasHeader`'s `scale` and `offset` read-only.
+
 ## [2.1.2] - 2021-11-12
 
 ### Fixed

cpp/include/copc-lib/geometry/vector3.hpp

@@ -37,7 +37,8 @@ struct Vector3
     std::string ToString() const
     {
         std::stringstream ss;
-        ss << "Vector3: x=" << x << ", y=" << y << ", z=" << z;
+        ss.precision(std::numeric_limits<double>::max_digits10);
+        ss << "(" << x << ", " << y << ", " << z << ")";
         return ss.str();
     }
 

cpp/include/copc-lib/io/writer.hpp

@@ -2,6 +2,7 @@
 #define COPCLIB_IO_WRITER_H_
 
 #include <array>
+#include <optional>
 #include <ostream>
 #include <stdexcept>
 #include <string>
@@ -24,9 +25,12 @@ class WriterInternal;
 class Writer : public BaseIO
 {
   public:
-    Writer(std::ostream &out_stream, CopcConfigWriter const &copc_file_writer)
+    Writer(std::ostream &out_stream, const CopcConfigWriter &copc_config_writer,
+           const std::optional<Vector3> &scale = {}, const std::optional<Vector3> &offset = {},
+           const std::optional<std::string> &wkt = {}, const std::optional<las::EbVlr> &extra_bytes_vlr = {},
+           const std::optional<bool> &has_extended_stats = {})
     {
-        InitWriter(out_stream, copc_file_writer);
+        InitWriter(out_stream, copc_config_writer, scale, offset, wkt, extra_bytes_vlr, has_extended_stats);
     }
 
     // Writes the file out
@@ -62,21 +66,27 @@ class Writer : public BaseIO
     };
 
     // Constructor helper function, initializes the file and hierarchy
-    void InitWriter(std::ostream &out_stream, const CopcConfigWriter &copc_file_writer);
+    void InitWriter(std::ostream &out_stream, const CopcConfigWriter &copc_file_writer,
+                    const std::optional<Vector3> &scale, const std::optional<Vector3> &offset,
+                    const std::optional<std::string> &wkt, const std::optional<las::EbVlr> &extra_bytes_vlr,
+                    const std::optional<bool> &has_extended_stats);
     // Gets the sum of the byte size the extra bytes will take up, for calculating point_record_len
     static int NumBytesFromExtraBytes(const std::vector<las::EbVlr::ebfield> &items);
 };
 
 class FileWriter : public Writer
 {
   public:
-    FileWriter(const std::string &file_path, const CopcConfigWriter &copc_file_writer)
+    FileWriter(const std::string &file_path, const CopcConfigWriter &copc_file_writer,
+               const std::optional<Vector3> &scale = {}, const std::optional<Vector3> &offset = {},
+               const std::optional<std::string> &wkt = {}, const std::optional<las::EbVlr> &extra_bytes_vlr = {},
+               const std::optional<bool> &has_extended_stats = {})
     {
 
         f_stream_.open(file_path.c_str(), std::ios::out | std::ios::binary);
         if (!f_stream_.good())
             throw std::runtime_error("FileWriter: Error while opening file path.");
-        InitWriter(f_stream_, copc_file_writer);
+        InitWriter(f_stream_, copc_file_writer, scale, offset, wkt, extra_bytes_vlr, has_extended_stats);
     }
 
     void Close() override;

cpp/include/copc-lib/las/header.hpp

@@ -24,15 +24,16 @@ class LasHeader
     LasHeader() = default;
     uint16_t EbByteSize() const;
     LasHeader(int8_t point_format_id, uint16_t point_record_length, const Vector3 &scale, const Vector3 &offset)
-        : point_format_id_(point_format_id), point_record_length_{point_record_length}, scale(scale), offset(offset){};
+        : point_format_id_(point_format_id), point_record_length_{point_record_length}, scale_(scale),
+          offset_(offset){};
 
     // Constructor for python pickling
     // TODO: Add a CMAKE flag to only compute python-specific code when python is compiled
     LasHeader(int8_t point_format_id, uint16_t point_record_length, uint32_t point_offset, uint64_t point_count,
               uint32_t vlr_count, const Vector3 &scale, const Vector3 &offset, uint64_t evlr_offset,
               uint32_t evlr_count)
         : point_format_id_(point_format_id), point_record_length_(point_record_length), point_offset_(point_offset),
-          point_count_(point_count), vlr_count_(vlr_count), scale(scale), offset(offset), evlr_offset_(evlr_offset),
+          point_count_(point_count), vlr_count_(vlr_count), scale_(scale), offset_(offset), evlr_offset_(evlr_offset),
           evlr_count_(evlr_count){};
 
     static LasHeader FromLazPerf(const lazperf::header14 &header);
@@ -45,8 +46,8 @@ class LasHeader
 
     uint8_t PointFormatId() const { return point_format_id_; }
     uint16_t PointRecordLength() const { return point_record_length_; }
-    Vector3 Scale() const { return scale; }
-    Vector3 Offset() const { return offset; }
+    Vector3 Scale() const { return scale_; }
+    Vector3 Offset() const { return offset_; }
     uint64_t PointCount() const { return point_count_; }
     uint32_t PointOffset() const { return point_offset_; }
     uint32_t VlrCount() const { return vlr_count_; }
@@ -84,14 +85,14 @@ class LasHeader
     Box Bounds() const;
 
     // Apply Las scale factors to Vector3 or double
-    Vector3 ApplyScale(const Vector3 &unscaled_value) const { return unscaled_value * scale + offset; }
-    Vector3 ApplyInverseScale(const Vector3 &scaled_value) const { return scaled_value / scale - offset; }
-    double ApplyScaleX(double unscaled_value) const { return unscaled_value * scale.x + offset.x; }
-    double ApplyScaleY(double unscaled_value) const { return unscaled_value * scale.y + offset.y; }
-    double ApplyScaleZ(double unscaled_value) const { return unscaled_value * scale.z + offset.z; }
-    double ApplyInverseScaleX(double scaled_value) const { return (scaled_value - offset.x) / scale.x; }
-    double ApplyInverseScaleY(double scaled_value) const { return (scaled_value - offset.y) / scale.y; }
-    double ApplyInverseScaleZ(double scaled_value) const { return (scaled_value - offset.z) / scale.z; }
+    Vector3 ApplyScale(const Vector3 &unscaled_value) const { return unscaled_value * scale_ + offset_; }
+    Vector3 ApplyInverseScale(const Vector3 &scaled_value) const { return scaled_value / scale_ - offset_; }
+    double ApplyScaleX(double unscaled_value) const { return unscaled_value * scale_.x + offset_.x; }
+    double ApplyScaleY(double unscaled_value) const { return unscaled_value * scale_.y + offset_.y; }
+    double ApplyScaleZ(double unscaled_value) const { return unscaled_value * scale_.z + offset_.z; }
+    double ApplyInverseScaleX(double scaled_value) const { return (scaled_value - offset_.x) / scale_.x; }
+    double ApplyInverseScaleY(double scaled_value) const { return (scaled_value - offset_.y) / scale_.y; }
+    double ApplyInverseScaleZ(double scaled_value) const { return (scaled_value - offset_.z) / scale_.z; }
 
     uint16_t file_source_id{};
     uint16_t global_encoding{};
@@ -103,14 +104,14 @@ class LasHeader
     Vector3 max{};
     Vector3 min{};
 
-    // xyz scale/offset
-    Vector3 scale{Vector3::DefaultScale()};
-    Vector3 offset{Vector3::DefaultOffset()};
-
     // # of points per return 0-14
     std::array<uint64_t, 15> points_by_return{};
 
   protected:
+    // xyz scale/offset
+    Vector3 scale_{Vector3::DefaultScale()};
+    Vector3 offset_{Vector3::DefaultOffset()};
+
     int8_t point_format_id_{6};
     uint16_t point_record_length_{};
 

cpp/src/geometry/box.cpp

@@ -92,6 +92,7 @@ bool Box::Within(const Box &other) const { return other.Contains(*this); }
 std::string Box::ToString() const
 {
     std::stringstream ss;
+    ss.precision(std::numeric_limits<double>::max_digits10);
     ss << "Box: x_min=" << x_min << " y_min=" << y_min << " z_min=" << z_min << " x_max=" << x_max << " y_max=" << y_max
        << " z_max=" << z_max;
     return ss.str();

cpp/src/hierarchy/key.cpp

@@ -7,7 +7,7 @@ namespace copc
 std::string VoxelKey::ToString() const
 {
     std::stringstream ss;
-    ss << d << "-" << x << "-" << y << "-" << z;
+    ss << "(" << d << ", " << x << ", " << y << ", " << z << ")";
     return ss.str();
 }
 

cpp/src/io/reader.cpp

@@ -1,4 +1,5 @@
 #include <cmath>
+#include <iomanip>
 #include <iostream>
 #include <stdexcept>
 
@@ -389,21 +390,38 @@ std::vector<Node> Reader::GetNodesWithinResolution(double resolution)
 
 bool Reader::ValidateSpatialBounds(bool verbose)
 {
-
     bool is_valid = true;
     auto header = config_.LasHeader();
 
+    int total_points_outside_header_bounds{0};
+    int total_points_outside_node_bounds{0};
+
+    // If verbose, set precision and print the las header.
+    if (verbose)
+    {
+        std::cout << std::setprecision(std::numeric_limits<double>::max_digits10);
+        std::cout << "Validating Spatial Bounds" << std::endl;
+        std::cout << "File info:" << std::endl;
+        std::cout << "\tPoint Count: " << header.PointCount() << std::endl;
+        std::cout << "\tScale: " << header.Scale().ToString() << std::endl;
+        std::cout << "\tOffset: " << header.Offset().ToString() << std::endl;
+        std::cout << "\tMin Bounds: " << header.min.ToString() << std::endl;
+        std::cout << "\tMax Bounds: " << header.max.ToString() << std::endl;
+        std::cout << std::endl << "Validating bounds..." << std::endl << std::endl;
+    }
+
     for (const auto &node : GetAllNodes())
     {
 
         // Check if node intersects las header bounds
         if (!Box(node.key, header).Intersects(header.Bounds()))
         {
             is_valid = false;
-            if (verbose)
-                std::cout << "Node " << node.key.ToString() << " is outside of las header bounds." << std::endl;
-            else
+            if (!verbose)
                 return false;
+            std::cout << "Node " << node.key.ToString() << " is outside of las header bounds ("
+                      << header.Bounds().ToString() << ")." << std::endl;
+            total_points_outside_header_bounds += node.point_count;
         }
         else
         {
@@ -416,30 +434,43 @@ bool Reader::ValidateSpatialBounds(bool verbose)
                     if (!point->Within(header.Bounds()))
                     {
                         is_valid = false;
-                        if (verbose)
-                            std::cout << "Point (" << point->X() << "," << point->Y() << "," << point->Z()
-                                      << ") from node " << node.key.ToString() << " is outside of las header bounds."
-                                      << std::endl;
-                        else
+                        if (!verbose)
                             return false;
+                        std::cout << "Point (" << point->X() << "," << point->Y() << "," << point->Z() << ") from node "
+                                  << node.key.ToString() << " is outside of las header bounds ("
+                                  << header.Bounds().ToString() << ")." << std::endl;
+                        total_points_outside_header_bounds++;
                     }
                 }
             }
             // Check that points fall within the node bounds
+            auto box = Box(node.key, header);
             for (auto const &point : points)
             {
-                if (!point->Within(Box(node.key, header)))
+                if (!point->Within(box))
                 {
                     is_valid = false;
-                    if (verbose)
-                        std::cout << "Point (" << point->X() << "," << point->Y() << "," << point->Z()
-                                  << ") is outside of node " << node.key.ToString() << " bounds." << std::endl;
-                    else
+                    if (!verbose)
                         return false;
+                    std::cout << "Point (" << point->X() << "," << point->Y() << "," << point->Z()
+                              << ") is outside of node " << node.key.ToString() << " bounds (" << box.ToString() << ")."
+                              << std::endl;
+                    total_points_outside_node_bounds++;
                 }
             }
         }
     }
+
+    if (verbose)
+    {
+        std::cout << std::endl;
+        std::cout << "...Bounds validation done." << std::endl << std::endl;
+        std::cout << "Number of points outside header bounds: " << total_points_outside_header_bounds << std::endl;
+        std::cout << "Number of points outside node bounds: " << total_points_outside_node_bounds << std::endl;
+        std::cout << std::endl;
+        is_valid ? std::cout << "Spatial bounds are valid!" : std::cout << "Spatial bounds are invalid!";
+        std::cout << std::endl;
+    }
     return is_valid;
 }
 

cpp/src/io/writer_public.cpp

@@ -8,9 +8,45 @@
 namespace copc
 {
 
-void Writer::InitWriter(std::ostream &out_stream, const CopcConfigWriter &copc_file_writer)
+void Writer::InitWriter(std::ostream &out_stream, const CopcConfigWriter &copc_config_writer,
+                        const std::optional<Vector3> &scale, const std::optional<Vector3> &offset,
+                        const std::optional<std::string> &wkt, const std::optional<las::EbVlr> &extra_bytes_vlr,
+                        const std::optional<bool> &has_extended_stats)
 {
-    this->config_ = std::make_shared<CopcConfigWriter>(copc_file_writer);
+
+    if (scale || offset || wkt || extra_bytes_vlr || has_extended_stats)
+    {
+        // If we need to update either parameter we need to create a new ConfigFileWriter
+        auto new_scale = copc_config_writer.LasHeader().Scale();
+        if (scale)
+            new_scale = *scale;
+
+        auto new_offset = copc_config_writer.LasHeader().Offset();
+        if (offset)
+            new_offset = *offset;
+
+        auto new_wkt = copc_config_writer.Wkt();
+        if (wkt)
+            new_wkt = *wkt;
+
+        auto new_extra_bytes_vlr = copc_config_writer.ExtraBytesVlr();
+        if (extra_bytes_vlr)
+            new_extra_bytes_vlr = *extra_bytes_vlr;
+
+        auto new_has_extended_stats = copc_config_writer.CopcExtents().HasExtendedStats();
+        if (has_extended_stats)
+            new_has_extended_stats = *has_extended_stats;
+
+        CopcConfigWriter cfg(copc_config_writer.LasHeader().PointFormatId(), new_scale, new_offset, new_wkt,
+                             new_extra_bytes_vlr, new_has_extended_stats);
+        this->config_ = std::make_shared<CopcConfigWriter>(cfg);
+    }
+    else
+    {
+        // If not we use the copc_config_writer provided
+        this->config_ = std::make_shared<CopcConfigWriter>(copc_config_writer);
+    }
+
     this->hierarchy_ = std::make_shared<Internal::Hierarchy>();
     this->writer_ = std::make_unique<Internal::WriterInternal>(out_stream, this->config_, this->hierarchy_);
 }

cpp/src/las/header.cpp

@@ -38,12 +38,12 @@ LasHeader LasHeader::FromLazPerf(const lazperf::header14 &header)
     h.point_format_id_ = static_cast<int8_t>(header.point_format_id);
     h.point_record_length_ = header.point_record_length;
     std::copy(std::begin(header.points_by_return), std::end(header.points_by_return), std::begin(h.points_by_return));
-    h.scale.x = header.scale.x;
-    h.scale.y = header.scale.y;
-    h.scale.z = header.scale.z;
-    h.offset.x = header.offset.x;
-    h.offset.y = header.offset.y;
-    h.offset.z = header.offset.z;
+    h.scale_.x = header.scale.x;
+    h.scale_.y = header.scale.y;
+    h.scale_.z = header.scale.z;
+    h.offset_.x = header.offset.x;
+    h.offset_.y = header.offset.y;
+    h.offset_.z = header.offset.z;
     h.max.x = header.maxx;
     h.min.x = header.minx;
     h.max.y = header.maxy;
@@ -91,13 +91,13 @@ lazperf::header14 LasHeader::ToLazPerf(uint32_t point_offset, uint64_t point_cou
         h.point_count = (uint32_t)point_count;
     std::fill(h.points_by_return, h.points_by_return + 5, 0); // Fill with zeros
 
-    h.offset.x = offset.x;
-    h.offset.y = offset.y;
-    h.offset.z = offset.z;
+    h.offset.x = offset_.x;
+    h.offset.y = offset_.y;
+    h.offset.z = offset_.z;
 
-    h.scale.x = scale.x;
-    h.scale.y = scale.y;
-    h.scale.z = scale.z;
+    h.scale.x = scale_.x;
+    h.scale.y = scale_.y;
+    h.scale.z = scale_.z;
 
     h.maxx = max.x;
     h.minx = min.x;
@@ -131,8 +131,8 @@ std::string LasHeader::ToString() const
     ss << "\tPoint Format ID: " << static_cast<int>(point_format_id_) << std::endl;
     ss << "\tPoint Record Length: " << point_record_length_ << std::endl;
     ss << "\tPoint Count: " << point_count_ << std::endl;
-    ss << "\tScale: " << scale.ToString() << std::endl;
-    ss << "\tOffset: " << offset.ToString() << std::endl;
+    ss << "\tScale: " << scale_.ToString() << std::endl;
+    ss << "\tOffset: " << offset_.ToString() << std::endl;
     ss << "\tMax: " << max.ToString() << std::endl;
     ss << "\tMin: " << min.ToString() << std::endl;
     ss << "\tEVLR Offset: " << evlr_offset_ << std::endl;

example/example-writer.py

@@ -234,6 +234,8 @@ def NewFileExample():
 
     # Now, we can create our COPC writer:
     writer = copc.FileWriter("new-copc.copc.laz", cfg)
+    # writer = copc.FileWriter("new-copc.copc.laz", cfg,None,None,None,None,None)
+    # writer = copc.FileWriter("new-copc.copc.laz", cfg,(1,1,1),(1,1,1),"test",)
     header = writer.copc_config.las_header
 
     # Set the COPC Extents