diff --git a/include/pdal/DbWriter.hpp b/include/pdal/DbWriter.hpp index 59953cda60..7c445adfb4 100644 --- a/include/pdal/DbWriter.hpp +++ b/include/pdal/DbWriter.hpp @@ -52,7 +52,7 @@ class PDAL_DLL DbWriter : public Writer DbWriter() {} - virtual void setAutoOffset(const PointViewPtr view); + virtual void setAutoXForm(const PointViewPtr view); XMLDimList dbDimTypes() const { return m_dbDims; } size_t readField(const PointView& view, char *pos, Dimension::Id::Enum id, diff --git a/include/pdal/PointView.hpp b/include/pdal/PointView.hpp index 6cad1b5533..b57ae66d60 100644 --- a/include/pdal/PointView.hpp +++ b/include/pdal/PointView.hpp @@ -413,24 +413,17 @@ inline T PointView::getFieldAs(Dimension::Id::Enum dim, val = 0; break; } -#ifdef PDAL_COMPILER_MSVC -// warning C4127: conditional expression is constant -#pragma warning(push) -#pragma warning(disable:4127) -#endif - try + + if (Utils::inRange(val)) { - if (std::is_same::value) - retval = val; - else + if (std::is_integral::value) { - if (std::is_integral::value == true ) - retval = boost::numeric_cast(lround(val)); - else - retval = boost::numeric_cast(val); + val = Utils::sround(val); } + + retval = static_cast(val); } - catch (boost::numeric::bad_numeric_cast& ) + else { std::ostringstream oss; oss << "Unable to fetch data and convert as requested: "; @@ -439,11 +432,8 @@ inline T PointView::getFieldAs(Dimension::Id::Enum dim, "(" << (double)val << ") -> " << Utils::typeidName(); throw pdal_error(oss.str()); } + return retval; -#ifdef PDAL_COMPILER_MSVC -// warning C4127: conditional expression is constant -#pragma warning(pop) -#endif } @@ -451,58 +441,21 @@ inline T PointView::getFieldAs(Dimension::Id::Enum dim, template bool PointView::convertAndSet(Dimension::Id::Enum dim, PointId idx, T_IN in) { -// This mess, instead of just using boost::numeric_cast, is here to: -// 1) Prevent the throwing of exceptions. The entrance/exit of the try -// block seemed somewhat expensive. -// 2) Round to nearest instead of truncation without rounding before -// invoking the converter. -// - using namespace boost; - static bool ok; - - struct RangeHandler + bool success(false); + + if (Utils::inRange(in)) { - void operator() (numeric::range_check_result r) + if (std::is_integral::value) { - ok = (r == numeric::cInRange); + in = Utils::sround(in); } - }; - - T_OUT out; - - typedef numeric::conversion_traits conv_traits; - typedef numeric::numeric_cast_traits cast_traits; - typedef numeric::converter< - T_OUT, - T_IN, - conv_traits, - RangeHandler, - numeric::RoundEven, - numeric::raw_converter, - typename cast_traits::range_checking_policy> - localConverter; -#ifdef PDAL_COMPILER_MSVC -// warning C4127: conditional expression is constant -#pragma warning(push) -#pragma warning(disable:4127) -#endif - ok = true; - // This is an optimization. - if (std::is_same::value == true) - out = in; - else - out = localConverter::convert(in); - if (!ok) - return false; - -#ifdef PDAL_COMPILER_MSVC -// warning C4127: conditional expression is constant -#pragma warning(pop) -#endif + const T_OUT out(static_cast(in)); + setFieldInternal(dim, idx, &out); + success = true; + } - setFieldInternal(dim, idx, (void *)&out); - return true; + return success; } diff --git a/include/pdal/Utils.hpp b/include/pdal/Utils.hpp index 7d46c2f98e..2626fa8ffb 100644 --- a/include/pdal/Utils.hpp +++ b/include/pdal/Utils.hpp @@ -353,6 +353,30 @@ namespace Utils out.rdbuf(redir.m_buf); redir.m_out->close(); } + + // Determine whether a value of a given input type may be safely + // statically casted to the given output type without over/underflow. If + // the output type is integral, inRange() will determine whether the + // rounded input value, rather than truncated, may be safely converted. + template + bool inRange(double in) + { + if (std::is_integral::value) + { + in = sround(in); + } + + return std::is_same::value || + (in >= static_cast(std::numeric_limits::lowest()) && + in <= static_cast(std::numeric_limits::max())); + } + + template + bool inRange(T_IN in) + { + return std::is_same::value || + inRange(static_cast(in)); + } }; } // namespace pdal diff --git a/include/pdal/Writer.hpp b/include/pdal/Writer.hpp index 2ecf5ef44f..2911f143d7 100644 --- a/include/pdal/Writer.hpp +++ b/include/pdal/Writer.hpp @@ -69,7 +69,7 @@ class PDAL_DLL Writer : public Stage XForm m_zXform; StringList m_outputDims; - virtual void setAutoOffset(const PointViewPtr view); + virtual void setAutoXForm(const PointViewPtr view); private: virtual PointViewSet run(PointViewPtr view) diff --git a/io/bpf/BpfWriter.cpp b/io/bpf/BpfWriter.cpp index 8ac03e5937..3761a19195 100644 --- a/io/bpf/BpfWriter.cpp +++ b/io/bpf/BpfWriter.cpp @@ -144,7 +144,7 @@ void BpfWriter::loadBpfDimensions(PointLayoutPtr layout) void BpfWriter::write(const PointViewPtr dataShared) { - setAutoOffset(dataShared); + setAutoXForm(dataShared); // Avoid reference count overhead internally. const PointView* data(dataShared.get()); diff --git a/io/las/LasWriter.cpp b/io/las/LasWriter.cpp index f111f28a9d..380f0f2be9 100644 --- a/io/las/LasWriter.cpp +++ b/io/las/LasWriter.cpp @@ -506,7 +506,7 @@ void LasWriter::openCompression() void LasWriter::write(const PointViewPtr view) { - setAutoOffset(view); + setAutoXForm(view); size_t pointLen = m_lasHeader.pointLen(); @@ -708,9 +708,12 @@ void LasWriter::done(PointTableRef table) out << evlr; } - // Reset the offset since it may have been auto-computed + // Reset the offset/scale since it may have been auto-computed m_lasHeader.setOffset(m_xXform.m_offset, m_yXform.m_offset, m_zXform.m_offset); + m_lasHeader.setScale(m_xXform.m_scale, m_yXform.m_scale, + m_zXform.m_scale); + // We didn't know the point count until we go through the points. m_lasHeader.setPointCount(m_numPointsWritten); // The summary is calculated as points are written. diff --git a/plugins/nitf/io/MetadataReader.hpp b/plugins/nitf/io/MetadataReader.hpp index 6eb63d0630..8ee56920b4 100644 --- a/plugins/nitf/io/MetadataReader.hpp +++ b/plugins/nitf/io/MetadataReader.hpp @@ -45,7 +45,7 @@ # pragma GCC diagnostic ignored "-Wcast-qual" // The following pragma doesn't actually work: // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61653 -# pragma GCC diagnostic ignored "-Wliteral-suffix" + //# pragma GCC diagnostic ignored "-Wliteral-suffix" #endif #ifdef PDAL_COMPILER_CLANG # pragma clang diagnostic push diff --git a/plugins/nitf/io/NitfFile.hpp b/plugins/nitf/io/NitfFile.hpp index 6d04438306..847cd05410 100644 --- a/plugins/nitf/io/NitfFile.hpp +++ b/plugins/nitf/io/NitfFile.hpp @@ -45,7 +45,7 @@ # pragma GCC diagnostic ignored "-Wcast-qual" // The following pragma doesn't actually work: // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61653 -# pragma GCC diagnostic ignored "-Wliteral-suffix" + //# pragma GCC diagnostic ignored "-Wliteral-suffix" #endif #ifdef PDAL_COMPILER_CLANG # pragma clang diagnostic push diff --git a/plugins/nitf/io/NitfWriter.cpp b/plugins/nitf/io/NitfWriter.cpp index 2020f07713..067e2612c4 100644 --- a/plugins/nitf/io/NitfWriter.cpp +++ b/plugins/nitf/io/NitfWriter.cpp @@ -49,7 +49,7 @@ # pragma GCC diagnostic ignored "-Wcast-qual" // The following pragma doesn't actually work: // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61653 -# pragma GCC diagnostic ignored "-Wliteral-suffix" + //# pragma GCC diagnostic ignored "-Wliteral-suffix" #endif #include #include diff --git a/plugins/nitf/io/tre_plugins.cpp b/plugins/nitf/io/tre_plugins.cpp index fe05c8a374..178f4f1d5f 100644 --- a/plugins/nitf/io/tre_plugins.cpp +++ b/plugins/nitf/io/tre_plugins.cpp @@ -41,7 +41,7 @@ # pragma GCC diagnostic ignored "-Wcast-qual" // The following pragma doesn't actually work: // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61653 -# pragma GCC diagnostic ignored "-Wliteral-suffix" + //# pragma GCC diagnostic ignored "-Wliteral-suffix" #endif #ifdef PDAL_COMPILER_CLANG # pragma clang diagnostic push diff --git a/plugins/oci/io/OciWriter.cpp b/plugins/oci/io/OciWriter.cpp index 99b429f2c4..057ecd1e41 100644 --- a/plugins/oci/io/OciWriter.cpp +++ b/plugins/oci/io/OciWriter.cpp @@ -590,7 +590,7 @@ void OciWriter::createPCEntry() OCILobLocator *schema_locator; OCILobLocator *boundary_locator; - statement->WriteCLob(&schema_locator, (char *)schemaData.c_str()); + statement->WriteCLob(&schema_locator, const_cast(schemaData.c_str())); statement->BindClob(&schema_locator); std::ostringstream wkt_s; @@ -623,7 +623,8 @@ void OciWriter::createPCEntry() if (!m_baseTableBoundaryColumn.empty()) { - statement->WriteCLob(&boundary_locator, (char *)wkt_s.str().c_str()); + statement->WriteCLob(&boundary_locator, + const_cast(wkt_s.str().c_str())); statement->BindClob(&boundary_locator); statement->Bind((int*)&m_srid); } @@ -723,7 +724,7 @@ void OciWriter::write(const PointViewPtr view) // While we'd like a separate offset for each tile, the schema is stored // for the entire point cloud. if (m_lastBlockId == 0) - setAutoOffset(view); + setAutoXForm(view); writeInit(); writeTile(view); } diff --git a/plugins/oci/test/OCITest.cpp b/plugins/oci/test/OCITest.cpp index 4345967059..e0ca4b570d 100644 --- a/plugins/oci/test/OCITest.cpp +++ b/plugins/oci/test/OCITest.cpp @@ -55,7 +55,7 @@ GTEST_API_ int main(int argc, char **argv) if (Utils::startsWith(s, "--connection")) { auto pos = s.find_first_of('='); - if (pos == string::npos) + if (pos == std::string::npos) throw pdal_error("Invalid command line connection string."); TestConfig::g_oracle_connection = s.substr(pos + 1); break; diff --git a/src/DbWriter.cpp b/src/DbWriter.cpp index 0b3129f5f0..ea9c5da487 100644 --- a/src/DbWriter.cpp +++ b/src/DbWriter.cpp @@ -136,11 +136,11 @@ void DbWriter::ready(PointTableRef /*table*/) /// Make sure that computed offsets are stored in the schema. -void DbWriter::setAutoOffset(const PointViewPtr view) +void DbWriter::setAutoXForm(const PointViewPtr view) { using namespace Dimension; - Writer::setAutoOffset(view); + Writer::setAutoXForm(view); for (auto& xmlDim : m_dbDims) { if (xmlDim.m_dimType.m_id == Id::X) diff --git a/src/Writer.cpp b/src/Writer.cpp index 36a30d9db1..153d58d254 100644 --- a/src/Writer.cpp +++ b/src/Writer.cpp @@ -58,51 +58,93 @@ void Writer::writerProcessOptions(const Options& options) xform.m_offset = boost::lexical_cast(offset); }; + auto setScale = [options](XForm& xform, const std::string& opName) + { + if (!options.hasOption(opName)) + return; + std::string scale = options.getValueOrThrow(opName); + if (scale == "auto") + xform.m_autoScale = true; + else + xform.m_scale = boost::lexical_cast(scale); + }; + setOffset(m_xXform, "offset_x"); setOffset(m_yXform, "offset_y"); setOffset(m_zXform, "offset_z"); + setScale(m_xXform, "scale_x"); + setScale(m_yXform, "scale_y"); + setScale(m_zXform, "scale_z"); + if (options.hasOption("filename")) m_filename = options.getValueOrThrow("filename"); - if (options.hasOption("scale_x")) - m_xXform.m_scale = options.getValueOrThrow("scale_x"); - if (options.hasOption("scale_y")) - m_yXform.m_scale = options.getValueOrThrow("scale_y"); - if (options.hasOption("scale_z")) - m_zXform.m_scale = options.getValueOrThrow("scale_z"); m_outputDims = options.getValueOrDefault("output_dims"); } -void Writer::setAutoOffset(const PointViewPtr view) +void Writer::setAutoXForm(const PointViewPtr view) { - if (!m_xXform.m_autoOffset && !m_yXform.m_autoOffset && - !m_zXform.m_autoOffset) + double xmin = (std::numeric_limits::max)(); + double xmax = (std::numeric_limits::lowest)(); + bool xmod = m_xXform.m_autoOffset || m_xXform.m_autoScale; + + double ymin = (std::numeric_limits::max)(); + double ymax = (std::numeric_limits::lowest)(); + bool ymod = m_yXform.m_autoOffset || m_yXform.m_autoScale; + + double zmin = (std::numeric_limits::max)(); + double zmax = (std::numeric_limits::lowest)(); + bool zmod = m_zXform.m_autoOffset || m_zXform.m_autoScale; + + if (!xmod && !ymod && !zmod) return; if (view->empty()) return; + for (PointId idx = 0; idx < view->size(); idx++) + { + if (xmod) + { + double x = view->getFieldAs(Dimension::Id::X, idx); + xmin = std::min(x, xmin); + xmax = std::max(x, xmax); + } + if (ymod) + { + double y = view->getFieldAs(Dimension::Id::Y, idx); + ymin = std::min(y, ymin); + ymax = std::max(y, ymax); + } + if (zmod) + { + double z = view->getFieldAs(Dimension::Id::Z, idx); + zmin = std::min(z, zmin); + zmax = std::max(z, zmax); + } + } + if (m_xXform.m_autoOffset) - m_xXform.m_offset = (std::numeric_limits::max)(); + { + m_xXform.m_offset = xmin; + xmax -= xmin; + } if (m_yXform.m_autoOffset) - m_yXform.m_offset = (std::numeric_limits::max)(); + { + m_yXform.m_offset = ymin; + ymax -= ymin; + } if (m_zXform.m_autoOffset) - m_zXform.m_offset = (std::numeric_limits::max)(); - for (PointId idx = 0; idx < view->size(); idx++) { - if (m_xXform.m_autoOffset) - m_xXform.m_offset = - std::min(view->getFieldAs(Dimension::Id::X, idx), - m_xXform.m_offset); - if (m_yXform.m_autoOffset) - m_yXform.m_offset = - std::min(view->getFieldAs(Dimension::Id::Y, idx), - m_yXform.m_offset); - if (m_zXform.m_autoOffset) - m_zXform.m_offset = - std::min(view->getFieldAs(Dimension::Id::Z, idx), - m_zXform.m_offset); + m_zXform.m_offset = zmin; + zmax -= zmin; } + if (m_xXform.m_autoScale) + m_xXform.m_scale = xmax / (std::numeric_limits::max)(); + if (m_yXform.m_autoScale) + m_yXform.m_scale = ymax / (std::numeric_limits::max)(); + if (m_zXform.m_autoScale) + m_zXform.m_scale = zmax / (std::numeric_limits::max)(); } diff --git a/test/unit/filters/FerryFilterTest.cpp b/test/unit/filters/FerryFilterTest.cpp index 66c819f2ef..57dba867ce 100644 --- a/test/unit/filters/FerryFilterTest.cpp +++ b/test/unit/filters/FerryFilterTest.cpp @@ -58,7 +58,7 @@ TEST(FerryFilterTest, test_ferry_copy) specReader.readPipeline(Support::configuredpath("filters/ferry.xml")); mgr.execute(); - const PointTableRef table(mgr.pointTable()); + ConstPointTableRef table(mgr.pointTable()); PointViewSet viewSet = mgr.views(); diff --git a/test/unit/io/las/LasWriterTest.cpp b/test/unit/io/las/LasWriterTest.cpp index ceba01e6f4..b307e9436c 100644 --- a/test/unit/io/las/LasWriterTest.cpp +++ b/test/unit/io/las/LasWriterTest.cpp @@ -81,6 +81,7 @@ TEST(LasWriterTest, auto_offset) Options writerOps; writerOps.add("filename", FILENAME); writerOps.add("offset_x", "auto"); + writerOps.add("scale_x", "auto"); LasWriter writer; writer.setOptions(writerOps); @@ -105,9 +106,9 @@ TEST(LasWriterTest, auto_offset) EXPECT_EQ(viewSet.size(), 1u); view = *viewSet.begin(); EXPECT_EQ(view->size(), 3u); - EXPECT_DOUBLE_EQ(125000.00, view->getFieldAs(Id::X, 0)); - EXPECT_DOUBLE_EQ(74529.00, view->getFieldAs(Id::X, 1)); - EXPECT_DOUBLE_EQ(523523.02, view->getFieldAs(Id::X, 2)); + EXPECT_NEAR(125000.00, view->getFieldAs(Id::X, 0), .0001); + EXPECT_NEAR(74529.00, view->getFieldAs(Id::X, 1), .0001); + EXPECT_NEAR(523523.02, view->getFieldAs(Id::X, 2), .0001); FileUtils::deleteFile(FILENAME); } diff --git a/test/unit/io/optech/OptechReaderTest.cpp b/test/unit/io/optech/OptechReaderTest.cpp index 8b8346aa85..2aeacd908f 100644 --- a/test/unit/io/optech/OptechReaderTest.cpp +++ b/test/unit/io/optech/OptechReaderTest.cpp @@ -129,10 +129,10 @@ TEST_F(OptechReaderTest, ReadingPoints) EXPECT_EQ(1, view->getFieldAs(Dimension::Id::ReturnNumber, 0)); EXPECT_EQ(1, view->getFieldAs(Dimension::Id::NumberOfReturns, 0)); EXPECT_FLOAT_EQ(8.27356689453125e2, - view->getFieldAs(Dimension::Id::EchoRange, 0)); + view->getFieldAs(Dimension::Id::EchoRange, 0)); EXPECT_EQ(384, view->getFieldAs(Dimension::Id::Intensity, 0)); - EXPECT_DOUBLE_EQ(-14.555161476135254, - view->getFieldAs(Dimension::Id::ScanAngleRank, 0)); + EXPECT_FLOAT_EQ(-14.55516, + view->getFieldAs(Dimension::Id::ScanAngleRank, 0)); }