From df3c502fe4882cff79420386d858608d4b970795 Mon Sep 17 00:00:00 2001
From: Andrew Bell <andrew.bell.ia@gmail.com>
Date: Mon, 5 Oct 2015 10:01:19 -0500
Subject: [PATCH] Flat options for colorization.

---
 doc/stages/filters.colorization.rst          |  46 ++---
 filters/colorization/ColorizationFilter.cpp  | 167 +++++++++++--------
 filters/colorization/ColorizationFilter.hpp  |  23 ++-
 include/pdal/GDALUtils.hpp                   |  15 --
 test/unit/filters/ColorizationFilterTest.cpp |  44 +----
 5 files changed, 125 insertions(+), 170 deletions(-)

diff --git a/doc/stages/filters.colorization.rst b/doc/stages/filters.colorization.rst
index 64bc0be08e..35ad2adb0e 100644
--- a/doc/stages/filters.colorization.rst
+++ b/doc/stages/filters.colorization.rst
@@ -20,26 +20,8 @@ The bands of the raster to apply to each are selected using the "band" option, a
     <Writer type="writers.las">
       <Option name="filename">colorized.las</Option>
       <Filter type="filters.colorization">
-        <Option name="dimension">
-          Red
-          <Options>
-            <Option name="band">1</Option>
-            <Option name="scale">1.0</Option>
-          </Options>
-        </Option>
-        <Option name="dimension">
-          Green
-          <Options>
-              <Option name="band">2</Option>
-              <Option name="scale">1.0</Option>
-          </Options>
-        </Option>
-        <Option name="dimension">
-          Blue
-          <Options>
-            <Option name="band">3</Option>
-            <Option name="scale">256</Option>
-          </Options>
+        <Option name="dimensions">
+          Red:1:1.0, Blue, Green::256.0
         </Option>
         <Option name="raster">aerial.tif</Option>
         <Reader type="readers.las">
@@ -50,27 +32,19 @@ The bands of the raster to apply to each are selected using the "band" option, a
   </Pipeline>
 
 
-
 Options
 -------
 
 raster
   The raster file to read the band from. Any format supported by `GDAL`_ may be read.
 
-dimension
-  A dimension to populate with values from the raster file. There may be multiple dimension options declared. The dimension name should be supplied, and an options list indicating the raster band to read from and the scaling to apply.
-  
-  band
-    The raster band from which to read values for this dimension.
-  
-  scale
-    The scaling factor to apply to the value read from the raster. [Default: **1.0**]
-
-x_dim
-  The point dimension to use for the x dimension [Default: **X**]
+dimensions
+  A comma separated list of dimensions to populate with values from the raster
+  file. The format of each dimension is <name>:<band_number>:<scale_factor>.
+  Either or both of band number and scale factor may be omitted as may ':'
+  separators if the data is not ambiguous.  If not supplied, band numbers
+  begin at 1 and increment from the band number of the previous dimension.
+  If not supplied, the scaling factor is 1.0.
+  [Default: "Red:1:1.0, Green:2:1.0, Blue:3:1.0"]
   
-y_dim
-  The point dimension to use for the y dimension [Default: **Y**]
-
-
 .. _GDAL: http://gdal.org
diff --git a/filters/colorization/ColorizationFilter.cpp b/filters/colorization/ColorizationFilter.cpp
index 9bb518cc3a..57747f8776 100644
--- a/filters/colorization/ColorizationFilter.cpp
+++ b/filters/colorization/ColorizationFilter.cpp
@@ -65,72 +65,104 @@ Options ColorizationFilter::getDefaultOptions()
 {
     Options options;
 
-    pdal::Option red("dimension", "Red", "");
-    pdal::Option b0("band",1, "");
-    pdal::Option s0("scale", 1.0f, "scale factor for this dimension");
-    pdal::Options redO;
-    redO.add(b0);
-    redO.add(s0);
-    red.setOptions(redO);
-
-    pdal::Option green("dimension", "Green", "");
-    pdal::Option b1("band",2, "");
-    pdal::Option s1("scale", 1.0f, "scale factor for this dimension");
-    pdal::Options greenO;
-    greenO.add(b1);
-    greenO.add(s1);
-    green.setOptions(greenO);
-
-    pdal::Option blue("dimension", "Blue", "");
-    pdal::Option b2("band",3, "");
-    pdal::Option s2("scale", 1.0f, "scale factor for this dimension");
-    pdal::Options blueO;
-    blueO.add(b2);
-    blueO.add(s2);
-    blue.setOptions(blueO);
-
-    pdal::Option reproject("reproject", false,
-        "Reproject the input data into the same coordinate system as "
-        "the raster?");
-
-    options.add(red);
-    options.add(green);
-    options.add(blue);
-    options.add(reproject);
+    options.add("dimensions", "Red:1:1.0, Green:2:1.0, Blue:3");
 
     return options;
 }
 
+namespace
+{
 
-void ColorizationFilter::processOptions(const Options& options)
+ColorizationFilter::BandInfo parseDim(const std::string& dim,
+    uint32_t defaultBand)
 {
-    m_rasterFilename = options.getValueOrThrow<std::string>("raster");
-    std::vector<Option> dimensions = options.getOptions("dimension");
+    std::string::size_type pos, count;
+    const char *start;
+    char *end;
+    std::string name;
+    uint32_t band = defaultBand;
+    double scale = 1.0;
+
+    try
+    {
+        pos = 0;
+        // Skip leading whitespace.
+        count = Utils::extract(dim, pos, (int(*)(int))std::isspace);
+        pos += count;
 
-    if (dimensions.size() == 0)
+        count = Utils::extract(dim, pos, (int(*)(int))std::isalpha);
+        if (count == 0)
+           throw std::string("No dimension name.");
+        name = dim.substr(pos, count);
+        pos += count;
+
+        count = Utils::extract(dim, pos, (int(*)(int))std::isspace);
+        pos += count;
+
+        if (pos < dim.size() && dim[pos] == ':')
+        {
+            pos++;
+            start = dim.data() + pos;
+            band = std::strtoul(start, &end, 10);
+            if (start == end)
+                band = defaultBand;
+            pos += (end - start);
+
+            count = Utils::extract(dim, pos, (int(*)(int))std::isspace);
+            pos += count;
+
+            if (pos < dim.size() && dim[pos] == ':')
+            {
+                pos++;
+                start = dim.data() + pos;
+                scale = std::strtod(start, &end);
+                if (start == end)
+                    scale = 1.0;
+                pos += (end - start);
+            }
+        }
+
+        count = Utils::extract(dim, pos, (int(*)(int))std::isspace);
+        pos += count;
+
+        if (pos != dim.size())
+            throw std::string("Invalid characters following dimension "
+                "specification.");
+    }
+    catch (std::string s)
     {
-        m_bands.emplace_back("Red", Dimension::Id::Red, 1, 1.0);
-        m_bands.emplace_back("Green", Dimension::Id::Green, 2, 1.0);
-        m_bands.emplace_back("Blue", Dimension::Id::Blue, 3, 1.0);
-        log()->get(LogLevel::Debug) << "No dimension mappings were given. "
-            "Using default mappings." << std::endl;
+        std::ostringstream oss;
+        oss << "filters.colorization: invalid --dimensions option: '" << dim <<
+            "': " << s;
+        throw pdal_error(oss.str());
     }
-    for (auto i = dimensions.begin(); i != dimensions.end(); ++i)
+    return ColorizationFilter::BandInfo(name, band, scale);
+}
+
+} // unnamed namespace
+
+void ColorizationFilter::processOptions(const Options& options)
+{
+    m_rasterFilename = options.getValueOrThrow<std::string>("raster");
+
+    if (options.hasOption("dimension") && !options.hasOption("dimensions"))
+        throw pdal_error("Option 'dimension' no longer supported.  Use "
+            "'dimensions' instead.");
+
+    StringList defaultDims;
+    defaultDims.push_back("Red");
+    defaultDims.push_back("Green");
+    defaultDims.push_back("Blue");
+
+    StringList dims =
+        options.getValueOrDefault<StringList>("dimensions", defaultDims);
+
+    uint32_t defaultBand = 1;
+    for (std::string& dim : dims)
     {
-        std::string name = i->getValue<std::string>();
-        boost::optional<Options const&> dimensionOptions = i->getOptions();
-        if (!dimensionOptions)
-        {
-            std::ostringstream oss;
-            oss << "No band and scaling information given for dimension '" <<
-                name << "'";
-            throw pdal_error(oss.str());
-        }
-        uint32_t bandId =
-            dimensionOptions->getValueOrThrow<uint32_t>("band");
-        double scale =
-            dimensionOptions->getValueOrDefault<double>("scale", 1.0);
-        m_bands.emplace_back(name, Dimension::Id::Unknown, bandId, scale);
+        BandInfo bi = parseDim(dim, defaultBand);
+        defaultBand = bi.m_band + 1;
+        m_bands.push_back(bi);
     }
 }
 
@@ -145,37 +177,28 @@ void ColorizationFilter::addDimensions(PointLayoutPtr layout)
 
 void ColorizationFilter::ready(PointTableRef table)
 {
-    m_raster = std::unique_ptr<gdal::Raster>(new gdal::Raster(m_rasterFilename));
+    m_raster =
+        std::unique_ptr<gdal::Raster>(new gdal::Raster(m_rasterFilename));
     m_raster->open();
-
-    for (auto bi = m_bands.begin(); bi != m_bands.end(); ++bi)
-    {
-        if (bi->m_dim == Dimension::Id::Unknown)
-            bi->m_dim = table.layout()->findDim(bi->m_name);
-        if (bi->m_dim == Dimension::Id::Unknown)
-            throw pdal_error((std::string)"Can't colorize - no dimension " +
-                bi->m_name);
-    }
 }
 
 
 void ColorizationFilter::filter(PointView& view)
 {
     std::vector<double> data;
-    int i(0);
+
     for (PointId idx = 0; idx < view.size(); ++idx)
     {
-        int i(0);
         double x = view.getFieldAs<double>(Dimension::Id::X, idx);
         double y = view.getFieldAs<double>(Dimension::Id::Y, idx);
 
-        bool bRead = m_raster->read(x, y, data);
-
-        if (!bRead) continue;
+        if (!m_raster->read(x, y, data))
+            continue;
 
+        int i(0);
         for (auto bi = m_bands.begin(); bi != m_bands.end(); ++bi)
         {
-            gdal::BandInfo& b = *bi;
+            BandInfo& b = *bi;
             view.setField(b.m_dim, idx, data[i] * b.m_scale);
             ++i;
         }
diff --git a/filters/colorization/ColorizationFilter.hpp b/filters/colorization/ColorizationFilter.hpp
index 695de6a9c5..ea4e686715 100644
--- a/filters/colorization/ColorizationFilter.hpp
+++ b/filters/colorization/ColorizationFilter.hpp
@@ -50,16 +50,29 @@ extern "C" PF_ExitFunc ColorizationFilter_InitPlugin();
 namespace pdal
 {
 
-namespace gdal
-{ class GlobalDebug; }
-
 // Provides GDAL-based raster overlay that places output data in
 // specified dimensions. It also supports scaling the data by a multiplier
 // on a per-dimension basis.
 class PDAL_DLL ColorizationFilter : public Filter
 {
-
 public:
+    struct BandInfo
+    {
+        BandInfo(const std::string& name, uint32_t band, double scale) :
+            m_name(name), m_band(band), m_scale(scale),
+            m_dim(Dimension::Id::Unknown)
+        {}
+
+        BandInfo() : m_band(0), m_scale(1.0), m_dim(Dimension::Id::Unknown)
+        {}
+
+        std::string m_name;
+        uint32_t m_band;
+        double m_scale;
+        Dimension::Id::Enum m_dim;
+    };
+
+
     ColorizationFilter()
     {}
 
@@ -77,7 +90,7 @@ class PDAL_DLL ColorizationFilter : public Filter
     virtual void filter(PointView& view);
 
     std::string m_rasterFilename;
-    std::vector<gdal::BandInfo> m_bands;
+    std::vector<BandInfo> m_bands;
 
     std::unique_ptr<gdal::Raster> m_raster;
 
diff --git a/include/pdal/GDALUtils.hpp b/include/pdal/GDALUtils.hpp
index de3a2d7db3..0a0f9e2612 100644
--- a/include/pdal/GDALUtils.hpp
+++ b/include/pdal/GDALUtils.hpp
@@ -174,21 +174,6 @@ class PDAL_DLL ErrorHandler
     pdal::LogPtr m_log;
 };
 
-
-struct BandInfo
-{
-    BandInfo(const std::string& name, pdal::Dimension::Id::Enum dim, uint32_t band,
-        double scale) : m_name(name), m_dim(dim), m_band(band), m_scale(scale)
-    {}
-
-    std::string m_name;
-    pdal::Dimension::Id::Enum m_dim;
-    uint32_t m_band;
-    double m_scale;
-    pdal::Dimension::Type::Enum m_type;
-};
-
-
 class PDAL_DLL Raster
 
 {
diff --git a/test/unit/filters/ColorizationFilterTest.cpp b/test/unit/filters/ColorizationFilterTest.cpp
index bd1d8887bc..926fb3c267 100644
--- a/test/unit/filters/ColorizationFilterTest.cpp
+++ b/test/unit/filters/ColorizationFilterTest.cpp
@@ -86,27 +86,7 @@ TEST(ColorizationFilterTest, test1)
 {
     Options options;
 
-    Option red("dimension", "Red");
-    Options subRed;
-    subRed.add("band", 1);
-    subRed.add("scale", 1.0f, "scale factor for this dimension");
-    red.setOptions(subRed);
-    options.add(red);
-
-    Option green("dimension", "Green");
-    Options subGreen;
-    subGreen.add("band", 2);
-    subGreen.add("scale", 1.0f, "scale factor for this dimension");
-    green.setOptions(subGreen);
-    options.add(green);
-
-    Option blue("dimension", "Blue", "");
-    Options subBlue;
-    subBlue.add("band", 3);
-    subBlue.add("scale", 255.0f, "scale factor for this dimension");
-    blue.setOptions(subBlue);
-    options.add(blue);
-
+    options.add("dimensions", "Red, Green,Blue::255  ");
     options.add("raster", Support::datapath("autzen/autzen.jpg"),
         "raster to read");
 
@@ -138,27 +118,7 @@ TEST(ColorizationFilterTest, test3)
 {
     Options options;
 
-    Option red("dimension", "Foo");
-    Options subRed;
-    subRed.add("band", 1);
-    subRed.add("scale", 1.0f, "scale factor for this dimension");
-    red.setOptions(subRed);
-    options.add(red);
-
-    Option green("dimension", "Bar");
-    Options subGreen;
-    subGreen.add("band", 2);
-    subGreen.add("scale", 1.0f, "scale factor for this dimension");
-    green.setOptions(subGreen);
-    options.add(green);
-
-    Option blue("dimension", "Baz", "");
-    Options subBlue;
-    subBlue.add("band", 3);
-    subBlue.add("scale", 255.0f, "scale factor for this dimension");
-    blue.setOptions(subBlue);
-    options.add(blue);
-
+    options.add("dimensions", "Foo:1,Bar:2,Baz:3:255");
     options.add("raster", Support::datapath("autzen/autzen.jpg"),
         "raster to read");