Working with tests.

Author: abellgithub

alg/viewshed/viewshed_executor.cpp

@@ -17,6 +17,8 @@
 #include <cmath>
 #include <limits>
 
+#include <iostream>
+
 #include "viewshed_executor.h"
 #include "progress.h"
 #include "util.h"
@@ -428,14 +430,15 @@ void ViewshedExecutor::processFirstLineLeft(const LineLimits &ll,
     // If the start cell is next to the observer, just mark it visible.
     if (iStart + 1 == m_nX || iStart + 1 == oCurExtent.xStop)
     {
+        double dfZ = *pThis;
         if (oOpts.outputMode == OutputMode::Normal)
             vResult[iStart] = oOpts.visibleVal;
         else
         {
-            maskLowPitch(*pThis, 1, 0);
-            setOutput(vResult[iStart], *pThis, *pThis);
+            maskLowPitch(dfZ, 1, 0);
+            setOutput(vResult[iStart], *pThis, dfZ);
         }
-        maskHighPitch(vResult[iStart], *pThis, 1, 0);
+        maskHighPitch(vResult[iStart], dfZ, 1, 0);
         iStart--;
         pThis--;
     }
@@ -622,14 +625,15 @@ void ViewshedExecutor::processFirstLineRight(const LineLimits &ll,
     // If the start cell is next to the observer, just mark it visible.
     if (iStart - 1 == m_nX || iStart == oCurExtent.xStart)
     {
+        double dfZ = *pThis;
         if (oOpts.outputMode == OutputMode::Normal)
             vResult[iStart] = oOpts.visibleVal;
         else
         {
-            maskLowPitch(*pThis, 1, 0);
-            setOutput(vResult[iStart], *pThis, *pThis);
+            maskLowPitch(dfZ, 1, 0);
+            setOutput(vResult[iStart], *pThis, dfZ);
         }
-        maskHighPitch(vResult[iStart], *pThis, 1, 0);
+        maskHighPitch(vResult[iStart], dfZ, 1, 0);
         iStart++;
         pThis++;
     }
@@ -662,7 +666,6 @@ void ViewshedExecutor::processLineLeft(int nYOffset, LineLimits &ll,
     int iStart = m_nX - 1;
     int iEnd = ll.left - 1;
     int nLine = m_nY + nYOffset;
-
     // If start to the left of end, everything is taken care of by processing right.
     if (iStart <= iEnd)
         return;
@@ -675,6 +678,7 @@ void ViewshedExecutor::processLineLeft(int nYOffset, LineLimits &ll,
     // If the observer is to the right of the raster, mark the first cell to the left as
     // visible. This may mark an out-of-range cell with a value, but this will be fixed
     // with the out of range assignment at the end.
+
     if (iStart == oCurExtent.xStop - 1)
     {
         if (oOpts.outputMode == OutputMode::Normal)
@@ -851,7 +855,6 @@ bool ViewshedExecutor::processLine(int nLine, std::vector<double> &vLastLineVal)
             processLineRight(nYOffset, ll, vResult, vThisLineVal, vLastLineVal);
         });
     pQueue->WaitCompletion();
-
     // Make the current line the last line.
     vLastLineVal = std::move(vThisLineVal);
 

alg/viewshed/viewshed_types.h

@@ -188,6 +188,7 @@ inline std::ostream &operator<<(std::ostream &out, const Window &w)
 /// The right side processing range is [rightMin, right).
 struct LineLimits
 {
+    /// Constructor that takes the members in order.
     LineLimits(int leftArg, int leftMinArg, int rightMinArg, int rightArg)
         : left(leftArg), leftMin(leftMinArg), rightMin(rightMinArg),
           right(rightArg)

apps/gdalalg_raster_viewshed.cpp

@@ -44,7 +44,7 @@ GDALRasterViewshedAlgorithm::GDALRasterViewshedAlgorithm()
     AddInputDatasetArg(&m_inputDataset, GDAL_OF_RASTER);
 
     AddOutputDatasetArg(&m_outputDataset, GDAL_OF_RASTER);
-    AddOutputFormatArg(&m_format, /* bStreamAllowed = */ false,
+    AddOutputFormatArg(&m_opts.outputFormat, /* bStreamAllowed = */ false,
                        /* bGDALGAllowed = */ false)
         .AddMetadataItem(GAAMDI_REQUIRED_CAPABILITIES,
                          {GDAL_DCAP_RASTER, GDAL_DCAP_CREATE});
@@ -56,13 +56,14 @@ GDALRasterViewshedAlgorithm::GDALRasterViewshedAlgorithm()
         .SetMetaVar("<X,Y> or <X,Y,H>")
         .SetMinCount(2)
         .SetMaxCount(3)
-        .SetRequired()
         .SetRepeatedArgAllowed(false);
+    AddArg("height", 'z', _("Observer height"), &m_opts.observer.z);
+
     AddArg("target-height", 0,
            _("Height of the target above the DEM surface in the height unit of "
              "the DEM."),
-           &m_targetHeight)
-        .SetDefault(m_targetHeight);
+           &m_opts.targetHeight)
+        .SetDefault(m_opts.targetHeight);
     AddArg("mode", 0, _("Sets what information the output contains."),
            &m_outputMode)
         .SetChoices("normal", "DEM", "ground", "cumulative")
@@ -71,41 +72,81 @@ GDALRasterViewshedAlgorithm::GDALRasterViewshedAlgorithm()
     AddArg("max-distance", 0,
            _("Maximum distance from observer to compute visibility. It is also "
              "used to clamp the extent of the output raster."),
-           &m_maxDistance)
+           &m_opts.maxDistance)
+        .SetMinValueIncluded(0);
+    AddArg("min-distance", 0,
+           _("Mask all cells less than this distance from the observer. Must "
+             "be less "
+             "than 'max-distance'."),
+           &m_opts.minDistance)
         .SetMinValueIncluded(0);
+
+    AddArg("start-angle", 0,
+           _("Mask all cells outside of the arc ('start-angle', 'end-angle'). "
+             "Clockwise degrees "
+             "from north. Also used to clamp the extent of the output raster."),
+           &m_opts.startAngle)
+        .SetMinValueIncluded(0)
+        .SetMaxValueExcluded(360);
+    AddArg("end-angle", 0,
+           _("Mask all cells outside of the arc ('start-angle', 'end-angle'). "
+             "Clockwise degrees "
+             "from north. Also used to clamp the extent of the output raster."),
+           &m_opts.endAngle)
+        .SetMinValueIncluded(0)
+        .SetMaxValueExcluded(360);
+
+    AddArg("high-pitch", 0,
+           _("Mark all cells out-of-range where the observable height would be "
+             "higher than the "
+             "'high-pitch' angle from the observer. Degrees from horizontal - "
+             "positive is up. "
+             "Must be greater than 'low-pitch'."),
+           &m_opts.highPitch)
+        .SetMaxValueIncluded(90)
+        .SetMinValueExcluded(-90);
+    AddArg("low-pitch", 0,
+           _("Bound observable height to be no lower than the 'low-pitch' "
+             "angle from the observer. "
+             "Degrees from horizontal - positive is up. Must be less than "
+             "'high-pitch'."),
+           &m_opts.lowPitch)
+        .SetMaxValueExcluded(90)
+        .SetMinValueIncluded(-90);
+
     AddArg("curvature-coefficient", 0,
            _("Coefficient to consider the effect of the curvature and "
              "refraction."),
-           &m_curveCoefficient)
+           &m_opts.curveCoeff)
         .SetMinValueIncluded(0);
 
     AddBandArg(&m_band).SetDefault(m_band);
     AddArg("visible-value", 0, _("Pixel value to set for visible areas"),
-           &m_visibleVal)
-        .SetDefault(m_visibleVal)
+           &m_opts.visibleVal)
+        .SetDefault(m_opts.visibleVal)
         .SetMinValueIncluded(0)
         .SetMaxValueIncluded(255);
     AddArg("invisible-value", 0, _("Pixel value to set for invisible areas"),
-           &m_invisibleVal)
-        .SetDefault(m_invisibleVal)
+           &m_opts.invisibleVal)
+        .SetDefault(m_opts.invisibleVal)
         .SetMinValueIncluded(0)
         .SetMaxValueIncluded(255);
     AddArg("out-of-range-value", 0,
            _("Pixel value to set for the cells that fall outside of the range "
              "specified by the observer location and the maximum distance"),
-           &m_outOfRangeVal)
-        .SetDefault(m_outOfRangeVal)
+           &m_opts.outOfRangeVal)
+        .SetDefault(m_opts.outOfRangeVal)
         .SetMinValueIncluded(0)
         .SetMaxValueIncluded(255);
     AddArg("dst-nodata", 0,
            _("The value to be set for the cells in the output raster that have "
              "no data."),
-           &m_dstNoData)
+           &m_opts.nodataVal)
         .SetMinValueIncluded(0)
         .SetMaxValueIncluded(255);
     AddArg("observer-spacing", 0, _("Cell Spacing between observers"),
-           &m_observerSpacing)
-        .SetDefault(m_observerSpacing)
+           &m_opts.observerSpacing)
+        .SetDefault(m_opts.observerSpacing)
         .SetMinValueIncluded(1);
 
     m_numThreadsStr = std::to_string(m_numThreads);
@@ -121,62 +162,76 @@ bool GDALRasterViewshedAlgorithm::RunImpl(GDALProgressFunc pfnProgress,
 {
     CPLAssert(!m_outputDataset.GetDatasetRef());
 
-    gdal::viewshed::Options opts;
-
-    opts.observer.x = m_observerPos[0];
-    opts.observer.y = m_observerPos[1];
-    if (m_observerPos.size() == 3)
-        opts.observer.z = m_observerPos[2];
-    else
-        opts.observer.z = 2;
-
-    opts.targetHeight = m_targetHeight;
+    if (GetArg("height")->IsExplicitlySet())
+    {
+        if (m_observerPos.size() == 3)
+        {
+            ReportError(CE_Failure, CPLE_AppDefined,
+                        "Height can't be specified in both 'position' and "
+                        "'height' arguments");
+            return false;
+        }
+    }
 
-    opts.maxDistance = m_maxDistance;
+    if (m_observerPos.size())
+    {
+        m_opts.observer.x = m_observerPos[0];
+        m_opts.observer.y = m_observerPos[1];
+        if (m_observerPos.size() == 3)
+            m_opts.observer.z = m_observerPos[2];
+        else
+            m_opts.observer.z = 2;
+    }
 
-    opts.curveCoeff = m_curveCoefficient;
     if (!GetArg("curvature-coefficient")->IsExplicitlySet())
     {
-        opts.curveCoeff = gdal::viewshed::adjustCurveCoeff(
-            opts.curveCoeff,
+        m_opts.curveCoeff = gdal::viewshed::adjustCurveCoeff(
+            m_opts.curveCoeff,
             GDALDataset::ToHandle(m_inputDataset.GetDatasetRef()));
     }
 
-    opts.visibleVal = m_visibleVal;
-    opts.invisibleVal = m_invisibleVal;
-    opts.outOfRangeVal = m_outOfRangeVal;
-    opts.nodataVal = m_dstNoData;
-
     if (m_outputMode == "normal")
-        opts.outputMode = gdal::viewshed::OutputMode::Normal;
+        m_opts.outputMode = gdal::viewshed::OutputMode::Normal;
     else if (m_outputMode == "DEM")
-        opts.outputMode = gdal::viewshed::OutputMode::DEM;
+        m_opts.outputMode = gdal::viewshed::OutputMode::DEM;
     else if (m_outputMode == "ground")
-        opts.outputMode = gdal::viewshed::OutputMode::Ground;
+        m_opts.outputMode = gdal::viewshed::OutputMode::Ground;
     else if (m_outputMode == "cumulative")
-        opts.outputMode = gdal::viewshed::OutputMode::Cumulative;
+        m_opts.outputMode = gdal::viewshed::OutputMode::Cumulative;
 
-    opts.observerSpacing = m_observerSpacing;
-    opts.numJobs = static_cast<uint8_t>(std::clamp(m_numThreads, 0, 255));
+    m_opts.numJobs = static_cast<uint8_t>(std::clamp(m_numThreads, 0, 255));
 
-    opts.outputFilename = m_outputDataset.GetName();
-    opts.outputFormat = m_format;
-    if (opts.outputFormat.empty())
+    m_opts.outputFilename = m_outputDataset.GetName();
+    if (m_opts.outputFormat.empty())
     {
-        opts.outputFormat =
-            GetOutputDriverForRaster(opts.outputFilename.c_str());
-        if (opts.outputFormat.empty())
+        m_opts.outputFormat =
+            GetOutputDriverForRaster(m_opts.outputFilename.c_str());
+        if (m_opts.outputFormat.empty())
         {
             ReportError(CE_Failure, CPLE_AppDefined,
                         "Cannot guess output driver from output filename");
             return false;
         }
     }
 
-    opts.creationOpts = CPLStringList(m_creationOptions);
+    m_opts.creationOpts = CPLStringList(m_creationOptions);
 
-    if (opts.outputMode == gdal::viewshed::OutputMode::Cumulative)
+    if (m_opts.outputMode == gdal::viewshed::OutputMode::Cumulative)
     {
+        static const std::vector<std::string> badArgs{
+            "visible-value", "invisible-value", "max-distance",
+            "min-distance",  "start-angle",     "end-angle",
+            "low-pitch",     "high-pitch",      "position"};
+
+        for (const auto &arg : badArgs)
+            if (GetArg(arg)->IsExplicitlySet())
+            {
+                std::string err =
+                    "Option '" + arg + "' can't be used in cumulative mode.";
+                ReportError(CE_Failure, CPLE_AppDefined, "%s", err.c_str());
+                return false;
+            }
+
         auto poSrcDS = m_inputDataset.GetDatasetRef();
         auto poSrcDriver = poSrcDS->GetDriver();
         if (EQUAL(poSrcDS->GetDescription(), "") || !poSrcDriver ||
@@ -187,29 +242,49 @@ bool GDALRasterViewshedAlgorithm::RunImpl(GDALProgressFunc pfnProgress,
                 "In cumulative mode, the input dataset must be opened by name");
             return false;
         }
-        if (EQUAL(opts.outputFormat.c_str(), "MEM"))
+        if (EQUAL(m_opts.outputFormat.c_str(), "MEM"))
         {
             ReportError(CE_Failure, CPLE_AppDefined,
                         "In cumulative mode, the output dataset cannot be a "
                         "MEM dataset");
             return false;
         }
-        gdal::viewshed::Cumulative oViewshed(opts);
+        gdal::viewshed::Cumulative oViewshed(m_opts);
         const bool bSuccess = oViewshed.run(
             m_inputDataset.GetName().c_str(),
             pfnProgress ? pfnProgress : GDALDummyProgress, pProgressData);
         if (bSuccess)
         {
             m_outputDataset.Set(std::unique_ptr<GDALDataset>(
-                GDALDataset::Open(opts.outputFilename.c_str(),
+                GDALDataset::Open(m_opts.outputFilename.c_str(),
                                   GDAL_OF_RASTER | GDAL_OF_VERBOSE_ERROR,
                                   nullptr, nullptr, nullptr)));
         }
         return bSuccess;
     }
     else
     {
-        gdal::viewshed::Viewshed oViewshed(opts);
+        static const std::vector<std::string> badArgs{"observer-spacing",
+                                                      "num-threads"};
+        for (const auto &arg : badArgs)
+            if (GetArg(arg)->IsExplicitlySet())
+            {
+                std::string err =
+                    "Option '" + arg + "' can't be used in standard mode.";
+                ReportError(CE_Failure, CPLE_AppDefined, "%s", err.c_str());
+                return false;
+            }
+        static const std::vector<std::string> goodArgs{"position"};
+        for (const auto &arg : goodArgs)
+            if (!GetArg(arg)->IsExplicitlySet())
+            {
+                std::string err =
+                    "Option '" + arg + "' must be specified in standard mode.";
+                ReportError(CE_Failure, CPLE_AppDefined, "%s", err.c_str());
+                return false;
+            }
+
+        gdal::viewshed::Viewshed oViewshed(m_opts);
         const bool bSuccess = oViewshed.run(
             GDALRasterBand::ToHandle(
                 m_inputDataset.GetDatasetRef()->GetRasterBand(m_band)),

apps/gdalalg_raster_viewshed.h

@@ -14,6 +14,7 @@
 #define GDALALG_RASTER_VIEWSHED_INCLUDED
 
 #include "gdalalgorithm.h"
+#include "viewshed/viewshed_types.h"
 
 //! @cond Doxygen_Suppress
 
@@ -39,25 +40,16 @@ class GDALRasterViewshedAlgorithm final : public GDALAlgorithm
     std::vector<std::string> m_openOptions{};
     std::vector<std::string> m_inputFormats{};
 
-    std::string m_format{};
     GDALArgDatasetValue m_outputDataset{};
     std::vector<std::string> m_creationOptions{};
     bool m_overwrite = false;
 
     std::vector<double> m_observerPos{};
-    double m_targetHeight = 0;
+    gdal::viewshed::Options m_opts;
 
     std::string m_outputMode = "normal";
-
     int m_band = 1;
-    double m_maxDistance = 0;
-    double m_curveCoefficient = 0.85714;
-    int m_observerSpacing = 10;
     int m_numThreads = 3;
-    int m_dstNoData = -1;
-    int m_visibleVal = 255;
-    int m_invisibleVal = 0;
-    int m_outOfRangeVal = 0;
 
     // Work variables
     std::string m_numThreadsStr{};