Merge pull request #11531 from rouault/fix_11526

gdal raster edit/reproject: use --bbox instead of --extent
OSGeo · Dec 21, 2024 · 4e36354 · 4e36354
2 parents 931c095 + 5c13e6d
commit 4e36354
Show file tree

Hide file tree

Showing 13 changed files with 81 additions and 106 deletions.
diff --git a/apps/gdalalg_raster_edit.cpp b/apps/gdalalg_raster_edit.cpp
@@ -44,29 +44,7 @@ GDALRasterEditAlgorithm::GDALRasterEditAlgorithm(bool standaloneStep)
         .AddHiddenAlias("a_srs")
         .SetIsCRSArg(/*noneAllowed=*/true);
 
-    {
-        auto &arg =
-            AddArg("extent", 0, _("Extent as xmin,ymin,xmax,ymax"), &m_extent)
-                .SetRepeatedArgAllowed(false)
-                .SetMinCount(4)
-                .SetMaxCount(4)
-                .SetDisplayHintAboutRepetition(false);
-        arg.AddValidationAction(
-            [&arg]()
-            {
-                const auto &val = arg.Get<std::vector<double>>();
-                CPLAssert(val.size() == 4);
-                if (!(val[0] <= val[2]) || !(val[1] <= val[3]))
-                {
-                    CPLError(
-                        CE_Failure, CPLE_AppDefined,
-                        "Value of 'extent' should be xmin,ymin,xmax,ymax with "
-                        "xmin <= xmax and ymin <= ymax");
-                    return false;
-                }
-                return true;
-            });
-    }
+    AddBBOXArg(&m_bbox);
 
     {
         auto &arg = AddArg("metadata", 0, _("Add/update dataset metadata item"),
@@ -122,7 +100,7 @@ bool GDALRasterEditAlgorithm::RunImpl(GDALProgressFunc pfnProgress,
             }
         }
 
-        if (!m_extent.empty())
+        if (!m_bbox.empty())
         {
             if (poDS->GetRasterXSize() == 0 || poDS->GetRasterYSize() == 0)
             {
@@ -132,12 +110,12 @@ bool GDALRasterEditAlgorithm::RunImpl(GDALProgressFunc pfnProgress,
                 return false;
             }
             double adfGT[6];
-            adfGT[0] = m_extent[0];
-            adfGT[1] = (m_extent[2] - m_extent[0]) / poDS->GetRasterXSize();
+            adfGT[0] = m_bbox[0];
+            adfGT[1] = (m_bbox[2] - m_bbox[0]) / poDS->GetRasterXSize();
             adfGT[2] = 0;
-            adfGT[3] = m_extent[3];
+            adfGT[3] = m_bbox[3];
             adfGT[4] = 0;
-            adfGT[5] = -(m_extent[3] - m_extent[1]) / poDS->GetRasterYSize();
+            adfGT[5] = -(m_bbox[3] - m_bbox[1]) / poDS->GetRasterYSize();
             if (poDS->SetGeoTransform(adfGT) != CE_None)
             {
                 ReportError(CE_Failure, CPLE_AppDefined,
@@ -193,15 +171,13 @@ bool GDALRasterEditAlgorithm::RunStep(GDALProgressFunc, void *)
         aosOptions.AddString("-a_srs");
         aosOptions.AddString(m_overrideCrs.c_str());
     }
-    if (!m_extent.empty())
+    if (!m_bbox.empty())
     {
         aosOptions.AddString("-a_ullr");
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[0]));  // upper-left X
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[3]));  // upper-left Y
-        aosOptions.AddString(
-            CPLSPrintf("%.17g", m_extent[2]));  // lower-right X
-        aosOptions.AddString(
-            CPLSPrintf("%.17g", m_extent[1]));  // lower-right Y
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[0]));  // upper-left X
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[3]));  // upper-left Y
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[2]));  // lower-right X
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[1]));  // lower-right Y
     }
 
     for (const auto &val : m_metadata)

diff --git a/apps/gdalalg_raster_edit.h b/apps/gdalalg_raster_edit.h
@@ -42,7 +42,7 @@ class GDALRasterEditAlgorithm /* non final */
 
     GDALArgDatasetValue m_dataset{};  // standalone mode only
     std::string m_overrideCrs{};
-    std::vector<double> m_extent{};
+    std::vector<double> m_bbox{};
     std::vector<std::string> m_metadata{};
     std::vector<std::string> m_unsetMetadata{};
 };

diff --git a/apps/gdalalg_raster_reproject.cpp b/apps/gdalalg_raster_reproject.cpp
@@ -62,28 +62,7 @@ GDALRasterReprojectAlgorithm::GDALRasterReprojectAlgorithm(bool standaloneStep)
             return true;
         });
 
-    auto &extentArg =
-        AddArg("extent", 0, _("Target extent (in destination CRS units)"),
-               &m_extent)
-            .SetMinCount(4)
-            .SetMaxCount(4)
-            .SetRepeatedArgAllowed(false)
-            .SetDisplayHintAboutRepetition(false)
-            .SetMetaVar("<xmin>,<ymin>,<xmax>,<ymax>");
-    extentArg.AddValidationAction(
-        [&extentArg]()
-        {
-            const auto &val = extentArg.Get<std::vector<double>>();
-            CPLAssert(val.size() == 4);
-            if (!(val[0] <= val[2]) || !(val[1] <= val[3]))
-            {
-                CPLError(CE_Failure, CPLE_AppDefined,
-                         "Value of 'extent' should be xmin,ymin,xmax,ymax with "
-                         "xmin <= xmax and ymin <= ymax");
-                return false;
-            }
-            return true;
-        });
+    AddBBOXArg(&m_bbox, _("Target bounding box (in destination CRS units)"));
 
     AddArg("target-aligned-pixels", 0,
            _("Round target extent to target resolution"),
@@ -125,13 +104,13 @@ bool GDALRasterReprojectAlgorithm::RunStep(GDALProgressFunc, void *)
         aosOptions.AddString(CPLSPrintf("%.17g", m_resolution[0]));
         aosOptions.AddString(CPLSPrintf("%.17g", m_resolution[1]));
     }
-    if (!m_extent.empty())
+    if (!m_bbox.empty())
     {
         aosOptions.AddString("-te");
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[0]));
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[1]));
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[2]));
-        aosOptions.AddString(CPLSPrintf("%.17g", m_extent[3]));
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[0]));
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[1]));
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[2]));
+        aosOptions.AddString(CPLSPrintf("%.17g", m_bbox[3]));
     }
     if (m_targetAlignedPixels)
     {

diff --git a/apps/gdalalg_raster_reproject.h b/apps/gdalalg_raster_reproject.h
@@ -44,7 +44,7 @@ class GDALRasterReprojectAlgorithm /* non final */
     std::string m_dstCrs{};
     std::string m_resampling{};
     std::vector<double> m_resolution{};
-    std::vector<double> m_extent{};
+    std::vector<double> m_bbox{};
     bool m_targetAlignedPixels = false;
 };
 

diff --git a/apps/gdalalg_vector_filter.cpp b/apps/gdalalg_vector_filter.cpp
@@ -29,26 +29,7 @@ GDALVectorFilterAlgorithm::GDALVectorFilterAlgorithm(bool standaloneStep)
     : GDALVectorPipelineStepAlgorithm(NAME, DESCRIPTION, HELP_URL,
                                       standaloneStep)
 {
-    auto &arg =
-        AddArg("bbox", 0, _("Bounding box as xmin,ymin,xmax,ymax"), &m_bbox)
-            .SetRepeatedArgAllowed(false)
-            .SetMinCount(4)
-            .SetMaxCount(4)
-            .SetDisplayHintAboutRepetition(false);
-    arg.AddValidationAction(
-        [&arg]()
-        {
-            const auto &val = arg.Get<std::vector<double>>();
-            CPLAssert(val.size() == 4);
-            if (!(val[0] <= val[2]) || !(val[1] <= val[3]))
-            {
-                CPLError(CE_Failure, CPLE_AppDefined,
-                         "Value of 'bbox' should be xmin,ymin,xmax,ymax with "
-                         "xmin <= xmax and ymin <= ymax");
-                return false;
-            }
-            return true;
-        });
+    AddBBOXArg(&m_bbox);
 }
 
 /************************************************************************/

diff --git a/autotest/cpp/test_gdal_algorithm.cpp b/autotest/cpp/test_gdal_algorithm.cpp
@@ -3106,7 +3106,7 @@ TEST_F(test_gdal_algorithm, raster_edit_failures_dataset_0_0)
     ASSERT_NE(datasetArg, nullptr);
     datasetArg->Get<GDALArgDatasetValue>().Set(std::make_unique<MyDataset>());
 
-    auto extentArg = edit->GetArg("extent");
+    auto extentArg = edit->GetArg("bbox");
     ASSERT_NE(extentArg, nullptr);
     extentArg->Set(std::vector<double>{2, 49, 3, 50});
 
@@ -3219,7 +3219,7 @@ TEST_F(test_gdal_algorithm, raster_edit_failures_set_geo_transform)
     ASSERT_NE(datasetArg, nullptr);
     datasetArg->Get<GDALArgDatasetValue>().Set(std::make_unique<MyDataset>());
 
-    auto extentArg = edit->GetArg("extent");
+    auto extentArg = edit->GetArg("bbox");
     ASSERT_NE(extentArg, nullptr);
     extentArg->Set(std::vector<double>{2, 49, 3, 50});
 

diff --git a/autotest/utilities/test_gdalalg_raster_edit.py b/autotest/utilities/test_gdalalg_raster_edit.py
@@ -69,15 +69,15 @@ def test_gdalalg_raster_edit_crs_none(tmp_vsimem):
         assert ds.GetSpatialRef() is None
 
 
-def test_gdalalg_raster_edit_extent(tmp_vsimem):
+def test_gdalalg_raster_edit_bbox(tmp_vsimem):
 
     tmp_filename = str(tmp_vsimem / "tmp.tif")
     gdal.FileFromMemBuffer(tmp_filename, open("../gcore/data/byte.tif", "rb").read())
 
     pipeline = get_edit_alg()
     assert pipeline.ParseRunAndFinalize(
         [
-            "--extent=1,2,10,200",
+            "--bbox=1,2,10,200",
             tmp_filename,
         ]
     )
@@ -86,19 +86,19 @@ def test_gdalalg_raster_edit_extent(tmp_vsimem):
         assert ds.GetGeoTransform() == pytest.approx((1.0, 0.45, 0.0, 200.0, 0.0, -9.9))
 
 
-def test_gdalalg_raster_edit_extent_invalid(tmp_vsimem):
+def test_gdalalg_raster_edit_bbox_invalid(tmp_vsimem):
 
     tmp_filename = str(tmp_vsimem / "tmp.tif")
     gdal.FileFromMemBuffer(tmp_filename, open("../gcore/data/byte.tif", "rb").read())
 
     pipeline = get_edit_alg()
     with pytest.raises(
         Exception,
-        match="Value of 'extent' should be xmin,ymin,xmax,ymax with xmin <= xmax and ymin <= ymax",
+        match="Value of 'bbox' should be xmin,ymin,xmax,ymax with xmin <= xmax and ymin <= ymax",
     ):
         pipeline.ParseRunAndFinalize(
             [
-                "--extent=1,200,10,2",
+                "--bbox=1,200,10,2",
                 tmp_filename,
             ]
         )
@@ -189,7 +189,7 @@ def test_gdalalg_raster_pipeline_edit_crs_none(tmp_vsimem):
         assert ds.GetSpatialRef() is None
 
 
-def test_gdalalg_raster_pipeline_edit_extent(tmp_vsimem):
+def test_gdalalg_raster_pipeline_edit_bbox(tmp_vsimem):
 
     out_filename = str(tmp_vsimem / "out.tif")
 
@@ -200,7 +200,7 @@ def test_gdalalg_raster_pipeline_edit_extent(tmp_vsimem):
             "../gcore/data/byte.tif",
             "!",
             "edit",
-            "--extent=1,2,10,200",
+            "--bbox=1,2,10,200",
             "!",
             "write",
             "--overwrite",

diff --git a/autotest/utilities/test_gdalalg_raster_pipeline.py b/autotest/utilities/test_gdalalg_raster_pipeline.py
@@ -458,30 +458,30 @@ def test_gdalalg_raster_pipeline_reproject_no_args(tmp_vsimem):
         assert ds.GetRasterBand(1).Checksum() == 4672
 
 
-def test_gdalalg_raster_pipeline_reproject_invalid_extent(tmp_vsimem):
+def test_gdalalg_raster_pipeline_reproject_invalid_bbox(tmp_vsimem):
 
     out_filename = str(tmp_vsimem / "out.tif")
 
     pipeline = get_pipeline_alg()
     with pytest.raises(
         Exception,
-        match="Value of 'extent' should be xmin,ymin,xmax,ymax with xmin <= xmax and ymin <= ymax",
+        match="Value of 'bbox' should be xmin,ymin,xmax,ymax with xmin <= xmax and ymin <= ymax",
     ):
         pipeline.ParseRunAndFinalize(
             [
                 "read",
                 "../gcore/data/byte.tif",
                 "!",
                 "reproject",
-                "--extent=3,4,2,1",
+                "--bbox=3,4,2,1",
                 "!",
                 "write",
                 out_filename,
             ]
         )
 
 
-def test_gdalalg_raster_pipeline_reproject_extent_arg(tmp_vsimem):
+def test_gdalalg_raster_pipeline_reproject_bbox_arg(tmp_vsimem):
 
     out_filename = str(tmp_vsimem / "out.tif")
 
@@ -494,7 +494,7 @@ def test_gdalalg_raster_pipeline_reproject_extent_arg(tmp_vsimem):
             "reproject",
             "--src-crs=EPSG:32611",
             "--dst-crs=EPSG:4326",
-            "--extent=-117.641,33.89,-117.628,33.9005",
+            "--bbox=-117.641,33.89,-117.628,33.9005",
             "!",
             "write",
             "--overwrite",

diff --git a/doc/source/programs/gdal_raster_edit.rst b/doc/source/programs/gdal_raster_edit.rst
@@ -32,7 +32,7 @@ Synopsis
 
     Options:
       --crs <CRS>               Override CRS (without reprojection)
-      --extent <EXTENT>         Extent as xmin,ymin,xmax,ymax
+      --bbox <EXTENT>           Bounding box as xmin,ymin,xmax,ymax
       --metadata <KEY>=<VALUE>  Add/update dataset metadata item [may be repeated]
       --unset-metadata <KEY>    Remove dataset metadata item [may be repeated]
 
@@ -61,9 +61,11 @@ This subcommand is also available as a potential step of :ref:`gdal_raster_pipel
 
     Note that the spatial extent is also left unchanged.
 
-.. option:: --extent <xmin>,<ymin>,<xmax>,ymax>
+.. option:: --bbox <xmin>,<ymin>,<xmax>,ymax>
 
-    Override the spatial extent, without reprojecting or subsetting.
+    Override the spatial bounding box, in CRS units, without reprojecting or subsetting.
+    'x' is longitude values for geographic CRS and easting for projected CRS.
+    'y' is latitude values for geographic CRS and northing for projected CRS.
 
 .. option:: --metadata <KEY>=<VALUE>
 
@@ -85,11 +87,11 @@ Examples
         $ gdal raster edit --crs=EPSG:32632 my.tif
 
 .. example::
-   :title: Override (without reprojecting or subsetting) the extent of a dataset
+   :title: Override (without reprojecting or subsetting) the bounding box of a dataset
 
    .. code-block:: bash
 
-        $ gdal raster edit --extent=2,49,3,50 my.tif
+        $ gdal raster edit --bbox=2,49,3,50 my.tif
 
 .. example::
    :title: Add a metadata item

diff --git a/doc/source/programs/gdal_raster_pipeline.rst b/doc/source/programs/gdal_raster_pipeline.rst
@@ -57,7 +57,7 @@ Potential steps are:
 
     Options:
       --crs <CRS>                                          Override CRS (without reprojection)
-      --extent <EXTENT>                                    Extent as xmin,ymin,xmax,ymax
+      --bbox <EXTENT>                                      Bounding box as xmin,ymin,xmax,ymax
       --metadata <KEY>=<VALUE>                             Add/update dataset metadata item [may be repeated]
       --unset-metadata <KEY>                               Remove dataset metadata item [may be repeated]
 
@@ -74,7 +74,7 @@ Details for options can be found in :ref:`gdal_raster_edit_subcommand`.
       -d, --dst-crs <DST-CRS>                              Destination CRS
       -r, --resampling <RESAMPLING>                        Resampling method. RESAMPLING=near|bilinear|cubic|cubicspline|lanczos|average|rms|mode|min|max|med|q1|q3|sum
       --resolution <xres>,<yres>                           Target resolution (in destination CRS units)
-      --extent <xmin>,<ymin>,<xmax>,<ymax>                 Target extent (in destination CRS units)
+      --bbox <xmin>,<ymin>,<xmax>,<ymax>                   Target bounding box (in destination CRS units)
       --target-aligned-pixels                              Round target extent to target resolution
 
 Details for options can be found in :ref:`gdal_raster_reproject_subcommand`.

diff --git a/doc/source/programs/gdal_raster_reproject.rst b/doc/source/programs/gdal_raster_reproject.rst
@@ -40,7 +40,7 @@ Synopsis
       -d, --dst-crs <DST-CRS>                              Destination CRS
       -r, --resampling <RESAMPLING>                        Resampling method. RESAMPLING=near|bilinear|cubic|cubicspline|lanczos|average|rms|mode|min|max|med|q1|q3|sum
       --resolution <xres>,<yres>                           Target resolution (in destination CRS units)
-      --extent <xmin>,<ymin>,<xmax>,<ymax>                 Target extent (in destination CRS units)
+      --bbox <xmin>,<ymin>,<xmax>,<ymax>                   Target bounding box (in destination CRS units)
       --target-aligned-pixels                              Round target extent to target resolution
 
     Advanced Options:

diff --git a/gcore/gdalalgorithm.cpp b/gcore/gdalalgorithm.cpp
@@ -2123,6 +2123,39 @@ GDALAlgorithm::AddLayerCreationOptionsArg(std::vector<std::string> *pValue)
     return arg;
 }
 
+/************************************************************************/
+/*                        GDALAlgorithm::AddBBOXArg()                   */
+/************************************************************************/
+
+/** Add bbox=xmin,ymin,xmax,ymax argument. */
+GDALInConstructionAlgorithmArg &
+GDALAlgorithm::AddBBOXArg(std::vector<double> *pValue, const char *helpMessage)
+{
+    auto &arg = AddArg("bbox", 0,
+                       helpMessage ? helpMessage
+                                   : _("Bounding box as xmin,ymin,xmax,ymax"),
+                       pValue)
+                    .SetRepeatedArgAllowed(false)
+                    .SetMinCount(4)
+                    .SetMaxCount(4)
+                    .SetDisplayHintAboutRepetition(false);
+    arg.AddValidationAction(
+        [&arg]()
+        {
+            const auto &val = arg.Get<std::vector<double>>();
+            CPLAssert(val.size() == 4);
+            if (!(val[0] <= val[2]) || !(val[1] <= val[3]))
+            {
+                CPLError(CE_Failure, CPLE_AppDefined,
+                         "Value of 'bbox' should be xmin,ymin,xmax,ymax with "
+                         "xmin <= xmax and ymin <= ymax");
+                return false;
+            }
+            return true;
+        });
+    return arg;
+}
+
 /************************************************************************/
 /*                  GDALAlgorithm::AddProgressArg()                     */
 /************************************************************************/