From 85614189333f990ed978126d9a2c9b585d5466e3 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 10 Feb 2020 08:47:53 +1000 Subject: [PATCH] Always disable GDAL side shapefile encoding handling And instead always do the decoding on QGIS' side. This unifies the encoding handling whether or not we are using the underlying shapefile declared encoding (e.g. via LDID or .cpg file) OR are overriding it manually by a user-set encoding. Why? - if we DON'T disable GDAL side encoding support, then there's NO way to change the encoding used when reading shapefiles. And unfortunately the embedded encoding (which is read by GDAL) is sometimes wrong (because shapefiles!), so we need to expose support for users to be able to change and correct this - we can't change this setting on-the-fly. If we don't set it upfront, we can't reverse this decision later when a user does want/need to manually specify the encoding This also removes a lot of confusing code logic in the provider! Fixes #21264, user frustration on mailing lists e.g. http://osgeo-org.1560.x6.nabble.com/Shapefile-with-file-cpg-codepage-td5275106.html http://osgeo-org.1560.x6.nabble.com/QGIS-ignore-the-cpg-files-when-loading-shapefiles-td5348021.html --- src/app/qgsoptions.cpp | 2 - src/core/providers/ogr/qgsogrprovider.cpp | 33 +++++--- src/core/qgsvectorfilewriter.cpp | 6 -- src/ui/qgsoptionsbase.ui | 93 ++++++++++------------- 4 files changed, 64 insertions(+), 70 deletions(-) diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp index eb8839551edd..ba099bea5618 100644 --- a/src/app/qgsoptions.cpp +++ b/src/app/qgsoptions.cpp @@ -687,7 +687,6 @@ QgsOptions::QgsOptions( QWidget *parent, Qt::WindowFlags fl, const QListaddItem( tr( "GeoJSON" ), QgsClipboard::GeoJSON ); mComboCopyFeatureFormat->setCurrentIndex( mComboCopyFeatureFormat->findData( mSettings->enumValue( QStringLiteral( "/qgis/copyFeatureFormat" ), QgsClipboard::AttributesWithWKT ) ) ); leNullValue->setText( QgsApplication::nullRepresentation() ); - cbxIgnoreShapeEncoding->setChecked( mSettings->value( QStringLiteral( "/qgis/ignoreShapeEncoding" ), true ).toBool() ); cmbLegendDoubleClickAction->setCurrentIndex( mSettings->value( QStringLiteral( "/qgis/legendDoubleClickAction" ), 0 ).toInt() ); @@ -1486,7 +1485,6 @@ void QgsOptions::saveOptions() cmbScanItemsInBrowser->currentData().toString() ); mSettings->setValue( QStringLiteral( "/qgis/scanZipInBrowser2" ), cmbScanZipInBrowser->currentData().toString() ); - mSettings->setValue( QStringLiteral( "/qgis/ignoreShapeEncoding" ), cbxIgnoreShapeEncoding->isChecked() ); mSettings->setValue( QStringLiteral( "/qgis/mainSnappingWidgetMode" ), mSnappingMainDialogComboBox->currentData() ); mSettings->setValue( QStringLiteral( "/qgis/compileExpressions" ), cbxCompileExpressions->isChecked() ); diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index eaccf7e03eee..b0fe6653cbff 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -489,7 +489,14 @@ QgsOgrProvider::QgsOgrProvider( QString const &uri, const ProviderOptions &optio QgsApplication::registerOgrDrivers(); QgsSettings settings; - CPLSetConfigOption( "SHAPE_ENCODING", settings.value( QStringLiteral( "qgis/ignoreShapeEncoding" ), true ).toBool() ? "" : nullptr ); + // we always disable GDAL side shapefile encoding handling, and do it on the QGIS side. + // why? it's not the ideal choice, but... + // - if we DON'T disable GDAL side encoding support, then there's NO way to change the encoding used when reading + // shapefiles. And unfortunately the embedded encoding (which is read by GDAL) is sometimes wrong, so we need + // to expose support for users to be able to change and correct this + // - we can't change this setting on-the-fly. If we don't set it upfront, we can't reverse this decision later when + // a user does want/need to manually specify the encoding + CPLSetConfigOption( "SHAPE_ENCODING", "" ); #ifndef QT_NO_NETWORKPROXY setupProxy(); @@ -1003,8 +1010,7 @@ QStringList QgsOgrProvider::_subLayers( bool withFeatureCount ) const void QgsOgrProvider::setEncoding( const QString &e ) { QgsSettings settings; - if ( ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) && - settings.value( QStringLiteral( "qgis/ignoreShapeEncoding" ), true ).toBool() ) || + if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) || ( mOgrLayer && !mOgrLayer->TestCapability( OLCStringsAsUTF8 ) ) ) { QgsVectorDataProvider::setEncoding( e ); @@ -3580,12 +3586,6 @@ bool QgsOgrProviderUtils::createEmptyDataSource( const QString &uri, layer = GDALDatasetCreateLayer( dataSource.get(), QFileInfo( uri ).completeBaseName().toUtf8().constData(), reference, OGRvectortype, papszOptions ); CSLDestroy( papszOptions ); - QgsSettings settings; - if ( !settings.value( QStringLiteral( "qgis/ignoreShapeEncoding" ), true ).toBool() ) - { - CPLSetConfigOption( "SHAPE_ENCODING", nullptr ); - } - if ( !layer ) { errorMessage = QString::fromUtf8( CPLGetLastErrorMsg() ); @@ -4531,7 +4531,20 @@ void QgsOgrProvider::open( OpenMode mode ) mOgrLayer = mOgrOrigLayer.get(); // check that the initial encoding setting is fit for this layer - setEncoding( encoding() ); + + if ( mode == OpenModeInitial && mGDALDriverName == QLatin1String( "ESRI Shapefile" ) ) + { + // determine encoding from shapefile cpg or LDID information, if possible + const QString shpEncoding = QgsOgrUtils::readShapefileEncoding( mFilePath ); + if ( !shpEncoding.isEmpty() ) + setEncoding( shpEncoding ); + else + setEncoding( encoding() ); + } + else + { + setEncoding( encoding() ); + } // Ensure subset is set (setSubsetString does nothing if the passed sql subset string is equal to mSubsetString, which is the case when reloading the dataset) QString origSubsetString = mSubsetString; diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp index 0bb0540cb10a..7e7437598f8d 100644 --- a/src/core/qgsvectorfilewriter.cpp +++ b/src/core/qgsvectorfilewriter.cpp @@ -497,12 +497,6 @@ void QgsVectorFileWriter::init( QString vectorFileName, options = nullptr; } - QgsSettings settings; - if ( !settings.value( QStringLiteral( "qgis/ignoreShapeEncoding" ), true ).toBool() ) - { - CPLSetConfigOption( "SHAPE_ENCODING", nullptr ); - } - if ( srs.isValid() ) { if ( mOgrDriverName == QLatin1String( "ESRI Shapefile" ) ) diff --git a/src/ui/qgsoptionsbase.ui b/src/ui/qgsoptionsbase.ui index 7b9fbafa255b..bcc27b48b49e 100644 --- a/src/ui/qgsoptionsbase.ui +++ b/src/ui/qgsoptionsbase.ui @@ -1995,44 +1995,46 @@ Data source handling - + - - - - Scan for contents of compressed files (.zip) in browser dock - + + + + + + + - - - - - 0 - 0 - + + + + Qt::Horizontal - - Prompt for raster sublayers when opening + + + 40 + 20 + - + - - + + - Execute expressions on server-side if possible + Scan for contents of compressed files (.zip) in browser dock - - + + - Disable OGR on-the-fly conversion from declared encoding to UTF-8 + <html><head/><body><p>When digitizing a new feature, default values are retrieved from the database. With this option turned on, the default values will be evaluated at the time of digitizing. With this option turned off, the default values will be evaluated at the time of saving.</p></body></html> - Ignore shapefile encoding declaration + Evaluate default values @@ -2043,44 +2045,32 @@ - - - - <html><head/><body><p>When digitizing a new feature, default values are retrieved from the database. With this option turned on, the default values will be evaluated at the time of digitizing. With this option turned off, the default values will be evaluated at the time of saving.</p></body></html> - + + - Evaluate default values + Scan for valid items in the browser dock - - - - - - - - - - - + + - Scan for valid items in the browser dock + Execute expressions on server-side if possible - - - - Qt::Horizontal + + + + + 0 + 0 + - - - 40 - 20 - + + Prompt for raster sublayers when opening - + @@ -5767,7 +5757,6 @@ p, li { white-space: pre-wrap; } cmbScanItemsInBrowser cmbScanZipInBrowser cmbPromptRasterSublayers - cbxIgnoreShapeEncoding cbxCompileExpressions cbxEvaluateDefaultValues mBtnRemoveHiddenPath