Skip to content

Commit e03f925

Browse files
committed
overview: warn in case of conflicting mask sources
Fixes OSGeo#13703 ``` $ gdal raster overview add --external sample2.tif -r average Warning 1: Raster band 1 of dataset sample2.tif has several conflicting mask sources: - nodata value - related to a raster band that is an alpha band Only the nodata value will be taken into account. 0...10...20...30...40...50...60...70...80...90...100 - done. ```
1 parent 0da2893 commit e03f925

File tree

8 files changed

+234
-33
lines changed

8 files changed

+234
-33
lines changed

apps/gdalalg_raster_reproject.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ bool GDALRasterReprojectAlgorithm::RunStep(GDALPipelineStepRunContext &ctxt)
207207

208208
CPLStringList aosOptions;
209209
std::string outputFilename;
210+
210211
if (ctxt.m_poNextUsableStep)
211212
{
212213
CPLAssert(CanHandleNextStep(ctxt.m_poNextUsableStep));

autotest/cpp/test_gdal.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6477,4 +6477,72 @@ TEST_F(test_gdal, GDALGeoTransform)
64776477
}
64786478
}
64796479

6480+
TEST_F(test_gdal, GDALRasterBand_HasConflictingMaskSources)
6481+
{
6482+
auto poMemDrv = GetGDALDriverManager()->GetDriverByName("MEM");
6483+
if (!poMemDrv)
6484+
{
6485+
GTEST_SKIP() << "MEM driver missing";
6486+
}
6487+
else
6488+
{
6489+
{
6490+
auto poDS = std::unique_ptr<GDALDataset>(
6491+
poMemDrv->Create("", 1, 1, 1, GDT_Byte, nullptr));
6492+
poDS->GetRasterBand(1)->SetNoDataValue(1);
6493+
6494+
EXPECT_FALSE(poDS->GetRasterBand(1)->HasConflictingMaskSources());
6495+
}
6496+
6497+
{
6498+
auto poDS = std::unique_ptr<GDALDataset>(
6499+
poMemDrv->Create("", 1, 1, 1, GDT_Byte, nullptr));
6500+
poDS->CreateMaskBand(GMF_PER_DATASET);
6501+
6502+
EXPECT_FALSE(poDS->GetRasterBand(1)->HasConflictingMaskSources());
6503+
}
6504+
6505+
{
6506+
auto poDS = std::unique_ptr<GDALDataset>(
6507+
poMemDrv->Create("", 1, 1, 2, GDT_Byte, nullptr));
6508+
poDS->GetRasterBand(2)->SetColorInterpretation(GCI_AlphaBand);
6509+
6510+
EXPECT_FALSE(poDS->GetRasterBand(1)->HasConflictingMaskSources());
6511+
}
6512+
6513+
{
6514+
auto poDS = std::unique_ptr<GDALDataset>(
6515+
poMemDrv->Create("", 1, 1, 1, GDT_Byte, nullptr));
6516+
poDS->SetMetadataItem("NODATA_VALUES", "0");
6517+
6518+
EXPECT_FALSE(poDS->GetRasterBand(1)->HasConflictingMaskSources());
6519+
}
6520+
6521+
for (int i = 0; i < 4; ++i)
6522+
{
6523+
for (int j = i + 1; j < 4; ++j)
6524+
{
6525+
auto poDS = std::unique_ptr<GDALDataset>(
6526+
poMemDrv->Create("", 1, 1, 2, GDT_Byte, nullptr));
6527+
if (i == 0)
6528+
poDS->GetRasterBand(1)->SetNoDataValue(1);
6529+
if (i == 1 || j == 1)
6530+
poDS->GetRasterBand(1)->CreateMaskBand(0);
6531+
if (i == 2 || j == 2)
6532+
poDS->GetRasterBand(2)->SetColorInterpretation(
6533+
GCI_AlphaBand);
6534+
if (i == 3 || j == 3)
6535+
poDS->SetMetadataItem("NODATA_VALUES", "0");
6536+
6537+
EXPECT_TRUE(
6538+
poDS->GetRasterBand(1)->HasConflictingMaskSources());
6539+
std::string osMsg;
6540+
EXPECT_TRUE(
6541+
poDS->GetRasterBand(1)->HasConflictingMaskSources(&osMsg));
6542+
EXPECT_TRUE(!osMsg.empty());
6543+
}
6544+
}
6545+
}
6546+
}
6547+
64806548
} // namespace

autotest/utilities/test_gdalalg_raster_overview.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,3 +491,19 @@ def test_gdalalg_overview_add_in_pipeline(tmp_vsimem):
491491
assert ds.GetRasterBand(1).GetOverview(0).Checksum() == 1192
492492

493493
assert src_ds.GetRasterBand(1).GetOverviewCount() == 0
494+
495+
496+
###############################################################################
497+
498+
499+
def test_gdalalg_overview_add_warn_conflicting_mask_sources():
500+
501+
src_ds = gdal.GetDriverByName("MEM").Create("foo", 20, 20, 2)
502+
src_ds.GetRasterBand(1).SetNoDataValue(255)
503+
src_ds.GetRasterBand(2).SetColorInterpretation(gdal.GCI_AlphaBand)
504+
505+
with gdaltest.error_raised(
506+
gdal.CE_Warning,
507+
match="Raster band 1 of dataset foo has several conflicting mask sources:\n- nodata value\n- related to a raster band that is an alpha band\nOnly the nodata value will be taken into account",
508+
):
509+
gdal.alg.raster.overview.add(input=src_ds, levels=[2, 4], resampling="average")

doc/source/api/gdalrasterband_cpp.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,9 @@ GDALRasterWindow class
7171
.. doxygenclass:: GDALRasterWindow
7272
:project: api
7373
:members:
74+
75+
76+
.. below is an allow-list for spelling checker.
77+
78+
.. spelling:word-list::
79+
posDetailMessage

frmts/gtiff/libtiff/tif_packbits.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,8 @@ static int PackBitsDecode(TIFF *tif, uint8_t *op, tmsize_t occ, uint16_t s)
243243
long n;
244244
int b;
245245

246+
uint8_t *op0 = op;
247+
tmsize_t occ0 = occ;
246248
(void)s;
247249
bp = (int8_t *)tif->tif_rawcp;
248250
cc = tif->tif_rawcc;

gcore/gdal_rasterband.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject
122122
GSpacing nPixelSpace, GSpacing nLineSpace,
123123
GDALRasterIOExtraArg *psExtraArg) CPL_WARN_UNUSED_RESULT;
124124

125+
CPL_INTERNAL bool HasNoData() const;
126+
125127
protected:
126128
GDALRasterBand();
127129
explicit GDALRasterBand(int bForceCachedIO);
@@ -552,6 +554,8 @@ class CPL_DLL GDALRasterBand : public GDALMajorObject
552554
virtual CPLErr CreateMaskBand(int nFlagsIn);
553555
virtual bool IsMaskBand() const;
554556
virtual GDALMaskValueRange GetMaskValueRange() const;
557+
bool HasConflictingMaskSources(std::string *posDetailMessage = nullptr,
558+
bool bMentionPrioritarySource = true) const;
555559

556560
virtual CPLVirtualMem *
557561
GetVirtualMemAuto(GDALRWFlag eRWFlag, int *pnPixelSpace,

gcore/gdalrasterband.cpp

Lines changed: 103 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9015,6 +9015,37 @@ CPLErr CPL_STDCALL GDALSetDefaultRAT(GDALRasterBandH hBand,
90159015
return poBand->SetDefaultRAT(GDALRasterAttributeTable::FromHandle(hRAT));
90169016
}
90179017

9018+
/************************************************************************/
9019+
/* HasNoData() */
9020+
/************************************************************************/
9021+
9022+
bool GDALRasterBand::HasNoData() const
9023+
{
9024+
int bHaveNoDataRaw = FALSE;
9025+
bool bHaveNoData = false;
9026+
GDALRasterBand *poThis = const_cast<GDALRasterBand *>(this);
9027+
if (eDataType == GDT_Int64)
9028+
{
9029+
CPL_IGNORE_RET_VAL(poThis->GetNoDataValueAsInt64(&bHaveNoDataRaw));
9030+
bHaveNoData = CPL_TO_BOOL(bHaveNoDataRaw);
9031+
}
9032+
else if (eDataType == GDT_UInt64)
9033+
{
9034+
CPL_IGNORE_RET_VAL(poThis->GetNoDataValueAsUInt64(&bHaveNoDataRaw));
9035+
bHaveNoData = CPL_TO_BOOL(bHaveNoDataRaw);
9036+
}
9037+
else
9038+
{
9039+
const double dfNoDataValue = poThis->GetNoDataValue(&bHaveNoDataRaw);
9040+
if (bHaveNoDataRaw &&
9041+
GDALNoDataMaskBand::IsNoDataInRange(dfNoDataValue, eDataType))
9042+
{
9043+
bHaveNoData = true;
9044+
}
9045+
}
9046+
return bHaveNoData;
9047+
}
9048+
90189049
/************************************************************************/
90199050
/* GetMaskBand() */
90209051
/************************************************************************/
@@ -9072,32 +9103,6 @@ CPLErr CPL_STDCALL GDALSetDefaultRAT(GDALRasterBandH hBand,
90729103
GDALRasterBand *GDALRasterBand::GetMaskBand()
90739104

90749105
{
9075-
const auto HasNoData = [this]()
9076-
{
9077-
int bHaveNoDataRaw = FALSE;
9078-
bool bHaveNoData = false;
9079-
if (eDataType == GDT_Int64)
9080-
{
9081-
CPL_IGNORE_RET_VAL(GetNoDataValueAsInt64(&bHaveNoDataRaw));
9082-
bHaveNoData = CPL_TO_BOOL(bHaveNoDataRaw);
9083-
}
9084-
else if (eDataType == GDT_UInt64)
9085-
{
9086-
CPL_IGNORE_RET_VAL(GetNoDataValueAsUInt64(&bHaveNoDataRaw));
9087-
bHaveNoData = CPL_TO_BOOL(bHaveNoDataRaw);
9088-
}
9089-
else
9090-
{
9091-
const double dfNoDataValue = GetNoDataValue(&bHaveNoDataRaw);
9092-
if (bHaveNoDataRaw &&
9093-
GDALNoDataMaskBand::IsNoDataInRange(dfNoDataValue, eDataType))
9094-
{
9095-
bHaveNoData = true;
9096-
}
9097-
}
9098-
return bHaveNoData;
9099-
};
9100-
91019106
if (poMask != nullptr)
91029107
{
91039108
if (poMask.IsOwned())
@@ -9599,6 +9604,78 @@ GDALMaskValueRange GDALRasterBand::GetMaskValueRange() const
95999604
return GMVR_UNKNOWN;
96009605
}
96019606

9607+
/************************************************************************/
9608+
/* HasConflictingMaskSources() */
9609+
/************************************************************************/
9610+
9611+
/**
9612+
* \brief Returns whether a raster band has conflicting mask sources.
9613+
*
9614+
* That is, if more than one of the following conditions is met:
9615+
* - it has a binary mask band (that is not an alpha band)
9616+
* - it has an external mask flags (.msk file)
9617+
* - it has a nodata value
9618+
* - it belongs to a dataset with the NODATA_VALUES metadata item set
9619+
* - it belongs to a dataset that has a band with a GCI_AlphaBand color interpretation
9620+
*
9621+
* @param[out] posDetailMessage Pointer to a string that will contain the
9622+
* details of the conflict.
9623+
* @param bMentionPrioritarySource Whether the mask source used should be
9624+
* mentioned in *posDetailMessage.
9625+
* @since GDAL 3.13.0
9626+
*/
9627+
9628+
bool GDALRasterBand::HasConflictingMaskSources(
9629+
std::string *posDetailMessage, bool bMentionPrioritarySource) const
9630+
{
9631+
const bool bHasExternalMask = poDS && poDS->oOvManager.HaveMaskFile();
9632+
const bool bHasBinaryMaskBand =
9633+
((const_cast<GDALRasterBand *>(this)->GetMaskFlags() &
9634+
(GMF_ALL_VALID | GMF_NODATA | GMF_ALPHA)) == 0) &&
9635+
(!bHasExternalMask || poDS->oOvManager.GetMaskBand(nBand) != this);
9636+
const bool bHasNoData = HasNoData();
9637+
const bool bHasNODATA_VALUES =
9638+
poDS && poDS->GetMetadataItem("NODATA_VALUES");
9639+
const bool bHasAlphaBand =
9640+
poDS &&
9641+
poDS->GetRasterBand(poDS->GetRasterCount())->GetColorInterpretation() ==
9642+
GCI_AlphaBand;
9643+
const bool abMaskSources[] = {bHasBinaryMaskBand, bHasExternalMask,
9644+
bHasNoData, bHasNODATA_VALUES, bHasAlphaBand};
9645+
const size_t nCount =
9646+
std::count(std::begin(abMaskSources), std::end(abMaskSources), true);
9647+
if (nCount >= 2)
9648+
{
9649+
if (posDetailMessage)
9650+
{
9651+
*posDetailMessage = "Raster band ";
9652+
*posDetailMessage += std::to_string(nBand);
9653+
if (poDS && poDS->GetDescription()[0])
9654+
{
9655+
*posDetailMessage += " of dataset ";
9656+
*posDetailMessage += poDS->GetDescription();
9657+
}
9658+
*posDetailMessage += " has several conflicting mask sources:\n";
9659+
if (bHasExternalMask)
9660+
*posDetailMessage += "- internal binary mask band\n";
9661+
if (bHasExternalMask)
9662+
*posDetailMessage += "- external mask band (.msk)\n";
9663+
if (bHasNoData)
9664+
*posDetailMessage += "- nodata value\n";
9665+
if (bHasNODATA_VALUES)
9666+
*posDetailMessage += "- NODATA_VALUES dataset metadata item\n";
9667+
if (bHasAlphaBand)
9668+
*posDetailMessage +=
9669+
"- related to a raster band that is an alpha band\n";
9670+
if (bMentionPrioritarySource)
9671+
*posDetailMessage +=
9672+
"Only the first listed one will be taken into account.";
9673+
}
9674+
return true;
9675+
}
9676+
return false;
9677+
}
9678+
96029679
/************************************************************************/
96039680
/* GetIndexColorTranslationTo() */
96049681
/************************************************************************/

gcore/overview.cpp

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4559,6 +4559,8 @@ static CPLErr GDALRegenerateCascadingOverviews(
45594559
/* -------------------------------------------------------------------- */
45604560
double dfPixelsProcessed = 0.0;
45614561

4562+
CPLStringList aosOptions(papszOptions);
4563+
aosOptions.SetNameValue("CASCADING", "YES");
45624564
for (int i = 0; i < nOverviews; ++i)
45634565
{
45644566
GDALRasterBand *poBaseBand = poSrcBand;
@@ -4577,7 +4579,7 @@ static CPLErr GDALRegenerateCascadingOverviews(
45774579
poBaseBand, 1,
45784580
reinterpret_cast<GDALRasterBandH *>(papoOvrBands) + i,
45794581
pszResampling, GDALScaledProgress, pScaledProgressData,
4580-
papszOptions);
4582+
aosOptions.List());
45814583
GDALDestroyScaledProgress(pScaledProgressData);
45824584

45834585
if (eErr != CE_None)
@@ -4896,6 +4898,26 @@ CPLErr GDALRegenerateOverviewsEx(GDALRasterBandH hSrcBand, int nOverviewCount,
48964898
}
48974899
}
48984900

4901+
int nHasNoData = 0;
4902+
const double dfNoDataValue = poSrcBand->GetNoDataValue(&nHasNoData);
4903+
const bool bHasNoData = CPL_TO_BOOL(nHasNoData);
4904+
const bool bPropagateNoData =
4905+
CPLTestBool(CPLGetConfigOption("GDAL_OVR_PROPAGATE_NODATA", "NO"));
4906+
4907+
if (poSrcBand->GetBand() == 1 && bUseNoDataMask &&
4908+
CSLFetchNameValue(papszOptions, "CASCADING") == nullptr)
4909+
{
4910+
std::string osDetailMessage;
4911+
if (poSrcBand->HasConflictingMaskSources(&osDetailMessage, false))
4912+
{
4913+
CPLError(
4914+
CE_Warning, CPLE_AppDefined, "%s%s", osDetailMessage.c_str(),
4915+
bHasNoData
4916+
? "Only the nodata value will be taken into account."
4917+
: "Only the first listed one will be taken into account.");
4918+
}
4919+
}
4920+
48994921
/* -------------------------------------------------------------------- */
49004922
/* If we are operating on multiple overviews, and using */
49014923
/* averaging, lets do them in cascading order to reduce the */
@@ -5027,12 +5049,6 @@ CPLErr GDALRegenerateOverviewsEx(GDALRasterBandH hSrcBand, int nOverviewCount,
50275049
}
50285050
}
50295051

5030-
int nHasNoData = 0;
5031-
const double dfNoDataValue = poSrcBand->GetNoDataValue(&nHasNoData);
5032-
const bool bHasNoData = CPL_TO_BOOL(nHasNoData);
5033-
const bool bPropagateNoData =
5034-
CPLTestBool(CPLGetConfigOption("GDAL_OVR_PROPAGATE_NODATA", "NO"));
5035-
50365052
// Structure describing a resampling job
50375053
struct OvrJob
50385054
{
@@ -5683,6 +5699,17 @@ CPLErr GDALRegenerateOverviewsMultiBand(
56835699
papoSrcBands[iBand]->GetNoDataValue(&nHasNoData);
56845700
abHasNoData[iBand] = CPL_TO_BOOL(nHasNoData);
56855701
}
5702+
5703+
std::string osDetailMessage;
5704+
if (bUseNoDataMask &&
5705+
papoSrcBands[0]->HasConflictingMaskSources(&osDetailMessage, false))
5706+
{
5707+
CPLError(CE_Warning, CPLE_AppDefined, "%s%s", osDetailMessage.c_str(),
5708+
abHasNoData[0]
5709+
? "Only the nodata value will be taken into account."
5710+
: "Only the first listed one will be taken into account.");
5711+
}
5712+
56865713
const bool bPropagateNoData =
56875714
CPLTestBool(CPLGetConfigOption("GDAL_OVR_PROPAGATE_NODATA", "NO"));
56885715

0 commit comments

Comments
 (0)