Merge pull request #13902 from rouault/fix_13862

rouault · web-flow · commit 7877247db4e1 · 2026-02-11T22:05:37.000+01:00
VRTDerivedRasterBand::IRasterIO(): fix output buffer zero intialization when nLineSpace != nBufXSize * nPixelSpace
diff --git a/autotest/gdrivers/vrtderived.py b/autotest/gdrivers/vrtderived.py
@@ -2504,3 +2504,39 @@ def test_vrt_derived_virtual_overviews(tmp_vsimem):
     with gdaltest.error_raised(gdal.CE_None):
         got = mask_band.ReadRaster(0, 0, width, height, 1, 1)
     assert got == b"\x02"
+
+
+def test_vrt_derived_zero_initialization(tmp_vsimem):
+
+    vrt_w = 20
+    vrt_h = 10
+    tile_w = vrt_w // 2
+    tile_h = vrt_h
+    tile_offset = vrt_w - tile_w
+    with gdal.GetDriverByName("GTiff").Create(
+        tmp_vsimem / "src1.tif", tile_w, tile_h, 1, gdal.GDT_UInt8
+    ) as src:
+        src.WriteRaster(0, 0, tile_w, tile_h, b"\x01" * (tile_w * tile_h))
+
+    xml = f"""
+    <VRTDataset rasterXSize="{vrt_w}" rasterYSize="{vrt_h}">
+      <VRTRasterBand dataType="Byte" band="1" subclass="VRTDerivedRasterBand">
+        <PixelFunctionType>sum</PixelFunctionType>
+        <SimpleSource>
+          <SourceFilename>{tmp_vsimem / "src1.tif"}</SourceFilename>
+          <SourceBand>1</SourceBand>
+          <SrcRect xOff="0" yOff="0" xSize="{tile_w}" ySize="{tile_h}" />
+          <DstRect xOff="{tile_offset}" yOff="0" xSize="{tile_w}" ySize="{tile_h}" />
+        </SimpleSource>
+      </VRTRasterBand>
+    </VRTDataset>"""
+
+    buf_obj = bytearray(b"\xFF" * ((vrt_h - 1) * vrt_w + tile_offset))
+    got = gdal.Open(xml).ReadRaster(
+        0, 0, tile_offset, vrt_h, buf_obj=buf_obj, buf_line_space=vrt_w
+    )
+    assert (
+        got
+        == (b"\x00" * tile_offset + b"\xFF" * tile_w) * (vrt_h - 1)
+        + b"\x00" * tile_offset
+    )
diff --git a/frmts/vrt/vrtdataset.h b/frmts/vrt/vrtdataset.h
@@ -955,6 +955,10 @@ class CPL_DLL VRTSourcedRasterBand CPL_NON_FINAL : public VRTRasterBand
   protected:
     bool SkipBufferInitialization();
 
+    void InitializeOutputBuffer(void *pData, int nBufXSize, int nBufYSize,
+                                GDALDataType eBufType, GSpacing nPixelSpace,
+                                GSpacing nLineSpace) const;
+
   public:
     std::vector<std::unique_ptr<VRTSource>> m_papoSources{};
 
diff --git a/frmts/vrt/vrtderivedrasterband.cpp b/frmts/vrt/vrtderivedrasterband.cpp
@@ -982,6 +982,56 @@ CPLErr VRTDerivedRasterBand::GetPixelFunctionArguments(
     return CE_None;
 }
 
+/************************************************************************/
+/*                   CreateMapBufferIdxToSourceIdx()                    */
+/************************************************************************/
+
+static bool CreateMapBufferIdxToSourceIdx(
+    const std::vector<std::unique_ptr<VRTSource>> &apoSources,
+    bool bSkipNonContributingSources, int nXOff, int nYOff, int nXSize,
+    int nYSize, int nBufXSize, int nBufYSize, GDALRasterIOExtraArg *psExtraArg,
+    bool &bCreateMapBufferIdxToSourceIdxHasRun,
+    std::vector<int> &anMapBufferIdxToSourceIdx,
+    bool &bSkipOutputBufferInitialization)
+{
+    CPLAssert(!bCreateMapBufferIdxToSourceIdxHasRun);
+    bCreateMapBufferIdxToSourceIdxHasRun = true;
+    anMapBufferIdxToSourceIdx.reserve(apoSources.size());
+    for (int iSource = 0; iSource < static_cast<int>(apoSources.size());
+         iSource++)
+    {
+        if (bSkipNonContributingSources &&
+            apoSources[iSource]->IsSimpleSource())
+        {
+            bool bError = false;
+            double dfReqXOff, dfReqYOff, dfReqXSize, dfReqYSize;
+            int nReqXOff, nReqYOff, nReqXSize, nReqYSize;
+            int nOutXOff, nOutYOff, nOutXSize, nOutYSize;
+            auto poSource =
+                static_cast<VRTSimpleSource *>(apoSources[iSource].get());
+            if (!poSource->GetSrcDstWindow(
+                    nXOff, nYOff, nXSize, nYSize, nBufXSize, nBufYSize,
+                    psExtraArg->eResampleAlg, &dfReqXOff, &dfReqYOff,
+                    &dfReqXSize, &dfReqYSize, &nReqXOff, &nReqYOff, &nReqXSize,
+                    &nReqYSize, &nOutXOff, &nOutYOff, &nOutXSize, &nOutYSize,
+                    bError))
+            {
+                if (bError)
+                {
+                    return false;
+                }
+
+                // Skip non contributing source
+                bSkipOutputBufferInitialization = false;
+                continue;
+            }
+        }
+
+        anMapBufferIdxToSourceIdx.push_back(iSource);
+    }
+    return true;
+}
+
 /************************************************************************/
 /*                             IRasterIO()                              */
 /************************************************************************/
@@ -1142,6 +1192,10 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
     }
     const int nSrcTypeSize = GDALGetDataTypeSizeBytes(eSrcType);
 
+    std::vector<int> anMapBufferIdxToSourceIdx;
+    bool bSkipOutputBufferInitialization = !m_papoSources.empty();
+    bool bCreateMapBufferIdxToSourceIdxHasRun = false;
+
     // If acquiring the region of interest in a single time is going
     // to consume too much RAM, split in halves, and that recursively
     // until we get below m_nAllowedRAMUsage.
@@ -1151,11 +1205,34 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
             m_poPrivate->m_nAllowedRAMUsage /
                 (static_cast<int>(m_papoSources.size()) * nSrcTypeSize))
     {
-        CPLErr eErr = SplitRasterIO(eRWFlag, nXOff, nYOff, nXSize, nYSize,
-                                    pData, nBufXSize, nBufYSize, eBufType,
-                                    nPixelSpace, nLineSpace, psExtraArg);
-        if (eErr != CE_Warning)
-            return eErr;
+        bool bSplit = true;
+        if (m_poPrivate->m_bSkipNonContributingSources)
+        {
+            // More accurate check by comparing against the number of
+            // actually contributing sources.
+            if (!CreateMapBufferIdxToSourceIdx(
+                    m_papoSources, m_poPrivate->m_bSkipNonContributingSources,
+                    nXOff, nYOff, nXSize, nYSize, nBufXSize, nBufYSize,
+                    psExtraArg, bCreateMapBufferIdxToSourceIdxHasRun,
+                    anMapBufferIdxToSourceIdx, bSkipOutputBufferInitialization))
+            {
+                return CE_Failure;
+            }
+            bSplit =
+                !anMapBufferIdxToSourceIdx.empty() &&
+                static_cast<GIntBig>(nBufXSize) * nBufYSize >
+                    m_poPrivate->m_nAllowedRAMUsage /
+                        (static_cast<int>(anMapBufferIdxToSourceIdx.size()) *
+                         nSrcTypeSize);
+        }
+        if (bSplit)
+        {
+            CPLErr eErr = SplitRasterIO(eRWFlag, nXOff, nYOff, nXSize, nYSize,
+                                        pData, nBufXSize, nBufYSize, eBufType,
+                                        nPixelSpace, nLineSpace, psExtraArg);
+            if (eErr != CE_Warning)
+                return eErr;
+        }
     }
 
     /* ---- Get pixel function for band ---- */
@@ -1196,51 +1273,32 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
     }
     const int nExtBufXSize = nBufXSize + 2 * nBufferRadius;
     const int nExtBufYSize = nBufYSize + 2 * nBufferRadius;
-    int nBufferCount = 0;
 
-    std::vector<std::unique_ptr<void, VSIFreeReleaser>> apBuffers(
-        m_papoSources.size());
-    std::vector<int> anMapBufferIdxToSourceIdx(m_papoSources.size());
-    bool bSkipOutputBufferInitialization = !m_papoSources.empty();
-    for (int iSource = 0; iSource < static_cast<int>(m_papoSources.size());
-         iSource++)
+    if (!bCreateMapBufferIdxToSourceIdxHasRun)
     {
-        if (m_poPrivate->m_bSkipNonContributingSources &&
-            m_papoSources[iSource]->IsSimpleSource())
+        if (!CreateMapBufferIdxToSourceIdx(
+                m_papoSources, m_poPrivate->m_bSkipNonContributingSources,
+                nXOff, nYOff, nXSize, nYSize, nBufXSize, nBufYSize, psExtraArg,
+                bCreateMapBufferIdxToSourceIdxHasRun, anMapBufferIdxToSourceIdx,
+                bSkipOutputBufferInitialization))
         {
-            bool bError = false;
-            double dfReqXOff, dfReqYOff, dfReqXSize, dfReqYSize;
-            int nReqXOff, nReqYOff, nReqXSize, nReqYSize;
-            int nOutXOff, nOutYOff, nOutXSize, nOutYSize;
-            auto poSource =
-                static_cast<VRTSimpleSource *>(m_papoSources[iSource].get());
-            if (!poSource->GetSrcDstWindow(
-                    nXOff, nYOff, nXSize, nYSize, nBufXSize, nBufYSize,
-                    psExtraArg->eResampleAlg, &dfReqXOff, &dfReqYOff,
-                    &dfReqXSize, &dfReqYSize, &nReqXOff, &nReqYOff, &nReqXSize,
-                    &nReqYSize, &nOutXOff, &nOutYOff, &nOutXSize, &nOutYSize,
-                    bError))
-            {
-                if (bError)
-                {
-                    return CE_Failure;
-                }
-
-                // Skip non contributing source
-                bSkipOutputBufferInitialization = false;
-                continue;
-            }
+            return CE_Failure;
         }
-
-        anMapBufferIdxToSourceIdx[nBufferCount] = iSource;
-        apBuffers[nBufferCount].reset(
+    }
+    std::vector<std::unique_ptr<void, VSIFreeReleaser>> apBuffers(
+        anMapBufferIdxToSourceIdx.size());
+    for (size_t iBuffer = 0; iBuffer < anMapBufferIdxToSourceIdx.size();
+         ++iBuffer)
+    {
+        apBuffers[iBuffer].reset(
             VSI_MALLOC3_VERBOSE(nSrcTypeSize, nExtBufXSize, nExtBufYSize));
-        if (apBuffers[nBufferCount] == nullptr)
+        if (apBuffers[iBuffer] == nullptr)
         {
             return CE_Failure;
         }
 
         bool bBufferInit = true;
+        const int iSource = anMapBufferIdxToSourceIdx[iBuffer];
         if (m_papoSources[iSource]->IsSimpleSource())
         {
             const auto poSS =
@@ -1279,53 +1337,36 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
             /* ------------------------------------------------------------ */
             if (!m_bNoDataValueSet || m_dfNoDataValue == 0)
             {
-                memset(apBuffers[nBufferCount].get(), 0,
+                memset(apBuffers[iBuffer].get(), 0,
                        static_cast<size_t>(nSrcTypeSize) * nExtBufXSize *
                            nExtBufYSize);
             }
             else
             {
-                GDALCopyWords64(
-                    &m_dfNoDataValue, GDT_Float64, 0,
-                    static_cast<GByte *>(apBuffers[nBufferCount].get()),
-                    eSrcType, nSrcTypeSize,
-                    static_cast<GPtrDiff_t>(nExtBufXSize) * nExtBufYSize);
+                GDALCopyWords64(&m_dfNoDataValue, GDT_Float64, 0,
+                                static_cast<GByte *>(apBuffers[iBuffer].get()),
+                                eSrcType, nSrcTypeSize,
+                                static_cast<GPtrDiff_t>(nExtBufXSize) *
+                                    nExtBufYSize);
             }
         }
-
-        ++nBufferCount;
     }
 
     /* -------------------------------------------------------------------- */
     /*      Initialize the buffer to some background value. Use the         */
     /*      nodata value if available.                                      */
     /* -------------------------------------------------------------------- */
-    if (bSkipOutputBufferInitialization)
-    {
-        // Do nothing
-    }
-    else if (nPixelSpace == nBufTypeSize &&
-             (!m_bNoDataValueSet || m_dfNoDataValue == 0))
+    if (!bSkipOutputBufferInitialization)
     {
-        memset(pData, 0,
-               static_cast<size_t>(nBufXSize) * nBufYSize * nBufTypeSize);
-    }
-    else if (m_bNoDataValueSet)
-    {
-        double dfWriteValue = m_dfNoDataValue;
-
-        for (int iLine = 0; iLine < nBufYSize; iLine++)
-        {
-            GDALCopyWords64(&dfWriteValue, GDT_Float64, 0,
-                            static_cast<GByte *>(pData) + nLineSpace * iLine,
-                            eBufType, static_cast<int>(nPixelSpace), nBufXSize);
-        }
+        InitializeOutputBuffer(pData, nBufXSize, nBufYSize, eBufType,
+                               nPixelSpace, nLineSpace);
     }
 
     // No contributing sources and SkipNonContributingSources mode ?
     // Do not call the pixel function and just return the 0/nodata initialized
     // output buffer.
-    if (nBufferCount == 0 && m_poPrivate->m_bSkipNonContributingSources)
+    if (anMapBufferIdxToSourceIdx.empty() &&
+        m_poPrivate->m_bSkipNonContributingSources)
     {
         return CE_None;
     }
@@ -1400,7 +1441,9 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
     // Load values for sources into packed buffers.
     CPLErr eErr = CE_None;
     VRTSource::WorkingState oWorkingState;
-    for (int iBuffer = 0; iBuffer < nBufferCount && eErr == CE_None; iBuffer++)
+    for (size_t iBuffer = 0;
+         iBuffer < anMapBufferIdxToSourceIdx.size() && eErr == CE_None;
+         iBuffer++)
     {
         const int iSource = anMapBufferIdxToSourceIdx[iBuffer];
         GByte *pabyBuffer = static_cast<GByte *>(apBuffers[iBuffer].get());
@@ -1486,6 +1529,7 @@ CPLErr VRTDerivedRasterBand::IRasterIO(
     }
 
     // Apply pixel function.
+    const int nBufferCount = static_cast<int>(anMapBufferIdxToSourceIdx.size());
     if (eErr == CE_None && EQUAL(m_poPrivate->m_osLanguage, "Python"))
     {
         // numpy doesn't have native cint16/cint32/cfloat16
diff --git a/frmts/vrt/vrtsourcedrasterband.cpp b/frmts/vrt/vrtsourcedrasterband.cpp
