From e0d9585c230e907c99595cca9e0322d8b816a8da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 19 Mar 2024 14:54:11 +0100 Subject: [PATCH 01/14] [dxf] Allow users to edit layer name in DXF Export (app) dialog --- src/app/qgsdxfexportdialog.cpp | 210 ++++++++++++++++++++------------- src/app/qgsdxfexportdialog.h | 1 + 2 files changed, 131 insertions(+), 80 deletions(-) diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 9db2a40c3c58..9f34cc5d751b 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -49,7 +49,23 @@ QWidget *FieldSelectorDelegate::createEditor( QWidget *parent, const QStyleOptio { Q_UNUSED( option ) - if ( index.column() == ALLOW_DD_SYMBOL_BLOCKS_COL ) + QgsVectorLayer *vl = indexToLayer( index.model(), index ); + if ( !vl ) + return nullptr; + + if ( index.column() == LAYER_COL ) + { + QLineEdit *le = new QLineEdit( parent ); + return le; + } + else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + { + QgsFieldComboBox *w = new QgsFieldComboBox( parent ); + w->setLayer( vl ); + w->setAllowEmptyFieldName( true ); + return w; + } + else if ( index.column() == ALLOW_DD_SYMBOL_BLOCKS_COL ) { return nullptr; } @@ -60,44 +76,68 @@ QWidget *FieldSelectorDelegate::createEditor( QWidget *parent, const QStyleOptio return le; } - QgsVectorLayer *vl = indexToLayer( index.model(), index ); - if ( !vl ) - return nullptr; - - QgsFieldComboBox *w = new QgsFieldComboBox( parent ); - w->setLayer( vl ); - w->setAllowEmptyFieldName( true ); - return w; + return nullptr; } void FieldSelectorDelegate::setEditorData( QWidget *editor, const QModelIndex &index ) const { - if ( index.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) + QgsVectorLayer *vl = indexToLayer( index.model(), index ); + if ( !vl ) + return; + + if ( index.column() == LAYER_COL ) + { + QLineEdit *le = qobject_cast< QLineEdit * >( editor ); + if ( le ) + { + le->setText( index.data().toString() ); + } + } + else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + { + QgsFieldComboBox *fcb = qobject_cast( editor ); + if ( !fcb ) + return; + + int idx = attributeIndex( index.model(), vl ); + if ( vl->fields().exists( idx ) ) + { + fcb->setField( vl->fields().at( idx ).name() ); + } + } + else if ( index.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) { QLineEdit *le = qobject_cast( editor ); if ( le ) { le->setText( index.data().toString() ); } - return; } +} +void FieldSelectorDelegate::setModelData( QWidget *editor, QAbstractItemModel *model, const QModelIndex &index ) const +{ QgsVectorLayer *vl = indexToLayer( index.model(), index ); if ( !vl ) return; - QgsFieldComboBox *fcb = qobject_cast( editor ); - if ( !fcb ) - return; - - int idx = attributeIndex( index.model(), vl ); - if ( vl->fields().exists( idx ) ) - fcb->setField( vl->fields().at( idx ).name() ); -} + if ( index.column() == LAYER_COL ) + { + QLineEdit *le = qobject_cast( editor ); + if ( le ) + { + model->setData( index, le->text() ); + } + } + else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + { + QgsFieldComboBox *fcb = qobject_cast( editor ); + if ( !fcb ) + return; -void FieldSelectorDelegate::setModelData( QWidget *editor, QAbstractItemModel *model, const QModelIndex &index ) const -{ - if ( index.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) + model->setData( index, vl->fields().lookupField( fcb->currentField() ) ); + } + else if ( index.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) { QLineEdit *le = qobject_cast( editor ); if ( le ) @@ -105,16 +145,6 @@ void FieldSelectorDelegate::setModelData( QWidget *editor, QAbstractItemModel *m model->setData( index, le->text().toInt() ); } } - - QgsVectorLayer *vl = indexToLayer( index.model(), index ); - if ( !vl ) - return; - - QgsFieldComboBox *fcb = qobject_cast( editor ); - if ( !fcb ) - return; - - model->setData( index, vl->fields().lookupField( fcb->currentField() ) ); } QgsVectorLayer *FieldSelectorDelegate::indexToLayer( const QAbstractItemModel *model, const QModelIndex &index ) const @@ -167,7 +197,7 @@ Qt::ItemFlags QgsVectorLayerAndAttributeModel::flags( const QModelIndex &index ) QgsVectorLayer *vl = vectorLayer( index ); if ( index.column() == LAYER_COL ) { - return Qt::ItemIsEnabled | Qt::ItemIsUserCheckable; + return vl ? Qt::ItemIsEditable | Qt::ItemIsEnabled | Qt::ItemIsUserCheckable : Qt::ItemIsEnabled | Qt::ItemIsUserCheckable; } else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) { @@ -229,6 +259,9 @@ QVariant QgsVectorLayerAndAttributeModel::headerData( int section, Qt::Orientati QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role ) const { QgsVectorLayer *vl = vectorLayer( idx ); + if ( !vl ) + return QVariant(); + if ( idx.column() == LAYER_COL ) { if ( role == Qt::CheckStateRole ) @@ -277,12 +310,37 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role Q_ASSERT( hasUnchecked ); return Qt::Unchecked; } + else if ( role == Qt::DisplayRole && mOverriddenName.contains( vl ) ) + { + return mOverriddenName[ vl ]; + } else + { return QgsLayerTreeModel::data( idx, role ); + } + } + else if ( idx.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + { + int idx = mAttributeIdx.value( vl, -1 ); + if ( role == Qt::EditRole ) + return idx; + + if ( role == Qt::DisplayRole ) + { + if ( vl->fields().exists( idx ) ) + return vl->fields().at( idx ).name(); + else + return mOverriddenName.contains( vl ) ? mOverriddenName[ vl ] : vl->name(); + } + + if ( role == Qt::ToolTipRole ) + { + return tr( "Attribute containing the name of the destination layer in the DXF output." ); + } } else if ( idx.column() == ALLOW_DD_SYMBOL_BLOCKS_COL ) { - if ( !vl || vl->geometryType() != Qgis::GeometryType::Point ) + if ( vl->geometryType() != Qgis::GeometryType::Point ) { return QVariant(); } @@ -299,7 +357,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role } else if ( idx.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) { - if ( !vl || vl->geometryType() != Qgis::GeometryType::Point ) + if ( vl->geometryType() != Qgis::GeometryType::Point ) { return QVariant(); } @@ -317,74 +375,66 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role } } - - if ( idx.column() == OUTPUT_LAYER_ATTRIBUTE_COL && vl ) - { - int idx = mAttributeIdx.value( vl, -1 ); - if ( role == Qt::EditRole ) - return idx; - - if ( role == Qt::DisplayRole ) - { - if ( vl->fields().exists( idx ) ) - return vl->fields().at( idx ).name(); - else - return vl->name(); - } - - if ( role == Qt::ToolTipRole ) - { - return tr( "Attribute containing the name of the destination layer in the DXF output." ); - } - } - return QVariant(); } bool QgsVectorLayerAndAttributeModel::setData( const QModelIndex &index, const QVariant &value, int role ) { - if ( index.column() == LAYER_COL && role == Qt::CheckStateRole ) + QgsVectorLayer *vl = vectorLayer( index ); + + if ( index.column() == LAYER_COL ) { - int i = 0; - for ( i = 0; ; i++ ) + if ( role == Qt::CheckStateRole ) { - QModelIndex child = QgsVectorLayerAndAttributeModel::index( i, 0, index ); - if ( !child.isValid() ) - break; + int i = 0; + for ( i = 0; ; i++ ) + { + QModelIndex child = QgsVectorLayerAndAttributeModel::index( i, 0, index ); + if ( !child.isValid() ) + break; - setData( child, value, role ); - } + setData( child, value, role ); + } + + if ( i == 0 ) + { + if ( value.toInt() == Qt::Checked ) + mCheckedLeafs.insert( index ); + else if ( value.toInt() == Qt::Unchecked ) + mCheckedLeafs.remove( index ); + else + Q_ASSERT( "expected checked or unchecked" ); - if ( i == 0 ) + emit dataChanged( QModelIndex(), index ); + } + + return true; + } + else if ( role == Qt::EditRole ) { - if ( value.toInt() == Qt::Checked ) - mCheckedLeafs.insert( index ); - else if ( value.toInt() == Qt::Unchecked ) - mCheckedLeafs.remove( index ); + if ( !value.toString().trimmed().isEmpty() && value.toString() != vl->name() ) + { + mOverriddenName[ vl ] = value.toString(); + } else - Q_ASSERT( "expected checked or unchecked" ); - - emit dataChanged( QModelIndex(), index ); + { + mOverriddenName.remove( vl ); + } + return true; } - - return true; } - - QgsVectorLayer *vl = vectorLayer( index ); - if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) { if ( role != Qt::EditRole ) return false; - if ( vl ) { mAttributeIdx[ vl ] = value.toInt(); return true; } } - - if ( index.column() == ALLOW_DD_SYMBOL_BLOCKS_COL && role == Qt::CheckStateRole ) + else if ( index.column() == ALLOW_DD_SYMBOL_BLOCKS_COL && role == Qt::CheckStateRole ) { if ( vl ) { diff --git a/src/app/qgsdxfexportdialog.h b/src/app/qgsdxfexportdialog.h index d58d0059b460..0a696617f653 100644 --- a/src/app/qgsdxfexportdialog.h +++ b/src/app/qgsdxfexportdialog.h @@ -81,6 +81,7 @@ class QgsVectorLayerAndAttributeModel : public QgsLayerTreeModel QHash mAttributeIdx; QHash mCreateDDBlockInfo; QHash mDDBlocksMaxNumberOfClasses; + QHash mOverriddenName; QSet mCheckedLeafs; void applyVisibility( QSet &visibleLayers, QgsLayerTreeNode *node ); From 4b2c7deedc7d6ea199beb027bcfa6ab94fcb62ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 19 Mar 2024 17:36:55 +0100 Subject: [PATCH 02/14] [core] Allow users to override a layer name when exporting to DXF; DxfLayerJob->layerTitle replaced by layerDerivedName, to avoid confusion with one of the final DXF layer name sources (layer metadata title, layer server properties title, layer name, layer's overridden name) --- .../auto_generated/dxf/qgsdxfexport.sip.in | 9 ++++++++- .../auto_generated/dxf/qgsdxfexport.sip.in | 9 ++++++++- src/app/qgsdxfexportdialog.cpp | 12 ++++++++++-- src/core/dxf/qgsdxfexport.cpp | 18 +++++++++++++----- src/core/dxf/qgsdxfexport.h | 15 ++++++++++++++- src/core/dxf/qgsdxfexport_p.h | 7 +++---- 6 files changed, 56 insertions(+), 14 deletions(-) diff --git a/python/PyQt6/core/auto_generated/dxf/qgsdxfexport.sip.in b/python/PyQt6/core/auto_generated/dxf/qgsdxfexport.sip.in index 98c957ce874b..9ca5450c6730 100644 --- a/python/PyQt6/core/auto_generated/dxf/qgsdxfexport.sip.in +++ b/python/PyQt6/core/auto_generated/dxf/qgsdxfexport.sip.in @@ -27,7 +27,7 @@ Exports QGIS layers to the DXF format. struct DxfLayer { - DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1 ); + DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1, QString overriddenName = QString() ); QgsVectorLayer *layer() const; %Docstring @@ -66,6 +66,13 @@ Returns the maximum number of data defined symbol classes for which blocks are c :return: +.. versionadded:: 3.38 +%End + + QString overriddenName() const; +%Docstring +Returns the overridden layer name to be used in the exported DXF. + .. versionadded:: 3.38 %End diff --git a/python/core/auto_generated/dxf/qgsdxfexport.sip.in b/python/core/auto_generated/dxf/qgsdxfexport.sip.in index 67ede91a94d6..61e474a533bd 100644 --- a/python/core/auto_generated/dxf/qgsdxfexport.sip.in +++ b/python/core/auto_generated/dxf/qgsdxfexport.sip.in @@ -27,7 +27,7 @@ Exports QGIS layers to the DXF format. struct DxfLayer { - DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1 ); + DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1, QString overriddenName = QString() ); QgsVectorLayer *layer() const; %Docstring @@ -66,6 +66,13 @@ Returns the maximum number of data defined symbol classes for which blocks are c :return: +.. versionadded:: 3.38 +%End + + QString overriddenName() const; +%Docstring +Returns the overridden layer name to be used in the exported DXF. + .. versionadded:: 3.38 %End diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 9f34cc5d751b..2f1f5ee85567 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -473,7 +473,11 @@ QList< QgsDxfExport::DxfLayer > QgsVectorLayerAndAttributeModel::layers() const if ( !layerIdx.contains( vl->id() ) ) { layerIdx.insert( vl->id(), layers.size() ); - layers << QgsDxfExport::DxfLayer( vl, mAttributeIdx.value( vl, -1 ), mCreateDDBlockInfo.value( vl, true ), mDDBlocksMaxNumberOfClasses.value( vl, -1 ) ); + layers << QgsDxfExport::DxfLayer( vl, + mAttributeIdx.value( vl, -1 ), + mCreateDDBlockInfo.value( vl, true ), + mDDBlocksMaxNumberOfClasses.value( vl, -1 ), + mOverriddenName.value( vl, QString() ) ); } } } @@ -484,7 +488,11 @@ QList< QgsDxfExport::DxfLayer > QgsVectorLayerAndAttributeModel::layers() const if ( !layerIdx.contains( vl->id() ) ) { layerIdx.insert( vl->id(), layers.size() ); - layers << QgsDxfExport::DxfLayer( vl, mAttributeIdx.value( vl, -1 ), mCreateDDBlockInfo.value( vl, true ), mDDBlocksMaxNumberOfClasses.value( vl, -1 ) ); + layers << QgsDxfExport::DxfLayer( vl, + mAttributeIdx.value( vl, -1 ), + mCreateDDBlockInfo.value( vl, true ), + mDDBlocksMaxNumberOfClasses.value( vl, -1 ), + mOverriddenName.value( vl, QString() ) ); } } } diff --git a/src/core/dxf/qgsdxfexport.cpp b/src/core/dxf/qgsdxfexport.cpp index c974fa15fed8..3485abbff7dd 100644 --- a/src/core/dxf/qgsdxfexport.cpp +++ b/src/core/dxf/qgsdxfexport.cpp @@ -89,17 +89,24 @@ void QgsDxfExport::addLayers( const QList &layers ) { mLayerList.clear(); mLayerNameAttribute.clear(); + mLayerOverriddenName.clear(); mLayerList.reserve( layers.size() ); for ( const DxfLayer &dxfLayer : layers ) { mLayerList << dxfLayer.layer(); if ( dxfLayer.layerOutputAttributeIndex() >= 0 ) + { mLayerNameAttribute.insert( dxfLayer.layer()->id(), dxfLayer.layerOutputAttributeIndex() ); + } if ( dxfLayer.buildDataDefinedBlocks() ) { mLayerDDBlockMaxNumberOfClasses.insert( dxfLayer.layer()->id(), dxfLayer.dataDefinedBlocksMaximumNumberOfClasses() ); } + if ( dxfLayer.overriddenName() != QString() ) + { + mLayerOverriddenName.insert( dxfLayer.layer()->id(), dxfLayer.overriddenName() ); + } } } @@ -776,7 +783,7 @@ void QgsDxfExport::writeEntities() while ( featureIt.nextFeature( fet ) ) { mRenderContext.expressionContext().setFeature( fet ); - QString lName( dxfLayerName( job->splitLayerAttribute.isNull() ? job->layerTitle : fet.attribute( job->splitLayerAttribute ).toString() ) ); + QString lName( dxfLayerName( job->splitLayerAttribute.isNull() ? job->layerDerivedName : fet.attribute( job->splitLayerAttribute ).toString() ) ); sctx.setFeature( &fet ); @@ -903,7 +910,7 @@ void QgsDxfExport::prepareRenderers() const QgsFields fields = vl->fields(); if ( splitLayerAttributeIndex >= 0 && splitLayerAttributeIndex < fields.size() ) splitLayerAttribute = fields.at( splitLayerAttributeIndex ).name(); - DxfLayerJob *job = new DxfLayerJob( vl, mMapSettings.layerStyleOverrides().value( vl->id() ), mRenderContext, this, splitLayerAttribute ); + DxfLayerJob *job = new DxfLayerJob( vl, mMapSettings.layerStyleOverrides().value( vl->id() ), mRenderContext, this, splitLayerAttribute, layerName( vl ) ); mJobs.append( job ); } } @@ -2384,9 +2391,10 @@ QStringList QgsDxfExport::encodings() QString QgsDxfExport::layerName( QgsVectorLayer *vl ) const { Q_ASSERT( vl ); - return mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) - ? ( !vl->metadata().title().isEmpty() ? vl->metadata().title() : vl->serverProperties()->title() ) - : vl->name(); + if ( mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) ) + return !vl->metadata().title().isEmpty() ? vl->metadata().title() : vl->serverProperties()->title(); + else + return mLayerOverriddenName.value( vl->id(), vl->name() ); } void QgsDxfExport::drawLabel( const QString &layerId, QgsRenderContext &context, pal::LabelPosition *label, const QgsPalLayerSettings &settings ) diff --git a/src/core/dxf/qgsdxfexport.h b/src/core/dxf/qgsdxfexport.h index 110ef2f7e1b9..1c99c1f0b8e6 100644 --- a/src/core/dxf/qgsdxfexport.h +++ b/src/core/dxf/qgsdxfexport.h @@ -73,11 +73,12 @@ class CORE_EXPORT QgsDxfExport : public QgsLabelSink */ struct CORE_EXPORT DxfLayer { - DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1 ) + DxfLayer( QgsVectorLayer *vl, int layerOutputAttributeIndex = -1, bool buildDDBlocks = true, int ddBlocksMaxNumberOfClasses = -1, QString overriddenName = QString() ) : mLayer( vl ) , mLayerOutputAttributeIndex( layerOutputAttributeIndex ) , mBuildDDBlocks( buildDDBlocks ) , mDDBlocksMaxNumberOfClasses( ddBlocksMaxNumberOfClasses ) + , mOverriddenName( overriddenName ) {} //! Returns the layer @@ -112,6 +113,12 @@ class CORE_EXPORT QgsDxfExport : public QgsLabelSink */ int dataDefinedBlocksMaximumNumberOfClasses() const { return mDDBlocksMaxNumberOfClasses; } + /** + * \brief Returns the overridden layer name to be used in the exported DXF. + * \since QGIS 3.38 + */ + QString overriddenName() const { return mOverriddenName; } + private: QgsVectorLayer *mLayer = nullptr; int mLayerOutputAttributeIndex = -1; @@ -125,6 +132,11 @@ class CORE_EXPORT QgsDxfExport : public QgsLabelSink * \brief Limit for the number of data defined symbol block classes (keep only the most used ones). -1 means no limit */ int mDDBlocksMaxNumberOfClasses = -1; + + /** + * \brief Overridden name of the layer to be exported to DXF + */ + QString mOverriddenName = QString(); }; //! Export flags @@ -667,6 +679,7 @@ class CORE_EXPORT QgsDxfExport : public QgsLabelSink QList mLayerList; QHash mLayerNameAttribute; QHash mLayerDDBlockMaxNumberOfClasses; + QHash mLayerOverriddenName; double mFactor = 1.0; bool mForce2d = false; diff --git a/src/core/dxf/qgsdxfexport_p.h b/src/core/dxf/qgsdxfexport_p.h index 742323aa3cb4..f315a97f09e2 100644 --- a/src/core/dxf/qgsdxfexport_p.h +++ b/src/core/dxf/qgsdxfexport_p.h @@ -33,7 +33,7 @@ */ struct DxfLayerJob { - DxfLayerJob( QgsVectorLayer *vl, const QString &layerStyleOverride, QgsRenderContext &renderContext, QgsDxfExport *dxfExport, const QString &splitLayerAttribute ) + DxfLayerJob( QgsVectorLayer *vl, const QString &layerStyleOverride, QgsRenderContext &renderContext, QgsDxfExport *dxfExport, const QString &splitLayerAttribute, const QString &layerDerivedName ) : renderContext( renderContext ) , styleOverride( vl ) , featureSource( vl ) @@ -41,8 +41,7 @@ struct DxfLayerJob , crs( vl->crs() ) , layerName( vl->name() ) , splitLayerAttribute( splitLayerAttribute ) - , layerTitle( !vl->metadata().title().isEmpty() ? vl->metadata().title() - : vl->serverProperties()->title().isEmpty() ? vl->name() : vl->serverProperties()->title() ) + , layerDerivedName( layerDerivedName ) { if ( !layerStyleOverride.isNull() ) { @@ -106,7 +105,7 @@ struct DxfLayerJob QgsLabelSinkProvider *labelProvider = nullptr; QgsRuleBasedLabelSinkProvider *ruleBasedLabelProvider = nullptr; QString splitLayerAttribute; - QString layerTitle; + QString layerDerivedName; // Obtained from layer title, name or overridden name QSet attributes; private: From d4e46009bbcfe5bac021888137df566ac1423203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 19 Mar 2024 18:15:07 +0100 Subject: [PATCH 03/14] Let group nodes show its name in DXF Export dialog --- src/app/qgsdxfexportdialog.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 2f1f5ee85567..577fb3901183 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -259,8 +259,6 @@ QVariant QgsVectorLayerAndAttributeModel::headerData( int section, Qt::Orientati QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role ) const { QgsVectorLayer *vl = vectorLayer( idx ); - if ( !vl ) - return QVariant(); if ( idx.column() == LAYER_COL ) { @@ -310,7 +308,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role Q_ASSERT( hasUnchecked ); return Qt::Unchecked; } - else if ( role == Qt::DisplayRole && mOverriddenName.contains( vl ) ) + else if ( role == Qt::DisplayRole && vl && mOverriddenName.contains( vl ) ) { return mOverriddenName[ vl ]; } @@ -319,7 +317,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role return QgsLayerTreeModel::data( idx, role ); } } - else if ( idx.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) + else if ( idx.column() == OUTPUT_LAYER_ATTRIBUTE_COL && vl ) { int idx = mAttributeIdx.value( vl, -1 ); if ( role == Qt::EditRole ) @@ -340,7 +338,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role } else if ( idx.column() == ALLOW_DD_SYMBOL_BLOCKS_COL ) { - if ( vl->geometryType() != Qgis::GeometryType::Point ) + if ( !vl || vl->geometryType() != Qgis::GeometryType::Point ) { return QVariant(); } @@ -357,7 +355,7 @@ QVariant QgsVectorLayerAndAttributeModel::data( const QModelIndex &idx, int role } else if ( idx.column() == MAXIMUM_DD_SYMBOL_BLOCKS_COL ) { - if ( vl->geometryType() != Qgis::GeometryType::Point ) + if ( !vl || vl->geometryType() != Qgis::GeometryType::Point ) { return QVariant(); } From 1fa0b1651ea43cf2f72cfa3d5c527fa83df345c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Fri, 22 Mar 2024 15:58:16 +0100 Subject: [PATCH 04/14] [tests] Add tests for output layer name precedence in DXF Export (1: attribute, 2: layer title, 3: overridden name, 4: layer name) --- tests/src/core/testqgsdxfexport.cpp | 159 ++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) diff --git a/tests/src/core/testqgsdxfexport.cpp b/tests/src/core/testqgsdxfexport.cpp index adfe17feb894..38ec4fb4b35a 100644 --- a/tests/src/core/testqgsdxfexport.cpp +++ b/tests/src/core/testqgsdxfexport.cpp @@ -55,6 +55,7 @@ class TestQgsDxfExport : public QObject void testPoints(); void testPointsDataDefinedSizeAngle(); void testPointsDataDefinedSizeSymbol(); + void testPointsOverriddenName(); void testLines(); void testPolygons(); void testMultiSurface(); @@ -81,6 +82,7 @@ class TestQgsDxfExport : public QObject void testSelectedPolygons(); void testMultipleLayersWithSelection(); void testExtentWithSelection(); + void testOutputLayerNamePrecedence(); private: QgsVectorLayer *mPointLayer = nullptr; @@ -283,6 +285,40 @@ void TestQgsDxfExport::testPointsDataDefinedSizeSymbol() QVERIFY( dxfString.contains( QStringLiteral( "50\n5.0" ) ) ); } +void TestQgsDxfExport::testPointsOverriddenName() +{ + QgsDxfExport d; + d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, -1, false, -1, QStringLiteral( "My Point Layer" ) ) ); + + QgsMapSettings mapSettings; + const QSize size( 640, 480 ); + mapSettings.setOutputSize( size ); + mapSettings.setExtent( mPointLayer->extent() ); + mapSettings.setLayers( QList() << mPointLayer ); + mapSettings.setOutputDpi( 96 ); + mapSettings.setDestinationCrs( mPointLayer->crs() ); + + d.setMapSettings( mapSettings ); + d.setSymbologyScale( 1000 ); + + const QString file = getTempFileName( "point_overridden_name_dxf" ); + QFile dxfFile( file ); + QCOMPARE( d.writeToFile( &dxfFile, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); + dxfFile.close(); + + QVERIFY( !fileContainsText( file, QStringLiteral( "nan.0" ) ) ); + QVERIFY( !fileContainsText( file, mPointLayer->name() ) ); // "points" + + // reload and compare + std::unique_ptr< QgsVectorLayer > result = std::make_unique< QgsVectorLayer >( file, "dxf" ); + QVERIFY( result->isValid() ); + QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); + QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); + QgsFeature feature; + result->getFeatures().nextFeature( feature ); + QCOMPARE( feature.attribute( "Layer" ), QStringLiteral( "My Point Layer" ) ); +} + void TestQgsDxfExport::testLines() { QgsDxfExport d; @@ -1773,6 +1809,129 @@ void TestQgsDxfExport::testExtentWithSelection() mPointLayer->removeSelection(); } +void TestQgsDxfExport::testOutputLayerNamePrecedence() +{ + // Test that output layer name precedence is: + // 1) Attribute (if any) + // 2) Layer title (if any) + // 3) Overridden name (if any) + // 4) Layer name + + const QString layerTitle = QStringLiteral( "Point Layer Title" ); + const QString layerOverriddenName = QStringLiteral( "My Point Layer" ); + + // A) All layer name options are set + QgsDxfExport d; + mPointLayer->setTitle( layerTitle ); + d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, + 0, // Class attribute, 3 unique values + false, + -1, + layerOverriddenName ) ); + + QgsMapSettings mapSettings; + const QSize size( 640, 480 ); + mapSettings.setOutputSize( size ); + mapSettings.setExtent( mPointLayer->extent() ); + mapSettings.setLayers( QList() << mPointLayer ); + mapSettings.setOutputDpi( 96 ); + mapSettings.setDestinationCrs( mPointLayer->crs() ); + + d.setMapSettings( mapSettings ); + d.setSymbologyScale( 1000 ); + d.setLayerTitleAsName( true ); + + const QString file = getTempFileName( "name_precedence_a_all_set_dxf" ); + QFile dxfFile( file ); + QCOMPARE( d.writeToFile( &dxfFile, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); + dxfFile.close(); + + QVERIFY( !fileContainsText( file, QStringLiteral( "nan.0" ) ) ); + QVERIFY( !fileContainsText( file, layerTitle ) ); + QVERIFY( !fileContainsText( file, layerOverriddenName ) ); + QVERIFY( !fileContainsText( file, mPointLayer->name() ) ); + + // reload and compare + std::unique_ptr< QgsVectorLayer > result = std::make_unique< QgsVectorLayer >( file, "dxf" ); + QVERIFY( result->isValid() ); + QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); + QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); + QSet values = result->uniqueValues( 0 ); // "Layer" field + QCOMPARE( values.count(), 3 ); + QVERIFY( values.contains( QVariant( "B52" ) ) ); + QVERIFY( values.contains( QVariant( "Jet" ) ) ); + QVERIFY( values.contains( QVariant( "Biplane" ) ) ); + + // B) No attribute given + d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, -1, false, -1, layerOverriddenName ) ); // this replaces layers + + const QString file2 = getTempFileName( "name_precedence_b_no_attr_dxf" ); + QFile dxfFile2( file2 ); + QCOMPARE( d.writeToFile( &dxfFile2, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); + dxfFile2.close(); + + QVERIFY( !fileContainsText( file2, QStringLiteral( "nan.0" ) ) ); + QVERIFY( fileContainsText( file2, layerTitle ) ); + QVERIFY( !fileContainsText( file2, layerOverriddenName ) ); + QVERIFY( !fileContainsText( file2, mPointLayer->name() ) ); + + // reload and compare + result = std::make_unique< QgsVectorLayer >( file2, "dxf" ); + QVERIFY( result->isValid() ); + QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); + QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); + QgsFeature feature; + result->getFeatures().nextFeature( feature ); + QCOMPARE( feature.attribute( "Layer" ), layerTitle ); + QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field + + // C) No attribute given, choose no title + d.setLayerTitleAsName( false ); + + const QString file3 = getTempFileName( "name_precedence_c_no_attr_no_title_dxf" ); + QFile dxfFile3( file3 ); + QCOMPARE( d.writeToFile( &dxfFile3, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); + dxfFile3.close(); + + QVERIFY( !fileContainsText( file3, QStringLiteral( "nan.0" ) ) ); + QVERIFY( !fileContainsText( file3, layerTitle ) ); + QVERIFY( fileContainsText( file3, layerOverriddenName ) ); + QVERIFY( !fileContainsText( file3, mPointLayer->name() ) ); + + // reload and compare + result = std::make_unique< QgsVectorLayer >( file3, "dxf" ); + QVERIFY( result->isValid() ); + QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); + QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); + result->getFeatures().nextFeature( feature ); + QCOMPARE( feature.attribute( "Layer" ), layerOverriddenName ); + QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field + + // D) No name options given, use default layer name + d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer ) ); // This replaces layers + + const QString file4 = getTempFileName( "name_precedence_d_no_anything_dxf" ); + QFile dxfFile4( file4 ); + QCOMPARE( d.writeToFile( &dxfFile4, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); + dxfFile4.close(); + + QVERIFY( !fileContainsText( file4, QStringLiteral( "nan.0" ) ) ); + QVERIFY( !fileContainsText( file4, layerTitle ) ); + QVERIFY( !fileContainsText( file4, layerOverriddenName ) ); + QVERIFY( fileContainsText( file4, mPointLayer->name() ) ); + + // reload and compare + result = std::make_unique< QgsVectorLayer >( file4, "dxf" ); + QVERIFY( result->isValid() ); + QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); + QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); + result->getFeatures().nextFeature( feature ); + QCOMPARE( feature.attribute( "Layer" ), mPointLayer->name() ); + QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field + + mPointLayer->setTitle( QString() ); // Leave the original empty title +} + bool TestQgsDxfExport::fileContainsText( const QString &path, const QString &text, QString *debugInfo ) const { QStringList debugLines; From f2137549a4736582779fa8d6a05f7ef881c2b21c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Mon, 25 Mar 2024 10:39:29 +0100 Subject: [PATCH 05/14] [processing] Allow users to override output layer name when exporting to DXF layers --- .../qgsprocessingparameterdxflayers.cpp | 12 +++++-- .../qgsprocessingdxflayerswidgetwrapper.cpp | 16 ++++++++- .../qgsprocessingdxflayerdetailswidgetbase.ui | 34 ++++++++++++------- 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/core/processing/qgsprocessingparameterdxflayers.cpp b/src/core/processing/qgsprocessingparameterdxflayers.cpp index 816e989363fe..ff9eb832d2ff 100644 --- a/src/core/processing/qgsprocessingparameterdxflayers.cpp +++ b/src/core/processing/qgsprocessingparameterdxflayers.cpp @@ -85,7 +85,7 @@ bool QgsProcessingParameterDxfLayers::checkValueIsAcceptable( const QVariant &in { const QVariantMap layerMap = variantLayer.toMap(); - if ( !layerMap.contains( QStringLiteral( "layer" ) ) && !layerMap.contains( QStringLiteral( "attributeIndex" ) ) ) + if ( !layerMap.contains( QStringLiteral( "layer" ) ) && !layerMap.contains( QStringLiteral( "attributeIndex" ) ) && !layerMap.contains( QStringLiteral( "overriddenLayerName" ) ) ) return false; if ( !context ) @@ -144,9 +144,12 @@ QString QgsProcessingParameterDxfLayers::valueAsPythonString( const QVariant &va { QStringList layerDefParts; layerDefParts << QStringLiteral( "'layer': " ) + QgsProcessingUtils::stringToPythonLiteral( QgsProcessingUtils::normalizeLayerSource( layer.layer()->source() ) ); + if ( layer.layerOutputAttributeIndex() >= -1 ) layerDefParts << QStringLiteral( "'attributeIndex': " ) + QgsProcessingUtils::variantToPythonLiteral( layer.layerOutputAttributeIndex() ); + layerDefParts << QStringLiteral( "'overriddenLayerName': " ) + QgsProcessingUtils::stringToPythonLiteral( layer.overriddenName() ); + const QString layerDef = QStringLiteral( "{%1}" ).arg( layerDefParts.join( ',' ) ); parts << layerDef; } @@ -239,7 +242,11 @@ QgsDxfExport::DxfLayer QgsProcessingParameterDxfLayers::variantMapAsLayer( const // bad } - QgsDxfExport::DxfLayer dxfLayer( inputLayer, layerVariantMap[ QStringLiteral( "attributeIndex" ) ].toInt() ); + QgsDxfExport::DxfLayer dxfLayer( inputLayer, + layerVariantMap[ QStringLiteral( "attributeIndex" ) ].toInt(), + false, + -1, + layerVariantMap[ QStringLiteral( "overriddenLayerName" ) ].toString() ); return dxfLayer; } @@ -251,5 +258,6 @@ QVariantMap QgsProcessingParameterDxfLayers::layerAsVariantMap( const QgsDxfExpo vm[ QStringLiteral( "layer" )] = layer.layer()->id(); vm[ QStringLiteral( "attributeIndex" ) ] = layer.layerOutputAttributeIndex(); + vm[ QStringLiteral( "overriddenLayerName" ) ] = layer.overriddenName(); return vm; } diff --git a/src/gui/processing/qgsprocessingdxflayerswidgetwrapper.cpp b/src/gui/processing/qgsprocessingdxflayerswidgetwrapper.cpp index a3b87d359741..56b4220ffbb6 100644 --- a/src/gui/processing/qgsprocessingdxflayerswidgetwrapper.cpp +++ b/src/gui/processing/qgsprocessingdxflayerswidgetwrapper.cpp @@ -51,14 +51,16 @@ QgsProcessingDxfLayerDetailsWidget::QgsProcessingDxfLayerDetailsWidget( const QV if ( mLayer->fields().exists( layer.layerOutputAttributeIndex() ) ) mFieldsComboBox->setField( mLayer->fields().at( layer.layerOutputAttributeIndex() ).name() ); + mOverriddenName->setText( layer.overriddenName() ); connect( mFieldsComboBox, &QgsFieldComboBox::fieldChanged, this, &QgsPanelWidget::widgetChanged ); + connect( mOverriddenName, &QLineEdit::textChanged, this, &QgsPanelWidget::widgetChanged ); } QVariant QgsProcessingDxfLayerDetailsWidget::value() const { const int index = mLayer->fields().lookupField( mFieldsComboBox->currentField() ); - const QgsDxfExport::DxfLayer layer( mLayer, index ); + const QgsDxfExport::DxfLayer layer( mLayer, index, false, -1, mOverriddenName->text().trimmed() ); return QgsProcessingParameterDxfLayers::layerAsVariantMap( layer ); } @@ -104,6 +106,7 @@ QgsProcessingDxfLayersPanelWidget::QgsProcessingDxfLayersPanelWidget( QVariantMap vm; vm["layer"] = layer->id(); vm["attributeIndex"] = -1; + vm["overriddenLayerName"] = QString(); const QString title = layer->name(); addOption( vm, title, false ); @@ -166,8 +169,19 @@ QString QgsProcessingDxfLayersPanelWidget::titleForLayer( const QgsDxfExport::Dx { QString title = layer.layer()->name(); + // if both options are set, the split attribute takes precedence, + // so hide overridden message to give users a hint on the result. if ( layer.layerOutputAttributeIndex() != -1 ) + { title += tr( " [split attribute: %1]" ).arg( layer.splitLayerAttribute() ); + } + else + { + if ( !layer.overriddenName().isEmpty() ) + { + title += tr( " [overridden name: %1]" ).arg( layer.overriddenName() ); + } + } return title; } diff --git a/src/ui/processing/qgsprocessingdxflayerdetailswidgetbase.ui b/src/ui/processing/qgsprocessingdxflayerdetailswidgetbase.ui index ef88ed6fcd66..542faa8f7484 100644 --- a/src/ui/processing/qgsprocessingdxflayerdetailswidgetbase.ui +++ b/src/ui/processing/qgsprocessingdxflayerdetailswidgetbase.ui @@ -7,28 +7,21 @@ 0 0 393 - 71 + 144 - - - - Attribute - - + + - + QDialogButtonBox::Cancel|QDialogButtonBox::Ok - - - - + Qt::Vertical @@ -41,6 +34,23 @@ + + + + Attribute + + + + + + + Output layer name + + + + + + From d8466a4d7b449aa47d95bf03f5531ba4e6a49db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Mon, 25 Mar 2024 10:41:03 +0100 Subject: [PATCH 06/14] [tests] DXF export processing algorithm allows users to override output layer name --- tests/src/analysis/testqgsprocessing.cpp | 17 +++++++++++++++-- tests/src/gui/testprocessinggui.cpp | 3 ++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp index 4dfbd740774d..9753c6bdc31e 100644 --- a/tests/src/analysis/testqgsprocessing.cpp +++ b/tests/src/analysis/testqgsprocessing.cpp @@ -11040,6 +11040,7 @@ void TestQgsProcessing::parameterDxfLayers() QVERIFY( !def->checkValueIsAcceptable( layerList ) ); layerMap["layer"] = "layerName"; layerMap["attributeIndex"] = -1; + layerMap["overriddenLayerName"] = QString(); layerList[0] = layerMap; QVERIFY( def->checkValueIsAcceptable( layerList ) ); QVERIFY( !def->checkValueIsAcceptable( layerList, &context ) ); //no corresponding layer in the context's project @@ -11052,6 +11053,10 @@ void TestQgsProcessing::parameterDxfLayers() layerList[0] = layerMap; QVERIFY( def->checkValueIsAcceptable( layerList, &context ) ); + layerMap["overriddenLayerName"] = QStringLiteral( "My Point Layer" ); + layerList[0] = layerMap; + QVERIFY( def->checkValueIsAcceptable( layerList, &context ) ); + // checkValueIsAcceptable on non-spatial layers QgsVectorLayer *nonSpatialLayer = new QgsVectorLayer( QStringLiteral( "None" ), QStringLiteral( "NonSpatialLayer" ), @@ -11073,15 +11078,16 @@ void TestQgsProcessing::parameterDxfLayers() QVariantMap wrongLayerMap; wrongLayerMap["layer"] = "NonSpatialLayer"; wrongLayerMap["attributeIndex"] = -1; + wrongLayerMap["overriddenLayerName"] = QString(); QVariantList wrongLayerMapList; wrongLayerMapList.append( wrongLayerMap ); QVERIFY( !def->checkValueIsAcceptable( wrongLayerMapList, &context ) ); // Check values const QString valueAsPythonString = def->valueAsPythonString( layerList, context ); - QCOMPARE( valueAsPythonString, QStringLiteral( "[{'layer': '%1','attributeIndex': -1}]" ).arg( vectorLayer->source() ) ); + QCOMPARE( valueAsPythonString, QStringLiteral( "[{'layer': '%1','attributeIndex': -1,'overriddenLayerName': 'My Point Layer'}]" ).arg( vectorLayer->source() ) ); QCOMPARE( QString::fromStdString( QgsJsonUtils::jsonFromVariant( def->valueAsJsonObject( layerList, context ) ).dump() ), - QStringLiteral( "[{\"attributeIndex\":-1,\"layer\":\"memory://%1\"}]" ).arg( vectorLayer->source() ) ); + QStringLiteral( "[{\"attributeIndex\":-1,\"layer\":\"memory://%1\",\"overriddenLayerName\":\"My Point Layer\"}]" ).arg( vectorLayer->source() ) ); bool ok = false; QCOMPARE( def->valueAsString( layerList, context, ok ), QString() ); QVERIFY( !ok ); @@ -11092,16 +11098,23 @@ void TestQgsProcessing::parameterDxfLayers() const QString pythonCode = def->asPythonString(); QCOMPARE( pythonCode, QStringLiteral( "QgsProcessingParameterDxfLayers('dxf input layer', '')" ) ); + // Default values for parameters other than the vector layer + layerMap["overriddenLayerName"] = QString(); + layerList[0] = layerMap; + const QgsDxfExport::DxfLayer dxfLayer( vectorLayer ); QList dxfList = def->parameterAsLayers( QVariant( vectorLayer->source() ), context ); QCOMPARE( dxfList.at( 0 ).layer()->source(), dxfLayer.layer()->source() ); QCOMPARE( dxfList.at( 0 ).layerOutputAttributeIndex(), dxfLayer.layerOutputAttributeIndex() ); + QCOMPARE( dxfList.at( 0 ).overriddenName(), dxfLayer.overriddenName() ); dxfList = def->parameterAsLayers( QVariant( QStringList() << vectorLayer->source() ), context ); QCOMPARE( dxfList.at( 0 ).layer()->source(), dxfLayer.layer()->source() ); QCOMPARE( dxfList.at( 0 ).layerOutputAttributeIndex(), dxfLayer.layerOutputAttributeIndex() ); + QCOMPARE( dxfList.at( 0 ).overriddenName(), dxfLayer.overriddenName() ); dxfList = def->parameterAsLayers( layerList, context ); QCOMPARE( dxfList.at( 0 ).layer()->source(), dxfLayer.layer()->source() ); QCOMPARE( dxfList.at( 0 ).layerOutputAttributeIndex(), dxfLayer.layerOutputAttributeIndex() ); + QCOMPARE( dxfList.at( 0 ).overriddenName(), dxfLayer.overriddenName() ); } void TestQgsProcessing::parameterAnnotationLayer() diff --git a/tests/src/gui/testprocessinggui.cpp b/tests/src/gui/testprocessinggui.cpp index a73b8ba6eb6d..00eb9bf2931d 100644 --- a/tests/src/gui/testprocessinggui.cpp +++ b/tests/src/gui/testprocessinggui.cpp @@ -9988,6 +9988,7 @@ void TestProcessingGui::testDxfLayersWrapper() QVariantMap layerMap; layerMap["layer"] = "PointLayer"; layerMap["attributeIndex"] = -1; + layerMap["overriddenLayerName"] = QString(); layerList.append( layerMap ); QVERIFY( definition.checkValueIsAcceptable( layerList, &context ) ); @@ -9998,7 +9999,7 @@ void TestProcessingGui::testDxfLayersWrapper() QVERIFY( definition.checkValueIsAcceptable( value, &context ) ); QString valueAsPythonString = definition.valueAsPythonString( value, context ); - QCOMPARE( valueAsPythonString, QStringLiteral( "[{'layer': '%1','attributeIndex': -1}]" ).arg( vectorLayer->source() ) ); + QCOMPARE( valueAsPythonString, QStringLiteral( "[{'layer': '%1','attributeIndex': -1,'overriddenLayerName': ''}]" ).arg( vectorLayer->source() ) ); } void TestProcessingGui::testAlignRasterLayersWrapper() From eaa5f3df256db2aaaf7073d4791a700032af6b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Wed, 27 Mar 2024 16:11:08 +0100 Subject: [PATCH 07/14] [dxf] Avoid unavailable layers in DXF Export dialog's layer tree --- src/app/qgsdxfexportdialog.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 577fb3901183..736dfdd31499 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -826,7 +826,8 @@ void QgsDxfExportDialog::cleanGroup( QgsLayerTreeNode *node ) { if ( QgsLayerTree::isLayer( child ) && ( QgsLayerTree::toLayer( child )->layer()->type() != Qgis::LayerType::Vector || - ! QgsLayerTree::toLayer( child )->layer()->isSpatial() ) ) + ! QgsLayerTree::toLayer( child )->layer()->isSpatial() || + ! QgsLayerTree::toLayer( child )->layer()->isValid() ) ) { toRemove << child; continue; From 7e4e09b86fdc48a319d35c8aedec1ea98f683da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Wed, 27 Mar 2024 16:53:27 +0100 Subject: [PATCH 08/14] [ux] Add hints for users on how the titleAsLayerName option works, since precedence is until now not clear for them --- src/analysis/processing/qgsalgorithmdxfexport.cpp | 3 ++- src/ui/qgsdxfexportdialogbase.ui | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/analysis/processing/qgsalgorithmdxfexport.cpp b/src/analysis/processing/qgsalgorithmdxfexport.cpp index 123781eb9ba2..a9ce7c69921e 100644 --- a/src/analysis/processing/qgsalgorithmdxfexport.cpp +++ b/src/analysis/processing/qgsalgorithmdxfexport.cpp @@ -47,7 +47,8 @@ QString QgsDxfExportAlgorithm::groupId() const QString QgsDxfExportAlgorithm::shortHelpString() const { - return QObject::tr( "Exports layers to DXF file. For each layer, you can choose a field whose values are used to split features in generated destination layers in the DXF output." ); + return QObject::tr( "Exports layers to DXF file. For each layer, you can choose a field whose values are used to split features in generated destination layers in the DXF output.\n\n" + "If no field is chosen, you can still override the output layer name by preferring layer title (set in layer properties) or by directly entering a new output layer name in the Configure Layer panel." ); } QgsDxfExportAlgorithm *QgsDxfExportAlgorithm::createInstance() const diff --git a/src/ui/qgsdxfexportdialogbase.ui b/src/ui/qgsdxfexportdialogbase.ui index 679c46bf1b2c..d8596064824c 100644 --- a/src/ui/qgsdxfexportdialogbase.ui +++ b/src/ui/qgsdxfexportdialogbase.ui @@ -171,6 +171,9 @@ + + If no attribute is chosen, prefer layer title (set in layer properties) to layer name. + Use layer title as name if set From b18c169ce5ace91363f648ef29032a1759d08882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Fri, 5 Apr 2024 14:24:51 +0200 Subject: [PATCH 09/14] [dxf] Apply review comments --- src/core/dxf/qgsdxfexport.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dxf/qgsdxfexport.h b/src/core/dxf/qgsdxfexport.h index 1c99c1f0b8e6..54a213bc4fa9 100644 --- a/src/core/dxf/qgsdxfexport.h +++ b/src/core/dxf/qgsdxfexport.h @@ -136,7 +136,7 @@ class CORE_EXPORT QgsDxfExport : public QgsLabelSink /** * \brief Overridden name of the layer to be exported to DXF */ - QString mOverriddenName = QString(); + QString mOverriddenName; }; //! Export flags From ed56c4c76827cb345cde02065a28b18409bd8ec8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 9 Apr 2024 16:03:28 +0200 Subject: [PATCH 10/14] [ux] Add help for DXF Export processing parameter 'Use layer title as name' --- src/analysis/processing/qgsalgorithmdxfexport.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/analysis/processing/qgsalgorithmdxfexport.cpp b/src/analysis/processing/qgsalgorithmdxfexport.cpp index a9ce7c69921e..8d2fa75f0af3 100644 --- a/src/analysis/processing/qgsalgorithmdxfexport.cpp +++ b/src/analysis/processing/qgsalgorithmdxfexport.cpp @@ -71,7 +71,9 @@ void QgsDxfExportAlgorithm::initAlgorithm( const QVariantMap & ) extentParam->setHelp( QObject::tr( "Limit exported features to those with geometries intersecting the provided extent" ) ); addParameter( extentParam.release() ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "SELECTED_FEATURES_ONLY" ), QObject::tr( "Use only selected features" ), false ) ); - addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "USE_LAYER_TITLE" ), QObject::tr( "Use layer title as name" ), false ) ); + std::unique_ptr useTitleParam = std::make_unique( QStringLiteral( "USE_LAYER_TITLE" ), QObject::tr( "Use layer title as name" ), false ); + useTitleParam->setHelp( QObject::tr( "If no attribute is chosen, prefer layer title (set in layer properties) to layer name" ) ); + addParameter( useTitleParam.release() ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "FORCE_2D" ), QObject::tr( "Force 2D output" ), false ) ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "MTEXT" ), QObject::tr( "Export labels as MTEXT elements" ), true ) ); addParameter( new QgsProcessingParameterFileDestination( QStringLiteral( "OUTPUT" ), QObject::tr( "DXF" ), QObject::tr( "DXF Files" ) + " (*.dxf *.DXF)" ) ); From a76439d76798abe29989c824ed3f18a7cc7b8ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 9 Apr 2024 20:45:07 +0200 Subject: [PATCH 11/14] Make it clear that we want a reference to avoid a compile warning --- src/app/qgsdxfexportdialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 736dfdd31499..25b1772e1ab6 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -1108,7 +1108,7 @@ void QgsDxfExportDialog::saveSettingsToXML( QDomDocument &doc ) const QgsVectorLayerRef vlRef; const QgsReadWriteContext rwContext = QgsReadWriteContext(); - for ( const auto dxfLayer : layers() ) + for ( const auto &dxfLayer : layers() ) { QDomElement layerElement = domDocument.createElement( QStringLiteral( "layer" ) ); vlRef.setLayer( dxfLayer.layer() ); From d9d6ad0079df77ba58de200ee59862eb95a6cfb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Wed, 10 Apr 2024 17:25:10 +0200 Subject: [PATCH 12/14] [tests] Replace setTitle() by serverProperties()->setTitle() (conflict resolution with PR 57103) --- tests/src/core/testqgsdxfexport.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/core/testqgsdxfexport.cpp b/tests/src/core/testqgsdxfexport.cpp index 38ec4fb4b35a..b8101b7b5763 100644 --- a/tests/src/core/testqgsdxfexport.cpp +++ b/tests/src/core/testqgsdxfexport.cpp @@ -1822,7 +1822,7 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() // A) All layer name options are set QgsDxfExport d; - mPointLayer->setTitle( layerTitle ); + mPointLayer->serverProperties()->setTitle( layerTitle ); d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, 0, // Class attribute, 3 unique values false, @@ -1929,7 +1929,7 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() QCOMPARE( feature.attribute( "Layer" ), mPointLayer->name() ); QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field - mPointLayer->setTitle( QString() ); // Leave the original empty title + mPointLayer->serverProperties()->setTitle( QString() ); // Leave the original empty title } bool TestQgsDxfExport::fileContainsText( const QString &path, const QString &text, QString *debugInfo ) const From c2e248befce59b81f0e6e2ce2b6d7cd6e7cdccd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 16 Apr 2024 11:32:50 +0200 Subject: [PATCH 13/14] [dxf] Allow users to recover original layer name after overriding it (by using the QgsFilterLineEdit with default value) --- src/app/qgsdxfexportdialog.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/app/qgsdxfexportdialog.cpp b/src/app/qgsdxfexportdialog.cpp index 25b1772e1ab6..4e58c1bc0118 100644 --- a/src/app/qgsdxfexportdialog.cpp +++ b/src/app/qgsdxfexportdialog.cpp @@ -55,7 +55,8 @@ QWidget *FieldSelectorDelegate::createEditor( QWidget *parent, const QStyleOptio if ( index.column() == LAYER_COL ) { - QLineEdit *le = new QLineEdit( parent ); + QgsFilterLineEdit *le = new QgsFilterLineEdit( parent, vl->name() ); + return le; } else if ( index.column() == OUTPUT_LAYER_ATTRIBUTE_COL ) @@ -87,7 +88,7 @@ void FieldSelectorDelegate::setEditorData( QWidget *editor, const QModelIndex &i if ( index.column() == LAYER_COL ) { - QLineEdit *le = qobject_cast< QLineEdit * >( editor ); + QgsFilterLineEdit *le = qobject_cast< QgsFilterLineEdit * >( editor ); if ( le ) { le->setText( index.data().toString() ); @@ -123,7 +124,7 @@ void FieldSelectorDelegate::setModelData( QWidget *editor, QAbstractItemModel *m if ( index.column() == LAYER_COL ) { - QLineEdit *le = qobject_cast( editor ); + QgsFilterLineEdit *le = qobject_cast( editor ); if ( le ) { model->setData( index, le->text() ); From 82fb6ccd24c9c36dbc2d7547850d3803e401c76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Carrillo?= Date: Tue, 16 Apr 2024 17:54:02 +0200 Subject: [PATCH 14/14] [dxf] Address review: prefer overridden layer name over layer title (the former can be changed in the export dialog, whereas the latter must be done in the layer properties, therefore, overridden layer name is closer to the export intention and should take the precedence) --- .../processing/qgsalgorithmdxfexport.cpp | 4 ++-- src/core/dxf/qgsdxfexport.cpp | 12 ++++++++-- src/core/dxf/qgsdxfexport_p.h | 2 +- src/ui/qgsdxfexportdialogbase.ui | 2 +- tests/src/core/testqgsdxfexport.cpp | 23 ++++++++++--------- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/analysis/processing/qgsalgorithmdxfexport.cpp b/src/analysis/processing/qgsalgorithmdxfexport.cpp index 8d2fa75f0af3..9e625715923d 100644 --- a/src/analysis/processing/qgsalgorithmdxfexport.cpp +++ b/src/analysis/processing/qgsalgorithmdxfexport.cpp @@ -48,7 +48,7 @@ QString QgsDxfExportAlgorithm::groupId() const QString QgsDxfExportAlgorithm::shortHelpString() const { return QObject::tr( "Exports layers to DXF file. For each layer, you can choose a field whose values are used to split features in generated destination layers in the DXF output.\n\n" - "If no field is chosen, you can still override the output layer name by preferring layer title (set in layer properties) or by directly entering a new output layer name in the Configure Layer panel." ); + "If no field is chosen, you can still override the output layer name by directly entering a new output layer name in the Configure Layer panel or by preferring layer title (set in layer properties) to layer name." ); } QgsDxfExportAlgorithm *QgsDxfExportAlgorithm::createInstance() const @@ -72,7 +72,7 @@ void QgsDxfExportAlgorithm::initAlgorithm( const QVariantMap & ) addParameter( extentParam.release() ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "SELECTED_FEATURES_ONLY" ), QObject::tr( "Use only selected features" ), false ) ); std::unique_ptr useTitleParam = std::make_unique( QStringLiteral( "USE_LAYER_TITLE" ), QObject::tr( "Use layer title as name" ), false ); - useTitleParam->setHelp( QObject::tr( "If no attribute is chosen, prefer layer title (set in layer properties) to layer name" ) ); + useTitleParam->setHelp( QObject::tr( "If no attribute is chosen and layer name is not being overridden, prefer layer title (set in layer properties) to layer name" ) ); addParameter( useTitleParam.release() ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "FORCE_2D" ), QObject::tr( "Force 2D output" ), false ) ); addParameter( new QgsProcessingParameterBoolean( QStringLiteral( "MTEXT" ), QObject::tr( "Export labels as MTEXT elements" ), true ) ); diff --git a/src/core/dxf/qgsdxfexport.cpp b/src/core/dxf/qgsdxfexport.cpp index 3485abbff7dd..b34c48c0b370 100644 --- a/src/core/dxf/qgsdxfexport.cpp +++ b/src/core/dxf/qgsdxfexport.cpp @@ -2391,10 +2391,18 @@ QStringList QgsDxfExport::encodings() QString QgsDxfExport::layerName( QgsVectorLayer *vl ) const { Q_ASSERT( vl ); - if ( mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) ) + if ( !mLayerOverriddenName.value( vl->id(), QString() ).isEmpty() ) + { + return mLayerOverriddenName.value( vl->id() ); + } + else if ( mLayerTitleAsName && ( !vl->metadata().title().isEmpty() || !vl->serverProperties()->title().isEmpty() ) ) + { return !vl->metadata().title().isEmpty() ? vl->metadata().title() : vl->serverProperties()->title(); + } else - return mLayerOverriddenName.value( vl->id(), vl->name() ); + { + return vl->name(); + } } void QgsDxfExport::drawLabel( const QString &layerId, QgsRenderContext &context, pal::LabelPosition *label, const QgsPalLayerSettings &settings ) diff --git a/src/core/dxf/qgsdxfexport_p.h b/src/core/dxf/qgsdxfexport_p.h index f315a97f09e2..cb00d13f0be8 100644 --- a/src/core/dxf/qgsdxfexport_p.h +++ b/src/core/dxf/qgsdxfexport_p.h @@ -105,7 +105,7 @@ struct DxfLayerJob QgsLabelSinkProvider *labelProvider = nullptr; QgsRuleBasedLabelSinkProvider *ruleBasedLabelProvider = nullptr; QString splitLayerAttribute; - QString layerDerivedName; // Obtained from layer title, name or overridden name + QString layerDerivedName; // Obtained from overridden name, title or layer name QSet attributes; private: diff --git a/src/ui/qgsdxfexportdialogbase.ui b/src/ui/qgsdxfexportdialogbase.ui index d8596064824c..39f0bd71b904 100644 --- a/src/ui/qgsdxfexportdialogbase.ui +++ b/src/ui/qgsdxfexportdialogbase.ui @@ -172,7 +172,7 @@ - If no attribute is chosen, prefer layer title (set in layer properties) to layer name. + If no attribute is chosen and layer name is not being overridden, prefer layer title (set in layer properties) to layer name. Use layer title as name if set diff --git a/tests/src/core/testqgsdxfexport.cpp b/tests/src/core/testqgsdxfexport.cpp index b8101b7b5763..f409bbc18e66 100644 --- a/tests/src/core/testqgsdxfexport.cpp +++ b/tests/src/core/testqgsdxfexport.cpp @@ -1813,8 +1813,8 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() { // Test that output layer name precedence is: // 1) Attribute (if any) - // 2) Layer title (if any) - // 3) Overridden name (if any) + // 2) Overridden name (if any) + // 3) Layer title (if any) // 4) Layer name const QString layerTitle = QStringLiteral( "Point Layer Title" ); @@ -1871,8 +1871,8 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() dxfFile2.close(); QVERIFY( !fileContainsText( file2, QStringLiteral( "nan.0" ) ) ); - QVERIFY( fileContainsText( file2, layerTitle ) ); - QVERIFY( !fileContainsText( file2, layerOverriddenName ) ); + QVERIFY( !fileContainsText( file2, layerTitle ) ); + QVERIFY( fileContainsText( file2, layerOverriddenName ) ); QVERIFY( !fileContainsText( file2, mPointLayer->name() ) ); // reload and compare @@ -1882,20 +1882,20 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); QgsFeature feature; result->getFeatures().nextFeature( feature ); - QCOMPARE( feature.attribute( "Layer" ), layerTitle ); + QCOMPARE( feature.attribute( "Layer" ), layerOverriddenName ); QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field - // C) No attribute given, choose no title - d.setLayerTitleAsName( false ); + // C) No attribute given, no override + d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer, -1, false, -1 ) ); // this replaces layers - const QString file3 = getTempFileName( "name_precedence_c_no_attr_no_title_dxf" ); + const QString file3 = getTempFileName( "name_precedence_c_no_attr_no_override_dxf" ); QFile dxfFile3( file3 ); QCOMPARE( d.writeToFile( &dxfFile3, QStringLiteral( "CP1252" ) ), QgsDxfExport::ExportResult::Success ); dxfFile3.close(); QVERIFY( !fileContainsText( file3, QStringLiteral( "nan.0" ) ) ); - QVERIFY( !fileContainsText( file3, layerTitle ) ); - QVERIFY( fileContainsText( file3, layerOverriddenName ) ); + QVERIFY( fileContainsText( file3, layerTitle ) ); + QVERIFY( !fileContainsText( file3, layerOverriddenName ) ); QVERIFY( !fileContainsText( file3, mPointLayer->name() ) ); // reload and compare @@ -1904,11 +1904,12 @@ void TestQgsDxfExport::testOutputLayerNamePrecedence() QCOMPARE( result->featureCount(), mPointLayer->featureCount() ); QCOMPARE( result->wkbType(), Qgis::WkbType::Point ); result->getFeatures().nextFeature( feature ); - QCOMPARE( feature.attribute( "Layer" ), layerOverriddenName ); + QCOMPARE( feature.attribute( "Layer" ), layerTitle ); QCOMPARE( result->uniqueValues( 0 ).count(), 1 ); // "Layer" field // D) No name options given, use default layer name d.addLayers( QList< QgsDxfExport::DxfLayer >() << QgsDxfExport::DxfLayer( mPointLayer ) ); // This replaces layers + d.setLayerTitleAsName( false ); const QString file4 = getTempFileName( "name_precedence_d_no_anything_dxf" ); QFile dxfFile4( file4 );