Skip to content

Commit

Permalink
[Backport release/3.10] MVT: emit warning when the maximum tile size …
Browse files Browse the repository at this point in the history
…or feature count is reached and the user didn't explicitly set MAX_SIZE or MAX_FEATURES layer creation options (#11408)

CPL_DEBUG=ON already warns about that, but better be more verbose when
the user didn't explicitly set limitations.

Should help addressing https://mastodon.social/@RegisHaubourg/113567187835337586

Co-authored-by: Alessandro Pasotti <elpaso@itopen.it>
  • Loading branch information
rouault and elpaso authored Dec 2, 2024
1 parent f27eb83 commit 8932458
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 15 deletions.
39 changes: 33 additions & 6 deletions autotest/ogr/ogr_mvt.py
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,9 @@ def test_ogr_mvt_write_mbtiles():

@pytest.mark.require_driver("SQLite")
@pytest.mark.require_geos
def test_ogr_mvt_write_limitations_max_size():
@pytest.mark.parametrize("implicit_limitation", [True, False])
@gdaltest.enable_exceptions()
def test_ogr_mvt_write_limitations_max_size(implicit_limitation, tmp_path):

src_ds = gdal.GetDriverByName("Memory").Create("", 0, 0, 0, gdal.GDT_Unknown)
lyr = src_ds.CreateLayer("mylayer")
Expand All @@ -1245,10 +1247,22 @@ def test_ogr_mvt_write_limitations_max_size():
out_ds = gdal.VectorTranslate(
"/vsimem/out.mbtiles",
src_ds,
datasetCreationOptions=["MAX_SIZE=100", "SIMPLIFICATION=1"],
datasetCreationOptions=[
"@MAX_SIZE_FOR_TEST=101" if implicit_limitation else "MAX_SIZE=101",
"SIMPLIFICATION=1",
],
)
assert out_ds is not None
out_ds = None
gdal.ErrorReset()
with gdal.quiet_errors():
out_ds.Close()
if implicit_limitation:
assert (
gdal.GetLastErrorMsg()
== "At least one tile exceeded the default maximum tile size of 101 bytes and was encoded at lower resolution"
)
else:
assert gdal.GetLastErrorMsg() == ""

out_ds = ogr.Open("/vsimem/out.mbtiles")
assert out_ds is not None
Expand Down Expand Up @@ -1442,7 +1456,9 @@ def test_ogr_mvt_write_limitations_max_size_polygon():

@pytest.mark.require_driver("SQLite")
@pytest.mark.require_geos
def test_ogr_mvt_write_limitations_max_features():
@pytest.mark.parametrize("implicit_limitation", [True, False])
@gdaltest.enable_exceptions()
def test_ogr_mvt_write_limitations_max_features(implicit_limitation):

src_ds = gdal.GetDriverByName("Memory").Create("", 0, 0, 0, gdal.GDT_Unknown)
lyr = src_ds.CreateLayer("mylayer")
Expand All @@ -1463,10 +1479,21 @@ def test_ogr_mvt_write_limitations_max_features():
"/vsimem/out.mbtiles",
src_ds,
format="MVT",
datasetCreationOptions=["MAX_FEATURES=1"],
datasetCreationOptions=[
"@MAX_FEATURES_FOR_TEST=1" if implicit_limitation else "MAX_FEATURES=1"
],
)
assert out_ds is not None
out_ds = None
gdal.ErrorReset()
with gdal.quiet_errors():
out_ds.Close()
if implicit_limitation:
assert (
gdal.GetLastErrorMsg()
== "At least one tile exceeded the default maximum number of features per tile (1) and was truncated to satisfy it."
)
else:
assert gdal.GetLastErrorMsg() == ""

out_ds = ogr.Open("/vsimem/out.mbtiles")
assert out_ds is not None
Expand Down
55 changes: 46 additions & 9 deletions ogr/ogrsf_frmts/mvt/ogrmvtdataset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3301,6 +3301,8 @@ class OGRMVTWriterDataset final : public GDALDataset
CPLString m_osType{"overlay"};
sqlite3 *m_hDBMBTILES = nullptr;
OGREnvelope m_oEnvelope;
bool m_bMaxTileSizeOptSpecified = false;
bool m_bMaxFeaturesOptSpecified = false;
unsigned m_nMaxTileSize = 500000;
unsigned m_nMaxFeatures = 200000;
std::map<std::string, std::string> m_oMapLayerNameToDesc;
Expand Down Expand Up @@ -4972,11 +4974,28 @@ std::string OGRMVTWriterDataset::EncodeTile(
const double dfCompressionRatio =
static_cast<double>(nSizeAfter) / nSizeBefore;

const bool bTooManyFeatures = nFeaturesInTile >= m_nMaxFeatures;
if (bTooManyFeatures && !m_bMaxFeaturesOptSpecified)
{
m_bMaxFeaturesOptSpecified = true;
CPLError(CE_Warning, CPLE_AppDefined,
"At least one tile exceeded the default maximum number of "
"features per tile (%u) and was truncated to satisfy it.",
m_nMaxFeatures);
}

// If the tile size is above the allowed values or there are too many
// features, then sort by descending area / length until we get to the
// limit.
bool bTooBigTile = oTileBuffer.size() > m_nMaxTileSize;
const bool bTooManyFeatures = nFeaturesInTile >= m_nMaxFeatures;
if (bTooBigTile && !m_bMaxTileSizeOptSpecified)
{
m_bMaxTileSizeOptSpecified = true;
CPLError(CE_Warning, CPLE_AppDefined,
"At least one tile exceeded the default maximum tile size of "
"%u bytes and was encoded at lower resolution",
m_nMaxTileSize);
}

GUInt32 nExtent = m_nExtent;
while (bTooBigTile && !bTooManyFeatures && nExtent >= 256)
Expand Down Expand Up @@ -6087,14 +6106,32 @@ GDALDataset *OGRMVTWriterDataset::Create(const char *pszFilename, int nXSize,
poDS->m_nBuffer = static_cast<unsigned>(atoi(CSLFetchNameValueDef(
papszOptions, "BUFFER", CPLSPrintf("%u", 5 * poDS->m_nExtent / 256))));

poDS->m_nMaxTileSize =
std::max(100U, static_cast<unsigned>(atoi(CSLFetchNameValueDef(
papszOptions, "MAX_SIZE",
CPLSPrintf("%u", poDS->m_nMaxTileSize)))));
poDS->m_nMaxFeatures =
std::max(1U, static_cast<unsigned>(atoi(CSLFetchNameValueDef(
papszOptions, "MAX_FEATURES",
CPLSPrintf("%u", poDS->m_nMaxFeatures)))));
{
const char *pszMaxSize = CSLFetchNameValue(papszOptions, "MAX_SIZE");
poDS->m_bMaxTileSizeOptSpecified = pszMaxSize != nullptr;
// This is used by unit tests
pszMaxSize = CSLFetchNameValueDef(papszOptions, "@MAX_SIZE_FOR_TEST",
pszMaxSize);
if (pszMaxSize)
{
poDS->m_nMaxTileSize =
std::max(100U, static_cast<unsigned>(atoi(pszMaxSize)));
}
}

{
const char *pszMaxFeatures =
CSLFetchNameValue(papszOptions, "MAX_FEATURES");
poDS->m_bMaxFeaturesOptSpecified = pszMaxFeatures != nullptr;
pszMaxFeatures = CSLFetchNameValueDef(
// This is used by unit tests
papszOptions, "@MAX_FEATURES_FOR_TEST", pszMaxFeatures);
if (pszMaxFeatures)
{
poDS->m_nMaxFeatures =
std::max(1U, static_cast<unsigned>(atoi(pszMaxFeatures)));
}
}

poDS->m_osName =
CSLFetchNameValueDef(papszOptions, "NAME", CPLGetBasename(pszFilename));
Expand Down

0 comments on commit 8932458

Please sign in to comment.