Merge pull request #1658 from tst-rwildes/merge-5.3.3.1-to-master

Merge 5.3.3.1 to master

earth_enterprise/BUILD_RHEL_CentOS.md

@@ -51,8 +51,6 @@ sudo subscription-manager repos --enable=rhel-server-dts2-6-rpms
 sudo subscription-manager repos --enable=rhel-6-server-optional-rpms
 sudo yum install -y https://dl.fedoraproject.org/pub/epel/epel-release-latest-6.noarch.rpm
 sudo yum install -y https://repo.ius.io/ius-release-el6.rpm
-
-
 ```
 
 ## Install Git

earth_enterprise/src/fusion/khgdal/SConscript

@@ -54,9 +54,11 @@ getranslate = env.executable('getranslate', 'getranslate.cpp',
 gereproject = env.executable('gereproject', 'gereproject.cpp',
                              LIBS=['gegdal', 'geutil', 'gdal', 'geos'])
 
-
 khgdal_test = env.test('khgdal_test', ['khgdal_test.cpp'], LIBS=['gegdal', 'gecommon', 'geos'])
 
+khGDALReader_unittest = env.test('khGDALReader_unittest',
+    ['khGDALReader_unittest.cpp'],
+    LIBS=['gegdal', 'gecommon', 'geos', 'gtest'])
 
 env.install('fusion_bin', [geinfo, gevirtualraster, gesplitkhvr,
                            getranslate, gereproject])

earth_enterprise/src/fusion/khgdal/khGDALReader.cpp

@@ -25,6 +25,8 @@ khGDALReader::khGDALReader(const khGDALDataset &srcDS_, uint numbands)
     numBands(numbands),
     gdalDatatype(srcDS->GetRasterBand(1)->GetRasterDataType()),
     topToBottom(srcDS.normalizedGeoExtents().topToBottom()),
+    no_data_set(false),
+    sanitized_no_data(0),
     paletteSize(0),
     palette(0)
 {
@@ -91,6 +93,9 @@ khGDALReader::khGDALReader(const khGDALDataset &srcDS_, uint numbands)
   }
 }
 
+void khGDALReader::GetNoDataFromSrc(double & no_data, int & nodata_exists) {
+  no_data = srcDS->GetRasterBand(1)->GetNoDataValue(&nodata_exists);
+}
 
 // ****************************************************************************
 // ***  khGDALSimpleReader

earth_enterprise/src/fusion/khgdal/khGDALReader.h

@@ -42,19 +42,27 @@ class khGDALReader
   template <class SrcPixelType, class TileType>
   void TypedRead(const khExtents<uint32> &readExtents, bool topToBottom,
                  TileType &tile, const khOffset<uint32> &tileOffset);
+  // virtual so it can be overridden by unit tests
+  virtual void GetNoDataFromSrc(double & no_data, int & nodata_exists);
+
  protected:
   khGDALDataset     srcDS;
   uint              numBands;
   std::vector<int>  gdalBandIndexes;
   GDALDataType      gdalDatatype;
   khTypes::StorageEnum storage;
   bool              topToBottom;
+  bool              no_data_set;
+  double            sanitized_no_data;
 
   uint paletteSize;
   const GDALColorEntry *palette;
 
   std::vector<uchar> rawReadBuf;
 
+  // Protected so it can be called from unit tests
+  template <class SrcPixelType> SrcPixelType GetNoDataOrZero();
+
   // reads numBands worth of gdalDatatype pixels into rawReadBuf
   // assumes rawReadBuf has already been sized appropriately
   // will succeed or throw

earth_enterprise/src/fusion/khgdal/khGDALReaderImpl.h

@@ -19,6 +19,7 @@
 
 #include "khGDALReader.h"
 #include "ReadHelpers.h"
+#include <limits>
 
 
 // ****************************************************************************
@@ -31,6 +32,8 @@ khGDALReader::TypedRead(const khExtents<uint32> &srcExtents, bool topToBottom,
                         TileType &tile, const khOffset<uint32> &tileOffset)
 {
   assert(storage == khTypes::Helper<SrcPixelType>::Storage);
+  // Make sure SrcPixelType has a min and max
+  assert(std::numeric_limits<SrcPixelType>::is_specialized);
 
   if (topToBottom != this->topToBottom) {
     throw khException(kh::tr("topToBottom doesn't match dataset"));
@@ -40,15 +43,12 @@ khGDALReader::TypedRead(const khExtents<uint32> &srcExtents, bool topToBottom,
   const uint pixelsPerBand = srcExtents.width() * srcExtents.height();
   const uint bandSize = (pixelsPerBand * sizeof(SrcPixelType));
   rawReadBuf.reserve(numBands * bandSize);
-  // Fill buffer with 0 (or NoData if it exists)
-  int nodata_exists = 0;
-  SrcPixelType no_data =
-      static_cast<SrcPixelType>(
-          srcDS->GetRasterBand(1)->GetNoDataValue(&nodata_exists));
-  if (!nodata_exists)
-    no_data = 0;
-  for (uint i = 0; i < pixelsPerBand * numBands; ++i)
+
+  // Fill buffer with NoData or zero
+  SrcPixelType no_data = GetNoDataOrZero<SrcPixelType>();
+  for (uint i = 0; i < pixelsPerBand * numBands; ++i) {
     ((SrcPixelType *) (&rawReadBuf[0]))[i] = no_data;
+  }
 
   // read pixels from all bands into rawReadBuf
   FetchPixels(srcExtents);
@@ -108,5 +108,35 @@ khGDALReader::TypedRead(const khExtents<uint32> &srcExtents, bool topToBottom,
   }
 }
 
+template <class SrcPixelType>
+SrcPixelType khGDALReader::GetNoDataOrZero() {
+  if (!no_data_set) {
+    double no_data;
+    int nodata_exists;
+    GetNoDataFromSrc(no_data, nodata_exists);
+    if (nodata_exists) {
+      // Check that nodata fits in the pixel type
+      SrcPixelType pixelMin = std::numeric_limits<SrcPixelType>::lowest();
+      SrcPixelType pixelMax = std::numeric_limits<SrcPixelType>::max();
+      if (no_data >= pixelMin && no_data <= pixelMax) {
+        sanitized_no_data = no_data;
+      }
+      else {
+        notify(NFY_WARN, "Ignoring NoData (%.2f) because it is too large for the pixel type.", no_data);
+      }
+    }
+
+    if (sanitized_no_data != 0) {
+      notify(NFY_NOTICE, "Using non-zero NoData (%.2f). If Fusion produces black regions "
+                         "around this image, consider creating your own mask and disabling "
+                         "Auto Masking for this resource.", no_data);
+    }
+
+    no_data_set = true;
+  }
+
+  return static_cast<SrcPixelType>(sanitized_no_data);
+}
+
 
 #endif /* __khGDALReaderImpl_h */

earth_enterprise/src/fusion/khgdal/khGDALReader_unittest.cpp

@@ -0,0 +1,91 @@
+// Copyright 2020 The Open GEE Contributors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+#include "khGDALReaderImpl.h"
+
+// Point the dataset at a valid image. We won't use this, but it prevents crashes
+// due to an invalid dataset.
+khGDALDataset dataset("fusion/testdata/i3SF15-meter.tif");
+
+class MockReader : public khGDALReader {
+  public:
+    double no_data_val;
+    int nodata_exists_val;
+    uint num_gets;
+    MockReader(double no_data, int nodata_exists) :
+        khGDALReader(dataset, 3), // Values don't matter; just making the constructor happy
+        no_data_val(no_data), nodata_exists_val(nodata_exists), num_gets(0) { }
+
+    // Give callers access to the functions we want to test
+    template<class SrcPixelType>
+    SrcPixelType CallGetNoDataOrZero() {
+      return GetNoDataOrZero<SrcPixelType>();
+    }
+  private:
+    virtual void GetNoDataFromSrc(double & no_data, int & nodata_exists) override {
+      no_data = no_data_val;
+      nodata_exists = nodata_exists_val;
+      ++num_gets;
+    }
+
+    // Override abstract functions so we can instantiate the class
+    virtual void FetchPixels(const khExtents<uint32> &srcExtents) override {}
+};
+
+TEST(khGDALReaderTest, NoDataDoesntExist) {
+  MockReader reader(255, false);
+  int8 nodata = reader.CallGetNoDataOrZero<int8>();
+  ASSERT_EQ(0, nodata);
+}
+
+TEST(khGDALReaderTest, NoDataZero) {
+  MockReader reader(0, true);
+  uint32 nodata = reader.CallGetNoDataOrZero<uint32>();
+  ASSERT_EQ(0, nodata);
+}
+
+TEST(khGDALReaderTest, NoDataNonZero) {
+  MockReader reader(-9999, true);
+  float32 nodata = reader.CallGetNoDataOrZero<float32>();
+  ASSERT_EQ(-9999, nodata);
+}
+
+TEST(khGDALReaderTest, NoDataTooBigPositive) {
+  MockReader reader(256, true);
+  uint8 nodata = reader.CallGetNoDataOrZero<uint8>();
+  ASSERT_EQ(0, nodata);
+}
+
+TEST(khGDALReaderTest, NoDataTooBigNegative) {
+  MockReader reader(-100000, true);
+  int16 nodata = reader.CallGetNoDataOrZero<int16>();
+  ASSERT_EQ(0, nodata);
+}
+
+TEST(khGDALReaderTest, NoDataMultipleCalls) {
+  MockReader reader(16, true);
+  float64 nodata = reader.CallGetNoDataOrZero<float64>();
+  ASSERT_EQ(16, nodata);
+  ASSERT_EQ(1, reader.num_gets);
+  nodata = reader.CallGetNoDataOrZero<float64>();
+  ASSERT_EQ(16, nodata);
+  // Make sure we only extract the NoData value once
+  ASSERT_EQ(1, reader.num_gets);
+}
+
+int main(int argc, char** argv) {
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

earth_enterprise/src/fusion/khgdal/khgdal.h

@@ -32,9 +32,9 @@
 // ****************************************************************************
 class CPLStrGuard
 {
-  // private and unimplemented - it would be a waste to copy the strings
-  CPLStrGuard(const CPLStrGuard &o);
-  CPLStrGuard& operator=(const CPLStrGuard &o);
+  // it would be a waste to copy the strings
+  CPLStrGuard(const CPLStrGuard &o) = delete;
+  CPLStrGuard& operator=(const CPLStrGuard &o) = delete;
 
   char *str;
  public: