From 715f58c123920b6169fd107f08965ef5210b6cae Mon Sep 17 00:00:00 2001 From: Andrew Date: Wed, 29 Apr 2015 09:34:50 -0600 Subject: [PATCH 1/3] Fixes for g++/Ubuntu compilation. --- include/pdal/PointView.hpp | 10 ++++++---- plugins/nitf/io/MetadataReader.hpp | 2 +- plugins/nitf/io/NitfFile.hpp | 2 +- plugins/nitf/io/NitfWriter.cpp | 2 +- plugins/nitf/io/tre_plugins.cpp | 2 +- plugins/oci/io/OciWriter.cpp | 5 +++-- plugins/oci/test/OCITest.cpp | 2 +- src/PointView.cpp | 2 ++ test/unit/io/optech/OptechReaderTest.cpp | 6 +++--- 9 files changed, 19 insertions(+), 14 deletions(-) diff --git a/include/pdal/PointView.hpp b/include/pdal/PointView.hpp index 6cad1b5533..cc3bf42886 100644 --- a/include/pdal/PointView.hpp +++ b/include/pdal/PointView.hpp @@ -265,6 +265,9 @@ class PDAL_DLL PointView inline PointId getTemp(PointId id); void freeTemp(PointId id) { m_temps.push(id); } + + // Awfulness to avoid exceptions in numeric cast. + static bool m_ok; }; struct PointViewLess @@ -458,13 +461,12 @@ bool PointView::convertAndSet(Dimension::Id::Enum dim, PointId idx, T_IN in) // invoking the converter. // using namespace boost; - static bool ok; struct RangeHandler { void operator() (numeric::range_check_result r) { - ok = (r == numeric::cInRange); + m_ok = (r == numeric::cInRange); } }; @@ -487,13 +489,13 @@ bool PointView::convertAndSet(Dimension::Id::Enum dim, PointId idx, T_IN in) #pragma warning(push) #pragma warning(disable:4127) #endif - ok = true; + m_ok = true; // This is an optimization. if (std::is_same::value == true) out = in; else out = localConverter::convert(in); - if (!ok) + if (!m_ok) return false; #ifdef PDAL_COMPILER_MSVC 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..90dacfeaf9 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); } 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/PointView.cpp b/src/PointView.cpp index 061f7dfb04..c3d69bb019 100644 --- a/src/PointView.cpp +++ b/src/PointView.cpp @@ -42,6 +42,8 @@ namespace pdal { +bool PointView::m_ok; + PointViewIter PointView::begin() { return PointViewIter(this, 0); 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)); } From 4f8f9cb203380945ea401a9fceff90ad60ef24e5 Mon Sep 17 00:00:00 2001 From: Connor Manning Date: Thu, 30 Apr 2015 15:51:45 -0500 Subject: [PATCH 2/3] Remove boost::numeric_cast. --- include/pdal/PointView.hpp | 85 ++++++++------------------------------ include/pdal/Utils.hpp | 24 +++++++++++ src/PointView.cpp | 2 - 3 files changed, 42 insertions(+), 69 deletions(-) diff --git a/include/pdal/PointView.hpp b/include/pdal/PointView.hpp index cc3bf42886..b57ae66d60 100644 --- a/include/pdal/PointView.hpp +++ b/include/pdal/PointView.hpp @@ -265,9 +265,6 @@ class PDAL_DLL PointView inline PointId getTemp(PointId id); void freeTemp(PointId id) { m_temps.push(id); } - - // Awfulness to avoid exceptions in numeric cast. - static bool m_ok; }; struct PointViewLess @@ -416,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: "; @@ -442,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 } @@ -454,57 +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; - - struct RangeHandler + bool success(false); + + if (Utils::inRange(in)) { - void operator() (numeric::range_check_result r) + if (std::is_integral::value) { - m_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 - m_ok = true; - // This is an optimization. - if (std::is_same::value == true) - out = in; - else - out = localConverter::convert(in); - if (!m_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/src/PointView.cpp b/src/PointView.cpp index c3d69bb019..061f7dfb04 100644 --- a/src/PointView.cpp +++ b/src/PointView.cpp @@ -42,8 +42,6 @@ namespace pdal { -bool PointView::m_ok; - PointViewIter PointView::begin() { return PointViewIter(this, 0); From a46515c5ca5b35f0cbd21a713cc82c9c10e43a51 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Fri, 1 May 2015 11:17:17 -0500 Subject: [PATCH 3/3] Add auto-scaling. Close #888 --- include/pdal/DbWriter.hpp | 2 +- include/pdal/Writer.hpp | 2 +- io/bpf/BpfWriter.cpp | 2 +- io/las/LasWriter.cpp | 7 +- plugins/oci/io/OciWriter.cpp | 2 +- src/DbWriter.cpp | 4 +- src/Writer.cpp | 92 +++++++++++++++++++-------- test/unit/filters/FerryFilterTest.cpp | 2 +- test/unit/io/las/LasWriterTest.cpp | 7 +- 9 files changed, 83 insertions(+), 37 deletions(-) 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/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 e09bd0156a..580152d727 100644 --- a/io/las/LasWriter.cpp +++ b/io/las/LasWriter.cpp @@ -504,7 +504,7 @@ void LasWriter::openCompression() void LasWriter::write(const PointViewPtr view) { - setAutoOffset(view); + setAutoXForm(view); size_t pointLen = m_lasHeader.pointLen(); @@ -706,9 +706,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/oci/io/OciWriter.cpp b/plugins/oci/io/OciWriter.cpp index 90dacfeaf9..057ecd1e41 100644 --- a/plugins/oci/io/OciWriter.cpp +++ b/plugins/oci/io/OciWriter.cpp @@ -724,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/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); }