From dac7cfed72f9c8d9f8a653af3da427eb59d9b9c9 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Mon, 5 Mar 2018 15:58:05 -0500 Subject: [PATCH] Issue 1824 (#1830) * Force initial cell I/J to be in bounds where not otherwise checked. * Remove unnecessary assignments. * Make sure width/height are in range. * Remove misplaced '='. * Add grid OOB test. * Add test file. --- io/GDALGrid.cpp | 47 ++++++++++++++--------- io/GDALGrid.hpp | 2 +- io/GDALWriter.cpp | 11 +++++- test/data/gdal/grid_bounds.txt | 40 ++++++++++++++++++++ test/unit/io/GDALWriterTest.cpp | 67 +++++++++++++++++++++++++-------- 5 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 test/data/gdal/grid_bounds.txt diff --git a/io/GDALGrid.cpp b/io/GDALGrid.cpp index 429fa45417..2b01dfa871 100644 --- a/io/GDALGrid.cpp +++ b/io/GDALGrid.cpp @@ -48,6 +48,15 @@ GDALGrid::GDALGrid(size_t width, size_t height, double edgeLength, m_width(width), m_height(height), m_windowSize(windowSize), m_edgeLength(edgeLength), m_radius(radius), m_outputTypes(outputTypes) { + if (width > std::numeric_limits::max() || + height > std::numeric_limits::max()) + { + std::ostringstream oss; + oss << "Grid width or height is too large. Width and height are " + "limited to " << std::numeric_limits::max() << " cells." + "Try setting bounds or increasing resolution."; + throw error(oss.str()); + } size_t size(width * height); m_count.reset(new DataVec(size)); @@ -199,9 +208,11 @@ void GDALGrid::addPoint(double x, double y, double z) // <- v + int i, j; + int iStart, jStart; // First quadrant; - int i = iOrigin + 1; - int j = jOrigin; + i = iStart = std::max(0, iOrigin + 1); + j = std::min(jOrigin, int(m_height - 1)); while (i < (int)m_width && j >= 0) { double d = distance(i, j, x, y); @@ -212,16 +223,16 @@ void GDALGrid::addPoint(double x, double y, double z) } else { - if (i == iOrigin + 1) + if (i == iStart) break; - i = iOrigin + 1; + i = iStart; j--; } } // Second quadrant; - i = iOrigin; - j = jOrigin - 1; + i = std::min(iOrigin, int(m_width - 1)); + j = jStart = std::min(jOrigin - 1, int(m_height - 1)); while (i >= 0 && j >= 0) { double d = distance(i, j, x, y); @@ -232,16 +243,16 @@ void GDALGrid::addPoint(double x, double y, double z) } else { - if (j == jOrigin - 1) + if (j == jStart) break; - j = jOrigin - 1; + j = jStart; i--; } } // Third quadrant; - i = iOrigin - 1; - j = jOrigin; + i = iStart = std::min(iOrigin - 1, int(m_width - 1)); + j = std::max(jOrigin, 0); while (i >= 0 && j < (int)m_height) { double d = distance(i, j, x, y); @@ -252,15 +263,15 @@ void GDALGrid::addPoint(double x, double y, double z) } else { - if (i == iOrigin - 1) + if (i == iStart) break; - i = iOrigin - 1; + i = iStart; j++; } } // Fourth quadrant; - i = iOrigin; - j = jOrigin + 1; + i = std::max(iOrigin, 0); + j = jStart = std::max(jOrigin + 1, 0); while (i < (int)m_width && j < (int)m_height) { double d = distance(i, j, x, y); @@ -271,9 +282,9 @@ void GDALGrid::addPoint(double x, double y, double z) } else { - if (j == jOrigin + 1) + if (j == jStart) break; - j = jOrigin + 1; + j = jStart; i++; } } @@ -283,11 +294,11 @@ void GDALGrid::addPoint(double x, double y, double z) double d = distance(iOrigin, jOrigin, x, y); if (d < m_radius && iOrigin >= 0 && jOrigin >= 0 && - iOrigin < (int)m_width && jOrigin <= (int)m_height) + iOrigin < (int)m_width && jOrigin < (int)m_height) update(iOrigin, jOrigin, z, d); } -void GDALGrid::update(int i, int j, double val, double dist) +void GDALGrid::update(size_t i, size_t j, double val, double dist) { // Once we determine that a point is close enough to a cell to count it, // this function does the actual math. We use the value of the diff --git a/io/GDALGrid.hpp b/io/GDALGrid.hpp index 99a4bc6bc6..583b49bcd2 100644 --- a/io/GDALGrid.hpp +++ b/io/GDALGrid.hpp @@ -139,7 +139,7 @@ class GDALGrid } // Update cell at i, j with value at a distance. - void update(int i, int j, double val, double dist); + void update(size_t i, size_t j, double val, double dist); // Fill cell at index \c i with the nondata value. void fillNodata(size_t i); diff --git a/io/GDALWriter.cpp b/io/GDALWriter.cpp index 65d9f86531..8709d2077f 100644 --- a/io/GDALWriter.cpp +++ b/io/GDALWriter.cpp @@ -143,8 +143,15 @@ void GDALWriter::createGrid(BOX2D bounds) m_curBounds = bounds; size_t width = ((m_curBounds.maxx - m_curBounds.minx) / m_edgeLength) + 1; size_t height = ((m_curBounds.maxy - m_curBounds.miny) / m_edgeLength) + 1; - m_grid.reset(new GDALGrid(width, height, m_edgeLength, m_radius, - m_outputTypes, m_windowSize)); + try + { + m_grid.reset(new GDALGrid(width, height, m_edgeLength, m_radius, + m_outputTypes, m_windowSize)); + } + catch (GDALGrid::error& err) + { + throwError(err.what()); + } } diff --git a/test/data/gdal/grid_bounds.txt b/test/data/gdal/grid_bounds.txt new file mode 100644 index 0000000000..69ceffe1ad --- /dev/null +++ b/test/data/gdal/grid_bounds.txt @@ -0,0 +1,40 @@ +X,Y,Z + +0, 0, 0 +.5, .5, 1 +1.5, .5, 2 +2.5, .5, 3 +3.5, .5, 4 +4.5, .5, 5 +3.5, 1, 4.4 +4.5, 1, 5.4 +.5, 1.5, 2 +1.5, 1.5, 3 +2.5, 1.5, 4 +3, 1.5, 4.4 +3.5, 1.5, 5 +4, 1.5, 5.4 +4.5, 1.5, 6 +3.5, 2, 5.4 +4.5, 2, 6.4 +.5, 2.5, 3 +1.5, 2.5, 4 +2.5, 2.5, 5 +3.5, 2.5, 6 +4.5, 2.5, 7 +.5, 3.5, 4 +2.5, 3.5, 6 +3.5, 3.5, 7 +4.5, 3.5, 8 +.5, 4.5, 5 +2.5, 4.5, 7 +3.5, 4.5, 8 +4.5, 4.6, 9.1 +4.7, 4.5, 8.9 +4.3, 4.5, 8.9 +-1000, 2, -45 +-1000, -10000, 4 +3.2, -1000, 7 +10000, 3, 4 +2, 10000, 45 +999, 9995, 45 diff --git a/test/unit/io/GDALWriterTest.cpp b/test/unit/io/GDALWriterTest.cpp index 579f85b963..47ba6ab78a 100644 --- a/test/unit/io/GDALWriterTest.cpp +++ b/test/unit/io/GDALWriterTest.cpp @@ -49,13 +49,13 @@ using namespace pdal; namespace { -void runGdalWriter(const Options& wo, const std::string& outfile, - const std::string& values) +void runGdalWriter(const Options& wo, const std::string& infile, + const std::string& outfile, const std::string& values) { FileUtils::deleteFile(outfile); Options ro; - ro.add("filename", Support::datapath("gdal/grid.txt")); + ro.add("filename", infile); TextReader r; r.setOptions(ro); @@ -162,6 +162,7 @@ void runGdalWriter2(const Options& wo, const std::string& outfile, TEST(GDALWriterTest, min) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -178,7 +179,7 @@ TEST(GDALWriterTest, min) "2.000 3.000 4.000 4.400 5.400 " "1.000 2.000 3.000 4.000 5.000 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, min2) @@ -213,6 +214,7 @@ TEST(GDALWriterTest, min2) TEST(GDALWriterTest, minWindow) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -230,11 +232,12 @@ TEST(GDALWriterTest, minWindow) "2.000 3.000 4.000 4.400 5.400 " "1.000 2.000 3.000 4.000 5.000 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, max) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -251,11 +254,12 @@ TEST(GDALWriterTest, max) "2.000 3.000 4.000 5.400 6.400 " "1.000 2.000 3.000 4.400 5.400 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, maxWindow) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -273,11 +277,12 @@ TEST(GDALWriterTest, maxWindow) "2.000 3.000 4.000 5.400 6.400 " "1.000 2.000 3.000 4.400 5.400 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, mean) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -294,11 +299,12 @@ TEST(GDALWriterTest, mean) "2.000 3.000 4.000 4.800 5.800 " "1.000 2.000 3.000 4.200 5.200 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, meanWindow) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -316,11 +322,12 @@ TEST(GDALWriterTest, meanWindow) "2.000 3.000 4.000 4.800 5.800 " "1.000 2.000 3.000 4.200 5.200 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, idw) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -337,11 +344,12 @@ TEST(GDALWriterTest, idw) "2.000 3.000 4.000 5.000 6.000 " "1.000 2.000 3.000 4.000 5.000 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, idwWindow) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -359,11 +367,12 @@ TEST(GDALWriterTest, idwWindow) "2.000 3.000 4.000 5.000 6.000 " "1.000 2.000 3.000 4.000 5.000 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, count) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -380,11 +389,12 @@ TEST(GDALWriterTest, count) "1.000 1.000 1.000 4.000 4.000 " "1.000 1.000 1.000 2.000 2.000 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, stdev) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -401,11 +411,12 @@ TEST(GDALWriterTest, stdev) "0.000 0.000 0.000 0.424 0.424 " "0.000 0.000 0.000 0.200 0.200 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, stdevWindow) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -423,7 +434,7 @@ TEST(GDALWriterTest, stdevWindow) "0.000 0.000 0.000 0.424 0.424 " "0.000 0.000 0.000 0.200 0.200 "; - runGdalWriter(wo, outfile, output); + runGdalWriter(wo, infile, outfile, output); } TEST(GDALWriterTest, additionalDim) @@ -468,6 +479,7 @@ TEST(GDALWriterTest, additionalDim) TEST(GDALWriterTest, btbad) { + std::string infile = Support::datapath("gdal/grid.txt"); std::string outfile = Support::temppath("tmp.tif"); Options wo; @@ -485,7 +497,7 @@ TEST(GDALWriterTest, btbad) "2.000 3.000 4.000 4.000 5.000 " "1.000 2.000 3.000 4.000 5.000 "; - EXPECT_THROW(runGdalWriter(wo, outfile, output), pdal_error); + EXPECT_THROW(runGdalWriter(wo, infile, outfile, output), pdal_error); } TEST(GDALWriterTest, btint) @@ -575,3 +587,28 @@ TEST(GDALWriterTest, no_points) EXPECT_THROW(w.execute(t), pdal_error); } +// Check that we don't crash with bad X/Y's in grid_bounds.txt and a preset +// grid bounds. +TEST(GDALWriterTest, bounds) +{ + std::string infile = Support::datapath("gdal/grid_bounds.txt"); + std::string outfile = Support::temppath("tmp.tif"); + + Options wo; + wo.add("gdaldriver", "GTiff"); + wo.add("output_type", "mean"); + wo.add("resolution", 1); + wo.add("radius", .7071); + wo.add("filename", outfile); + wo.add("bounds", "([0, 4.5],[0, 4.5])"); + + const std::string output = + "5.000 -9999.000 7.000 8.000 8.967 " + "4.000 -9999.000 6.000 7.000 8.000 " + "3.000 4.000 5.000 5.700 6.700 " + "2.000 3.000 4.000 4.800 5.800 " + "1.000 2.000 3.000 4.200 5.200 "; + + runGdalWriter(wo, infile, outfile, output); +} +