From 61101b28736ef1e8402b22f7a22ca6cc14ac46b8 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 22 Jul 2024 17:39:53 +0200 Subject: [PATCH 01/14] GDALCopyWords(): Fix double->uint64 when input value > UINT64_MAX that was wrongly converted to 0 --- autotest/cpp/testcopywords.cpp | 145 +++++++++++++++++++++++++++++++++ gcore/gdal_priv_templates.hpp | 80 +++++++++++++----- 2 files changed, 203 insertions(+), 22 deletions(-) diff --git a/autotest/cpp/testcopywords.cpp b/autotest/cpp/testcopywords.cpp index 7e00bf98946d..d145098b0d56 100644 --- a/autotest/cpp/testcopywords.cpp +++ b/autotest/cpp/testcopywords.cpp @@ -32,6 +32,7 @@ #include #include +#include #include "gtest_include.h" @@ -594,6 +595,150 @@ TEST_F(TestCopyWords, GDT_Float32and64) FROM_R(intype, CST_5000000000, GDT_CFloat64, CST_5000000000); FROM_R(intype, -CST_5000000000, GDT_CFloat64, -CST_5000000000); } + + // Float32 to Int64 + { + float in_value = std::numeric_limits::quiet_NaN(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + float in_value = -std::numeric_limits::infinity(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MIN); + } + + { + float in_value = -std::numeric_limits::max(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MIN); + } + + { + float in_value = std::numeric_limits::max(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MAX); + } + + { + float in_value = std::numeric_limits::infinity(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MAX); + } + + // Float64 to Int64 + { + double in_value = std::numeric_limits::quiet_NaN(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + double in_value = -std::numeric_limits::infinity(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MIN); + } + + { + double in_value = -std::numeric_limits::max(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MIN); + } + + { + double in_value = std::numeric_limits::max(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MAX); + } + + { + double in_value = std::numeric_limits::infinity(); + int64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_Int64, 0, 1); + EXPECT_EQ(out_value, INT64_MAX); + } + + // Float32 to UInt64 + { + float in_value = std::numeric_limits::quiet_NaN(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + float in_value = -std::numeric_limits::infinity(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + float in_value = -std::numeric_limits::max(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + float in_value = std::numeric_limits::max(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, UINT64_MAX); + } + + { + float in_value = std::numeric_limits::infinity(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float32, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, UINT64_MAX); + } + + // Float64 to UInt64 + { + double in_value = -std::numeric_limits::quiet_NaN(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + double in_value = -std::numeric_limits::infinity(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + double in_value = -std::numeric_limits::max(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, 0); + } + + { + double in_value = std::numeric_limits::max(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, UINT64_MAX); + } + + { + double in_value = std::numeric_limits::infinity(); + uint64_t out_value = 0; + GDALCopyWords(&in_value, GDT_Float64, 0, &out_value, GDT_UInt64, 0, 1); + EXPECT_EQ(out_value, UINT64_MAX); + } } TEST_F(TestCopyWords, GDT_CInt16) diff --git a/gcore/gdal_priv_templates.hpp b/gcore/gdal_priv_templates.hpp index dcb2ceb9c8c0..dd41417855a3 100644 --- a/gcore/gdal_priv_templates.hpp +++ b/gcore/gdal_priv_templates.hpp @@ -371,16 +371,45 @@ template <> struct sGDALCopyWord { static inline void f(const double dfValueIn, std::int64_t &nValueOut) { - if (CPLIsNan(dfValueIn)) + if (std::isnan(dfValueIn)) { nValueOut = 0; - return; } - double dfMaxVal, dfMinVal; - GDALGetDataLimits(dfMaxVal, dfMinVal); - double dfValue = dfValueIn >= 0.0 ? dfValueIn + 0.5 : dfValueIn - 0.5; - nValueOut = static_cast( - GDALClampValue(dfValue, dfMaxVal, dfMinVal)); + else if (dfValueIn >= + static_cast(std::numeric_limits::max())) + { + nValueOut = std::numeric_limits::max(); + } + else if (dfValueIn <= + static_cast(std::numeric_limits::min())) + { + nValueOut = std::numeric_limits::min(); + } + else + { + nValueOut = static_cast( + dfValueIn > 0.0f ? dfValueIn + 0.5f : dfValueIn - 0.5f); + } + } +}; + +template <> struct sGDALCopyWord +{ + static inline void f(const double dfValueIn, std::uint64_t &nValueOut) + { + if (!(dfValueIn > 0)) + { + nValueOut = 0; + } + else if (dfValueIn > + static_cast(std::numeric_limits::max())) + { + nValueOut = std::numeric_limits::max(); + } + else + { + nValueOut = static_cast(dfValueIn + 0.5); + } } }; @@ -424,7 +453,12 @@ template <> struct sGDALCopyWord { static inline void f(const float fValueIn, int &nValueOut) { - if (fValueIn >= static_cast(std::numeric_limits::max())) + if (std::isnan(fValueIn)) + { + nValueOut = 0; + } + else if (fValueIn >= + static_cast(std::numeric_limits::max())) { nValueOut = std::numeric_limits::max(); } @@ -447,15 +481,14 @@ template <> struct sGDALCopyWord { static inline void f(const float fValueIn, unsigned int &nValueOut) { - if (fValueIn >= - static_cast(std::numeric_limits::max())) + if (!(fValueIn > 0)) { - nValueOut = std::numeric_limits::max(); + nValueOut = 0; } - else if (fValueIn <= - static_cast(std::numeric_limits::min())) + else if (fValueIn >= + static_cast(std::numeric_limits::max())) { - nValueOut = std::numeric_limits::min(); + nValueOut = std::numeric_limits::max(); } else { @@ -470,8 +503,12 @@ template <> struct sGDALCopyWord { static inline void f(const float fValueIn, std::int64_t &nValueOut) { - if (fValueIn >= - static_cast(std::numeric_limits::max())) + if (std::isnan(fValueIn)) + { + nValueOut = 0; + } + else if (fValueIn >= + static_cast(std::numeric_limits::max())) { nValueOut = std::numeric_limits::max(); } @@ -494,15 +531,14 @@ template <> struct sGDALCopyWord { static inline void f(const float fValueIn, std::uint64_t &nValueOut) { - if (fValueIn >= - static_cast(std::numeric_limits::max())) + if (!(fValueIn > 0)) { - nValueOut = std::numeric_limits::max(); + nValueOut = 0; } - else if (fValueIn <= - static_cast(std::numeric_limits::min())) + else if (fValueIn >= + static_cast(std::numeric_limits::max())) { - nValueOut = std::numeric_limits::min(); + nValueOut = std::numeric_limits::max(); } else { From a61c346f9ead92152420ecbf504b5653c3c5bece Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 23 Jul 2024 17:54:57 +0200 Subject: [PATCH 02/14] autotest/cpp/CMakeLists.txt: move dependency to gdal_plugins to test-unit --- autotest/cpp/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/autotest/cpp/CMakeLists.txt b/autotest/cpp/CMakeLists.txt index cf5410d1c8d2..74f10ca99290 100644 --- a/autotest/cpp/CMakeLists.txt +++ b/autotest/cpp/CMakeLists.txt @@ -103,7 +103,6 @@ target_link_libraries(gdal_unit_test PRIVATE $ Date: Mon, 22 Jul 2024 14:08:46 +0200 Subject: [PATCH 03/14] GDALRasterBand: add convenience ReadRaster() methods ```cpp /** Read a region of image data for this band. * * This is a slightly more convenient alternative to GDALRasterBand::RasterIO() * for common use cases, like reading a whole band. * It infers the GDAL data type of the buffer from the C/C++ type of the buffer. * This template is instantiated for the following types: [u?]int[8|16|32|64]_t, * float, double, std::complex. * * To read a whole raster (assuming it fits into memory), as a vector of double: * \code std::vector myArray; if (poBand->ReadRaster(myArray) == CE_None) { // do something } \endcode * * To read 128x128 pixels starting at (col=12, line=24) as a vector of double: * \code{.cpp} std::vector myArray; if (poBand->ReadRaster(myArray, 12, 24, 128, 128) == CE_None) { // do something } \endcode * ``` --- autotest/cpp/test_gdal.cpp | 323 ++++++++++++++++++++++++ gcore/gdal_priv.h | 89 +++++++ gcore/gdalrasterband.cpp | 491 +++++++++++++++++++++++++++++++++++++ 3 files changed, 903 insertions(+) diff --git a/autotest/cpp/test_gdal.cpp b/autotest/cpp/test_gdal.cpp index 8e8af2c85dd3..be310432af49 100644 --- a/autotest/cpp/test_gdal.cpp +++ b/autotest/cpp/test_gdal.cpp @@ -4021,4 +4021,327 @@ TEST_F(test_gdal, gdal_gcp_class) } } +// Test GDALRasterBand::ReadRaster +TEST_F(test_gdal, ReadRaster) +{ + GDALDatasetUniquePtr poDS(GDALDriver::FromHandle(GDALGetDriverByName("MEM")) + ->Create("", 2, 3, 1, GDT_Float64, nullptr)); + std::array buffer = { + -1e300, -1, ////////////////////////////////////////////// + 1, 128, ////////////////////////////////////////////// + 32768, 1e300, ////////////////////////////////////////////// + }; + GDALRasterIOExtraArg sExtraArg; + INIT_RASTERIO_EXTRA_ARG(sExtraArg); + EXPECT_EQ(poDS->GetRasterBand(1)->RasterIO( + GF_Write, 0, 0, 2, 3, buffer.data(), 2, 3, GDT_Float64, + sizeof(double), 2 * sizeof(double), &sExtraArg), + CE_None); + + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = std::vector{0, 0, 1, 128, 255, 255}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 0, 0, 2, 3, 2, 3), + CE_None); + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 0, 0, 2, 3), CE_None); + EXPECT_EQ(res, expected_res); + +#if __cplusplus >= 202002L + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(std::span(res)), + CE_None); + EXPECT_EQ(res, expected_res); +#endif + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data(), res.size()), + CE_None); + EXPECT_EQ(res, expected_res); + + CPLPushErrorHandler(CPLQuietErrorHandler); + // Too small buffer size + EXPECT_EQ( + poDS->GetRasterBand(1)->ReadRaster(res.data(), res.size() - 1), + CE_Failure); + CPLPopErrorHandler(); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ( + poDS->GetRasterBand(1)->ReadRaster(res.data(), 0, 0, 0, 2, 3, 2, 3), + CE_None); + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data(), 0, 0, 0, 2, 3), + CE_None); + EXPECT_EQ(res, expected_res); + } + + { + std::vector res; + CPLPushErrorHandler(CPLQuietErrorHandler); + // Too large nBufXSize + EXPECT_EQ( + poDS->GetRasterBand(1)->ReadRaster(res, 0, 0, 1, 1, UINT32_MAX, 1), + CE_Failure); + + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data(), UINT32_MAX, 0, + 0, 1, 1, UINT32_MAX, 1), + CE_Failure); + + // Too large nBufYSize + EXPECT_EQ( + poDS->GetRasterBand(1)->ReadRaster(res, 0, 0, 1, 1, 1, UINT32_MAX), + CE_Failure); + + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data(), UINT32_MAX, 0, + 0, 1, 1, 1, UINT32_MAX), + CE_Failure); + + CPLPopErrorHandler(); + } + + { + std::vector res; + CPLPushErrorHandler(CPLQuietErrorHandler); + // Huge nBufXSize x nBufYSize + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 0, 0, 1, 1, INT32_MAX, + INT32_MAX), + CE_Failure); + CPLPopErrorHandler(); + } + + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 1, 2, 1, 1), CE_None); + const auto expected_res = std::vector{1e300}; + EXPECT_EQ(res, expected_res); + } + + { + std::vector res; + CPLPushErrorHandler(CPLQuietErrorHandler); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 1.1, 2.1, 0.9, 0.9), + CE_Failure); + CPLPopErrorHandler(); + + EXPECT_EQ( + poDS->GetRasterBand(1)->ReadRaster(res, 1.1, 2.1, 0.9, 0.9, 1, 1), + CE_None); + const auto expected_res = std::vector{1e300}; + EXPECT_EQ(res, expected_res); + } + + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res, 0.4, 0.5, 1.4, 1.5, 1, + 1, GRIORA_Bilinear), + CE_None); + ASSERT_EQ(res.size(), 1U); + const double expected_res = -8.64198e+298; + EXPECT_NEAR(res[0], expected_res, std::fabs(expected_res) * 1e-6); + } + + // Test int8_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{-128, -1, 1, 127, 127, 127}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test uint16_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{0, 0, 1, 128, 32768, 65535}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data(), res.size()), + CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test int16_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{-32768, -1, 1, 128, 32767, 32767}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + +#if 0 + // Not allowed by C++ standard + // Test complex + { + std::vector> res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = std::vector>{ + -32768, -1, 1, 128, 32767, 32767}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } +#endif + + // Test uint32_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{0, 0, 1, 128, 32768, UINT32_MAX}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test int32_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{INT32_MIN, -1, 1, 128, 32768, INT32_MAX}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + +#if 0 + // Not allowed by C++ standard + // Test complex + { + std::vector> res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = std::vector>{ + INT32_MIN, -1, 1, 128, 32768, INT32_MAX}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } +#endif + + // Test uint64_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{0, 0, 1, 128, 32768, UINT64_MAX}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test int64_t + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{INT64_MIN, -1, 1, 128, 32768, INT64_MAX}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test float + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{-std::numeric_limits::infinity(), + -1.0f, + 1.0f, + 128.0f, + 32768.0f, + std::numeric_limits::infinity()}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0.0f); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test complex + { + std::vector> res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = std::vector>{ + -std::numeric_limits::infinity(), + -1.0f, + 1.0f, + 128.0f, + 32768.0f, + std::numeric_limits::infinity()}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0.0f); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test double + { + std::vector res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector{-1e300, -1, 1, 128, 32768, 1e300}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } + + // Test complex + { + std::vector> res; + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res), CE_None); + const auto expected_res = + std::vector>{-1e300, -1, 1, 128, 32768, 1e300}; + EXPECT_EQ(res, expected_res); + + std::fill(res.begin(), res.end(), 0); + EXPECT_EQ(poDS->GetRasterBand(1)->ReadRaster(res.data()), CE_None); + EXPECT_EQ(res, expected_res); + } +} + } // namespace diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index be27a8645519..ad34720b9949 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -71,12 +71,16 @@ class GDALRelationship; #include #include +#include #include #include #include #include #include #include +#if __cplusplus >= 202002L +#include +#endif #include #include "ogr_core.h" @@ -1694,6 +1698,42 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject GSpacing nLineSpace, GDALRasterIOExtraArg *psExtraArg) CPL_WARN_UNUSED_RESULT; #endif + + template + CPLErr ReadRaster(T *pData, size_t nArrayEltCount = 0, double dfXOff = 0, + double dfYOff = 0, double dfXSize = 0, double dfYSize = 0, + size_t nBufXSize = 0, size_t nBufYSize = 0, + GDALRIOResampleAlg eResampleAlg = GRIORA_NearestNeighbour, + GDALProgressFunc pfnProgress = nullptr, + void *pProgressData = nullptr) const; + + template + CPLErr ReadRaster(std::vector &vData, double dfXOff = 0, + double dfYOff = 0, double dfXSize = 0, double dfYSize = 0, + size_t nBufXSize = 0, size_t nBufYSize = 0, + GDALRIOResampleAlg eResampleAlg = GRIORA_NearestNeighbour, + GDALProgressFunc pfnProgress = nullptr, + void *pProgressData = nullptr) const; + +#if __cplusplus >= 202002L + //! @cond Doxygen_Suppress + template + inline CPLErr + ReadRaster(std::span pData, double dfXOff = 0, double dfYOff = 0, + double dfXSize = 0, double dfYSize = 0, size_t nBufXSize = 0, + size_t nBufYSize = 0, + GDALRIOResampleAlg eResampleAlg = GRIORA_NearestNeighbour, + GDALProgressFunc pfnProgress = nullptr, + void *pProgressData = nullptr) const + { + return ReadRaster(pData.data(), pData.size(), dfXOff, dfYOff, dfXSize, + dfYSize, nBufXSize, nBufYSize, eResampleAlg, + pfnProgress, pProgressData); + } + + //! @endcond +#endif + CPLErr ReadBlock(int nXBlockOff, int nYBlockOff, void *pImage) CPL_WARN_UNUSED_RESULT; @@ -1848,6 +1888,55 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject CPL_DISALLOW_COPY_ASSIGN(GDALRasterBand) }; +//! @cond Doxygen_Suppress +#define GDAL_EXTERN_TEMPLATE_READ_RASTER(T) \ + extern template CPLErr GDALRasterBand::ReadRaster( \ + T * pData, size_t nArrayEltCount, double dfXOff, double dfYOff, \ + double dfXSize, double dfYSize, size_t nBufXSize, size_t nBufYSize, \ + GDALRIOResampleAlg eResampleAlg, GDALProgressFunc pfnProgress, \ + void *pProgressData) const; + +GDAL_EXTERN_TEMPLATE_READ_RASTER(uint8_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(int8_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(uint16_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(int16_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(uint32_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(int32_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(uint64_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(int64_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER(float) +GDAL_EXTERN_TEMPLATE_READ_RASTER(double) +// Not allowed by C++ standard +// GDAL_EXTERN_TEMPLATE_READ_RASTER(std::complex) +// GDAL_EXTERN_TEMPLATE_READ_RASTER(std::complex) +GDAL_EXTERN_TEMPLATE_READ_RASTER(std::complex) +GDAL_EXTERN_TEMPLATE_READ_RASTER(std::complex) + +#define GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(T) \ + extern template CPLErr GDALRasterBand::ReadRaster( \ + std::vector & vData, double dfXOff, double dfYOff, double dfXSize, \ + double dfYSize, size_t nBufXSize, size_t nBufYSize, \ + GDALRIOResampleAlg eResampleAlg, GDALProgressFunc pfnProgress, \ + void *pProgressData) const; + +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(uint8_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(int8_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(uint16_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(int16_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(uint32_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(int32_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(uint64_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(int64_t) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(float) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(double) +// Not allowed by C++ standard +// GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(std::complex) +// GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(std::complex) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(std::complex) +GDAL_EXTERN_TEMPLATE_READ_RASTER_VECTOR(std::complex) + +//! @endcond + //! @cond Doxygen_Suppress /* ******************************************************************** */ /* GDALAllValidMaskBand */ diff --git a/gcore/gdalrasterband.cpp b/gcore/gdalrasterband.cpp index 702f695d1ee3..8a6171bea755 100644 --- a/gcore/gdalrasterband.cpp +++ b/gcore/gdalrasterband.cpp @@ -263,6 +263,13 @@ GDALRasterBand::~GDALRasterBand() * This method is the same as the C GDALRasterIO() or GDALRasterIOEx() * functions. * + * Starting with GDAL 3.10, the GDALRasterBand::ReadRaster() methods may be + * more convenient to use for most common use cases. + * + * As nearly all GDAL methods, this method is *NOT* thread-safe, that is it cannot + * be called on the same GDALRasterBand instance (or another GDALRasterBand + * instance of this dataset) concurrently from several threads. + * * @param eRWFlag Either GF_Read to read a region of data, or GF_Write to * write a region of data. * @@ -307,6 +314,8 @@ GDALRasterBand::~GDALRasterBand() * to one of BILINEAR, CUBIC, CUBICSPLINE, LANCZOS, AVERAGE or MODE. * * @return CE_Failure if the access fails, otherwise CE_None. + * + * @see GDALRasterBand::ReadRaster() */ CPLErr GDALRasterBand::RasterIO(GDALRWFlag eRWFlag, int nXOff, int nYOff, @@ -491,6 +500,488 @@ CPLErr CPL_STDCALL GDALRasterIOEx(GDALRasterBandH hBand, GDALRWFlag eRWFlag, nLineSpace, psExtraArg)); } +/************************************************************************/ +/* GetGDTFromCppType() */ +/************************************************************************/ + +namespace +{ +template struct GetGDTFromCppType; + +#define DEFINE_GetGDTFromCppType(T, eDT) \ + template <> struct GetGDTFromCppType \ + { \ + static constexpr GDALDataType GDT = eDT; \ + } + +DEFINE_GetGDTFromCppType(uint8_t, GDT_Byte); +DEFINE_GetGDTFromCppType(int8_t, GDT_Int8); +DEFINE_GetGDTFromCppType(uint16_t, GDT_UInt16); +DEFINE_GetGDTFromCppType(int16_t, GDT_Int16); +DEFINE_GetGDTFromCppType(uint32_t, GDT_UInt32); +DEFINE_GetGDTFromCppType(int32_t, GDT_Int32); +DEFINE_GetGDTFromCppType(uint64_t, GDT_UInt64); +DEFINE_GetGDTFromCppType(int64_t, GDT_Int64); +DEFINE_GetGDTFromCppType(float, GDT_Float32); +DEFINE_GetGDTFromCppType(double, GDT_Float64); +// Not allowed by C++ standard +//DEFINE_GetGDTFromCppType(std::complex, GDT_CInt16); +//DEFINE_GetGDTFromCppType(std::complex, GDT_CInt32); +DEFINE_GetGDTFromCppType(std::complex, GDT_CFloat32); +DEFINE_GetGDTFromCppType(std::complex, GDT_CFloat64); +} // namespace + +/************************************************************************/ +/* ReadRaster() */ +/************************************************************************/ + +// clang-format off +/** Read a region of image data for this band. + * + * This is a slightly more convenient alternative to GDALRasterBand::RasterIO() + * for common use cases, like reading a whole band. + * It infers the GDAL data type of the buffer from the C/C++ type of the buffer. + * This template is instantiated for the following types: [u?]int[8|16|32|64]_t, + * float, double, std::complex. + * + * When possible prefer the ReadRaster(std::vector& vData, double dfXOff, double dfYOff, double dfXSize, double dfYSize, size_t nBufXSize, size_t nBufYSize, GDALRIOResampleAlg eResampleAlg, GDALProgressFunc pfnProgress, void *pProgressData) const variant that takes a std::vector&, + * and can allocate memory automatically. + * + * To read a whole band (assuming it fits into memory), as an array of double: + * +\code{.cpp} + double* myArray = static_cast( + VSI_MALLOC3_VERBOSE(sizeof(double), poBand->GetXSize(), poBand->GetYSize())); + // TODO: check here that myArray != nullptr + const size_t nArrayEltCount = + static_cast(poBand->GetXSize()) * poBand->GetYSize()); + if (poBand->ReadRaster(myArray, nArrayEltCount) == CE_None) + { + // do something + } + VSIFree(myArray) +\endcode + * + * To read 128x128 pixels starting at (col=12, line=24) as an array of double: + * +\code{.cpp} + double* myArray = static_cast( + VSI_MALLOC3_VERBOSE(sizeof(double), 128, 128)); + // TODO: check here that myArray != nullptr + const size_t nArrayEltCount = 128 * 128; + if (poBand->ReadRaster(myArray, nArrayEltCount, 12, 24, 128, 128) == CE_None) + { + // do something + } + VSIFree(myArray) +\endcode + * + * As nearly all GDAL methods, this method is *NOT* thread-safe, that is it cannot + * be called on the same GDALRasterBand instance (or another GDALRasterBand + * instance of this dataset) concurrently from several threads. + * + * @param[out] pData The buffer into which the data should be written. + * This buffer must contain at least nBufXSize * + * nBufYSize words of type T. It is organized in left to right, + * top to bottom pixel order, and fully packed. + * The type of the buffer does not need to be the one of GetDataType(). The + * method will perform data type translation (with potential rounding, clamping) + * if needed. + * + * @param nArrayEltCount Number of values of pData. If non zero, the method will + * check that it is at least greater or equal to nBufXSize * nBufYSize, and + * return in error if it is not. If set to zero, then pData is trusted to be + * large enough. + * + * @param dfXOff The pixel offset to the top left corner of the region + * of the band to be accessed. This would be zero to start from the left side. + * Defaults to 0. + * + * @param dfYOff The line offset to the top left corner of the region + * of the band to be accessed. This would be zero to start from the top. + * Defaults to 0. + * + * @param dfXSize The width of the region of the band to be accessed in pixels. + * If all of dfXOff, dfYOff, dfXSize and dfYSize are left to their zero default value, + * dfXSize is set to the band width. + * + * @param dfYSize The height of the region of the band to be accessed in lines. + * If all of dfXOff, dfYOff, dfXSize and dfYSize are left to their zero default value, + * dfYSize is set to the band height. + * + * @param nBufXSize the width of the buffer image into which the desired region + * is to be read. If set to zero, and both dfXSize and dfYSize are integer values, + * then nBufXSize is initialized with dfXSize. + * + * @param nBufYSize the height of the buffer image into which the desired region + * is to be read. If set to zero, and both dfXSize and dfYSize are integer values, + * then nBufYSize is initialized with dfYSize. + * + * @param eResampleAlg Resampling algorithm. Defaults to GRIORA_NearestNeighbour. + * + * @param pfnProgress Progress function. May be nullptr. + * + * @param pProgressData User data of pfnProgress. May be nullptr. + * + * @return CE_Failure if the access fails, otherwise CE_None. + * + * @see GDALRasterBand::RasterIO() + * @since GDAL 3.10 + */ +// clang-format on + +template +CPLErr GDALRasterBand::ReadRaster(T *pData, size_t nArrayEltCount, + double dfXOff, double dfYOff, double dfXSize, + double dfYSize, size_t nBufXSize, + size_t nBufYSize, + GDALRIOResampleAlg eResampleAlg, + GDALProgressFunc pfnProgress, + void *pProgressData) const +{ + if (((nBufXSize | nBufYSize) >> 31) != 0) + { + return CE_Failure; + } + + if (dfXOff == 0 && dfYOff == 0 && dfXSize == 0 && dfYSize == 0) + { + dfXSize = nRasterXSize; + dfYSize = nRasterYSize; + } + else if (!(dfXOff >= 0 && dfXOff <= INT_MAX) || + !(dfYOff >= 0 && dfYOff <= INT_MAX) || !(dfXSize >= 0) || + !(dfYSize >= 0) || dfXOff + dfXSize > INT_MAX || + dfYOff + dfYSize > INT_MAX) + { + return CE_Failure; + } + + GDALRasterIOExtraArg sExtraArg; + sExtraArg.nVersion = 1; + sExtraArg.eResampleAlg = eResampleAlg; + sExtraArg.pfnProgress = pfnProgress; + sExtraArg.pProgressData = pProgressData; + sExtraArg.bFloatingPointWindowValidity = true; + sExtraArg.dfXOff = dfXOff; + sExtraArg.dfYOff = dfYOff; + sExtraArg.dfXSize = dfXSize; + sExtraArg.dfYSize = dfYSize; + const int nXOff = static_cast(dfXOff); + const int nYOff = static_cast(dfYOff); + const int nXSize = std::max(1, static_cast(dfXSize + 0.5)); + const int nYSize = std::max(1, static_cast(dfYSize + 0.5)); + if (nBufXSize == 0 && nBufYSize == 0) + { + if (static_cast(dfXSize) == dfXSize && + static_cast(dfYSize) == dfYSize) + { + nBufXSize = static_cast(dfXSize); + nBufYSize = static_cast(dfYSize); + } + else + { + CPLError(CE_Failure, CPLE_AppDefined, + "nBufXSize and nBufYSize must be provided if dfXSize or " + "dfYSize is not an integer value"); + return CE_Failure; + } + } + if (nBufXSize == 0 || nBufYSize == 0) + { + CPLDebug("GDAL", + "RasterIO() skipped for odd window or buffer size.\n" + " Window = (%d,%d)x%dx%d\n" + " Buffer = %dx%d\n", + nXOff, nYOff, nXSize, nYSize, static_cast(nBufXSize), + static_cast(nBufYSize)); + + return CE_None; + } + + if (nArrayEltCount > 0 && nBufXSize > nArrayEltCount / nBufYSize) + { + CPLError(CE_Failure, CPLE_AppDefined, + "Provided array is not large enough"); + return CE_Failure; + } + + constexpr GSpacing nPixelSpace = sizeof(T); + const GSpacing nLineSpace = nPixelSpace * nBufXSize; + constexpr GDALDataType eBufType = GetGDTFromCppType::GDT; + + GDALRasterBand *pThis = const_cast(this); + + const bool bCallLeaveReadWrite = + CPL_TO_BOOL(pThis->EnterReadWrite(GF_Read)); + CPLErr eErr; + if (bForceCachedIO) + eErr = pThis->GDALRasterBand::IRasterIO( + GF_Read, nXOff, nYOff, nXSize, nYSize, pData, + static_cast(nBufXSize), static_cast(nBufYSize), eBufType, + nPixelSpace, nLineSpace, &sExtraArg); + else + eErr = pThis->IRasterIO(GF_Read, nXOff, nYOff, nXSize, nYSize, pData, + static_cast(nBufXSize), + static_cast(nBufYSize), eBufType, + nPixelSpace, nLineSpace, &sExtraArg); + + if (bCallLeaveReadWrite) + pThis->LeaveReadWrite(); + + return eErr; +} + +//! @cond Doxygen_Suppress + +#define INSTANTIATE_READ_RASTER(T) \ + template CPLErr CPL_DLL GDALRasterBand::ReadRaster( \ + T *vData, size_t nArrayEltCount, double dfXOff, double dfYOff, \ + double dfXSize, double dfYSize, size_t nBufXSize, size_t nBufYSize, \ + GDALRIOResampleAlg eResampleAlg, GDALProgressFunc pfnProgress, \ + void *pProgressData) const; + +INSTANTIATE_READ_RASTER(uint8_t) +INSTANTIATE_READ_RASTER(int8_t) +INSTANTIATE_READ_RASTER(uint16_t) +INSTANTIATE_READ_RASTER(int16_t) +INSTANTIATE_READ_RASTER(uint32_t) +INSTANTIATE_READ_RASTER(int32_t) +INSTANTIATE_READ_RASTER(uint64_t) +INSTANTIATE_READ_RASTER(int64_t) +INSTANTIATE_READ_RASTER(float) +INSTANTIATE_READ_RASTER(double) +// Not allowed by C++ standard +// INSTANTIATE_READ_RASTER(std::complex) +// INSTANTIATE_READ_RASTER(std::complex) +INSTANTIATE_READ_RASTER(std::complex) +INSTANTIATE_READ_RASTER(std::complex) + +//! @endcond + +/************************************************************************/ +/* ReadRaster() */ +/************************************************************************/ + +/** Read a region of image data for this band. + * + * This is a slightly more convenient alternative to GDALRasterBand::RasterIO() + * for common use cases, like reading a whole band. + * It infers the GDAL data type of the buffer from the C/C++ type of the buffer. + * This template is instantiated for the following types: [u?]int[8|16|32|64]_t, + * float, double, std::complex. + * + * To read a whole band (assuming it fits into memory), as a vector of double: + * +\code + std::vector myArray; + if (poBand->ReadRaster(myArray) == CE_None) + { + // do something + } +\endcode + * + * To read 128x128 pixels starting at (col=12, line=24) as a vector of double: + * +\code{.cpp} + std::vector myArray; + if (poBand->ReadRaster(myArray, 12, 24, 128, 128) == CE_None) + { + // do something + } +\endcode + * + * As nearly all GDAL methods, this method is *NOT* thread-safe, that is it cannot + * be called on the same GDALRasterBand instance (or another GDALRasterBand + * instance of this dataset) concurrently from several threads. + * + * @param[out] vData The vector into which the data should be written. + * The vector will be resized, if needed, to contain at least nBufXSize * + * nBufYSize values. The values in the vector are organized in left to right, + * top to bottom pixel order, and fully packed. + * The type of the vector does not need to be the one of GetDataType(). The + * method will perform data type translation (with potential rounding, clamping) + * if needed. + * + * @param dfXOff The pixel offset to the top left corner of the region + * of the band to be accessed. This would be zero to start from the left side. + * Defaults to 0. + * + * @param dfYOff The line offset to the top left corner of the region + * of the band to be accessed. This would be zero to start from the top. + * Defaults to 0. + * + * @param dfXSize The width of the region of the band to be accessed in pixels. + * If all of dfXOff, dfYOff, dfXSize and dfYSize are left to their zero default value, + * dfXSize is set to the band width. + * + * @param dfYSize The height of the region of the band to be accessed in lines. + * If all of dfXOff, dfYOff, dfXSize and dfYSize are left to their zero default value, + * dfYSize is set to the band height. + * + * @param nBufXSize the width of the buffer image into which the desired region + * is to be read. If set to zero, and both dfXSize and dfYSize are integer values, + * then nBufXSize is initialized with dfXSize. + * + * @param nBufYSize the height of the buffer image into which the desired region + * is to be read. If set to zero, and both dfXSize and dfYSize are integer values, + * then nBufYSize is initialized with dfYSize. + * + * @param eResampleAlg Resampling algorithm. Defaults to GRIORA_NearestNeighbour. + * + * @param pfnProgress Progress function. May be nullptr. + * + * @param pProgressData User data of pfnProgress. May be nullptr. + * + * @return CE_Failure if the access fails, otherwise CE_None. + * + * @see GDALRasterBand::RasterIO() + * @since GDAL 3.10 + */ +template +CPLErr GDALRasterBand::ReadRaster(std::vector &vData, double dfXOff, + double dfYOff, double dfXSize, double dfYSize, + size_t nBufXSize, size_t nBufYSize, + GDALRIOResampleAlg eResampleAlg, + GDALProgressFunc pfnProgress, + void *pProgressData) const +{ + if (((nBufXSize | nBufYSize) >> 31) != 0) + { + return CE_Failure; + } + + if (dfXOff == 0 && dfYOff == 0 && dfXSize == 0 && dfYSize == 0) + { + dfXSize = nRasterXSize; + dfYSize = nRasterYSize; + } + else if (!(dfXOff >= 0 && dfXOff <= INT_MAX) || + !(dfYOff >= 0 && dfYOff <= INT_MAX) || !(dfXSize >= 0) || + !(dfYSize >= 0) || dfXOff + dfXSize > INT_MAX || + dfYOff + dfYSize > INT_MAX) + { + return CE_Failure; + } + + GDALRasterIOExtraArg sExtraArg; + sExtraArg.nVersion = 1; + sExtraArg.eResampleAlg = eResampleAlg; + sExtraArg.pfnProgress = pfnProgress; + sExtraArg.pProgressData = pProgressData; + sExtraArg.bFloatingPointWindowValidity = true; + sExtraArg.dfXOff = dfXOff; + sExtraArg.dfYOff = dfYOff; + sExtraArg.dfXSize = dfXSize; + sExtraArg.dfYSize = dfYSize; + const int nXOff = static_cast(dfXOff); + const int nYOff = static_cast(dfYOff); + const int nXSize = std::max(1, static_cast(dfXSize + 0.5)); + const int nYSize = std::max(1, static_cast(dfYSize + 0.5)); + if (nBufXSize == 0 && nBufYSize == 0) + { + if (static_cast(dfXSize) == dfXSize && + static_cast(dfYSize) == dfYSize) + { + nBufXSize = static_cast(dfXSize); + nBufYSize = static_cast(dfYSize); + } + else + { + CPLError(CE_Failure, CPLE_AppDefined, + "nBufXSize and nBufYSize must be provided if " + "dfXSize or dfYSize is not an integer value"); + return CE_Failure; + } + } + if (nBufXSize == 0 || nBufYSize == 0) + { + CPLDebug("GDAL", + "RasterIO() skipped for odd window or buffer size.\n" + " Window = (%d,%d)x%dx%d\n" + " Buffer = %dx%d\n", + nXOff, nYOff, nXSize, nYSize, static_cast(nBufXSize), + static_cast(nBufYSize)); + + return CE_None; + } + + if constexpr (SIZEOF_VOIDP < 8) + { + if (nBufXSize > std::numeric_limits::max() / nBufYSize) + { + CPLError(CE_Failure, CPLE_OutOfMemory, "Too large buffer"); + return CE_Failure; + } + } + + if (vData.size() < nBufXSize * nBufYSize) + { + try + { + vData.resize(nBufXSize * nBufYSize); + } + catch (const std::exception &) + { + CPLError(CE_Failure, CPLE_OutOfMemory, "Cannot resize array"); + return CE_Failure; + } + } + + constexpr GSpacing nPixelSpace = sizeof(T); + const GSpacing nLineSpace = nPixelSpace * nBufXSize; + constexpr GDALDataType eBufType = GetGDTFromCppType::GDT; + + GDALRasterBand *pThis = const_cast(this); + + const bool bCallLeaveReadWrite = + CPL_TO_BOOL(pThis->EnterReadWrite(GF_Read)); + + CPLErr eErr; + if (bForceCachedIO) + eErr = pThis->GDALRasterBand::IRasterIO( + GF_Read, nXOff, nYOff, nXSize, nYSize, vData.data(), + static_cast(nBufXSize), static_cast(nBufYSize), eBufType, + nPixelSpace, nLineSpace, &sExtraArg); + else + eErr = pThis->IRasterIO(GF_Read, nXOff, nYOff, nXSize, nYSize, + vData.data(), static_cast(nBufXSize), + static_cast(nBufYSize), eBufType, + nPixelSpace, nLineSpace, &sExtraArg); + + if (bCallLeaveReadWrite) + pThis->LeaveReadWrite(); + + return eErr; +} + +//! @cond Doxygen_Suppress + +#define INSTANTIATE_READ_RASTER_VECTOR(T) \ + template CPLErr CPL_DLL GDALRasterBand::ReadRaster( \ + std::vector &vData, double dfXOff, double dfYOff, double dfXSize, \ + double dfYSize, size_t nBufXSize, size_t nBufYSize, \ + GDALRIOResampleAlg eResampleAlg, GDALProgressFunc pfnProgress, \ + void *pProgressData) const; + +INSTANTIATE_READ_RASTER_VECTOR(uint8_t) +INSTANTIATE_READ_RASTER_VECTOR(int8_t) +INSTANTIATE_READ_RASTER_VECTOR(uint16_t) +INSTANTIATE_READ_RASTER_VECTOR(int16_t) +INSTANTIATE_READ_RASTER_VECTOR(uint32_t) +INSTANTIATE_READ_RASTER_VECTOR(int32_t) +INSTANTIATE_READ_RASTER_VECTOR(uint64_t) +INSTANTIATE_READ_RASTER_VECTOR(int64_t) +INSTANTIATE_READ_RASTER_VECTOR(float) +INSTANTIATE_READ_RASTER_VECTOR(double) +// Not allowed by C++ standard +// INSTANTIATE_READ_RASTER_VECTOR(std::complex) +// INSTANTIATE_READ_RASTER_VECTOR(std::complex) +INSTANTIATE_READ_RASTER_VECTOR(std::complex) +INSTANTIATE_READ_RASTER_VECTOR(std::complex) + +//! @endcond + /************************************************************************/ /* ReadBlock() */ /************************************************************************/ From 9a545987c613289e3da3d38baa9d129573c0d367 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 11 Aug 2024 13:18:06 +0200 Subject: [PATCH 04/14] Multidimensional API: add GDALMDArray::GetMeshGrid() and map it to C GDALMDArrayGetMeshGrid() and Python gdal.MDArray.GetMeshGrid() ```cpp /** Return a list of multidimensional arrays from a list of one-dimensional * arrays. * * This is typically used to transform one-dimensional longitude, latitude * arrays into 2D ones. * * More formally, for one-dimensional arrays x1, x2,..., xn with lengths * Ni=len(xi), returns (N1, N2, ..., Nn) shaped arrays if indexing="ij" or * (N2, N1, ..., Nn) shaped arrays if indexing="xy" with the elements of xi * repeated to fill the matrix along the first dimension for x1, the second * for x2 and so on. * * For example, if x = [1, 2], and y = [3, 4, 5], * GetMeshGrid([x, y], ["INDEXING=xy"]) will return [xm, ym] such that * xm=[[1, 2],[1, 2],[1, 2]] and ym=[[3, 3],[4, 4],[5, 5]], * or more generally xm[any index][i] = x[i] and ym[i][any index]=y[i] * * and * GetMeshGrid([x, y], ["INDEXING=ij"]) will return [xm, ym] such that * xm=[[1, 1, 1],[2, 2, 2]] and ym=[[3, 4, 5],[3, 4, 5]], * or more generally xm[i][any index] = x[i] and ym[any index][i]=y[i] * * The currently supported options are: *
    *
  • INDEXING=xy/ij: Cartesian ("xy", default) or matrix ("ij") indexing of * output. *
    • *
* * This is the same as * numpy.meshgrid() * function. * * This is the same as the C function GDALMDArrayGetMeshGrid() * * @param apoArrays Input arrays * @param papszOptions NULL, or NULL terminated list of options. * * @return an array of coordinate matrices * @since 3.10 */ /* static */ std::vector> GDALMDArray::GetMeshGrid( const std::vector> &apoArrays, CSLConstList papszOptions) ``` --- autotest/gcore/multidim.py | 193 ++++++++++++++++++ gcore/CMakeLists.txt | 1 + gcore/gdal.h | 6 + gcore/gdal_priv.h | 4 + gcore/gdalmultidim.cpp | 75 +++++++ gcore/gdalmultidim_meshgrid.cpp | 340 ++++++++++++++++++++++++++++++++ swig/include/MultiDimensional.i | 12 ++ 7 files changed, 631 insertions(+) create mode 100644 gcore/gdalmultidim_meshgrid.cpp diff --git a/autotest/gcore/multidim.py b/autotest/gcore/multidim.py index 0382144a4a4e..c70fd40ad6dc 100644 --- a/autotest/gcore/multidim.py +++ b/autotest/gcore/multidim.py @@ -29,6 +29,7 @@ ############################################################################### import array +import itertools import json import math @@ -1140,3 +1141,195 @@ def test_multidim_CreateRasterAttributeTableFromMDArrays(): assert rat.GetUsageOfCol(1) == gdal.GFU_PixelCount assert rat.GetColOfUsage(gdal.GFU_PixelCount) == 1 assert rat.GetColOfUsage(gdal.GFU_Min) == -1 + + +@gdaltest.enable_exceptions() +def test_multidim_GetMeshGrid(): + + drv = gdal.GetDriverByName("MEM") + mem_ds = drv.CreateMultiDimensional("myds") + rg = mem_ds.GetRootGroup() + dim2 = rg.CreateDimension("dim2", None, None, 2) + dim3 = rg.CreateDimension("dim3", None, None, 3) + ar2 = rg.CreateMDArray( + "ar2", [dim2], gdal.ExtendedDataType.Create(gdal.GDT_Float64) + ) + ar2_vals = [1, 2] + ar2.SetNoDataValueDouble(0) + ar2.SetUnit("m") + ar2.SetOffset(1) + ar2.SetScale(10) + ar2.CreateAttribute("attr", [], gdal.ExtendedDataType.Create(gdal.GDT_Float64)) + ar2.Write(ar2_vals) + ar3 = rg.CreateMDArray( + "ar3", [dim3], gdal.ExtendedDataType.Create(gdal.GDT_Float32) + ) + ar3_vals = [3, 4, 5] + ar3.Write(ar3_vals) + + with pytest.raises(Exception, match="Only 1-D input arrays are accepted"): + gdal.MDArray.GetMeshGrid( + [ + rg.CreateMDArray( + "2d_array", + [dim2, dim3], + gdal.ExtendedDataType.Create(gdal.GDT_Float64), + ) + ] + ) + + with pytest.raises(Exception, match="Only INDEXING=xy or ij is accepted"): + gdal.MDArray.GetMeshGrid([ar2, ar3], ["INDEXING=unsupported"]) + + ret = gdal.MDArray.GetMeshGrid([]) + assert len(ret) == 0 + + ret = gdal.MDArray.GetMeshGrid([ar2, ar3], ["INDEXING=IJ"]) + assert len(ret) == 2 + + assert ret[0].GetDataType().GetNumericDataType() == gdal.GDT_Float64 + assert ret[0].GetDimensionCount() == 2 + assert ret[0].GetDimensions()[0].GetSize() == dim2.GetSize() + assert ret[0].GetDimensions()[1].GetSize() == dim3.GetSize() + assert ret[0].GetNoDataValueAsDouble() == 0 + assert ret[0].GetUnit() == "m" + assert ret[0].GetOffset() == 1 + assert ret[0].GetScale() == 10 + assert len(ret[0].GetAttributes()) == 1 + assert ret[0].GetAttribute("attr") + with pytest.raises(Exception, match="Cannot cache an array with an empty filename"): + ret[0].Cache() # Test GetFilename() + with pytest.raises( + Exception, + match="Write operation not permitted on dataset opened in read-only mode", + ): + ret[0].AsClassicDataset(0, 1).WriteRaster( + 0, 0, 2, 3, array.array("d", [0] * 6) + ) # Test IsWritable() + + assert ret[1].GetDataType().GetNumericDataType() == gdal.GDT_Float32 + assert ret[1].GetDimensionCount() == 2 + assert ret[1].GetDimensions()[0].GetSize() == dim2.GetSize() + assert ret[1].GetDimensions()[1].GetSize() == dim3.GetSize() + + assert ret[0].Read() == array.array( + "d", + list(itertools.chain.from_iterable([[v] * dim3.GetSize() for v in ar2_vals])), + ) + assert ret[1].Read() == array.array("f", ar3_vals * dim2.GetSize()) + + # Check interoperability with numpy.meshgrid() + try: + import numpy as np + + from osgeo import gdal_array # NOQA + + has_numpy = True + except ImportError: + has_numpy = False + + if has_numpy: + xv, yv = np.meshgrid(ar2.ReadAsArray(), ar3.ReadAsArray(), indexing="ij") + assert np.all(xv == ret[0].ReadAsArray()) + assert np.all(yv == ret[1].ReadAsArray()) + + ret = gdal.MDArray.GetMeshGrid([ar2, ar3], ["INDEXING=XY"]) + assert len(ret) == 2 + assert ret[0].GetDataType().GetNumericDataType() == gdal.GDT_Float64 + assert ret[0].GetDimensionCount() == 2 + assert ret[0].GetDimensions()[0].GetSize() == dim3.GetSize() + assert ret[0].GetDimensions()[1].GetSize() == dim2.GetSize() + assert ret[1].GetDataType().GetNumericDataType() == gdal.GDT_Float32 + assert ret[1].GetDimensionCount() == 2 + assert ret[1].GetDimensions()[0].GetSize() == dim3.GetSize() + assert ret[1].GetDimensions()[1].GetSize() == dim2.GetSize() + + assert ret[0].Read() == array.array("d", ar2_vals * dim3.GetSize()) + assert ret[1].Read() == array.array( + "f", + list(itertools.chain.from_iterable([[v] * dim2.GetSize() for v in ar3_vals])), + ) + + # Check interoperability with numpy.meshgrid() + if has_numpy: + xv, yv = np.meshgrid(ar2.ReadAsArray(), ar3.ReadAsArray(), indexing="xy") + assert np.all(xv == ret[0].ReadAsArray()) + assert np.all(yv == ret[1].ReadAsArray()) + + # Test 3D + + dim4 = rg.CreateDimension("dim4", None, None, 4) + ar4 = rg.CreateMDArray("ar4", [dim4], gdal.ExtendedDataType.CreateString()) + ar4_vals = ["a", "bc", "def", "ghij"] + ar4.Write(ar4_vals) + + ret = gdal.MDArray.GetMeshGrid([ar2, ar3, ar4], ["INDEXING=IJ"]) + assert len(ret) == 3 + assert ret[0].GetDimensionCount() == 3 + assert ret[0].GetDimensions()[0].GetSize() == dim2.GetSize() + assert ret[0].GetDimensions()[1].GetSize() == dim3.GetSize() + assert ret[0].GetDimensions()[2].GetSize() == dim4.GetSize() + + # print(ret[0].ReadAsArray()) + # print(ret[1].ReadAsArray()) + # print(ret[2].Read()) + + assert ret[0].Read() == array.array( + "d", + list( + itertools.chain.from_iterable( + [[v] * (dim3.GetSize() * dim4.GetSize()) for v in ar2_vals] + ) + ), + ) + assert ret[1].Read() == array.array( + "f", + list(itertools.chain.from_iterable([[v] * dim4.GetSize() for v in ar3_vals])) + * dim2.GetSize(), + ) + assert ret[2].Read() == ar4_vals * (dim2.GetSize() * dim3.GetSize()) + + # Check interoperability with numpy.meshgrid() + if has_numpy: + xv, yv, zv = np.meshgrid( + ar2.ReadAsArray(), ar3.ReadAsArray(), ar4_vals, indexing="ij" + ) + assert np.all(xv == ret[0].ReadAsArray()) + assert np.all(yv == ret[1].ReadAsArray()) + assert np.all(zv == np.array(ret[2].Read()).reshape(zv.shape)) + + ret = gdal.MDArray.GetMeshGrid([ar2, ar3, ar4], ["INDEXING=XY"]) + assert len(ret) == 3 + assert ret[0].GetDimensionCount() == 3 + assert ret[0].GetDimensions()[0].GetSize() == dim3.GetSize() + assert ret[0].GetDimensions()[1].GetSize() == dim2.GetSize() + assert ret[0].GetDimensions()[2].GetSize() == dim4.GetSize() + + # print(ret[0].ReadAsArray()) + # print(ret[1].ReadAsArray()) + # print(ret[2].Read()) + + assert ret[0].Read() == array.array( + "d", + list(itertools.chain.from_iterable([[v] * dim4.GetSize() for v in ar2_vals])) + * dim3.GetSize(), + ) + assert ret[1].Read() == array.array( + "f", + list( + itertools.chain.from_iterable( + [[v] * (dim2.GetSize() * dim4.GetSize()) for v in ar3_vals] + ) + ), + ) + assert ret[2].Read() == ar4_vals * (dim2.GetSize() * dim3.GetSize()) + + # Check interoperability with numpy.meshgrid() + + if has_numpy: + xv, yv, zv = np.meshgrid( + ar2.ReadAsArray(), ar3.ReadAsArray(), ar4_vals, indexing="xy" + ) + assert np.all(xv == ret[0].ReadAsArray()) + assert np.all(yv == ret[1].ReadAsArray()) + assert np.all(zv == np.array(ret[2].Read()).reshape(zv.shape)) diff --git a/gcore/CMakeLists.txt b/gcore/CMakeLists.txt index f8258bbdec4c..7253fd9cea9e 100644 --- a/gcore/CMakeLists.txt +++ b/gcore/CMakeLists.txt @@ -46,6 +46,7 @@ add_library( gdalmultidim.cpp gdalmultidim_gridded.cpp gdalmultidim_gltorthorectification.cpp + gdalmultidim_meshgrid.cpp gdalmultidim_subsetdimension.cpp gdalmultidim_rat.cpp gdalpython.cpp diff --git a/gcore/gdal.h b/gcore/gdal.h index 17aa103df3ff..21d1a4da18d7 100644 --- a/gcore/gdal.h +++ b/gcore/gdal.h @@ -2494,6 +2494,12 @@ GDALMDArrayH CPL_DLL GDALMDArrayGetGridded( GDALMDArrayH CPL_DLL * GDALMDArrayGetCoordinateVariables(GDALMDArrayH hArray, size_t *pnCount) CPL_WARN_UNUSED_RESULT; + +GDALMDArrayH CPL_DLL * +GDALMDArrayGetMeshGrid(const GDALMDArrayH *pahInputArrays, + size_t nCountInputArrays, size_t *pnCountOutputArrays, + CSLConstList papszOptions) CPL_WARN_UNUSED_RESULT; + void CPL_DLL GDALReleaseArrays(GDALMDArrayH *arrays, size_t nCount); int CPL_DLL GDALMDArrayCache(GDALMDArrayH hArray, CSLConstList papszOptions); bool CPL_DLL GDALMDArrayRename(GDALMDArrayH hArray, const char *pszNewName); diff --git a/gcore/gdal_priv.h b/gcore/gdal_priv.h index 736c95d18e2b..af52fca80b62 100644 --- a/gcore/gdal_priv.h +++ b/gcore/gdal_priv.h @@ -3569,6 +3569,10 @@ class CPL_DLL GDALMDArray : virtual public GDALAbstractMDArray, const std::shared_ptr &poYArray = nullptr, CSLConstList papszOptions = nullptr) const; + static std::vector> + GetMeshGrid(const std::vector> &apoArrays, + CSLConstList papszOptions = nullptr); + virtual GDALDataset * AsClassicDataset(size_t iXDim, size_t iYDim, const std::shared_ptr &poRootGroup = nullptr, diff --git a/gcore/gdalmultidim.cpp b/gcore/gdalmultidim.cpp index 8c760b814b69..6d3d550bf2c1 100644 --- a/gcore/gdalmultidim.cpp +++ b/gcore/gdalmultidim.cpp @@ -12629,6 +12629,81 @@ GDALMDArrayH GDALMDArrayGetGridded(GDALMDArrayH hArray, return new GDALMDArrayHS(gridded); } +/************************************************************************/ +/* GDALMDArrayGetMeshGrid() */ +/************************************************************************/ + +/** Return a list of multidimensional arrays from a list of one-dimensional + * arrays. + * + * This is typically used to transform one-dimensional longitude, latitude + * arrays into 2D ones. + * + * More formally, for one-dimensional arrays x1, x2,..., xn with lengths + * Ni=len(xi), returns (N1, N2, ..., Nn) shaped arrays if indexing="ij" or + * (N2, N1, ..., Nn) shaped arrays if indexing="xy" with the elements of xi + * repeated to fill the matrix along the first dimension for x1, the second + * for x2 and so on. + * + * For example, if x = [1, 2], and y = [3, 4, 5], + * GetMeshGrid([x, y], ["INDEXING=xy"]) will return [xm, ym] such that + * xm=[[1, 2],[1, 2],[1, 2]] and ym=[[3, 3],[4, 4],[5, 5]], + * or more generally xm[any index][i] = x[i] and ym[i][any index]=y[i] + * + * and + * GetMeshGrid([x, y], ["INDEXING=ij"]) will return [xm, ym] such that + * xm=[[1, 1, 1],[2, 2, 2]] and ym=[[3, 4, 5],[3, 4, 5]], + * or more generally xm[i][any index] = x[i] and ym[any index][i]=y[i] + * + * The currently supported options are: + *
    + *
  • INDEXING=xy/ij: Cartesian ("xy", default) or matrix ("ij") indexing of + * output. + *
    • + *
+ * + * This is the same as + * numpy.meshgrid() + * function. + * + * The returned array (of arrays) must be freed with GDALReleaseArrays(). + * If only the array itself needs to be freed, CPLFree() should be called + * (and GDALMDArrayRelease() on individual array members). + * + * This is the same as the C++ method GDALMDArray::GetMeshGrid() + * + * @param pahInputArrays Input arrays + * @param nCountInputArrays Number of input arrays + * @param pnCountOutputArrays Pointer to the number of values returned. Must NOT be NULL. + * @param papszOptions NULL, or NULL terminated list of options. + * + * @return an array of *pnCountOutputArrays arrays. + * @since 3.10 + */ +GDALMDArrayH *GDALMDArrayGetMeshGrid(const GDALMDArrayH *pahInputArrays, + size_t nCountInputArrays, + size_t *pnCountOutputArrays, + CSLConstList papszOptions) +{ + VALIDATE_POINTER1(pahInputArrays, __func__, nullptr); + VALIDATE_POINTER1(pnCountOutputArrays, __func__, nullptr); + + std::vector> apoInputArrays; + for (size_t i = 0; i < nCountInputArrays; ++i) + apoInputArrays.push_back(pahInputArrays[i]->m_poImpl); + + const auto apoOutputArrays = + GDALMDArray::GetMeshGrid(apoInputArrays, papszOptions); + auto ret = static_cast( + CPLMalloc(sizeof(GDALMDArrayH) * apoOutputArrays.size())); + for (size_t i = 0; i < apoOutputArrays.size(); i++) + { + ret[i] = new GDALMDArrayHS(apoOutputArrays[i]); + } + *pnCountOutputArrays = apoOutputArrays.size(); + return ret; +} + /************************************************************************/ /* GDALReleaseArrays() */ /************************************************************************/ diff --git a/gcore/gdalmultidim_meshgrid.cpp b/gcore/gdalmultidim_meshgrid.cpp new file mode 100644 index 000000000000..43ae7b0ec2b9 --- /dev/null +++ b/gcore/gdalmultidim_meshgrid.cpp @@ -0,0 +1,340 @@ +/****************************************************************************** + * Name: gdalmultiDim_meshgrid.cpp + * Project: GDAL Core + * Purpose: Return a vector of coordinate matrices from coordinate vectors. + * Author: Even Rouault + * + ****************************************************************************** + * Copyright (c) 2024, Even Rouault + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + ****************************************************************************/ + +#include "gdal_priv.h" + +#include +#include + +/************************************************************************/ +/* GetConcatenatedNames() */ +/************************************************************************/ + +static std::string +GetConcatenatedNames(const std::vector> &apoArrays) +{ + std::string ret; + for (const auto &poArray : apoArrays) + { + if (!ret.empty()) + ret += ", "; + ret += poArray->GetFullName(); + } + return ret; +} + +/************************************************************************/ +/* GDALMDArrayMeshGrid */ +/************************************************************************/ + +class GDALMDArrayMeshGrid final : public GDALMDArray +{ + const std::vector> m_apoArrays; + std::vector> m_apoDims{}; + const size_t m_iDim; + const bool m_bIJIndexing; + + protected: + explicit GDALMDArrayMeshGrid( + const std::vector> &apoArrays, + const std::vector> &apoDims, size_t iDim, + bool bIJIndexing) + : GDALAbstractMDArray(std::string(), + "Mesh grid view of " + + GetConcatenatedNames(apoArrays)), + GDALMDArray(std::string(), + "Mesh grid view of " + GetConcatenatedNames(apoArrays)), + m_apoArrays(apoArrays), m_apoDims(apoDims), m_iDim(iDim), + m_bIJIndexing(bIJIndexing) + { + } + + bool IRead(const GUInt64 *arrayStartIdx, const size_t *count, + const GInt64 *arrayStep, const GPtrDiff_t *bufferStride, + const GDALExtendedDataType &bufferDataType, + void *pDstBuffer) const override; + + public: + static std::shared_ptr + Create(const std::vector> &apoArrays, + size_t iDim, bool bIJIndexing) + { + std::vector> apoDims; + for (size_t i = 0; i < apoArrays.size(); ++i) + { + const size_t iTranslatedDim = (!bIJIndexing && i <= 1) ? 1 - i : i; + apoDims.push_back(apoArrays[iTranslatedDim]->GetDimensions()[0]); + } + auto newAr(std::shared_ptr( + new GDALMDArrayMeshGrid(apoArrays, apoDims, iDim, bIJIndexing))); + newAr->SetSelf(newAr); + return newAr; + } + + bool IsWritable() const override + { + return false; + } + + const std::string &GetFilename() const override + { + return m_apoArrays[m_iDim]->GetFilename(); + } + + const std::vector> & + GetDimensions() const override + { + return m_apoDims; + } + + const GDALExtendedDataType &GetDataType() const override + { + return m_apoArrays[m_iDim]->GetDataType(); + } + + std::shared_ptr + GetAttribute(const std::string &osName) const override + { + return m_apoArrays[m_iDim]->GetAttribute(osName); + } + + std::vector> + GetAttributes(CSLConstList papszOptions = nullptr) const override + { + return m_apoArrays[m_iDim]->GetAttributes(papszOptions); + } + + const std::string &GetUnit() const override + { + return m_apoArrays[m_iDim]->GetUnit(); + } + + const void *GetRawNoDataValue() const override + { + return m_apoArrays[m_iDim]->GetRawNoDataValue(); + } + + double GetOffset(bool *pbHasOffset, + GDALDataType *peStorageType) const override + { + return m_apoArrays[m_iDim]->GetOffset(pbHasOffset, peStorageType); + } + + double GetScale(bool *pbHasScale, + GDALDataType *peStorageType) const override + { + return m_apoArrays[m_iDim]->GetScale(pbHasScale, peStorageType); + } +}; + +/************************************************************************/ +/* IRead() */ +/************************************************************************/ + +bool GDALMDArrayMeshGrid::IRead(const GUInt64 *arrayStartIdx, + const size_t *count, const GInt64 *arrayStep, + const GPtrDiff_t *bufferStride, + const GDALExtendedDataType &bufferDataType, + void *pDstBuffer) const +{ + const size_t nBufferDTSize = bufferDataType.GetSize(); + const size_t iTranslatedDim = + (!m_bIJIndexing && m_iDim <= 1) ? 1 - m_iDim : m_iDim; + std::vector abyTmpData(nBufferDTSize * count[iTranslatedDim]); + const GPtrDiff_t strideOne[] = {1}; + if (!m_apoArrays[m_iDim]->Read(&arrayStartIdx[iTranslatedDim], + &count[iTranslatedDim], + &arrayStep[iTranslatedDim], strideOne, + bufferDataType, abyTmpData.data())) + return false; + + const auto nDims = GetDimensionCount(); + + struct Stack + { + size_t nIters = 0; + GByte *dst_ptr = nullptr; + GPtrDiff_t dst_inc_offset = 0; + }; + + // +1 to avoid -Werror=null-dereference + std::vector stack(nDims + 1); + for (size_t i = 0; i < nDims; i++) + { + stack[i].dst_inc_offset = + static_cast(bufferStride[i] * nBufferDTSize); + } + stack[0].dst_ptr = static_cast(pDstBuffer); + size_t dimIdx = 0; + size_t valIdx = 0; + +lbl_next_depth: + if (dimIdx == nDims - 1) + { + auto nIters = count[dimIdx]; + GByte *dst_ptr = stack[dimIdx].dst_ptr; + if (dimIdx == iTranslatedDim) + { + valIdx = 0; + while (true) + { + GDALExtendedDataType::CopyValue( + &abyTmpData[nBufferDTSize * valIdx], bufferDataType, + dst_ptr, bufferDataType); + if ((--nIters) == 0) + break; + ++valIdx; + dst_ptr += stack[dimIdx].dst_inc_offset; + } + } + else + { + while (true) + { + GDALExtendedDataType::CopyValue( + &abyTmpData[nBufferDTSize * valIdx], bufferDataType, + dst_ptr, bufferDataType); + if ((--nIters) == 0) + break; + dst_ptr += stack[dimIdx].dst_inc_offset; + } + } + } + else + { + if (dimIdx == iTranslatedDim) + valIdx = 0; + stack[dimIdx].nIters = count[dimIdx]; + while (true) + { + dimIdx++; + stack[dimIdx].dst_ptr = stack[dimIdx - 1].dst_ptr; + goto lbl_next_depth; + lbl_return_to_caller: + dimIdx--; + if ((--stack[dimIdx].nIters) == 0) + break; + if (dimIdx == iTranslatedDim) + ++valIdx; + stack[dimIdx].dst_ptr += stack[dimIdx].dst_inc_offset; + } + } + if (dimIdx > 0) + { + goto lbl_return_to_caller; + } + + if (bufferDataType.NeedsFreeDynamicMemory()) + { + for (size_t i = 0; i < count[iTranslatedDim]; ++i) + { + bufferDataType.FreeDynamicMemory(&abyTmpData[i * nBufferDTSize]); + } + } + + return true; +} + +/************************************************************************/ +/* GDALMDArrayGetMeshGrid() */ +/************************************************************************/ + +/** Return a list of multidimensional arrays from a list of one-dimensional + * arrays. + * + * This is typically used to transform one-dimensional longitude, latitude + * arrays into 2D ones. + * + * More formally, for one-dimensional arrays x1, x2,..., xn with lengths + * Ni=len(xi), returns (N1, N2, ..., Nn) shaped arrays if indexing="ij" or + * (N2, N1, ..., Nn) shaped arrays if indexing="xy" with the elements of xi + * repeated to fill the matrix along the first dimension for x1, the second + * for x2 and so on. + * + * For example, if x = [1, 2], and y = [3, 4, 5], + * GetMeshGrid([x, y], ["INDEXING=xy"]) will return [xm, ym] such that + * xm=[[1, 2],[1, 2],[1, 2]] and ym=[[3, 3],[4, 4],[5, 5]], + * or more generally xm[any index][i] = x[i] and ym[i][any index]=y[i] + * + * and + * GetMeshGrid([x, y], ["INDEXING=ij"]) will return [xm, ym] such that + * xm=[[1, 1, 1],[2, 2, 2]] and ym=[[3, 4, 5],[3, 4, 5]], + * or more generally xm[i][any index] = x[i] and ym[any index][i]=y[i] + * + * The currently supported options are: + *
    + *
  • INDEXING=xy/ij: Cartesian ("xy", default) or matrix ("ij") indexing of + * output. + *
    • + *
+ * + * This is the same as + * numpy.meshgrid() + * function. + * + * This is the same as the C function GDALMDArrayGetMeshGrid() + * + * @param apoArrays Input arrays + * @param papszOptions NULL, or NULL terminated list of options. + * + * @return an array of coordinate matrices + * @since 3.10 + */ + +/* static */ std::vector> GDALMDArray::GetMeshGrid( + const std::vector> &apoArrays, + CSLConstList papszOptions) +{ + std::vector> ret; + for (const auto &poArray : apoArrays) + { + if (poArray->GetDimensionCount() != 1) + { + CPLError(CE_Failure, CPLE_NotSupported, + "Only 1-D input arrays are accepted"); + return ret; + } + } + + const char *pszIndexing = + CSLFetchNameValueDef(papszOptions, "INDEXING", "xy"); + if (!EQUAL(pszIndexing, "xy") && !EQUAL(pszIndexing, "ij")) + { + CPLError(CE_Failure, CPLE_NotSupported, + "Only INDEXING=xy or ij is accepted"); + return ret; + } + const bool bIJIndexing = EQUAL(pszIndexing, "ij"); + + for (size_t i = 0; i < apoArrays.size(); ++i) + { + ret.push_back(GDALMDArrayMeshGrid::Create(apoArrays, i, bIJIndexing)); + } + + return ret; +} diff --git a/swig/include/MultiDimensional.i b/swig/include/MultiDimensional.i index cdcd08550fc9..53d6b446578b 100644 --- a/swig/include/MultiDimensional.i +++ b/swig/include/MultiDimensional.i @@ -1160,6 +1160,18 @@ public: %clear OSRSpatialReferenceShadow**; #endif + +#if defined(SWIGPYTHON) +%newobject GetMeshGrid; +%apply (int object_list_count, GDALMDArrayHS **poObjects) {(int nInputArrays, GDALMDArrayHS **ahInputArrays)}; +%apply (GDALMDArrayHS*** parrays, size_t* pnCount) {(GDALMDArrayHS*** outputArrays, size_t* pnCountOutputArrays)}; + static void GetMeshGrid(int nInputArrays, GDALMDArrayHS **ahInputArrays, + GDALMDArrayHS*** outputArrays, size_t* pnCountOutputArrays, char **options = 0) + { + *outputArrays = GDALMDArrayGetMeshGrid(ahInputArrays, nInputArrays, pnCountOutputArrays, options); + } +#endif + bool Cache( char** options = NULL ) { return GDALMDArrayCache(self, options); From 295f54779e688c694f95b5578bec6e40a98e27d0 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sun, 11 Aug 2024 19:16:29 +0200 Subject: [PATCH 05/14] NITF: CreateCopy(): support Blue,Green,Red[,other] color interpretation ordering Fixes #10508 --- autotest/gdrivers/nitf.py | 143 +++++++++++++++++++++++++------------ frmts/nitf/nitfdataset.cpp | 18 ++++- 2 files changed, 113 insertions(+), 48 deletions(-) diff --git a/autotest/gdrivers/nitf.py b/autotest/gdrivers/nitf.py index e211cd829025..9ca57a6a3634 100755 --- a/autotest/gdrivers/nitf.py +++ b/autotest/gdrivers/nitf.py @@ -63,11 +63,6 @@ def setup_and_cleanup(): yield - try: - gdal.GetDriverByName("NITF").Delete("tmp/test_create.ntf") - except RuntimeError: - pass - try: gdal.GetDriverByName("NITF").Delete("tmp/nitf9.ntf") except RuntimeError: @@ -249,21 +244,20 @@ def test_nitf_3(): # Test direction creation of an NITF file. -def nitf_create(creation_options, set_inverted_color_interp=True, createcopy=False): +def nitf_create( + filename, + creation_options, + set_inverted_color_interp=True, + createcopy=False, + nbands=3, +): drv = gdal.GetDriverByName("NITF") - try: - os.remove("tmp/test_create.ntf") - except OSError: - pass - if createcopy: - ds = gdal.GetDriverByName("MEM").Create("", 200, 100, 3, gdal.GDT_Byte) + ds = gdal.GetDriverByName("MEM").Create("", 200, 100, nbands, gdal.GDT_Byte) else: - ds = drv.Create( - "tmp/test_create.ntf", 200, 100, 3, gdal.GDT_Byte, creation_options - ) + ds = drv.Create(filename, 200, 100, nbands, gdal.GDT_Byte, creation_options) ds.SetGeoTransform((100, 0.1, 0.0, 30.0, 0.0, -0.1)) if set_inverted_color_interp: @@ -276,23 +270,27 @@ def nitf_create(creation_options, set_inverted_color_interp=True, createcopy=Fal ds.GetRasterBand(3).SetRasterColorInterpretation(gdal.GCI_BlueBand) my_list = list(range(200)) + list(range(20, 220)) + list(range(30, 230)) - try: - raw_data = array.array("h", my_list).tobytes() - except Exception: - # Python 2 - raw_data = array.array("h", my_list).tostring() + if nbands == 4: + my_list += list(range(40, 240)) + raw_data = array.array("h", my_list).tobytes() for line in range(100): ds.WriteRaster( - 0, line, 200, 1, raw_data, buf_type=gdal.GDT_Int16, band_list=[1, 2, 3] + 0, + line, + 200, + 1, + raw_data, + buf_type=gdal.GDT_Int16, ) assert ds.FlushCache() == gdal.CE_None if createcopy: - ds = drv.CreateCopy("tmp/test_create.ntf", ds, options=creation_options) + ds = drv.CreateCopy(filename, ds, options=creation_options) ds = None + gdal.Unlink(filename + ".aux.xml") ############################################################################### @@ -300,11 +298,12 @@ def nitf_create(creation_options, set_inverted_color_interp=True, createcopy=Fal def nitf_check_created_file( + filename, checksum1, checksum2, checksum3, - filename="tmp/test_create.ntf", set_inverted_color_interp=True, + createcopy=False, ): ds = gdal.Open(filename) @@ -331,6 +330,10 @@ def nitf_check_created_file( ), "geotransform differs from expected" if set_inverted_color_interp: + + if createcopy: + assert ds.GetMetadataItem("NITF_IREP") == "MULTI" + assert ( ds.GetRasterBand(1).GetRasterColorInterpretation() == gdal.GCI_BlueBand ), "Got wrong color interpretation." @@ -343,6 +346,11 @@ def nitf_check_created_file( ds.GetRasterBand(3).GetRasterColorInterpretation() == gdal.GCI_RedBand ), "Got wrong color interpretation." + if ds.RasterCount == 4: + assert ( + ds.GetRasterBand(4).GetRasterColorInterpretation() == gdal.GCI_GrayIndex + ), "Got wrong color interpretation." + ds = None @@ -350,11 +358,29 @@ def nitf_check_created_file( # Test direction creation of an non-compressed NITF file. -def test_nitf_5(): +@pytest.mark.parametrize("createcopy", [False, True]) +@pytest.mark.parametrize("set_inverted_color_interp", [False, True]) +@pytest.mark.parametrize("nbands", [3, 4]) +def test_nitf_5(tmp_path, createcopy, set_inverted_color_interp, nbands): + + filename = str(tmp_path / "test.ntf") - nitf_create(["ICORDS=G"]) + nitf_create( + filename, + ["ICORDS=G"], + set_inverted_color_interp=set_inverted_color_interp, + createcopy=createcopy, + nbands=nbands, + ) - nitf_check_created_file(32498, 42602, 38982) + nitf_check_created_file( + filename, + 32498, + 42602, + 38982, + set_inverted_color_interp=set_inverted_color_interp, + createcopy=createcopy, + ) ############################################################################### @@ -827,11 +853,13 @@ def test_nitf_26(): # Test Create() with IC=NC compression, and multi-blocks -def test_nitf_27(): +def test_nitf_27(tmp_path): - nitf_create(["ICORDS=G", "IC=NC", "BLOCKXSIZE=10", "BLOCKYSIZE=10"]) + filename = str(tmp_path / "test.ntf") - nitf_check_created_file(32498, 42602, 38982) + nitf_create(filename, ["ICORDS=G", "IC=NC", "BLOCKXSIZE=10", "BLOCKYSIZE=10"]) + + nitf_check_created_file(filename, 32498, 42602, 38982) ############################################################################### @@ -839,7 +867,7 @@ def test_nitf_27(): @pytest.mark.require_driver("JP2ECW") -def test_nitf_28_jp2ecw(): +def test_nitf_28_jp2ecw(tmp_path): import ecw @@ -849,9 +877,17 @@ def test_nitf_28_jp2ecw(): # Deregister other potential conflicting JPEG2000 drivers gdaltest.deregister_all_jpeg2000_drivers_but("JP2ECW") try: - nitf_create(["ICORDS=G", "IC=C8", "TARGET=75"], set_inverted_color_interp=False) + filename = str(tmp_path / "test.ntf") + + nitf_create( + filename, + ["ICORDS=G", "IC=C8", "TARGET=75"], + set_inverted_color_interp=False, + ) - nitf_check_created_file(32398, 42502, 38882, set_inverted_color_interp=False) + nitf_check_created_file( + filename, 32398, 42502, 38882, set_inverted_color_interp=False + ) tmpfilename = "/vsimem/nitf_28_jp2ecw.ntf" src_ds = gdal.GetDriverByName("MEM").Create("", 1025, 1025) @@ -879,10 +915,10 @@ def test_nitf_28_jp2mrsid(): gdaltest.deregister_all_jpeg2000_drivers_but("JP2MrSID") nitf_check_created_file( + "data/nitf/test_jp2_ecw33.ntf", 32398, 42502, 38882, - filename="data/nitf/test_jp2_ecw33.ntf", set_inverted_color_interp=False, ) @@ -900,10 +936,10 @@ def test_nitf_28_jp2kak(): gdaltest.deregister_all_jpeg2000_drivers_but("JP2KAK") nitf_check_created_file( + "data/nitf/test_jp2_ecw33.ntf", 32398, 42502, 38882, - filename="data/nitf/test_jp2_ecw33.ntf", set_inverted_color_interp=False, ) @@ -921,10 +957,10 @@ def test_nitf_28_jp2openjpeg(): gdaltest.deregister_all_jpeg2000_drivers_but("JP2OpenJPEG") try: nitf_check_created_file( + "data/nitf/test_jp2_ecw33.ntf", 32398, 42502, 38882, - filename="data/nitf/test_jp2_ecw33.ntf", set_inverted_color_interp=False, ) finally: @@ -936,17 +972,20 @@ def test_nitf_28_jp2openjpeg(): @pytest.mark.require_driver("JP2OpenJPEG") -def test_nitf_28_jp2openjpeg_bis(): +def test_nitf_28_jp2openjpeg_bis(tmp_path): # Deregister other potential conflicting JPEG2000 drivers gdaltest.deregister_all_jpeg2000_drivers_but("JP2OpenJPEG") try: + filename = str(tmp_path / "test.ntf") + nitf_create( + filename, ["ICORDS=G", "IC=C8", "QUALITY=25"], set_inverted_color_interp=False, createcopy=True, ) - ds = gdal.Open("tmp/test_create.ntf") + ds = gdal.Open(filename) assert ds.GetRasterBand(1).Checksum() in (31604, 31741) ds = None @@ -1386,11 +1425,15 @@ def test_nitf_30(): # Verify we can write a file with a custom TRE and read it back properly. -def test_nitf_31(): +def test_nitf_31(tmp_path): + + filename = str(tmp_path / "test.ntf") - nitf_create(["TRE=CUSTOM= Test TRE1\\0MORE", "TRE=TOTEST=SecondTRE", "ICORDS=G"]) + nitf_create( + filename, ["TRE=CUSTOM= Test TRE1\\0MORE", "TRE=TOTEST=SecondTRE", "ICORDS=G"] + ) - ds = gdal.Open("tmp/test_create.ntf") + ds = gdal.Open(filename) md = ds.GetMetadata("TRE") assert len(md) == 2, "Did not get expected TRE count" @@ -1408,27 +1451,31 @@ def test_nitf_31(): ), "Did not get expected TRE contents" ds = None - return nitf_check_created_file(32498, 42602, 38982) + return nitf_check_created_file(filename, 32498, 42602, 38982) ############################################################################### # Test Create() with ICORDS=D -def test_nitf_32(): +def test_nitf_32(tmp_path): - nitf_create(["ICORDS=D"]) + filename = str(tmp_path / "test.ntf") - return nitf_check_created_file(32498, 42602, 38982) + nitf_create(filename, ["ICORDS=D"]) + + return nitf_check_created_file(filename, 32498, 42602, 38982) ############################################################################### # Test Create() with ICORDS=D and a consistent BLOCKA -def test_nitf_33(): +def test_nitf_33(tmp_path): + filename = str(tmp_path / "test.ntf") nitf_create( + filename, [ "ICORDS=D", "BLOCKA_BLOCK_COUNT=01", @@ -1438,10 +1485,10 @@ def test_nitf_33(): "BLOCKA_LRLC_LOC_01=+20.050000+119.950000", "BLOCKA_LRFC_LOC_01=+20.050000+100.050000", "BLOCKA_FRFC_LOC_01=+29.950000+100.050000", - ] + ], ) - return nitf_check_created_file(32498, 42602, 38982) + return nitf_check_created_file(filename, 32498, 42602, 38982) ############################################################################### @@ -2940,6 +2987,7 @@ def test_nitf_72(): src_md = src_md_max_precision src_ds.SetMetadata(src_md, "RPC") + gdal.ErrorReset() gdal.GetDriverByName("NITF").CreateCopy("/vsimem/nitf_72.ntf", src_ds) assert gdal.GetLastErrorMsg() == "", "fail: did not expect warning" @@ -5367,6 +5415,7 @@ def test_nitf_create_three_images_final_uncompressed(): src_ds_8193.GetRasterBand(1).Fill(2) # Write first image segment, reserve space for two other ones and a DES + gdal.ErrorReset() ds = gdal.GetDriverByName("NITF").CreateCopy( "/vsimem/out.ntf", src_ds_2049, options=["NUMI=3", "NUMDES=1"] ) diff --git a/frmts/nitf/nitfdataset.cpp b/frmts/nitf/nitfdataset.cpp index 34ee8afd6dda..c5835ef47244 100644 --- a/frmts/nitf/nitfdataset.cpp +++ b/frmts/nitf/nitfdataset.cpp @@ -4593,7 +4593,6 @@ GDALDataset *NITFDataset::NITFCreateCopy(const char *pszFilename, { if (((poSrcDS->GetRasterCount() == 3 && bJPEG) || (poSrcDS->GetRasterCount() >= 3 && !bJPEG)) && - eType == GDT_Byte && poSrcDS->GetRasterBand(1)->GetColorInterpretation() == GCI_RedBand && poSrcDS->GetRasterBand(2)->GetColorInterpretation() == @@ -4607,6 +4606,23 @@ GDALDataset *NITFDataset::NITFCreateCopy(const char *pszFilename, papszFullOptions = CSLSetNameValue(papszFullOptions, "IREP", "RGB"); } + else if (poSrcDS->GetRasterCount() >= 3 && !bJPEG && + poSrcDS->GetRasterBand(1)->GetColorInterpretation() == + GCI_BlueBand && + poSrcDS->GetRasterBand(2)->GetColorInterpretation() == + GCI_GreenBand && + poSrcDS->GetRasterBand(3)->GetColorInterpretation() == + GCI_RedBand && + CSLFetchNameValue(papszFullOptions, "IREPBAND") == nullptr) + { + papszFullOptions = + CSLSetNameValue(papszFullOptions, "IREP", "MULTI"); + std::string osIREPBAND = "B,G,R"; + for (int i = 4; i <= poSrcDS->GetRasterCount(); ++i) + osIREPBAND += ",M"; + papszFullOptions = CSLSetNameValue(papszFullOptions, "IREPBAND", + osIREPBAND.c_str()); + } else if (poSrcDS->GetRasterCount() == 1 && eType == GDT_Byte && poBand1->GetColorTable() != nullptr) { From 401237424eb34872f74d2c97c4b8ac421c8d0e8e Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 12 Aug 2024 18:40:29 +0200 Subject: [PATCH 06/14] OpenFileGDB: error out explicitly when attempting to create an index on multiple columns Refs #10590 --- autotest/ogr/ogr_openfilegdb_write.py | 45 ++++++++++--------- doc/source/drivers/vector/openfilegdb.rst | 1 + .../openfilegdb/ogropenfilegdbdatasource.cpp | 7 +++ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/autotest/ogr/ogr_openfilegdb_write.py b/autotest/ogr/ogr_openfilegdb_write.py index 08ef72a8221f..7157bcfdc696 100755 --- a/autotest/ogr/ogr_openfilegdb_write.py +++ b/autotest/ogr/ogr_openfilegdb_write.py @@ -1546,6 +1546,7 @@ def test_ogr_openfilegdb_write_spatial_index( ############################################################################### +@gdaltest.enable_exceptions() def test_ogr_openfilegdb_write_attribute_index(tmp_vsimem): dirname = tmp_vsimem / "out.gdb" @@ -1589,31 +1590,35 @@ def test_ogr_openfilegdb_write_attribute_index(tmp_vsimem): f = None # Errors of index creation - with gdal.quiet_errors(): - gdal.ErrorReset() + with pytest.raises( + Exception, match="Invalid index name: cannot be greater than 16 characters" + ): ds.ExecuteSQL("CREATE INDEX this_name_is_wayyyyy_tooo_long ON test(int16)") - assert gdal.GetLastErrorMsg() != "" - gdal.ErrorReset() + with pytest.raises(Exception, match="Invalid layer name: non_existing_layer"): ds.ExecuteSQL("CREATE INDEX idx_int16 ON non_existing_layer(int16)") - assert gdal.GetLastErrorMsg() != "" - gdal.ErrorReset() + with pytest.raises(Exception, match="Cannot find field invalid_field"): ds.ExecuteSQL("CREATE INDEX invalid_field ON test(invalid_field)") - assert gdal.GetLastErrorMsg() != "" - # Reserved keyword - gdal.ErrorReset() + with pytest.raises( + Exception, match="Invalid index name: must not be a reserved keyword" + ): ds.ExecuteSQL("CREATE INDEX SELECT ON test(int16)") - assert gdal.GetLastErrorMsg() != "" - gdal.ErrorReset() + with pytest.raises(Exception, match="Invalid index name: must start with a letter"): ds.ExecuteSQL("CREATE INDEX _starting_by_ ON test(int16)") - assert gdal.GetLastErrorMsg() != "" - gdal.ErrorReset() + with pytest.raises( + Exception, + match="Invalid index name: must contain only alpha numeric character or _", + ): ds.ExecuteSQL("CREATE INDEX a&b ON test(int16)") - assert gdal.GetLastErrorMsg() != "" + + with pytest.raises( + Exception, match="Creation of multiple-column indices is not supported" + ): + ds.ExecuteSQL("CREATE INDEX index_on_two_cols ON test(int16, int32)") # Create indexes gdal.ErrorReset() @@ -1631,20 +1636,18 @@ def test_ogr_openfilegdb_write_attribute_index(tmp_vsimem): fld_defn = ogr.FieldDefn("unindexed", ogr.OFTString) assert lyr.CreateField(fld_defn) == ogr.OGRERR_NONE - with gdal.quiet_errors(): + with pytest.raises(Exception, match="An index with same name already exists"): # Re-using an index name - gdal.ErrorReset() ds.ExecuteSQL("CREATE INDEX idx_int16 ON test(unindexed)") - assert gdal.GetLastErrorMsg() != "" + with pytest.raises(Exception, match="Field int16 has already a registered index"): # Trying to index twice a field - gdal.ErrorReset() ds.ExecuteSQL("CREATE INDEX int16_again ON test(int16)") - assert gdal.GetLastErrorMsg() != "" - gdal.ErrorReset() + with pytest.raises( + Exception, match="Field lower_str has already a registered index" + ): ds.ExecuteSQL("CREATE INDEX lower_str_again ON test(lower_str)") - assert gdal.GetLastErrorMsg() != "" ds = None diff --git a/doc/source/drivers/vector/openfilegdb.rst b/doc/source/drivers/vector/openfilegdb.rst index 6dea6fbda869..f4a73dfbe0a7 100644 --- a/doc/source/drivers/vector/openfilegdb.rst +++ b/doc/source/drivers/vector/openfilegdb.rst @@ -68,6 +68,7 @@ Geodatabases created with ArcGIS 10 or above) The "CREATE INDEX idx_name ON layer_name(field_name)" SQL request can be used to create an attribute index. idx_name must have 16 characters or less, start with a letter and contain only alphanumeric characters or underscore. +Multiple column indices are not supported. The "RECOMPUTE EXTENT ON layer_name" SQL request can be used to trigger an update of the layer extent in layer metadata. This is useful when updating diff --git a/ogr/ogrsf_frmts/openfilegdb/ogropenfilegdbdatasource.cpp b/ogr/ogrsf_frmts/openfilegdb/ogropenfilegdbdatasource.cpp index 9b30d45eb46a..b1832b84d68b 100644 --- a/ogr/ogrsf_frmts/openfilegdb/ogropenfilegdbdatasource.cpp +++ b/ogr/ogrsf_frmts/openfilegdb/ogropenfilegdbdatasource.cpp @@ -1716,6 +1716,13 @@ OGRLayer *OGROpenFileGDBDataSource::ExecuteSQL(const char *pszSQLCommand, const std::string osLayerName = afterOn.substr(0, nOpenParPos); const std::string osExpression = afterOn.substr( nOpenParPos + 1, afterOn.size() - nOpenParPos - 2); + if (osExpression.find(',') != std::string::npos) + { + CPLError( + CE_Failure, CPLE_NotSupported, + "Creation of multiple-column indices is not supported"); + return nullptr; + } auto poLayer = GetLayerByName(osLayerName.c_str()); if (poLayer) { From c73952c3ded2de2648f6a2acb41898df6db49f60 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 12 Aug 2024 19:32:32 +0200 Subject: [PATCH 07/14] Fix various false positive or minor issues raised by Coverity Scan --- frmts/netcdf/netcdfdataset.cpp | 4 +- frmts/netcdf/netcdfmultidim.cpp | 2 +- frmts/netcdf/netcdfvirtual.cpp | 15 +++---- .../arrow_common/ograrrowwriterlayer.hpp | 39 ++++++++----------- .../gpkg/gdalgeopackagerasterband.cpp | 1 + .../gpkg/ogrgeopackagetablelayer.cpp | 6 ++- .../openfilegdb/filegdbtable_freelist.cpp | 3 ++ ogr/ogrsf_frmts/parquet/ogrparquetdriver.cpp | 2 +- port/cpl_worker_thread_pool.cpp | 16 ++++++-- 9 files changed, 51 insertions(+), 37 deletions(-) diff --git a/frmts/netcdf/netcdfdataset.cpp b/frmts/netcdf/netcdfdataset.cpp index ae3dbf3f9e2e..5e5ce8d0cbe4 100644 --- a/frmts/netcdf/netcdfdataset.cpp +++ b/frmts/netcdf/netcdfdataset.cpp @@ -1922,7 +1922,7 @@ void netCDFRasterBand::CreateMetadataFromAttributes() // Get attribute metadata. int nAtt = 0; - nc_inq_varnatts(cdfid, nZId, &nAtt); + NCDF_ERR(nc_inq_varnatts(cdfid, nZId, &nAtt)); for (int i = 0; i < nAtt; i++) { @@ -7210,7 +7210,7 @@ bool netCDFDataset::GrowDim(int nLayerId, int nDimIdToGrow, size_t nNewSize) { char szGroupName[NC_MAX_NAME + 1]; szGroupName[0] = 0; - nc_inq_grpname(panGroupIds[i], szGroupName); + NCDF_ERR(nc_inq_grpname(panGroupIds[i], szGroupName)); int nNewGrpId = -1; status = nc_def_grp(new_cdfid, szGroupName, &nNewGrpId); NCDF_ERR(status); diff --git a/frmts/netcdf/netcdfmultidim.cpp b/frmts/netcdf/netcdfmultidim.cpp index 304928c5236c..fb6507135f2b 100644 --- a/frmts/netcdf/netcdfmultidim.cpp +++ b/frmts/netcdf/netcdfmultidim.cpp @@ -766,7 +766,7 @@ netCDFGroup::netCDFGroup(const std::shared_ptr &poShared, if (m_gid == m_poShared->GetCDFId()) { int nFormat = 0; - nc_inq_format(m_gid, &nFormat); + NCDF_ERR(nc_inq_format(m_gid, &nFormat)); if (nFormat == NC_FORMAT_CLASSIC) { m_aosStructuralInfo.SetNameValue("NC_FORMAT", "CLASSIC"); diff --git a/frmts/netcdf/netcdfvirtual.cpp b/frmts/netcdf/netcdfvirtual.cpp index eec057e54275..8577bb8bcafa 100644 --- a/frmts/netcdf/netcdfvirtual.cpp +++ b/frmts/netcdf/netcdfvirtual.cpp @@ -154,12 +154,12 @@ void netCDFVID::nc_resize_vdim(int dimid, size_t dimlen) void netCDFVID::nc_set_define_mode() { - nc_redef(ncid); + NCDF_ERR(nc_redef(ncid)); } void netCDFVID::nc_set_data_mode() { - nc_enddef(ncid); + NCDF_ERR(nc_enddef(ncid)); } void netCDFVID::nc_vmap() @@ -168,7 +168,7 @@ void netCDFVID::nc_vmap() for (size_t itr_d = 0; itr_d < dimList.size(); itr_d++) { - int realDimID; + int realDimID = -1; netCDFVDimension &dim = dimList[itr_d]; if (!dim.isValid()) @@ -176,13 +176,14 @@ void netCDFVID::nc_vmap() continue; // don't do anywork if variable is invalid } - nc_def_dim(ncid, dim.getName().c_str(), dim.getLen(), &realDimID); + NCDF_ERR( + nc_def_dim(ncid, dim.getName().c_str(), dim.getLen(), &realDimID)); dimList[itr_d].setRealID(realDimID); } for (size_t itr_v = 0; itr_v < varList.size(); itr_v++) { - int realVarID; + int realVarID = -1; netCDFVVariable &var = varList[itr_v]; if (!var.isValid()) @@ -199,8 +200,8 @@ void netCDFVID::nc_vmap() virtualDIDToDim(var.getDimIds()[dimct]).getRealID(); } - nc_def_var(ncid, var.getName().c_str(), var.getType(), - var.getDimCount(), newdims.get(), &realVarID); + NCDF_ERR(nc_def_var(ncid, var.getName().c_str(), var.getType(), + var.getDimCount(), newdims.get(), &realVarID)); var.setRealID(realVarID); // Now write each of its attributes diff --git a/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp b/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp index 827babbc1d65..2a30a8e71dc0 100644 --- a/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp +++ b/ogr/ogrsf_frmts/arrow_common/ograrrowwriterlayer.hpp @@ -294,19 +294,16 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() const bool pointFieldNullable = GetDriverUCName() == "PARQUET"; // Fixed Size List GeoArrow encoding - std::shared_ptr pointField; - if (nDim == 2) - pointField = - arrow::field("xy", arrow::float64(), pointFieldNullable); - else if (nDim == 3 && OGR_GT_HasZ(eGType)) - pointField = - arrow::field("xyz", arrow::float64(), pointFieldNullable); - else if (nDim == 3 && OGR_GT_HasM(eGType)) - pointField = - arrow::field("xym", arrow::float64(), pointFieldNullable); - else - pointField = - arrow::field("xyzm", arrow::float64(), pointFieldNullable); + const auto getFixedSizeListOfPoint = + [nDim, eGType, pointFieldNullable]() + { + return arrow::fixed_size_list( + arrow::field(nDim == 2 ? "xy" + : nDim == 3 ? (OGR_GT_HasZ(eGType) ? "xyz" : "xym") + : "xyzm", + arrow::float64(), pointFieldNullable), + nDim); + }; // Struct GeoArrow encoding auto xField(arrow::field("x", arrow::float64(), false)); @@ -339,30 +336,28 @@ inline void OGRArrowWriterLayer::CreateSchemaCommon() break; case OGRArrowGeomEncoding::GEOARROW_FSL_POINT: - dt = arrow::fixed_size_list(pointField, nDim); + dt = getFixedSizeListOfPoint(); break; case OGRArrowGeomEncoding::GEOARROW_FSL_LINESTRING: - dt = arrow::list(arrow::fixed_size_list(pointField, nDim)); + dt = arrow::list(getFixedSizeListOfPoint()); break; case OGRArrowGeomEncoding::GEOARROW_FSL_POLYGON: - dt = arrow::list( - arrow::list(arrow::fixed_size_list(pointField, nDim))); + dt = arrow::list(arrow::list(getFixedSizeListOfPoint())); break; case OGRArrowGeomEncoding::GEOARROW_FSL_MULTIPOINT: - dt = arrow::list(arrow::fixed_size_list(pointField, nDim)); + dt = arrow::list(getFixedSizeListOfPoint()); break; case OGRArrowGeomEncoding::GEOARROW_FSL_MULTILINESTRING: - dt = arrow::list( - arrow::list(arrow::fixed_size_list(pointField, nDim))); + dt = arrow::list(arrow::list(getFixedSizeListOfPoint())); break; case OGRArrowGeomEncoding::GEOARROW_FSL_MULTIPOLYGON: - dt = arrow::list(arrow::list( - arrow::list(arrow::fixed_size_list(pointField, nDim)))); + dt = arrow::list( + arrow::list(arrow::list(getFixedSizeListOfPoint()))); break; case OGRArrowGeomEncoding::GEOARROW_STRUCT_POINT: diff --git a/ogr/ogrsf_frmts/gpkg/gdalgeopackagerasterband.cpp b/ogr/ogrsf_frmts/gpkg/gdalgeopackagerasterband.cpp index 26fb858c4703..c11cca9f82ca 100644 --- a/ogr/ogrsf_frmts/gpkg/gdalgeopackagerasterband.cpp +++ b/ogr/ogrsf_frmts/gpkg/gdalgeopackagerasterband.cpp @@ -1324,6 +1324,7 @@ int GDALGPKGMBTilesLikeRasterBand::IGetDataCoverageStatus(int nXOff, int nYOff, { for (int iX = iColMin; iX <= iColMax; ++iX) { + // coverity[swapped_arguments] if (oSetTiles.find(std::pair(iY, iX)) == oSetTiles.end()) { nStatus |= GDAL_DATA_COVERAGE_STATUS_EMPTY; diff --git a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp index a46f47c6765b..5383eb2e758d 100644 --- a/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp +++ b/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp @@ -7805,7 +7805,11 @@ void OGR_GPKG_FillArrowArray_Step(sqlite3_context *pContext, int /*argc*/, const int SQLITE_MAX_FUNCTION_ARG = sqlite3_limit(psFillArrowArray->hDB, SQLITE_LIMIT_FUNCTION_ARG, -1); begin: - const int iFeat = psFillArrowArray->nCountRows; + const int iFeat = [psFillArrowArray]() + { + std::unique_lock oLock(psFillArrowArray->oMutex); + return psFillArrowArray->nCountRows; + }(); auto psHelper = psFillArrowArray->psHelper.get(); int iCol = 0; const int iFieldStart = sqlite3_value_int(argv[iCol]); diff --git a/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp b/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp index fc845c98383c..4fa15a4fe424 100644 --- a/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp +++ b/ogr/ogrsf_frmts/openfilegdb/filegdbtable_freelist.cpp @@ -666,6 +666,9 @@ bool FileGDBTable::CheckFreeListConsistency() { const uint32_t nFreeAreaSize = GetUInt32( abyPage.data() + nPageHeaderSize + i * nEntrySize, 0); + assert(iSlot + 1 < + static_cast(CPL_ARRAYSIZE(anHoleSizes))); + // coverity[overrun-local] if (nFreeAreaSize < anHoleSizes[iSlot] || nFreeAreaSize >= anHoleSizes[iSlot + 1]) { diff --git a/ogr/ogrsf_frmts/parquet/ogrparquetdriver.cpp b/ogr/ogrsf_frmts/parquet/ogrparquetdriver.cpp index 9ee2558a40a6..ad9a63b88901 100644 --- a/ogr/ogrsf_frmts/parquet/ogrparquetdriver.cpp +++ b/ogr/ogrsf_frmts/parquet/ogrparquetdriver.cpp @@ -167,7 +167,7 @@ OpenParquetDatasetWithoutMetadata(const std::string &osBasePathIn, std::move(partitioningFactory)); arrow::fs::FileSelector selector; - selector.base_dir = osFSFilename; + selector.base_dir = std::move(osFSFilename); selector.recursive = true; PARQUET_ASSIGN_OR_THROW( diff --git a/port/cpl_worker_thread_pool.cpp b/port/cpl_worker_thread_pool.cpp index ee086fabd99a..90db56f418bb 100644 --- a/port/cpl_worker_thread_pool.cpp +++ b/port/cpl_worker_thread_pool.cpp @@ -669,10 +669,20 @@ void CPLJobQueue::WaitCompletion(int nMaxRemainingJobs) bool CPLJobQueue::WaitEvent() { std::unique_lock oGuard(m_mutex); - // coverity[missing_lock:FALSE] - if (m_nPendingJobs > 0) + while (true) { + // coverity[missing_lock:FALSE] + const int nPendingJobsBefore = m_nPendingJobs; + if (nPendingJobsBefore == 0) + { + return false; + } m_cv.wait(oGuard); + // cppcheck-suppress knownConditionTrueFalse + // coverity[missing_lock:FALSE] + if (m_nPendingJobs < nPendingJobsBefore) + { + return m_nPendingJobs > 0; + } } - return m_nPendingJobs > 0; } From 8b2282ade847333645811bca5b628f4697daad8c Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 14 Aug 2024 12:39:55 +0200 Subject: [PATCH 08/14] gdaldem color-relief: fix issues with entry at 0 and -exact_color_entry mode, and other issues Fixes https://lists.osgeo.org/pipermail/gdal-dev/2024-August/059324.html --- apps/gdaldem_lib.cpp | 104 +++++++++++++++---------- autotest/utilities/test_gdaldem_lib.py | 78 ++++++++++++++++++- 2 files changed, 138 insertions(+), 44 deletions(-) diff --git a/apps/gdaldem_lib.cpp b/apps/gdaldem_lib.cpp index ff613b1113f9..de51acf3fddf 100644 --- a/apps/gdaldem_lib.cpp +++ b/apps/gdaldem_lib.cpp @@ -1437,7 +1437,8 @@ static int GDALColorReliefSortColors(const ColorAssociation &pA, static void GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, int *pnColorAssociation, int bSrcHasNoData, - double dfSrcNoDataValue) + double dfSrcNoDataValue, + ColorSelectionMode eColorSelectionMode) { ColorAssociation *pasColorAssociation = *ppasColorAssociation; int nColorAssociation = *pnColorAssociation; @@ -1455,12 +1456,13 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, ColorAssociation *pCurrent = &pasColorAssociation[i]; // NaN comparison is always false, so it handles itself - if (bSrcHasNoData && pCurrent->dfVal == dfSrcNoDataValue) + if (eColorSelectionMode != COLOR_SELECTION_EXACT_ENTRY && + bSrcHasNoData && pCurrent->dfVal == dfSrcNoDataValue) { // Check if there is enough distance between the nodata value and // its predecessor. - const double dfNewValue = - pCurrent->dfVal - std::abs(pCurrent->dfVal) * DBL_EPSILON; + const double dfNewValue = std::nextafter( + pCurrent->dfVal, -std::numeric_limits::infinity()); if (dfNewValue > pPrevious->dfVal) { // add one just below the nodata value @@ -1476,12 +1478,13 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, dfNewValue; } } - else if (bSrcHasNoData && pPrevious->dfVal == dfSrcNoDataValue) + else if (eColorSelectionMode != COLOR_SELECTION_EXACT_ENTRY && + bSrcHasNoData && pPrevious->dfVal == dfSrcNoDataValue) { // Check if there is enough distance between the nodata value and // its successor. - const double dfNewValue = - pPrevious->dfVal + std::abs(pPrevious->dfVal) * DBL_EPSILON; + const double dfNewValue = std::nextafter( + pPrevious->dfVal, std::numeric_limits::infinity()); if (dfNewValue < pCurrent->dfVal) { // add one just above the nodata value @@ -1652,8 +1655,25 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, } else { - if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY && - pasColorAssociation[i - 1].dfVal != dfVal) + if (pasColorAssociation[i - 1].dfVal == dfVal) + { + *pnR = pasColorAssociation[i - 1].nR; + *pnG = pasColorAssociation[i - 1].nG; + *pnB = pasColorAssociation[i - 1].nB; + *pnA = pasColorAssociation[i - 1].nA; + return true; + } + + if (pasColorAssociation[i].dfVal == dfVal) + { + *pnR = pasColorAssociation[i].nR; + *pnG = pasColorAssociation[i].nG; + *pnB = pasColorAssociation[i].nB; + *pnA = pasColorAssociation[i].nA; + return true; + } + + if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY) { *pnR = 0; *pnG = 0; @@ -1679,15 +1699,6 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, return true; } - if (pasColorAssociation[i - 1].dfVal == dfVal) - { - *pnR = pasColorAssociation[i - 1].nR; - *pnG = pasColorAssociation[i - 1].nG; - *pnB = pasColorAssociation[i - 1].nB; - *pnA = pasColorAssociation[i - 1].nA; - return true; - } - if (CPLIsNan(pasColorAssociation[i - 1].dfVal)) { *pnR = pasColorAssociation[i].nR; @@ -1791,7 +1802,8 @@ static bool GDALColorReliefFindNamedColor(const char *pszColorName, int *pnR, static ColorAssociation * GDALColorReliefParseColorFile(GDALRasterBandH hSrcBand, - const char *pszColorFilename, int *pnColors) + const char *pszColorFilename, int *pnColors, + ColorSelectionMode eColorSelectionMode) { VSILFILE *fpColorFile = VSIFOpenL(pszColorFilename, "rt"); if (fpColorFile == nullptr) @@ -1972,7 +1984,8 @@ GDALColorReliefParseColorFile(GDALRasterBandH hSrcBand, } GDALColorReliefProcessColors(&pasColorAssociation, &nColorAssociation, - bSrcHasNoData, dfSrcNoDataValue); + bSrcHasNoData, dfSrcNoDataValue, + eColorSelectionMode); *pnColors = nColorAssociation; return pasColorAssociation; @@ -2082,7 +2095,7 @@ GDALColorReliefDataset::GDALColorReliefDataset( panSourceBuf(nullptr), nCurBlockXOff(-1), nCurBlockYOff(-1) { pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation); + hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); nRasterXSize = GDALGetRasterXSize(hSrcDS); nRasterYSize = GDALGetRasterYSize(hSrcDS); @@ -2222,7 +2235,7 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, int nColorAssociation = 0; ColorAssociation *pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation); + hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); if (pasColorAssociation == nullptr) return CE_Failure; @@ -2427,7 +2440,7 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, { int nColorAssociation = 0; ColorAssociation *pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation); + hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); if (pasColorAssociation == nullptr) return CE_Failure; @@ -2513,29 +2526,23 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, for (int iColor = 0; iColor < nColorAssociation; iColor++) { - if (eColorSelectionMode == COLOR_SELECTION_NEAREST_ENTRY) - { - if (iColor > 1) - bOK &= VSIFPrintfL(fp, ",") > 0; - } - else if (iColor > 0) - bOK &= VSIFPrintfL(fp, ",") > 0; - const double dfVal = pasColorAssociation[iColor].dfVal; + if (iColor > 0) + bOK &= VSIFPrintfL(fp, ",") > 0; - if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY) - { - bOK &= VSIFPrintfL(fp, "%.18g:0,", - dfVal - fabs(dfVal) * DBL_EPSILON) > 0; - } - else if (iColor > 0 && - eColorSelectionMode == COLOR_SELECTION_NEAREST_ENTRY) + if (iColor > 0 && + eColorSelectionMode == COLOR_SELECTION_NEAREST_ENTRY && + dfVal != + std::nextafter(pasColorAssociation[iColor - 1].dfVal, + std::numeric_limits::infinity())) { const double dfMidVal = (dfVal + pasColorAssociation[iColor - 1].dfVal) / 2.0; bOK &= VSIFPrintfL( - fp, "%.18g:%d", dfMidVal - fabs(dfMidVal) * DBL_EPSILON, + fp, "%.18g:%d", + std::nextafter( + dfMidVal, -std::numeric_limits::infinity()), (iBand == 0) ? pasColorAssociation[iColor - 1].nR : (iBand == 1) ? pasColorAssociation[iColor - 1].nG : (iBand == 2) @@ -2548,9 +2555,17 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, : (iBand == 2) ? pasColorAssociation[iColor].nB : pasColorAssociation[iColor].nA) > 0; } - - if (eColorSelectionMode != COLOR_SELECTION_NEAREST_ENTRY) + else { + if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY) + { + bOK &= + VSIFPrintfL( + fp, "%.18g:0,", + std::nextafter( + dfVal, + -std::numeric_limits::infinity())) > 0; + } if (dfVal != static_cast(static_cast(dfVal))) bOK &= VSIFPrintfL(fp, "%.18g", dfVal) > 0; else @@ -2565,8 +2580,11 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY) { - bOK &= VSIFPrintfL(fp, ",%.18g:0", - dfVal + fabs(dfVal) * DBL_EPSILON) > 0; + bOK &= VSIFPrintfL( + fp, ",%.18g:0", + std::nextafter( + dfVal, + std::numeric_limits::infinity())) > 0; } } bOK &= VSIFPrintfL(fp, "\n") > 0; diff --git a/autotest/utilities/test_gdaldem_lib.py b/autotest/utilities/test_gdaldem_lib.py index 101ff987b22b..af3170465c1e 100755 --- a/autotest/utilities/test_gdaldem_lib.py +++ b/autotest/utilities/test_gdaldem_lib.py @@ -469,7 +469,7 @@ def test_gdaldem_lib_color_relief(): colorFilename="data/color_file.txt", colorSelection="exact_color_entry", ) - assert ds.GetRasterBand(1).Checksum() == 0 + assert ds.GetRasterBand(1).Checksum() == 8073 ds = gdal.DEMProcessing( "", @@ -526,6 +526,82 @@ def test_gdaldem_lib_color_relief_nodata_value(tmp_vsimem): gdal.Unlink(colorFilename) +@pytest.mark.parametrize( + "colorSelection", + ["nearest_color_entry", "exact_color_entry", "linear_interpolation"], +) +@pytest.mark.parametrize("format", ["MEM", "VRT"]) +def test_gdaldem_lib_color_relief_synthetic(tmp_path, colorSelection, format): + + src_filename = "" if format == "MEM" else str(tmp_path / "in.tif") + src_ds = gdal.GetDriverByName("MEM" if format == "MEM" else "GTiff").Create( + src_filename, 4, 1 + ) + src_ds.GetRasterBand(1).SetNoDataValue(0) + src_ds.GetRasterBand(1).WriteRaster(0, 0, 4, 1, b"\x00\x01\x02\x03") + if format != "MEM": + src_ds.Close() + src_ds = gdal.Open(src_filename) + colorFilename = tmp_path / "color_file.txt" + with open(colorFilename, "wb") as f: + f.write(b"""0 0 0 0\n1 10 11 12\n2 20 21 22\n3 30 31 32\n""") + + out_filename = "" if format == "MEM" else str(tmp_path / "out.vrt") + ds = gdal.DEMProcessing( + out_filename, + src_ds, + "color-relief", + format=format, + colorFilename=colorFilename, + colorSelection=colorSelection, + ) + if format != "MEM": + ds.Close() + ds = gdal.Open(out_filename) + assert struct.unpack("B" * 4, ds.GetRasterBand(1).ReadRaster()) == (0, 10, 20, 30) + assert struct.unpack("B" * 4, ds.GetRasterBand(2).ReadRaster()) == (0, 11, 21, 31) + assert struct.unpack("B" * 4, ds.GetRasterBand(3).ReadRaster()) == (0, 12, 22, 32) + + +@pytest.mark.parametrize( + "colorSelection", + ["nearest_color_entry", "exact_color_entry", "linear_interpolation"], +) +@pytest.mark.parametrize("format", ["MEM", "VRT"]) +def test_gdaldem_lib_color_relief_synthetic_nodata_255( + tmp_path, colorSelection, format +): + + src_filename = "" if format == "MEM" else str(tmp_path / "in.tif") + src_ds = gdal.GetDriverByName("MEM" if format == "MEM" else "GTiff").Create( + src_filename, 4, 1 + ) + src_ds.GetRasterBand(1).SetNoDataValue(255) + src_ds.GetRasterBand(1).WriteRaster(0, 0, 4, 1, b"\x00\x01\x02\xFF") + if format != "MEM": + src_ds.Close() + src_ds = gdal.Open(src_filename) + colorFilename = tmp_path / "color_file.txt" + with open(colorFilename, "wb") as f: + f.write(b"""0 0 1 2\n1 10 11 12\n2 20 21 22\n255 255 255 255\n""") + + out_filename = "" if format == "MEM" else str(tmp_path / "out.vrt") + ds = gdal.DEMProcessing( + out_filename, + src_ds, + "color-relief", + format=format, + colorFilename=colorFilename, + colorSelection=colorSelection, + ) + if format != "MEM": + ds.Close() + ds = gdal.Open(out_filename) + assert struct.unpack("B" * 4, ds.GetRasterBand(1).ReadRaster()) == (0, 10, 20, 255) + assert struct.unpack("B" * 4, ds.GetRasterBand(2).ReadRaster()) == (1, 11, 21, 255) + assert struct.unpack("B" * 4, ds.GetRasterBand(3).ReadRaster()) == (2, 12, 22, 255) + + ############################################################################### # Test gdaldem tpi From 22eb99b8cd8965133530feac45dbcb8dfd2c17e2 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 14 Aug 2024 20:41:36 +0200 Subject: [PATCH 09/14] [Lint] C++ify some aspects of gdaldem color-relief --- apps/gdaldem_lib.cpp | 471 ++++++++++++++++++------------------------- 1 file changed, 192 insertions(+), 279 deletions(-) diff --git a/apps/gdaldem_lib.cpp b/apps/gdaldem_lib.cpp index de51acf3fddf..c4f3284a000e 100644 --- a/apps/gdaldem_lib.cpp +++ b/apps/gdaldem_lib.cpp @@ -116,6 +116,7 @@ #include "cpl_progress.h" #include "cpl_string.h" #include "cpl_vsi.h" +#include "cpl_vsi_virtual.h" #include "gdal.h" #include "gdal_priv.h" @@ -1435,25 +1436,19 @@ static int GDALColorReliefSortColors(const ColorAssociation &pA, } static void -GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, - int *pnColorAssociation, int bSrcHasNoData, - double dfSrcNoDataValue, +GDALColorReliefProcessColors(std::vector &asColorAssociation, + int bSrcHasNoData, double dfSrcNoDataValue, ColorSelectionMode eColorSelectionMode) { - ColorAssociation *pasColorAssociation = *ppasColorAssociation; - int nColorAssociation = *pnColorAssociation; - - std::stable_sort(pasColorAssociation, - pasColorAssociation + nColorAssociation, + std::stable_sort(asColorAssociation.begin(), asColorAssociation.end(), GDALColorReliefSortColors); - ColorAssociation *pPrevious = - (nColorAssociation > 0) ? &pasColorAssociation[0] : nullptr; - int nAdded = 0; - int nRepeatedEntryIndex = 0; - for (int i = 1; i < nColorAssociation; ++i) + size_t nRepeatedEntryIndex = 0; + const size_t nInitialSize = asColorAssociation.size(); + for (size_t i = 1; i < nInitialSize; ++i) { - ColorAssociation *pCurrent = &pasColorAssociation[i]; + const ColorAssociation *pPrevious = &asColorAssociation[i - 1]; + const ColorAssociation *pCurrent = &asColorAssociation[i]; // NaN comparison is always false, so it handles itself if (eColorSelectionMode != COLOR_SELECTION_EXACT_ENTRY && @@ -1466,16 +1461,9 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, if (dfNewValue > pPrevious->dfVal) { // add one just below the nodata value - ++nAdded; - ColorAssociation sPrevious = *pPrevious; - pasColorAssociation = - static_cast(CPLRealloc( - pasColorAssociation, (nColorAssociation + nAdded) * - sizeof(ColorAssociation))); - pCurrent = &pasColorAssociation[i]; - pasColorAssociation[nColorAssociation + nAdded - 1] = sPrevious; - pasColorAssociation[nColorAssociation + nAdded - 1].dfVal = - dfNewValue; + ColorAssociation sNew = *pPrevious; + sNew.dfVal = dfNewValue; + asColorAssociation.push_back(std::move(sNew)); } } else if (eColorSelectionMode != COLOR_SELECTION_EXACT_ENTRY && @@ -1488,16 +1476,9 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, if (dfNewValue < pCurrent->dfVal) { // add one just above the nodata value - ++nAdded; - ColorAssociation sCurrent = *pCurrent; - pasColorAssociation = - static_cast(CPLRealloc( - pasColorAssociation, (nColorAssociation + nAdded) * - sizeof(ColorAssociation))); - pCurrent = &pasColorAssociation[i]; - pasColorAssociation[nColorAssociation + nAdded - 1] = sCurrent; - pasColorAssociation[nColorAssociation + nAdded - 1].dfVal = - dfNewValue; + ColorAssociation sNew = *pCurrent; + sNew.dfVal = dfNewValue; + asColorAssociation.push_back(std::move(sNew)); } } else if (nRepeatedEntryIndex == 0 && @@ -1515,8 +1496,8 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, double dfLeftDist = 0.0; if (nRepeatedEntryIndex >= 2) { - ColorAssociation *pLower = - &pasColorAssociation[nRepeatedEntryIndex - 2]; + const ColorAssociation *pLower = + &asColorAssociation[nRepeatedEntryIndex - 2]; dfTotalDist = pCurrent->dfVal - pLower->dfVal; dfLeftDist = pPrevious->dfVal - pLower->dfVal; } @@ -1526,16 +1507,16 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, } // check if this distance is enough - const int nEquivalentCount = i - nRepeatedEntryIndex + 1; + const size_t nEquivalentCount = i - nRepeatedEntryIndex + 1; if (dfTotalDist > std::abs(pPrevious->dfVal) * nEquivalentCount * DBL_EPSILON) { // balance the alterations double dfMultiplier = - 0.5 - nEquivalentCount * dfLeftDist / dfTotalDist; - for (int j = nRepeatedEntryIndex - 1; j < i; ++j) + 0.5 - double(nEquivalentCount) * dfLeftDist / dfTotalDist; + for (auto j = nRepeatedEntryIndex - 1; j < i; ++j) { - pasColorAssociation[j].dfVal += + asColorAssociation[j].dfVal += (std::abs(pPrevious->dfVal) * dfMultiplier) * DBL_EPSILON; dfMultiplier += 1.0; @@ -1549,35 +1530,33 @@ GDALColorReliefProcessColors(ColorAssociation **ppasColorAssociation, nRepeatedEntryIndex = 0; } - pPrevious = pCurrent; } - if (nAdded) + if (nInitialSize != asColorAssociation.size()) { - std::stable_sort(pasColorAssociation, - pasColorAssociation + nColorAssociation + nAdded, + std::stable_sort(asColorAssociation.begin(), asColorAssociation.end(), GDALColorReliefSortColors); - *ppasColorAssociation = pasColorAssociation; - *pnColorAssociation = nColorAssociation + nAdded; } } -static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, - int nColorAssociation, double dfVal, - ColorSelectionMode eColorSelectionMode, - int *pnR, int *pnG, int *pnB, int *pnA) +static bool +GDALColorReliefGetRGBA(const std::vector &asColorAssociation, + double dfVal, ColorSelectionMode eColorSelectionMode, + int *pnR, int *pnG, int *pnB, int *pnA) { - int lower = 0; + CPLAssert(!asColorAssociation.empty()); + + size_t lower = 0; // Special case for NaN - if (CPLIsNan(pasColorAssociation[0].dfVal)) + if (CPLIsNan(asColorAssociation[0].dfVal)) { if (CPLIsNan(dfVal)) { - *pnR = pasColorAssociation[0].nR; - *pnG = pasColorAssociation[0].nG; - *pnB = pasColorAssociation[0].nB; - *pnA = pasColorAssociation[0].nA; + *pnR = asColorAssociation[0].nR; + *pnG = asColorAssociation[0].nG; + *pnB = asColorAssociation[0].nB; + *pnA = asColorAssociation[0].nA; return true; } else @@ -1588,22 +1567,22 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, // Find the index of the first element in the LUT input array that // is not smaller than the dfVal value. - int i = 0; - int upper = nColorAssociation - 1; + size_t i = 0; + size_t upper = asColorAssociation.size() - 1; while (true) { - const int mid = (lower + upper) / 2; + const size_t mid = (lower + upper) / 2; if (upper - lower <= 1) { - if (dfVal <= pasColorAssociation[lower].dfVal) + if (dfVal <= asColorAssociation[lower].dfVal) i = lower; - else if (dfVal <= pasColorAssociation[upper].dfVal) + else if (dfVal <= asColorAssociation[upper].dfVal) i = upper; else i = upper + 1; break; } - else if (pasColorAssociation[mid].dfVal >= dfVal) + else if (asColorAssociation[mid].dfVal >= dfVal) { upper = mid; } @@ -1616,7 +1595,7 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, if (i == 0) { if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY && - pasColorAssociation[0].dfVal != dfVal) + asColorAssociation[0].dfVal != dfVal) { *pnR = 0; *pnG = 0; @@ -1626,17 +1605,17 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, } else { - *pnR = pasColorAssociation[0].nR; - *pnG = pasColorAssociation[0].nG; - *pnB = pasColorAssociation[0].nB; - *pnA = pasColorAssociation[0].nA; + *pnR = asColorAssociation[0].nR; + *pnG = asColorAssociation[0].nG; + *pnB = asColorAssociation[0].nB; + *pnA = asColorAssociation[0].nA; return true; } } - else if (i == nColorAssociation) + else if (i == asColorAssociation.size()) { if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY && - pasColorAssociation[i - 1].dfVal != dfVal) + asColorAssociation[i - 1].dfVal != dfVal) { *pnR = 0; *pnG = 0; @@ -1646,30 +1625,30 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, } else { - *pnR = pasColorAssociation[i - 1].nR; - *pnG = pasColorAssociation[i - 1].nG; - *pnB = pasColorAssociation[i - 1].nB; - *pnA = pasColorAssociation[i - 1].nA; + *pnR = asColorAssociation[i - 1].nR; + *pnG = asColorAssociation[i - 1].nG; + *pnB = asColorAssociation[i - 1].nB; + *pnA = asColorAssociation[i - 1].nA; return true; } } else { - if (pasColorAssociation[i - 1].dfVal == dfVal) + if (asColorAssociation[i - 1].dfVal == dfVal) { - *pnR = pasColorAssociation[i - 1].nR; - *pnG = pasColorAssociation[i - 1].nG; - *pnB = pasColorAssociation[i - 1].nB; - *pnA = pasColorAssociation[i - 1].nA; + *pnR = asColorAssociation[i - 1].nR; + *pnG = asColorAssociation[i - 1].nG; + *pnB = asColorAssociation[i - 1].nB; + *pnA = asColorAssociation[i - 1].nA; return true; } - if (pasColorAssociation[i].dfVal == dfVal) + if (asColorAssociation[i].dfVal == dfVal) { - *pnR = pasColorAssociation[i].nR; - *pnG = pasColorAssociation[i].nG; - *pnB = pasColorAssociation[i].nB; - *pnA = pasColorAssociation[i].nA; + *pnR = asColorAssociation[i].nR; + *pnG = asColorAssociation[i].nG; + *pnB = asColorAssociation[i].nB; + *pnA = asColorAssociation[i].nA; return true; } @@ -1683,58 +1662,58 @@ static bool GDALColorReliefGetRGBA(ColorAssociation *pasColorAssociation, } if (eColorSelectionMode == COLOR_SELECTION_NEAREST_ENTRY && - pasColorAssociation[i - 1].dfVal != dfVal) + asColorAssociation[i - 1].dfVal != dfVal) { - int index = i; - if (dfVal - pasColorAssociation[i - 1].dfVal < - pasColorAssociation[i].dfVal - dfVal) + size_t index = i; + if (dfVal - asColorAssociation[i - 1].dfVal < + asColorAssociation[i].dfVal - dfVal) { --index; } - *pnR = pasColorAssociation[index].nR; - *pnG = pasColorAssociation[index].nG; - *pnB = pasColorAssociation[index].nB; - *pnA = pasColorAssociation[index].nA; + *pnR = asColorAssociation[index].nR; + *pnG = asColorAssociation[index].nG; + *pnB = asColorAssociation[index].nB; + *pnA = asColorAssociation[index].nA; return true; } - if (CPLIsNan(pasColorAssociation[i - 1].dfVal)) + if (CPLIsNan(asColorAssociation[i - 1].dfVal)) { - *pnR = pasColorAssociation[i].nR; - *pnG = pasColorAssociation[i].nG; - *pnB = pasColorAssociation[i].nB; - *pnA = pasColorAssociation[i].nA; + *pnR = asColorAssociation[i].nR; + *pnG = asColorAssociation[i].nG; + *pnB = asColorAssociation[i].nB; + *pnA = asColorAssociation[i].nA; return true; } const double dfRatio = - (dfVal - pasColorAssociation[i - 1].dfVal) / - (pasColorAssociation[i].dfVal - pasColorAssociation[i - 1].dfVal); - *pnR = static_cast(0.45 + pasColorAssociation[i - 1].nR + - dfRatio * (pasColorAssociation[i].nR - - pasColorAssociation[i - 1].nR)); + (dfVal - asColorAssociation[i - 1].dfVal) / + (asColorAssociation[i].dfVal - asColorAssociation[i - 1].dfVal); + *pnR = static_cast(0.45 + asColorAssociation[i - 1].nR + + dfRatio * (asColorAssociation[i].nR - + asColorAssociation[i - 1].nR)); if (*pnR < 0) *pnR = 0; else if (*pnR > 255) *pnR = 255; - *pnG = static_cast(0.45 + pasColorAssociation[i - 1].nG + - dfRatio * (pasColorAssociation[i].nG - - pasColorAssociation[i - 1].nG)); + *pnG = static_cast(0.45 + asColorAssociation[i - 1].nG + + dfRatio * (asColorAssociation[i].nG - + asColorAssociation[i - 1].nG)); if (*pnG < 0) *pnG = 0; else if (*pnG > 255) *pnG = 255; - *pnB = static_cast(0.45 + pasColorAssociation[i - 1].nB + - dfRatio * (pasColorAssociation[i].nB - - pasColorAssociation[i - 1].nB)); + *pnB = static_cast(0.45 + asColorAssociation[i - 1].nB + + dfRatio * (asColorAssociation[i].nB - + asColorAssociation[i - 1].nB)); if (*pnB < 0) *pnB = 0; else if (*pnB > 255) *pnB = 255; - *pnA = static_cast(0.45 + pasColorAssociation[i - 1].nA + - dfRatio * (pasColorAssociation[i].nA - - pasColorAssociation[i - 1].nA)); + *pnA = static_cast(0.45 + asColorAssociation[i - 1].nA + + dfRatio * (asColorAssociation[i].nA - + asColorAssociation[i - 1].nA)); if (*pnA < 0) *pnA = 0; else if (*pnA > 255) @@ -1800,22 +1779,21 @@ static bool GDALColorReliefFindNamedColor(const char *pszColorName, int *pnR, return false; } -static ColorAssociation * +static std::vector GDALColorReliefParseColorFile(GDALRasterBandH hSrcBand, - const char *pszColorFilename, int *pnColors, + const char *pszColorFilename, ColorSelectionMode eColorSelectionMode) { - VSILFILE *fpColorFile = VSIFOpenL(pszColorFilename, "rt"); + auto fpColorFile = + VSIVirtualHandleUniquePtr(VSIFOpenL(pszColorFilename, "rt")); if (fpColorFile == nullptr) { CPLError(CE_Failure, CPLE_AppDefined, "Cannot find %s", pszColorFilename); - *pnColors = 0; - return nullptr; + return {}; } - ColorAssociation *pasColorAssociation = nullptr; - int nColorAssociation = 0; + std::vector asColorAssociation; int bSrcHasNoData = FALSE; double dfSrcNoDataValue = @@ -1823,7 +1801,8 @@ GDALColorReliefParseColorFile(GDALRasterBandH hSrcBand, const char *pszLine = nullptr; bool bIsGMT_CPT = false; - while ((pszLine = CPLReadLineL(fpColorFile)) != nullptr) + ColorAssociation sColor; + while ((pszLine = CPLReadLineL(fpColorFile.get())) != nullptr) { if (pszLine[0] == '#' && strstr(pszLine, "COLOR_MODEL")) { @@ -1831,171 +1810,127 @@ GDALColorReliefParseColorFile(GDALRasterBandH hSrcBand, { CPLError(CE_Failure, CPLE_AppDefined, "Only COLOR_MODEL = RGB is supported"); - CPLFree(pasColorAssociation); - *pnColors = 0; - return nullptr; + return {}; } bIsGMT_CPT = true; } - char **papszFields = - CSLTokenizeStringComplex(pszLine, " ,\t:", FALSE, FALSE); + const CPLStringList aosFields( + CSLTokenizeStringComplex(pszLine, " ,\t:", FALSE, FALSE)); /* Skip comment lines */ - const int nTokens = CSLCount(papszFields); - if (nTokens >= 1 && - (papszFields[0][0] == '#' || papszFields[0][0] == '/')) + const int nTokens = aosFields.size(); + if (nTokens >= 1 && (aosFields[0][0] == '#' || aosFields[0][0] == '/')) { - CSLDestroy(papszFields); continue; } if (bIsGMT_CPT && nTokens == 8) { - pasColorAssociation = static_cast( - CPLRealloc(pasColorAssociation, - (nColorAssociation + 2) * sizeof(ColorAssociation))); - - pasColorAssociation[nColorAssociation].dfVal = - CPLAtof(papszFields[0]); - pasColorAssociation[nColorAssociation].nR = atoi(papszFields[1]); - pasColorAssociation[nColorAssociation].nG = atoi(papszFields[2]); - pasColorAssociation[nColorAssociation].nB = atoi(papszFields[3]); - pasColorAssociation[nColorAssociation].nA = 255; - nColorAssociation++; - - pasColorAssociation[nColorAssociation].dfVal = - CPLAtof(papszFields[4]); - pasColorAssociation[nColorAssociation].nR = atoi(papszFields[5]); - pasColorAssociation[nColorAssociation].nG = atoi(papszFields[6]); - pasColorAssociation[nColorAssociation].nB = atoi(papszFields[7]); - pasColorAssociation[nColorAssociation].nA = 255; - nColorAssociation++; + sColor.dfVal = CPLAtof(aosFields[0]); + sColor.nR = atoi(aosFields[1]); + sColor.nG = atoi(aosFields[2]); + sColor.nB = atoi(aosFields[3]); + sColor.nA = 255; + asColorAssociation.push_back(sColor); + + sColor.dfVal = CPLAtof(aosFields[4]); + sColor.nR = atoi(aosFields[5]); + sColor.nG = atoi(aosFields[6]); + sColor.nB = atoi(aosFields[7]); + sColor.nA = 255; + asColorAssociation.push_back(sColor); } else if (bIsGMT_CPT && nTokens == 4) { // The first token might be B (background), F (foreground) or N // (nodata) Just interested in N. - if (EQUAL(papszFields[0], "N") && bSrcHasNoData) + if (EQUAL(aosFields[0], "N") && bSrcHasNoData) { - pasColorAssociation = - static_cast(CPLRealloc( - pasColorAssociation, - (nColorAssociation + 1) * sizeof(ColorAssociation))); - - pasColorAssociation[nColorAssociation].dfVal = dfSrcNoDataValue; - pasColorAssociation[nColorAssociation].nR = - atoi(papszFields[1]); - pasColorAssociation[nColorAssociation].nG = - atoi(papszFields[2]); - pasColorAssociation[nColorAssociation].nB = - atoi(papszFields[3]); - pasColorAssociation[nColorAssociation].nA = 255; - nColorAssociation++; + sColor.dfVal = dfSrcNoDataValue; + sColor.nR = atoi(aosFields[1]); + sColor.nG = atoi(aosFields[2]); + sColor.nB = atoi(aosFields[3]); + sColor.nA = 255; + asColorAssociation.push_back(sColor); } } else if (!bIsGMT_CPT && nTokens >= 2) { - pasColorAssociation = static_cast( - CPLRealloc(pasColorAssociation, - (nColorAssociation + 1) * sizeof(ColorAssociation))); - if (EQUAL(papszFields[0], "nv")) + if (EQUAL(aosFields[0], "nv")) { if (!bSrcHasNoData) { CPLError(CE_Warning, CPLE_AppDefined, "Input dataset has no nodata value. " "Ignoring 'nv' entry in color palette"); - CSLDestroy(papszFields); continue; } - pasColorAssociation[nColorAssociation].dfVal = dfSrcNoDataValue; + sColor.dfVal = dfSrcNoDataValue; } - else if (strlen(papszFields[0]) > 1 && - papszFields[0][strlen(papszFields[0]) - 1] == '%') + else if (strlen(aosFields[0]) > 1 && + aosFields[0][strlen(aosFields[0]) - 1] == '%') { - const double dfPct = CPLAtof(papszFields[0]) / 100.0; + const double dfPct = CPLAtof(aosFields[0]) / 100.0; if (dfPct < 0.0 || dfPct > 1.0) { CPLError(CE_Failure, CPLE_AppDefined, - "Wrong value for a percentage : %s", - papszFields[0]); - CSLDestroy(papszFields); - VSIFCloseL(fpColorFile); - CPLFree(pasColorAssociation); - *pnColors = 0; - return nullptr; + "Wrong value for a percentage : %s", aosFields[0]); + return {}; } - pasColorAssociation[nColorAssociation].dfVal = + sColor.dfVal = GDALColorReliefGetAbsoluteValFromPct(hSrcBand, dfPct); } else { - pasColorAssociation[nColorAssociation].dfVal = - CPLAtof(papszFields[0]); + sColor.dfVal = CPLAtof(aosFields[0]); } if (nTokens >= 4) { - pasColorAssociation[nColorAssociation].nR = - atoi(papszFields[1]); - pasColorAssociation[nColorAssociation].nG = - atoi(papszFields[2]); - pasColorAssociation[nColorAssociation].nB = - atoi(papszFields[3]); - pasColorAssociation[nColorAssociation].nA = - (CSLCount(papszFields) >= 5) ? atoi(papszFields[4]) : 255; + sColor.nR = atoi(aosFields[1]); + sColor.nG = atoi(aosFields[2]); + sColor.nB = atoi(aosFields[3]); + sColor.nA = + (CSLCount(aosFields) >= 5) ? atoi(aosFields[4]) : 255; } else { int nR = 0; int nG = 0; int nB = 0; - if (!GDALColorReliefFindNamedColor(papszFields[1], &nR, &nG, - &nB)) + if (!GDALColorReliefFindNamedColor(aosFields[1], &nR, &nG, &nB)) { CPLError(CE_Failure, CPLE_AppDefined, "Unknown color : %s", - papszFields[1]); - CSLDestroy(papszFields); - VSIFCloseL(fpColorFile); - CPLFree(pasColorAssociation); - *pnColors = 0; - return nullptr; + aosFields[1]); + return {}; } - pasColorAssociation[nColorAssociation].nR = nR; - pasColorAssociation[nColorAssociation].nG = nG; - pasColorAssociation[nColorAssociation].nB = nB; - pasColorAssociation[nColorAssociation].nA = - (CSLCount(papszFields) >= 3) ? atoi(papszFields[2]) : 255; + sColor.nR = nR; + sColor.nG = nG; + sColor.nB = nB; + sColor.nA = + (CSLCount(aosFields) >= 3) ? atoi(aosFields[2]) : 255; } - - nColorAssociation++; + asColorAssociation.push_back(sColor); } - CSLDestroy(papszFields); } - VSIFCloseL(fpColorFile); - if (nColorAssociation == 0) + if (asColorAssociation.empty()) { CPLError(CE_Failure, CPLE_AppDefined, "No color association found in %s", pszColorFilename); - CPLFree(pasColorAssociation); - *pnColors = 0; - return nullptr; + return {}; } - GDALColorReliefProcessColors(&pasColorAssociation, &nColorAssociation, - bSrcHasNoData, dfSrcNoDataValue, - eColorSelectionMode); + GDALColorReliefProcessColors(asColorAssociation, bSrcHasNoData, + dfSrcNoDataValue, eColorSelectionMode); - *pnColors = nColorAssociation; - return pasColorAssociation; + return asColorAssociation; } -static GByte *GDALColorReliefPrecompute(GDALRasterBandH hSrcBand, - ColorAssociation *pasColorAssociation, - int nColorAssociation, - ColorSelectionMode eColorSelectionMode, - int *pnIndexOffset) +static GByte *GDALColorReliefPrecompute( + GDALRasterBandH hSrcBand, + const std::vector &asColorAssociation, + ColorSelectionMode eColorSelectionMode, int *pnIndexOffset) { const GDALDataType eDT = GDALGetRasterDataType(hSrcBand); GByte *pabyPrecomputed = nullptr; @@ -2016,9 +1951,8 @@ static GByte *GDALColorReliefPrecompute(GDALRasterBandH hSrcBand, int nG = 0; int nB = 0; int nA = 0; - GDALColorReliefGetRGBA(pasColorAssociation, nColorAssociation, - i - nIndexOffset, eColorSelectionMode, - &nR, &nG, &nB, &nA); + GDALColorReliefGetRGBA(asColorAssociation, i - nIndexOffset, + eColorSelectionMode, &nR, &nG, &nB, &nA); pabyPrecomputed[4 * i] = static_cast(nR); pabyPrecomputed[4 * i + 1] = static_cast(nG); pabyPrecomputed[4 * i + 2] = static_cast(nB); @@ -2043,8 +1977,7 @@ class GDALColorReliefDataset : public GDALDataset GDALDatasetH hSrcDS; GDALRasterBandH hSrcBand; - int nColorAssociation; - ColorAssociation *pasColorAssociation; + std::vector asColorAssociation{}; ColorSelectionMode eColorSelectionMode; GByte *pabyPrecomputed; int nIndexOffset; @@ -2089,13 +2022,13 @@ GDALColorReliefDataset::GDALColorReliefDataset( GDALDatasetH hSrcDSIn, GDALRasterBandH hSrcBandIn, const char *pszColorFilename, ColorSelectionMode eColorSelectionModeIn, int bAlpha) - : hSrcDS(hSrcDSIn), hSrcBand(hSrcBandIn), nColorAssociation(0), - pasColorAssociation(nullptr), eColorSelectionMode(eColorSelectionModeIn), - pabyPrecomputed(nullptr), nIndexOffset(0), pafSourceBuf(nullptr), - panSourceBuf(nullptr), nCurBlockXOff(-1), nCurBlockYOff(-1) + : hSrcDS(hSrcDSIn), hSrcBand(hSrcBandIn), + eColorSelectionMode(eColorSelectionModeIn), pabyPrecomputed(nullptr), + nIndexOffset(0), pafSourceBuf(nullptr), panSourceBuf(nullptr), + nCurBlockXOff(-1), nCurBlockYOff(-1) { - pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); + asColorAssociation = GDALColorReliefParseColorFile( + hSrcBand, pszColorFilename, eColorSelectionMode); nRasterXSize = GDALGetRasterXSize(hSrcDS); nRasterYSize = GDALGetRasterYSize(hSrcDS); @@ -2105,8 +2038,7 @@ GDALColorReliefDataset::GDALColorReliefDataset( GDALGetBlockSize(hSrcBand, &nBlockXSize, &nBlockYSize); pabyPrecomputed = GDALColorReliefPrecompute( - hSrcBand, pasColorAssociation, nColorAssociation, eColorSelectionMode, - &nIndexOffset); + hSrcBand, asColorAssociation, eColorSelectionMode, &nIndexOffset); for (int i = 0; i < ((bAlpha) ? 4 : 3); i++) { @@ -2123,7 +2055,6 @@ GDALColorReliefDataset::GDALColorReliefDataset( GDALColorReliefDataset::~GDALColorReliefDataset() { - CPLFree(pasColorAssociation); CPLFree(pabyPrecomputed); CPLFree(panSourceBuf); CPLFree(pafSourceBuf); @@ -2203,10 +2134,9 @@ CPLErr GDALColorReliefRasterBand::IReadBlock(int nBlockXOff, int nBlockYOff, for (int x = 0; x < nReqXSize; x++) { GDALColorReliefGetRGBA( - poGDS->pasColorAssociation, poGDS->nColorAssociation, - poGDS->pafSourceBuf[j], poGDS->eColorSelectionMode, - &anComponents[0], &anComponents[1], &anComponents[2], - &anComponents[3]); + poGDS->asColorAssociation, poGDS->pafSourceBuf[j], + poGDS->eColorSelectionMode, &anComponents[0], + &anComponents[1], &anComponents[2], &anComponents[3]); static_cast(pImage)[y * nBlockXSize + x] = static_cast(anComponents[nBand - 1]); j++; @@ -2233,10 +2163,9 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, hDstBand3 == nullptr) return CE_Failure; - int nColorAssociation = 0; - ColorAssociation *pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); - if (pasColorAssociation == nullptr) + const auto asColorAssociation = GDALColorReliefParseColorFile( + hSrcBand, pszColorFilename, eColorSelectionMode); + if (asColorAssociation.empty()) return CE_Failure; if (pfnProgress == nullptr) @@ -2248,8 +2177,7 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, /* -------------------------------------------------------------------- */ int nIndexOffset = 0; GByte *pabyPrecomputed = GDALColorReliefPrecompute( - hSrcBand, pasColorAssociation, nColorAssociation, eColorSelectionMode, - &nIndexOffset); + hSrcBand, asColorAssociation, eColorSelectionMode, &nIndexOffset); /* -------------------------------------------------------------------- */ /* Initialize progress counter. */ @@ -2279,7 +2207,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return CE_Failure; } @@ -2291,7 +2218,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return CE_Failure; } @@ -2315,7 +2241,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return eErr; } @@ -2334,9 +2259,8 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, { for (int j = 0; j < nXSize; j++) { - GDALColorReliefGetRGBA(pasColorAssociation, nColorAssociation, - pafSourceBuf[j], eColorSelectionMode, - &nR, &nG, &nB, &nA); + GDALColorReliefGetRGBA(asColorAssociation, pafSourceBuf[j], + eColorSelectionMode, &nR, &nG, &nB, &nA); pabyDestBuf1[j] = static_cast(nR); pabyDestBuf2[j] = static_cast(nG); pabyDestBuf3[j] = static_cast(nB); @@ -2355,7 +2279,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return eErr; } @@ -2368,7 +2291,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return eErr; } @@ -2381,7 +2303,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return eErr; } @@ -2396,7 +2317,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return eErr; } @@ -2410,7 +2330,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return CE_Failure; } @@ -2422,7 +2341,6 @@ GDALColorRelief(GDALRasterBandH hSrcBand, GDALRasterBandH hDstBand1, CPLFree(pafSourceBuf); CPLFree(panSourceBuf); CPLFree(pabyDestBuf1); - CPLFree(pasColorAssociation); return CE_None; } @@ -2438,16 +2356,14 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, ColorSelectionMode eColorSelectionMode, bool bAddAlpha) { - int nColorAssociation = 0; - ColorAssociation *pasColorAssociation = GDALColorReliefParseColorFile( - hSrcBand, pszColorFilename, &nColorAssociation, eColorSelectionMode); - if (pasColorAssociation == nullptr) + const auto asColorAssociation = GDALColorReliefParseColorFile( + hSrcBand, pszColorFilename, eColorSelectionMode); + if (asColorAssociation.empty()) return CE_Failure; VSILFILE *fp = VSIFOpenL(pszDstFilename, "wt"); if (fp == nullptr) { - CPLFree(pasColorAssociation); return CE_Failure; } @@ -2524,36 +2440,35 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, bOK &= VSIFPrintfL(fp, " ") > 0; - for (int iColor = 0; iColor < nColorAssociation; iColor++) + for (size_t iColor = 0; iColor < asColorAssociation.size(); iColor++) { - const double dfVal = pasColorAssociation[iColor].dfVal; + const double dfVal = asColorAssociation[iColor].dfVal; if (iColor > 0) bOK &= VSIFPrintfL(fp, ",") > 0; if (iColor > 0 && eColorSelectionMode == COLOR_SELECTION_NEAREST_ENTRY && dfVal != - std::nextafter(pasColorAssociation[iColor - 1].dfVal, + std::nextafter(asColorAssociation[iColor - 1].dfVal, std::numeric_limits::infinity())) { const double dfMidVal = - (dfVal + pasColorAssociation[iColor - 1].dfVal) / 2.0; + (dfVal + asColorAssociation[iColor - 1].dfVal) / 2.0; bOK &= VSIFPrintfL( fp, "%.18g:%d", std::nextafter( dfMidVal, -std::numeric_limits::infinity()), - (iBand == 0) ? pasColorAssociation[iColor - 1].nR - : (iBand == 1) ? pasColorAssociation[iColor - 1].nG - : (iBand == 2) - ? pasColorAssociation[iColor - 1].nB - : pasColorAssociation[iColor - 1].nA) > 0; + (iBand == 0) ? asColorAssociation[iColor - 1].nR + : (iBand == 1) ? asColorAssociation[iColor - 1].nG + : (iBand == 2) ? asColorAssociation[iColor - 1].nB + : asColorAssociation[iColor - 1].nA) > 0; bOK &= VSIFPrintfL( fp, ",%.18g:%d", dfMidVal, - (iBand == 0) ? pasColorAssociation[iColor].nR - : (iBand == 1) ? pasColorAssociation[iColor].nG - : (iBand == 2) ? pasColorAssociation[iColor].nB - : pasColorAssociation[iColor].nA) > 0; + (iBand == 0) ? asColorAssociation[iColor].nR + : (iBand == 1) ? asColorAssociation[iColor].nG + : (iBand == 2) ? asColorAssociation[iColor].nB + : asColorAssociation[iColor].nA) > 0; } else { @@ -2572,10 +2487,10 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, bOK &= VSIFPrintfL(fp, "%d", static_cast(dfVal)) > 0; bOK &= VSIFPrintfL( fp, ":%d", - (iBand == 0) ? pasColorAssociation[iColor].nR - : (iBand == 1) ? pasColorAssociation[iColor].nG - : (iBand == 2) ? pasColorAssociation[iColor].nB - : pasColorAssociation[iColor].nA) > 0; + (iBand == 0) ? asColorAssociation[iColor].nR + : (iBand == 1) ? asColorAssociation[iColor].nG + : (iBand == 2) ? asColorAssociation[iColor].nB + : asColorAssociation[iColor].nA) > 0; } if (eColorSelectionMode == COLOR_SELECTION_EXACT_ENTRY) @@ -2599,8 +2514,6 @@ static CPLErr GDALGenerateVRTColorRelief(const char *pszDstFilename, VSIFCloseL(fp); - CPLFree(pasColorAssociation); - return (bOK) ? CE_None : CE_Failure; } From dbae84844413a04995d01024f239982ab104fc25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Thu, 15 Aug 2024 13:02:25 +0300 Subject: [PATCH 10/14] Doc: fix typo in dev_practices.rst --- doc/source/development/dev_practices.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/development/dev_practices.rst b/doc/source/development/dev_practices.rst index a7319b0c4c08..3121e138bce9 100644 --- a/doc/source/development/dev_practices.rst +++ b/doc/source/development/dev_practices.rst @@ -284,7 +284,7 @@ Source tree layout - :file:`swig/python/gdal-utils/osgeo_utils`: Core code for GDAL Python utilities. Available in the PyPI gdal and gdal-utils packages. - :file:`swig/python/gdal-utils/samples`: Scripts that are not installed and generally not or very little documented. May serve as a staging area for future scripts that are going to be promoted as official. - :file:`swig/python/gdal-utils/auxiliary`: Helper methods and classes used by GDAL Python utilities. Available in the PyPI gdal and gdal-utils packages. -- :file:`third_pary`: Third-party libraries used by libgdal. Other may be found in: +- :file:`third_party`: Third-party libraries used by libgdal. Other may be found in: * :file:`alg/internal_libqhull` * :file:`apps/argparse` From 30e8a86cef94da1df06462b8580523add4d622db Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 15 Aug 2024 15:32:50 +0200 Subject: [PATCH 11/14] doc_build.yml: install ssh (now that it is no longer in proj-docs Docker image) --- .github/workflows/doc_build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/doc_build.yml b/.github/workflows/doc_build.yml index 91f6db8b444b..ad8310b46c70 100644 --- a/.github/workflows/doc_build.yml +++ b/.github/workflows/doc_build.yml @@ -106,6 +106,7 @@ jobs: if: ${{ github.ref_name == 'master' && github.repository == 'OSGeo/gdal' }} shell: bash -l {0} run: | + apt install -y ssh mkdir -p /root/.ssh && echo "${{ secrets.SSH_KEY_DOCS }}" > /root/.ssh/id_rsa chmod 700 /root/.ssh && chmod 600 /root/.ssh/id_rsa ssh-keyscan -t rsa github.com >> /root/.ssh/known_hosts From 4caab40505ce4fdf248b5f7f6b0fcb83b58f1925 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 15 Aug 2024 23:40:17 +0200 Subject: [PATCH 12/14] Internal libtiff: resync with upstream --- frmts/gtiff/libtiff/tif_dir.c | 32 ++++++++++++++++-------- frmts/gtiff/libtiff/tif_dirwrite.c | 6 ++--- frmts/gtiff/libtiff/tif_fax3.c | 15 +++++++++-- frmts/gtiff/libtiff/tif_getimage.c | 5 ++-- frmts/gtiff/libtiff/tif_jpeg.c | 18 +++++++++++--- frmts/gtiff/libtiff/tif_lerc.c | 2 ++ frmts/gtiff/libtiff/tif_lzma.c | 22 ++++++++++++++++ frmts/gtiff/libtiff/tif_lzw.c | 4 +++ frmts/gtiff/libtiff/tif_ojpeg.c | 12 +++++++++ frmts/gtiff/libtiff/tif_packbits.c | 1 + frmts/gtiff/libtiff/tif_pixarlog.c | 7 ++++++ frmts/gtiff/libtiff/tif_read.c | 39 ++++++++++++++++++++++------- frmts/gtiff/libtiff/tif_strip.c | 20 ++++++++++++++- frmts/gtiff/libtiff/tif_thunder.c | 16 +++++++----- frmts/gtiff/libtiff/tif_webp.c | 40 +++++++++++++++++++++++++++++- frmts/gtiff/libtiff/tif_zip.c | 27 +++++++++++++++++++- frmts/gtiff/libtiff/tif_zstd.c | 2 ++ 17 files changed, 229 insertions(+), 39 deletions(-) diff --git a/frmts/gtiff/libtiff/tif_dir.c b/frmts/gtiff/libtiff/tif_dir.c index a31a596b1821..459f9696fd3e 100644 --- a/frmts/gtiff/libtiff/tif_dir.c +++ b/frmts/gtiff/libtiff/tif_dir.c @@ -1864,7 +1864,9 @@ static int TIFFAdvanceDirectory(TIFF *tif, uint64_t *nextdiroff, uint64_t *off, if (((uint64_t)poffa != poff) || (poffb < poffa) || (poffb < (tmsize_t)sizeof(uint16_t)) || (poffb > tif->tif_size)) { - TIFFErrorExtR(tif, module, "Error fetching directory count"); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); *nextdiroff = 0; return (0); } @@ -1893,14 +1895,18 @@ static int TIFFAdvanceDirectory(TIFF *tif, uint64_t *nextdiroff, uint64_t *off, uint16_t dircount16; if (poff > (uint64_t)TIFF_TMSIZE_T_MAX - sizeof(uint64_t)) { - TIFFErrorExtR(tif, module, "Error fetching directory count"); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); return (0); } poffa = (tmsize_t)poff; poffb = poffa + sizeof(uint64_t); if (poffb > tif->tif_size) { - TIFFErrorExtR(tif, module, "Error fetching directory count"); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); return (0); } _TIFFmemcpy(&dircount64, tif->tif_base + poffa, sizeof(uint64_t)); @@ -1942,8 +1948,9 @@ static int TIFFAdvanceDirectory(TIFF *tif, uint64_t *nextdiroff, uint64_t *off, if (!SeekOK(tif, *nextdiroff) || !ReadOK(tif, &dircount, sizeof(uint16_t))) { - TIFFErrorExtR(tif, module, "%s: Error fetching directory count", - tif->tif_name); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); return (0); } if (tif->tif_flags & TIFF_SWAB) @@ -1969,15 +1976,18 @@ static int TIFFAdvanceDirectory(TIFF *tif, uint64_t *nextdiroff, uint64_t *off, if (!SeekOK(tif, *nextdiroff) || !ReadOK(tif, &dircount64, sizeof(uint64_t))) { - TIFFErrorExtR(tif, module, "%s: Error fetching directory count", - tif->tif_name); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); return (0); } if (tif->tif_flags & TIFF_SWAB) TIFFSwabLong8(&dircount64); if (dircount64 > 0xFFFF) { - TIFFErrorExtR(tif, module, "Error fetching directory count"); + TIFFErrorExtR(tif, module, + "%s:%d: %s: Error fetching directory count", + __FILE__, __LINE__, tif->tif_name); return (0); } dircount16 = (uint16_t)dircount64; @@ -2169,8 +2179,10 @@ int TIFFSetSubDirectory(TIFF *tif, uint64_t diroff) probablySubIFD = 1; } /* -1 because TIFFReadDirectory() will increment tif_curdir. */ - tif->tif_curdir = - curdir == 0 ? TIFF_NON_EXISTENT_DIR_NUMBER : curdir - 1; + if (curdir >= 1) + tif->tif_curdir = curdir - 1; + else + tif->tif_curdir = TIFF_NON_EXISTENT_DIR_NUMBER; } curdir = tif->tif_curdir; diff --git a/frmts/gtiff/libtiff/tif_dirwrite.c b/frmts/gtiff/libtiff/tif_dirwrite.c index 546fb48ad696..0701f40d97ef 100644 --- a/frmts/gtiff/libtiff/tif_dirwrite.c +++ b/frmts/gtiff/libtiff/tif_dirwrite.c @@ -2074,7 +2074,7 @@ static void EvaluateIFDdatasizeWrite(TIFF *tif, uint32_t count, uint64_t datalength = (uint64_t)count * typesize; if (datalength > ((tif->tif_flags & TIFF_BIGTIFF) ? 0x8U : 0x4U)) { - /* LibTIFF increments write address to an even offset, thus datalenght + /* LibTIFF increments write address to an even offset, thus datalength * written is also incremented. */ if (datalength & 1) datalength++; @@ -3198,7 +3198,7 @@ static int TIFFLinkDirectory(TIFF *tif) /* * Not the first directory, search to the last and append. */ - tdir_t dirn = -1; + tdir_t dirn = 0; if (tif->tif_lastdiroff != 0 && _TIFFGetDirNumberFromOffset(tif, tif->tif_lastdiroff, &dirn)) { @@ -3276,7 +3276,7 @@ static int TIFFLinkDirectory(TIFF *tif) /* * Not the first directory, search to the last and append. */ - tdir_t dirn = -1; + tdir_t dirn = 0; if (tif->tif_lastdiroff != 0 && _TIFFGetDirNumberFromOffset(tif, tif->tif_lastdiroff, &dirn)) { diff --git a/frmts/gtiff/libtiff/tif_fax3.c b/frmts/gtiff/libtiff/tif_fax3.c index 668e96a33073..01a784730bdc 100644 --- a/frmts/gtiff/libtiff/tif_fax3.c +++ b/frmts/gtiff/libtiff/tif_fax3.c @@ -569,7 +569,11 @@ static int Fax3SetupState(TIFF *tif) TIFFroundup and TIFFSafeMultiply return zero on integer overflow */ - dsp->runs = (uint32_t *)NULL; + if (dsp->runs != NULL) + { + _TIFFfreeExt(tif, dsp->runs); + dsp->runs = (uint32_t *)NULL; + } dsp->nruns = TIFFroundup_32(rowpixels + 1, 32); if (needsRefLine) { @@ -611,6 +615,10 @@ static int Fax3SetupState(TIFF *tif) * is referenced. The reference line must * be initialized to be ``white'' (done elsewhere). */ + if (esp->refline != NULL) + { + _TIFFfreeExt(tif, esp->refline); + } esp->refline = (unsigned char *)_TIFFmallocExt(tif, rowbytes); if (esp->refline == NULL) { @@ -1556,6 +1564,7 @@ static int Fax4Decode(TIFF *tif, uint8_t *buf, tmsize_t occ, uint16_t s) return (-1); } CACHE_STATE(tif, sp); + int start = sp->line; while (occ > 0) { a0 = 0; @@ -1604,7 +1613,9 @@ static int Fax4Decode(TIFF *tif, uint8_t *buf, tmsize_t occ, uint16_t s) } (*sp->fill)(buf, thisrun, pa, lastx); UNCACHE_STATE(tif, sp); - return (sp->line ? 1 : -1); /* don't error on badly-terminated strips */ + return (sp->line != start + ? 1 + : -1); /* don't error on badly-terminated strips */ } UNCACHE_STATE(tif, sp); return (1); diff --git a/frmts/gtiff/libtiff/tif_getimage.c b/frmts/gtiff/libtiff/tif_getimage.c index 0ada2697259c..6c7b5031a193 100644 --- a/frmts/gtiff/libtiff/tif_getimage.c +++ b/frmts/gtiff/libtiff/tif_getimage.c @@ -766,7 +766,6 @@ static int gtTileContig(TIFFRGBAImage *img, uint32_t *raster, uint32_t w, return (0); } - /* * Leftmost tile is clipped on left side if col_offset > 0. */ @@ -1111,7 +1110,6 @@ static int gtStripContig(TIFFRGBAImage *img, uint32_t *raster, uint32_t w, return (0); } - scanline = TIFFScanlineSize(tif); fromskew = (w < imagewidth ? imagewidth - w : 0); for (row = 0; row < h; row += nrow) @@ -3323,7 +3321,8 @@ int TIFFReadRGBATileExt(TIFF *tif, uint32_t col, uint32_t row, uint32_t *raster, TIFFGetFieldDefaulted(tif, TIFFTAG_TILELENGTH, &tile_ysize); if (tile_xsize == 0 || tile_ysize == 0) { - TIFFErrorExtR(tif, TIFFFileName(tif), "tile_xsize or tile_ysize is zero"); + TIFFErrorExtR(tif, TIFFFileName(tif), + "tile_xsize or tile_ysize is zero"); return (0); } diff --git a/frmts/gtiff/libtiff/tif_jpeg.c b/frmts/gtiff/libtiff/tif_jpeg.c index d80bbc6f012d..10aed5463589 100644 --- a/frmts/gtiff/libtiff/tif_jpeg.c +++ b/frmts/gtiff/libtiff/tif_jpeg.c @@ -88,18 +88,18 @@ int TIFFJPEGIsFullStripRequired_12(TIFF *tif); * 16bit value? */ -/* HAVE_JPEGTURBO_DUAL_MODE_8_12 is defined for libjpeg-turbo >= 2.2 which +/* HAVE_JPEGTURBO_DUAL_MODE_8_12 is defined for libjpeg-turbo >= 3.0 which * adds a dual-mode 8/12 bit API in the same library. */ #if defined(HAVE_JPEGTURBO_DUAL_MODE_8_12) #define JPEG_DUAL_MODE_8_12 /* Start by undefining BITS_IN_JSAMPLE which is always set to 8 in libjpeg-turbo - * >= 2.2 Cf + * >= 3.0 Cf * https://github.com/libjpeg-turbo/libjpeg-turbo/commit/8b9bc4b9635a2a047fb23ebe70c9acd728d3f99b */ #undef BITS_IN_JSAMPLE -/* libjpeg-turbo >= 2.2 adds J12xxxx datatypes for the 12-bit mode. */ +/* libjpeg-turbo >= 3.0 adds J12xxxx datatypes for the 12-bit mode. */ #if defined(FROM_TIF_JPEG_12) #define BITS_IN_JSAMPLE 12 #define TIFF_JSAMPLE J12SAMPLE @@ -1451,7 +1451,10 @@ static int JPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) sp->src.bytes_in_buffer = (size_t)tif->tif_rawcc; if (sp->bytesperline == 0) + { + memset(buf, 0, (size_t)cc); return 0; + } nrows = cc / sp->bytesperline; if (cc % sp->bytesperline) @@ -1472,7 +1475,10 @@ static int JPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) JSAMPROW bufptr = (JSAMPROW)buf; if (TIFFjpeg_read_scanlines(sp, &bufptr, 1) != 1) + { + memset(buf, 0, (size_t)cc); return (0); + } ++tif->tif_row; buf += sp->bytesperline; @@ -1506,7 +1512,10 @@ static int JPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) sp->src.bytes_in_buffer = (size_t)tif->tif_rawcc; if (sp->bytesperline == 0) + { + memset(buf, 0, (size_t)cc); return 0; + } nrows = cc / sp->bytesperline; if (cc % sp->bytesperline) @@ -1542,7 +1551,10 @@ static int JPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) * for 12bit data, which we need to repack. */ if (TIFFjpeg_read_scanlines(sp, &line_work_buf, 1) != 1) + { + memset(buf, 0, (size_t)cc); return (0); + } if (sp->cinfo.d.data_precision == 12) { diff --git a/frmts/gtiff/libtiff/tif_lerc.c b/frmts/gtiff/libtiff/tif_lerc.c index d9d8d3342eb3..d57b1d253de5 100644 --- a/frmts/gtiff/libtiff/tif_lerc.c +++ b/frmts/gtiff/libtiff/tif_lerc.c @@ -754,6 +754,7 @@ static int LERCDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (sp->uncompressed_buffer == 0) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Uncompressed buffer not allocated"); return 0; } @@ -761,6 +762,7 @@ static int LERCDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if ((uint64_t)sp->uncompressed_offset + (uint64_t)occ > sp->uncompressed_size) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Too many bytes read"); return 0; } diff --git a/frmts/gtiff/libtiff/tif_lzma.c b/frmts/gtiff/libtiff/tif_lzma.c index 6fdc92802627..db1c8b6829ff 100644 --- a/frmts/gtiff/libtiff/tif_lzma.c +++ b/frmts/gtiff/libtiff/tif_lzma.c @@ -44,6 +44,8 @@ typedef struct { TIFFPredictorState predict; + int read_error; /* whether a read error has occurred, and which should cause + further reads in the same strip/tile to be aborted */ lzma_stream stream; lzma_filter filters[LZMA_FILTERS_MAX + 1]; lzma_options_delta opt_delta; /* delta filter options */ @@ -156,6 +158,9 @@ static int LZMAPreDecode(TIFF *tif, uint16_t s) LZMAStrerror(ret)); return 0; } + + sp->read_error = 0; + return 1; } @@ -168,6 +173,16 @@ static int LZMADecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) assert(sp != NULL); assert(sp->state == LSTATE_INIT_DECODE); + if (sp->read_error) + { + memset(op, 0, (size_t)occ); + TIFFErrorExtR(tif, module, + "LZMADecode: Scanline %" PRIu32 " cannot be read due to " + "previous error", + tif->tif_row); + return 0; + } + sp->stream.next_in = tif->tif_rawcp; sp->stream.avail_in = (size_t)tif->tif_rawcc; @@ -175,6 +190,9 @@ static int LZMADecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) sp->stream.avail_out = (size_t)occ; if ((tmsize_t)sp->stream.avail_out != occ) { + // read_error not set here as this is a usage issue that can be + // recovered in a following call. + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Liblzma cannot deal with buffers this size"); return 0; @@ -198,6 +216,8 @@ static int LZMADecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) lzma_stream_decoder(&sp->stream, lzma_memusage(&sp->stream), 0); if (r != LZMA_OK) { + sp->read_error = 1; + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Error initializing the stream decoder, %s", LZMAStrerror(r)); @@ -217,6 +237,8 @@ static int LZMADecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) } while (sp->stream.avail_out > 0); if (sp->stream.avail_out != 0) { + sp->read_error = 1; + memset(sp->stream.next_out, 0, sp->stream.avail_out); TIFFErrorExtR(tif, module, "Not enough data at scanline %" PRIu32 " (short %" TIFF_SIZE_FORMAT " bytes)", diff --git a/frmts/gtiff/libtiff/tif_lzw.c b/frmts/gtiff/libtiff/tif_lzw.c index 05ac8aa1abe7..4baf78e50b20 100644 --- a/frmts/gtiff/libtiff/tif_lzw.c +++ b/frmts/gtiff/libtiff/tif_lzw.c @@ -417,6 +417,7 @@ static int LZWDecode(TIFF *tif, uint8_t *op0, tmsize_t occ0, uint16_t s) if (sp->read_error) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "LZWDecode: Scanline %" PRIu32 " cannot be read due to " "previous error", @@ -731,6 +732,7 @@ static int LZWDecode(TIFF *tif, uint8_t *op0, tmsize_t occ0, uint16_t s) if (occ > 0) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Not enough data at scanline %" PRIu32 " (short %" PRIu64 " bytes)", @@ -740,12 +742,14 @@ static int LZWDecode(TIFF *tif, uint8_t *op0, tmsize_t occ0, uint16_t s) return (1); no_eoi: + memset(op, 0, (size_t)occ); sp->read_error = 1; TIFFErrorExtR(tif, module, "LZWDecode: Strip %" PRIu32 " not terminated with EOI code", tif->tif_curstrip); return 0; error_code: + memset(op, 0, (size_t)occ); sp->read_error = 1; TIFFErrorExtR(tif, tif->tif_name, "Using code not yet in table"); return 0; diff --git a/frmts/gtiff/libtiff/tif_ojpeg.c b/frmts/gtiff/libtiff/tif_ojpeg.c index e4547cf09137..f94d2a4e45f7 100644 --- a/frmts/gtiff/libtiff/tif_ojpeg.c +++ b/frmts/gtiff/libtiff/tif_ojpeg.c @@ -755,6 +755,9 @@ static int OJPEGPreDecode(TIFF *tif, uint16_t s) if (OJPEGWriteHeaderInfo(tif) == 0) return (0); } + + sp->subsampling_convert_state = 0; + while (sp->write_curstrile < m) { if (sp->libjpeg_jpeg_query_style == 0) @@ -840,12 +843,14 @@ static int OJPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) (void)s; if (!sp->decoder_ok) { + memset(buf, 0, (size_t)cc); TIFFErrorExtR(tif, module, "Cannot decode: decoder not correctly initialized"); return 0; } if (sp->libjpeg_session_active == 0) { + memset(buf, 0, (size_t)cc); /* This should normally not happen, except that it does when */ /* using TIFFReadScanline() which calls OJPEGPostDecode() for */ /* each scanline, which assumes that a whole strile was read */ @@ -859,17 +864,24 @@ static int OJPEGDecode(TIFF *tif, uint8_t *buf, tmsize_t cc, uint16_t s) } if (sp->error_in_raw_data_decoding) { + memset(buf, 0, (size_t)cc); return 0; } if (sp->libjpeg_jpeg_query_style == 0) { if (OJPEGDecodeRaw(tif, buf, cc) == 0) + { + memset(buf, 0, (size_t)cc); return (0); + } } else { if (OJPEGDecodeScanlines(tif, buf, cc) == 0) + { + memset(buf, 0, (size_t)cc); return (0); + } } return (1); } diff --git a/frmts/gtiff/libtiff/tif_packbits.c b/frmts/gtiff/libtiff/tif_packbits.c index 62849f8f3c16..1ae50cbd47ce 100644 --- a/frmts/gtiff/libtiff/tif_packbits.c +++ b/frmts/gtiff/libtiff/tif_packbits.c @@ -300,6 +300,7 @@ static int PackBitsDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) tif->tif_rawcc = cc; if (occ > 0) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Not enough data for scanline %" PRIu32, tif->tif_row); return (0); diff --git a/frmts/gtiff/libtiff/tif_pixarlog.c b/frmts/gtiff/libtiff/tif_pixarlog.c index 5ed921eb3004..56cf416a7f6b 100644 --- a/frmts/gtiff/libtiff/tif_pixarlog.c +++ b/frmts/gtiff/libtiff/tif_pixarlog.c @@ -859,6 +859,7 @@ static int PixarLogDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) TIFFErrorExtR(tif, module, "%" PRIu16 " bit input not supported in PixarLog", td->td_bitspersample); + memset(op, 0, (size_t)occ); return 0; } @@ -879,12 +880,14 @@ static int PixarLogDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (sp->stream.avail_out != nsamples * sizeof(uint16_t)) { TIFFErrorExtR(tif, module, "ZLib cannot deal with buffers this size"); + memset(op, 0, (size_t)occ); return (0); } /* Check that we will not fill more than what was allocated */ if ((tmsize_t)sp->stream.avail_out > sp->tbuf_size) { TIFFErrorExtR(tif, module, "sp->stream.avail_out > sp->tbuf_size"); + memset(op, 0, (size_t)occ); return (0); } do @@ -899,12 +902,14 @@ static int PixarLogDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) TIFFErrorExtR( tif, module, "Decoding error at scanline %" PRIu32 ", %s", tif->tif_row, sp->stream.msg ? sp->stream.msg : "(null)"); + memset(op, 0, (size_t)occ); return (0); } if (state != Z_OK) { TIFFErrorExtR(tif, module, "ZLib error: %s", sp->stream.msg ? sp->stream.msg : "(null)"); + memset(op, 0, (size_t)occ); return (0); } } while (sp->stream.avail_out > 0); @@ -916,6 +921,7 @@ static int PixarLogDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) "Not enough data at scanline %" PRIu32 " (short %u bytes)", tif->tif_row, sp->stream.avail_out); + memset(op, 0, (size_t)occ); return (0); } @@ -977,6 +983,7 @@ static int PixarLogDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) default: TIFFErrorExtR(tif, module, "Unsupported bits/sample: %" PRIu16, td->td_bitspersample); + memset(op, 0, (size_t)occ); return (0); } } diff --git a/frmts/gtiff/libtiff/tif_read.c b/frmts/gtiff/libtiff/tif_read.c index df4bac69253c..7efab59c6a38 100644 --- a/frmts/gtiff/libtiff/tif_read.c +++ b/frmts/gtiff/libtiff/tif_read.c @@ -464,6 +464,10 @@ int TIFFReadScanline(TIFF *tif, void *buf, uint32_t row, uint16_t sample) if (e) (*tif->tif_postdecode)(tif, (uint8_t *)buf, tif->tif_scanlinesize); } + else + { + memset(buf, 0, (size_t)tif->tif_scanlinesize); + } return (e > 0 ? 1 : -1); } @@ -549,7 +553,10 @@ tmsize_t TIFFReadEncodedStrip(TIFF *tif, uint32_t strip, void *buf, if ((size != (tmsize_t)(-1)) && (size < stripsize)) stripsize = size; if (!TIFFFillStrip(tif, strip)) + { + memset(buf, 0, (size_t)stripsize); return ((tmsize_t)(-1)); + } if ((*tif->tif_decodestrip)(tif, buf, stripsize, plane) <= 0) return ((tmsize_t)(-1)); (*tif->tif_postdecode)(tif, buf, stripsize); @@ -967,9 +974,13 @@ tmsize_t TIFFReadEncodedTile(TIFF *tif, uint32_t tile, void *buf, tmsize_t size) size = tilesize; else if (size > tilesize) size = tilesize; - if (TIFFFillTile(tif, tile) && - (*tif->tif_decodetile)(tif, (uint8_t *)buf, size, - (uint16_t)(tile / td->td_stripsperimage))) + if (!TIFFFillTile(tif, tile)) + { + memset(buf, 0, (size_t)size); + return ((tmsize_t)(-1)); + } + else if ((*tif->tif_decodetile)(tif, (uint8_t *)buf, size, + (uint16_t)(tile / td->td_stripsperimage))) { (*tif->tif_postdecode)(tif, (uint8_t *)buf, size); return (size); @@ -1555,9 +1566,14 @@ int TIFFReadFromUserBuffer(TIFF *tif, uint32_t strile, void *inbuf, if (TIFFIsTiled(tif)) { - if (!TIFFStartTile(tif, strile) || - !(*tif->tif_decodetile)(tif, (uint8_t *)outbuf, outsize, - (uint16_t)(strile / td->td_stripsperimage))) + if (!TIFFStartTile(tif, strile)) + { + ret = 0; + memset(outbuf, 0, (size_t)outsize); + } + else if (!(*tif->tif_decodetile)( + tif, (uint8_t *)outbuf, outsize, + (uint16_t)(strile / td->td_stripsperimage))) { ret = 0; } @@ -1577,9 +1593,14 @@ int TIFFReadFromUserBuffer(TIFF *tif, uint32_t strile, void *inbuf, { stripsperplane = TIFFhowmany_32_maxuint_compat(td->td_imagelength, rowsperstrip); - if (!TIFFStartStrip(tif, strile) || - !(*tif->tif_decodestrip)(tif, (uint8_t *)outbuf, outsize, - (uint16_t)(strile / stripsperplane))) + if (!TIFFStartStrip(tif, strile)) + { + ret = 0; + memset(outbuf, 0, (size_t)outsize); + } + else if (!(*tif->tif_decodestrip)( + tif, (uint8_t *)outbuf, outsize, + (uint16_t)(strile / stripsperplane))) { ret = 0; } diff --git a/frmts/gtiff/libtiff/tif_strip.c b/frmts/gtiff/libtiff/tif_strip.c index ee31892af269..7ae7ce41dfa2 100644 --- a/frmts/gtiff/libtiff/tif_strip.c +++ b/frmts/gtiff/libtiff/tif_strip.c @@ -294,7 +294,25 @@ uint64_t TIFFScanlineSize64(TIFF *tif) else { uint64_t scanline_samples; - scanline_samples = _TIFFMultiply64(tif, td->td_imagewidth, + uint32_t scanline_width = td->td_imagewidth; + +#if 0 + // Tries to fix https://gitlab.com/libtiff/libtiff/-/merge_requests/564 + // but causes regression when decoding legit files with tiffcp -c none + // Cf https://gitlab.com/libtiff/libtiff/-/merge_requests/644 + if (td->td_photometric == PHOTOMETRIC_YCBCR) + { + uint16_t subsampling_hor; + uint16_t ignored; + TIFFGetFieldDefaulted(tif, TIFFTAG_YCBCRSUBSAMPLING, + &subsampling_hor, &ignored); + if (subsampling_hor > 1) // roundup width for YCbCr + scanline_width = + TIFFroundup_32(scanline_width, subsampling_hor); + } +#endif + + scanline_samples = _TIFFMultiply64(tif, scanline_width, td->td_samplesperpixel, module); scanline_size = TIFFhowmany_64(_TIFFMultiply64(tif, scanline_samples, diff --git a/frmts/gtiff/libtiff/tif_thunder.c b/frmts/gtiff/libtiff/tif_thunder.c index 1f97362ca39d..bac0607dbe54 100644 --- a/frmts/gtiff/libtiff/tif_thunder.c +++ b/frmts/gtiff/libtiff/tif_thunder.c @@ -82,13 +82,14 @@ static int ThunderSetupDecode(TIFF *tif) return (1); } -static int ThunderDecode(TIFF *tif, uint8_t *op, tmsize_t maxpixels) +static int ThunderDecode(TIFF *tif, uint8_t *op0, tmsize_t maxpixels) { static const char module[] = "ThunderDecode"; register unsigned char *bp; register tmsize_t cc; unsigned int lastpixel; tmsize_t npixels; + uint8_t *op = op0; bp = (unsigned char *)tif->tif_rawcp; cc = tif->tif_rawcc; @@ -107,6 +108,8 @@ static int ThunderDecode(TIFF *tif, uint8_t *op, tmsize_t maxpixels) * Replicate the last pixel n times, * where n is the lower-order 6 bits. */ + if (n == 0) + break; if (npixels & 1) { op[0] |= lastpixel; @@ -117,11 +120,10 @@ static int ThunderDecode(TIFF *tif, uint8_t *op, tmsize_t maxpixels) else lastpixel |= lastpixel << 4; npixels += n; - if (npixels < maxpixels) - { - for (; n > 0; n -= 2) - *op++ = (uint8_t)lastpixel; - } + if (npixels > maxpixels) + break; + for (; n > 0; n -= 2) + *op++ = (uint8_t)lastpixel; if (n == -1) *--op &= 0xf0; lastpixel &= 0xf; @@ -154,6 +156,8 @@ static int ThunderDecode(TIFF *tif, uint8_t *op, tmsize_t maxpixels) tif->tif_rawcc = cc; if (npixels != maxpixels) { + uint8_t *op_end = op0 + (maxpixels + 1) / 2; + memset(op, 0, (size_t)(op_end - op)); TIFFErrorExtR(tif, module, "%s data at scanline %lu (%" PRIu64 " != %" PRIu64 ")", npixels < maxpixels ? "Not enough" : "Too much", diff --git a/frmts/gtiff/libtiff/tif_webp.c b/frmts/gtiff/libtiff/tif_webp.c index bf9d77eb9beb..ccffef070377 100644 --- a/frmts/gtiff/libtiff/tif_webp.c +++ b/frmts/gtiff/libtiff/tif_webp.c @@ -47,7 +47,9 @@ typedef struct { uint16_t nSamples; /* number of samples per pixel */ - int lossless; /* lossy/lossless compression */ + int read_error; /* whether a read error has occurred, and which should cause + further reads in the same strip/tile to be aborted */ + int lossless; /* lossy/lossless compression */ int lossless_exact; /* lossless exact mode. If TRUE, R,G,B values in areas with alpha = 0 will be preserved */ int quality_level; /* compression level */ @@ -133,6 +135,16 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) assert(sp != NULL); assert(sp->state == LSTATE_INIT_DECODE); + if (sp->read_error) + { + memset(op, 0, (size_t)occ); + TIFFErrorExtR(tif, module, + "ZIPDecode: Scanline %" PRIu32 " cannot be read due to " + "previous error", + tif->tif_row); + return 0; + } + if (sp->psDecoder == NULL) { TIFFDirectory *td = &tif->tif_dir; @@ -158,12 +170,16 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) : (uint32_t)tif->tif_rawcc, &webp_width, &webp_height)) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "WebPGetInfo() failed"); return 0; } if ((uint32_t)webp_width != segment_width || (uint32_t)webp_height != segment_height) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR( tif, module, "WebP blob dimension is %dx%d. Expected %ux%u", webp_width, webp_height, segment_width, segment_height); @@ -174,6 +190,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) WebPDecoderConfig config; if (!WebPInitDecoderConfig(&config)) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "WebPInitDecoderConfig() failed"); return 0; } @@ -189,6 +207,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (!bWebPGetFeaturesOK) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "WebPInitDecoderConfig() failed"); return 0; } @@ -202,6 +222,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) */ !(webp_bands == 3 && sp->nSamples == 4)) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "WebP blob band count is %d. Expected %d", webp_bands, sp->nSamples); @@ -228,6 +250,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (!sp->pBuffer) { TIFFErrorExtR(tif, module, "Cannot allocate buffer"); + memset(op, 0, (size_t)occ); + sp->read_error = 1; return 0; } sp->buffer_size = buffer_size; @@ -257,6 +281,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (sp->psDecoder == NULL) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "Unable to allocate WebP decoder."); return 0; } @@ -264,6 +290,10 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (occ % sp->sDecBuffer.u.RGBA.stride) { + // read_error not set here as this is a usage issue that can be + // recovered in a following call. + memset(op, 0, (size_t)occ); + /* Do not set read_error as could potentially be recovered */ TIFFErrorExtR(tif, module, "Fractional scanlines cannot be read"); return 0; } @@ -284,6 +314,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) { TIFFErrorExtR(tif, module, "Unrecognized error."); } + memset(op, 0, (size_t)occ); + sp->read_error = 1; return 0; } else @@ -303,6 +335,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) { if (current_y != numberOfExpectedLines) { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "Unable to decode WebP data: less lines than " "expected."); @@ -332,6 +366,8 @@ static int TWebPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) } else { + memset(op, 0, (size_t)occ); + sp->read_error = 1; TIFFErrorExtR(tif, module, "Unable to decode WebP data."); return 0; } @@ -451,6 +487,8 @@ static int TWebPPreDecode(TIFF *tif, uint16_t s) sp->psDecoder = NULL; } + sp->read_error = 0; + return 1; } diff --git a/frmts/gtiff/libtiff/tif_zip.c b/frmts/gtiff/libtiff/tif_zip.c index a351ba5649fc..2a2a1d787f69 100644 --- a/frmts/gtiff/libtiff/tif_zip.c +++ b/frmts/gtiff/libtiff/tif_zip.c @@ -67,6 +67,8 @@ typedef struct { TIFFPredictorState predict; z_stream stream; + int read_error; /* whether a read error has occurred, and which should cause + further reads in the same strip/tile to be aborted */ int zipquality; /* compression level */ int state; /* state flags */ int subcodec; /* DEFLATE_SUBCODEC_ZLIB or DEFLATE_SUBCODEC_LIBDEFLATE */ @@ -150,7 +152,12 @@ static int ZIPPreDecode(TIFF *tif, uint16_t s) sp->stream.avail_in = (uint64_t)tif->tif_rawcc < 0xFFFFFFFFU ? (uInt)tif->tif_rawcc : 0xFFFFFFFFU; - return (inflateReset(&sp->stream) == Z_OK); + if (inflateReset(&sp->stream) == Z_OK) + { + sp->read_error = 0; + return 1; + } + return 0; } static int ZIPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) @@ -162,6 +169,16 @@ static int ZIPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) assert(sp != NULL); assert(sp->state == ZSTATE_INIT_DECODE); + if (sp->read_error) + { + memset(op, 0, (size_t)occ); + TIFFErrorExtR(tif, module, + "ZIPDecode: Scanline %" PRIu32 " cannot be read due to " + "previous error", + tif->tif_row); + return 0; + } + #if LIBDEFLATE_SUPPORT if (sp->libdeflate_state == 1) return 0; @@ -227,8 +244,10 @@ static int ZIPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (res != LIBDEFLATE_SUCCESS && res != LIBDEFLATE_INSUFFICIENT_SPACE) { + memset(op, 0, (size_t)occ); TIFFErrorExtR(tif, module, "Decoding error at scanline %lu", (unsigned long)tif->tif_row); + sp->read_error = 1; return 0; } @@ -263,13 +282,17 @@ static int ZIPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) break; if (state == Z_DATA_ERROR) { + memset(sp->stream.next_out, 0, sp->stream.avail_out); TIFFErrorExtR(tif, module, "Decoding error at scanline %lu, %s", (unsigned long)tif->tif_row, SAFE_MSG(sp)); + sp->read_error = 1; return (0); } if (state != Z_OK) { + memset(sp->stream.next_out, 0, sp->stream.avail_out); TIFFErrorExtR(tif, module, "ZLib error: %s", SAFE_MSG(sp)); + sp->read_error = 1; return (0); } } while (occ > 0); @@ -279,6 +302,8 @@ static int ZIPDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) "Not enough data at scanline %lu (short %" PRIu64 " bytes)", (unsigned long)tif->tif_row, (uint64_t)occ); + memset(sp->stream.next_out, 0, sp->stream.avail_out); + sp->read_error = 1; return (0); } diff --git a/frmts/gtiff/libtiff/tif_zstd.c b/frmts/gtiff/libtiff/tif_zstd.c index 5aaf4c44bb3a..fc73ce9cf53e 100644 --- a/frmts/gtiff/libtiff/tif_zstd.c +++ b/frmts/gtiff/libtiff/tif_zstd.c @@ -146,6 +146,7 @@ static int ZSTDDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) zstd_ret = ZSTD_decompressStream(sp->dstream, &out_buffer, &in_buffer); if (ZSTD_isError(zstd_ret)) { + memset(op + out_buffer.pos, 0, out_buffer.size - out_buffer.pos); TIFFErrorExtR(tif, module, "Error in ZSTD_decompressStream(): %s", ZSTD_getErrorName(zstd_ret)); return 0; @@ -155,6 +156,7 @@ static int ZSTDDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s) if (out_buffer.pos < (size_t)occ) { + memset(op + out_buffer.pos, 0, out_buffer.size - out_buffer.pos); TIFFErrorExtR(tif, module, "Not enough data at scanline %lu (short %lu bytes)", (unsigned long)tif->tif_row, From fe7c842f431051689d6beede64aae31cf67ddc32 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 16 Aug 2024 13:00:03 +0200 Subject: [PATCH 13/14] NEWS.md: update with 3.9.2 --- NEWS.md | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) diff --git a/NEWS.md b/NEWS.md index cb81e534f3ce..89236bf03253 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,166 @@ +# GDAL/OGR 3.9.2 Release Notes + +GDAL 3.9.2 is a bugfix release. + +## Build + +* Fix compilation against openssl-libs-3.2.2-3 of fedora:rawhide +* Fix compilation against libarchive-3.3.3-5 as shipped by RHEL 8 (#10428) +* Fix -Wnull-dereference warnings of gcc 14.2 + +## GDAL 3.9.2 + +### Port + +* CPLFormFilename()/CPLGetDirname()/CPLGetPath(): make it work with + 'vsicurl/http://example.com?foo' type of filename, to fix Zarr driver + +### Core + +* GDALCopyWords(): Fix double->uint64 when input value > UINT64_MAX that was + wrongly converted to 0 + +### Raster utilities + +* gdalinfo_output.schema.json: pin stac-extensions/eo to v1.1.0 +* gdal_translate: fix -a_nodata to accept '-inf' as input (3.9.0 regression) +* gdalwarp: fix -srcnodata/-dstnodata to accept negative value in first position + (3.9.0 regression) +* gdalbuildvrt: -fix -srcnodata/-vrtnodata to accept negative value in first + position (3.9.0 regression) +* gdallocationinfo: avoid extra newline character in -valonly mode if coordinate + is outside raster extent (3.9.0 regression) +* gdal_rasterize: on a int64 band, set nodata value as int64 (#10306) +* gdal_rasterize: restrict to defaulting to Int64 raster data type only if + output driver supports it (and the burned field is Int64) +* gdal_retile: error out with clear message when trying to retile a file with a + geotransform with rotation terms, or several input files with inconsistent SRS + (#10333) +* gdallocationinfo: in -E echo mode, always report input coordinates, not pixel,line +* gdal2tiles: update links in generate_leaflet(), remove OSM Toner (#10304) +* gdal2tiles: uUse correct OpenStreetMap tile url (openstreetmap/operations#737) +* gdal2tiles: fix exception with --nodata-values-pct-threshold but not + --excluded-values on a multi-band raster + +### Raster drivers + +COG driver: + * properly deal with mask bands w.r.t resampling when generating JPEG output, + or when input has a mask band (3.5 regression) (#10536) + +GTI driver: + * start looking for OVERVIEW__xxxx metadata items at index 0 + * automatically add overviews of overviews, unless the OVERVIEW_LEVEL=NONE open + option is specified + +GTiff driver: + * make SetNoDataValue(double) work on a Int64/UInt64 band (#10306) + +KEA driver: + * don't derive from PAM classes (#10355) + +JPEG driver: + * ReadFLIRMetadata(): avoid potential infinite loop + +netCDF driver: + * multidim: fix use-after-free on string variables in ReadOneElement() + +NITF driver: + * 12-bit JPEG writer: fix crash if raster width > block width (#10441) + +OGCAPI driver: + * do not emit 'Server does not support specified IMAGE_FORMAT: AUTO' when + opening a vector layer with MVT tiles only + * fix reading encodingInfo/dataType for Coverage API + +SRTMHGT driver: + * add support for 0.5 deg resolution datasets (#10514) + +VRT driver: + * fix reading from virtual overviews when SrcRect / DstRect elements are missing + +WMTS driver: + * make sure not to request tiles outside of tile matrix / tile matrix limits, + if the dataset extent goes beyond them + * clip layer extent with union of extent of tile matrices (#10348) + +## OGR 3.9.2 + +### Core + +* WKT geometry importer: accept (non conformant) PointZ/PointZM without space as + generated by current QGIS versions +* OGR SQL: do not make backslash a special character inside single-quoted + strings, to improve compliance with SQL92 (#10416) +* OGRSQL: return in error when 'Did not find end-of-string character' is emitted + (#10515) + +### OGRSpatialReference + +* EPSGTreatsAsNorthingEasting(): make it work correctly with compound CRS + +### Vector drivers + +GeoJSON driver: + * writer: make sure geometry is reprojected even when it is invalid after + coordinate rounding (3.9.0 regression) (qgis/QGIS#58169) + +GML driver: + * writer: fix missing SRS in Featurecollection's boundedBy element (3.9.1 + regression) (#10332) + +GMLAS driver: + * avoid crash on a OSSFuzz generated xsd (ossfuzz#70511) + +GTFS: + * fix error when applying an attribute filter on a field whose advertized data + type is not string (duckdb/duckdb_spatial#343) + +JSONFG driver: + * fix reading non-FeatureCollection documents larger than 6,000 bytes (#10525) + +LIBKML driver: + * fix writing a .kmz to cloud storage (#10313) + * fix LIBKML_RESOLVE_STYLE=YES when reading a KML file with dataset-level + styles (3.9.1 regression) (qgis/qgis#58208) + * fix reading/writing style URL in KMZ files (#10478) + +MiraMonVector driver: + * fix oss-fuzz#70860 + +OAPIF driver: + * fix resolving of relative links (#10410) + +OpenFileGDB driver: + * fix attribute filter when an equality comparison is involved with a field + with an index created with a LOWER(field_name) expression (#10345) + +OSM driver: + * avoid creating /vsimem/osm_importer directory (#10566) + +PG driver: + * fix ogr2ogr scenarios to PostgreSQL when there are several input layer names + like schema_name.layer_name (3.9.0 regression) + * fix support for geometry/geography OID in range between 2^31 and 2^32 (#10486) + +PG/PGDump drivers: + * make sure spatial index name is unique, even if several tables have a long + enough prefix (#10522, #7629) + +Shapefile driver: + * fix recognizing an empty string as a NULL date (#10405) + * fix off-by-one write heap buffer overflow when deleting exactly 128, 232, + etc. features and repacking (#10451) + +XLSX driver: + * support documents whose XML elements have a prefix (duckdb +/duckdb_spatial#362) + +## Python bindings + +* fix typos in gdal.Footprint() help message +* make MDArray.Write(array_of_strings) work with a 0-d string variable + # GDAL/OGR 3.9.1 Release Notes GDAL 3.9.1 is a bugfix release. From 1196ed30952a1126d6abdc6dc9bae10b44f7acaa Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 16 Aug 2024 13:00:11 +0200 Subject: [PATCH 14/14] Doc: advertize 3.9.2 --- doc/source/about_no_title.rst | 4 ++-- doc/source/download.rst | 8 ++++---- doc/source/download_past.rst | 6 ++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/doc/source/about_no_title.rst b/doc/source/about_no_title.rst index d7163f525f89..2a98d1a4cfb3 100644 --- a/doc/source/about_no_title.rst +++ b/doc/source/about_no_title.rst @@ -1,11 +1,11 @@ -GDAL is a translator library for raster and vector geospatial data formats that is released under an MIT style Open Source :ref:`license` by the `Open Source Geospatial Foundation`_. As a library, it presents a single raster abstract data model and single vector abstract data model to the calling application for all supported formats. It also comes with a variety of useful command line utilities for data translation and processing. The `NEWS`_ page describes the June 2024 GDAL/OGR 3.9.1 release. +GDAL is a translator library for raster and vector geospatial data formats that is released under an MIT style Open Source :ref:`license` by the `Open Source Geospatial Foundation`_. As a library, it presents a single raster abstract data model and single vector abstract data model to the calling application for all supported formats. It also comes with a variety of useful command line utilities for data translation and processing. The `NEWS`_ page describes the August 2024 GDAL/OGR 3.9.2 release. .. image:: ../images/OSGeo_project.png :alt: OSGeo project :target: `Open Source Geospatial Foundation`_ .. _`Open Source Geospatial Foundation`: http://www.osgeo.org/ -.. _`NEWS`: https://github.com/OSGeo/gdal/blob/v3.9.1/NEWS.md +.. _`NEWS`: https://github.com/OSGeo/gdal/blob/v3.9.2/NEWS.md See :ref:`software_using_gdal` diff --git a/doc/source/download.rst b/doc/source/download.rst index d67a20bd4e69..3bc444622f68 100644 --- a/doc/source/download.rst +++ b/doc/source/download.rst @@ -18,11 +18,11 @@ Source Code Current Release ............... -* **2024-06-26** `gdal-3.9.1.tar.gz`_ `3.9.1 Release Notes`_ (`3.9.1 md5`_) +* **2024-08-16** `gdal-3.9.2.tar.gz`_ `3.9.2 Release Notes`_ (`3.9.2 md5`_) -.. _`3.9.1 Release Notes`: https://github.com/OSGeo/gdal/blob/v3.9.1/NEWS.md -.. _`gdal-3.9.1.tar.gz`: https://github.com/OSGeo/gdal/releases/download/v3.9.1/gdal-3.9.1.tar.gz -.. _`3.9.1 md5`: https://github.com/OSGeo/gdal/releases/download/v3.9.1/gdal-3.9.1.tar.gz.md5 +.. _`3.9.2 Release Notes`: https://github.com/OSGeo/gdal/blob/v3.9.2/NEWS.md +.. _`gdal-3.9.2.tar.gz`: https://github.com/OSGeo/gdal/releases/download/v3.9.2/gdal-3.9.2.tar.gz +.. _`3.9.2 md5`: https://github.com/OSGeo/gdal/releases/download/v3.9.2/gdal-3.9.2.tar.gz.md5 Past Releases ............. diff --git a/doc/source/download_past.rst b/doc/source/download_past.rst index 608ddfccfad4..340924f953a1 100644 --- a/doc/source/download_past.rst +++ b/doc/source/download_past.rst @@ -5,6 +5,12 @@ Past Releases ============= +* **2024-06-26** `gdal-3.9.1.tar.gz`_ `3.9.1 Release Notes`_ (`3.9.1 md5`_) + +.. _`3.9.1 Release Notes`: https://github.com/OSGeo/gdal/blob/v3.9.1/NEWS.md +.. _`gdal-3.9.1.tar.gz`: https://github.com/OSGeo/gdal/releases/download/v3.9.1/gdal-3.9.1.tar.gz +.. _`3.9.1 md5`: https://github.com/OSGeo/gdal/releases/download/v3.9.1/gdal-3.9.1.tar.gz.md5 + * **2024-05-10** `gdal-3.9.0.tar.gz`_ `3.9.0 Release Notes`_ (`3.9.0 md5`_) .. _`3.9.0 Release Notes`: https://github.com/OSGeo/gdal/blob/v3.9.0/NEWS.md