Skip to content

Commit 2973735

Browse files
domi4484domi4484
committedJan 10, 2022
Apply suggestions from code review
·
ltr-3_40final-3_24_0
1 parent 34fd68f commit 2973735

File tree

10 files changed

+205
-270
lines changed

10 files changed

+205
-270
lines changed
 

‎python/gui/auto_generated/qgstextformatwidget.sip.in

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,12 @@ Sets the background color for the text preview widget.
145145
:param color: background color
146146
%End
147147

148-
void updateDataDefinedAlignment();
148+
void enableDataDefinedAlignment( bool enable ) /Deprecated/;
149149
%Docstring
150150
Update the enabled state of the data defined alignment buttons.
151+
152+
.. deprecated::
153+
QGIS 3.24
151154
%End
152155

153156
virtual QgsExpressionContext createExpressionContext() const;

‎src/app/labeling/qgsmaptoollabel.cpp

Lines changed: 12 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include "qgsstatusbar.h"
3737
#include "qgslabelingresults.h"
3838
#include "qgsexpressionnodeimpl.h"
39+
#include "qgsreferencedgeometry.h"
3940

4041
#include <QMouseEvent>
4142

@@ -632,50 +633,8 @@ QVariant QgsMapToolLabel::evaluateDataDefinedProperty( QgsPalLayerSettings::Prop
632633

633634
bool QgsMapToolLabel::currentLabelDataDefinedPosition( double &x, bool &xSuccess, double &y, bool &ySuccess, int &xCol, int &yCol ) const
634635
{
635-
QgsVectorLayer *vlayer = mCurrentLabel.layer;
636-
QgsFeatureId featureId = mCurrentLabel.pos.featureId;
637-
638-
xSuccess = false;
639-
ySuccess = false;
640-
641-
if ( !vlayer )
642-
{
643-
return false;
644-
}
645-
646-
if ( mCurrentLabel.pos.isDiagram )
647-
{
648-
if ( !diagramMoveable( vlayer, xCol, yCol ) )
649-
{
650-
return false;
651-
}
652-
}
653-
else if ( !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol ) )
654-
{
655-
return false;
656-
}
657-
658-
QgsFeature f;
659-
if ( !vlayer->getFeatures( QgsFeatureRequest().setFilterFid( featureId ).setFlags( QgsFeatureRequest::NoGeometry ) ).nextFeature( f ) )
660-
{
661-
return false;
662-
}
663-
664-
if ( mCurrentLabel.pos.isUnplaced )
665-
{
666-
xSuccess = false;
667-
ySuccess = false;
668-
}
669-
else
670-
{
671-
QgsAttributes attributes = f.attributes();
672-
if ( !attributes.at( xCol ).isNull() )
673-
x = attributes.at( xCol ).toDouble( &xSuccess );
674-
if ( !attributes.at( yCol ).isNull() )
675-
y = attributes.at( yCol ).toDouble( &ySuccess );
676-
}
677-
678-
return true;
636+
int pointCol = -1;
637+
return currentLabelDataDefinedPosition( x, xSuccess, y, ySuccess, xCol, yCol, pointCol );
679638
}
680639

681640
bool QgsMapToolLabel::currentLabelDataDefinedPosition( double &x, bool &xSuccess, double &y, bool &ySuccess, int &xCol, int &yCol, int &pointCol ) const
@@ -715,30 +674,18 @@ bool QgsMapToolLabel::currentLabelDataDefinedPosition( double &x, bool &xSuccess
715674

716675
if ( mCurrentLabel.settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionPoint ) )
717676
{
718-
if ( !attributes.at( pointCol ).isNull() )
677+
if ( pointCol >= 0
678+
&& !attributes.at( pointCol ).isNull() )
719679
{
720680
QVariant pointAsVariant = attributes.at( pointCol );
721681
if ( pointAsVariant.canConvert<QgsGeometry>() )
722682
{
723-
const QgsGeometry geometry = pointAsVariant.value<QgsGeometry>();
724-
if ( const QgsPoint *point = ( geometry.constGet() ? qgsgeometry_cast<QgsPoint *>( geometry.constGet()->simplifiedTypeRef() ) : nullptr ) )
725-
{
683+
const QgsGeometry geometry = pointAsVariant.value<QgsGeometry>();
684+
if ( const QgsPoint *point = ( geometry.constGet() ? qgsgeometry_cast<QgsPoint *>( geometry.constGet()->simplifiedTypeRef() ) : nullptr ) )
685+
{
726686
x = point->x();
727687
y = point->y();
728688

729-
xSuccess = true;
730-
ySuccess = true;
731-
}
732-
}
733-
else if ( !pointAsVariant.toByteArray().isEmpty() )
734-
{
735-
QgsPoint point;
736-
QgsConstWkbPtr wkbPtr( pointAsVariant.toByteArray() );
737-
if ( point.fromWkb( wkbPtr ) )
738-
{
739-
x = point.x();
740-
y = point.y();
741-
742689
xSuccess = true;
743690
ySuccess = true;
744691
}
@@ -809,7 +756,7 @@ bool QgsMapToolLabel::changeCurrentLabelDataDefinedPosition( const QVariant &x,
809756
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, mCurrentLabel.settings, mCurrentLabel.layer );
810757
int pointCol = mCurrentLabel.layer->fields().lookupField( pointColName );
811758

812-
if ( !mCurrentLabel.layer->changeAttributeValue( mCurrentLabel.pos.featureId, pointCol, QgsPoint( x.toDouble(), y.toDouble() ).asWkt() ) )
759+
if ( !mCurrentLabel.layer->changeAttributeValue( mCurrentLabel.pos.featureId, pointCol, QVariant::fromValue( QgsReferencedGeometry( QgsGeometry::fromPointXY( QgsPoint( x.toDouble(), y.toDouble() ) ), mCurrentLabel.layer->crs() ) ) ) )
813760
return false;
814761
}
815762
else
@@ -905,12 +852,8 @@ bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, int &xCol, int &yCo
905852

906853
bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, const QgsPalLayerSettings &settings, int &xCol, int &yCol ) const
907854
{
908-
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, settings, vlayer );
909-
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, settings, vlayer );
910-
//return !xColName.isEmpty() && !yColName.isEmpty();
911-
xCol = vlayer->fields().lookupField( xColName );
912-
yCol = vlayer->fields().lookupField( yColName );
913-
return ( xCol != -1 && yCol != -1 );
855+
int pointCol = -1;
856+
return labelMoveable( vlayer, settings, xCol, yCol, pointCol );
914857
}
915858

916859
bool QgsMapToolLabel::layerCanPin( QgsVectorLayer *vlayer, int &xCol, int &yCol ) const
@@ -1071,7 +1014,7 @@ bool QgsMapToolLabel::createAuxiliaryFields( LabelDetails &details, QgsPalIndexe
10711014
{
10721015
index = vlayer->fields().lookupField( prop.field() );
10731016
}
1074-
else if ( prop.propertyType() != QgsProperty::ExpressionBasedProperty )
1017+
else
10751018
{
10761019
index = QgsAuxiliaryLayer::createProperty( p, vlayer, false );
10771020
changed = true;

‎src/app/labeling/qgsmaptoollabel.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,6 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapToolAdvancedDigitizing
8383
*/
8484
bool labelCanShowHide( QgsVectorLayer *vlayer, int &showCol ) const;
8585

86-
/**
87-
* Checks if labels in a layer can be rotated
88-
* \param rotationCol out: attribute column for data defined label rotation
89-
*/
90-
bool layerIsRotatable( QgsVectorLayer *vlayer, int &rotationCol ) const;
91-
9286
/**
9387
* Checks if labels in a layer can be rotated
9488
* \param rotationCol out: attribute column for data defined label rotation

‎src/app/labeling/qgsmaptoolmovelabel.cpp

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -187,48 +187,30 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )
187187
if ( !mCurrentLabel.pos.isDiagram && !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol, pointCol ) )
188188
{
189189
if ( mCurrentLabel.settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionPoint ) )
190-
{
191-
mPalProperties.clear();
192-
mPalProperties << QgsPalLayerSettings::PositionPoint;
193-
QgsPalIndexes indexes;
194-
if ( createAuxiliaryFields( indexes ) )
195-
return;
190+
mCurrentLabel.settings.dataDefinedProperties().property( QgsPalLayerSettings::PositionPoint ).setActive( false );
196191

197-
if ( !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol, pointCol ) )
198-
{
199-
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, mCurrentLabel.settings, vlayer );
200-
if ( pointCol < 0 )
201-
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label Point column “%1” does not exist in the layer" ).arg( pointColName ) );
202-
return;
203-
}
192+
mPalProperties.clear();
193+
mPalProperties << QgsPalLayerSettings::PositionX;
194+
mPalProperties << QgsPalLayerSettings::PositionY;
195+
QgsPalIndexes indexes;
196+
if ( createAuxiliaryFields( indexes ) )
197+
return;
204198

205-
pointCol = indexes[ QgsPalLayerSettings::PositionPoint ];
206-
}
207-
else
199+
if ( !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol ) )
208200
{
209-
mPalProperties.clear();
210-
mPalProperties << QgsPalLayerSettings::PositionX;
211-
mPalProperties << QgsPalLayerSettings::PositionY;
212-
QgsPalIndexes indexes;
213-
if ( createAuxiliaryFields( indexes ) )
214-
return;
215-
216-
if ( !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol ) )
217-
{
218-
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, vlayer );
219-
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, vlayer );
220-
if ( xCol < 0 && yCol < 0 )
221-
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label X/Y columns “%1” and “%2” do not exist in the layer" ).arg( xColName, yColName ) );
222-
else if ( xCol < 0 )
223-
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label X column “%1” does not exist in the layer" ).arg( xColName ) );
224-
else if ( yCol < 0 )
225-
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label Y column “%1” does not exist in the layer" ).arg( yColName ) );
226-
return;
227-
}
228-
229-
xCol = indexes[ QgsPalLayerSettings::PositionX ];
230-
yCol = indexes[ QgsPalLayerSettings::PositionY ];
201+
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, vlayer );
202+
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, vlayer );
203+
if ( xCol < 0 && yCol < 0 )
204+
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label X/Y columns “%1” and “%2” do not exist in the layer" ).arg( xColName, yColName ) );
205+
else if ( xCol < 0 )
206+
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label X column “%1” does not exist in the layer" ).arg( xColName ) );
207+
else if ( yCol < 0 )
208+
QgisApp::instance()->messageBar()->pushWarning( tr( "Move Label" ), tr( "The label Y column “%1” does not exist in the layer" ).arg( yColName ) );
209+
return;
231210
}
211+
212+
xCol = indexes[ QgsPalLayerSettings::PositionX ];
213+
yCol = indexes[ QgsPalLayerSettings::PositionY ];
232214
}
233215
else if ( mCurrentLabel.pos.isDiagram && !diagramMoveable( vlayer, xCol, yCol ) )
234216
{
@@ -244,8 +226,11 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )
244226
yCol = indexes[ QgsDiagramLayerSettings::PositionY ];
245227
}
246228

247-
bool usesAuxFields = vlayer->fields().fieldOrigin( xCol ) == QgsFields::OriginJoin
248-
&& vlayer->fields().fieldOrigin( yCol ) == QgsFields::OriginJoin;
229+
bool usesAuxFields = false;
230+
if ( xCol >= 0 && yCol >= 0 )
231+
usesAuxFields = vlayer->fields().fieldOrigin( xCol ) == QgsFields::OriginJoin
232+
&& vlayer->fields().fieldOrigin( yCol ) == QgsFields::OriginJoin;
233+
249234
if ( mCurrentLabel.settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionPoint )
250235
&& !mCurrentLabel.pos.isDiagram )
251236
usesAuxFields = vlayer->fields().fieldOrigin( pointCol ) == QgsFields::OriginJoin;

‎src/core/labeling/qgspallabeling.cpp

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,48 +2383,54 @@ std::unique_ptr<QgsLabelFeature> QgsPalLayerSettings::registerFeatureWithDetails
23832383
bool ddPosition = false;
23842384

23852385
if ( mDataDefinedProperties.isActive( QgsPalLayerSettings::PositionX )
2386-
&& mDataDefinedProperties.isActive( QgsPalLayerSettings::PositionY )
2387-
&& !mDataDefinedProperties.value( QgsPalLayerSettings::PositionX, context.expressionContext() ).isNull()
2388-
&& !mDataDefinedProperties.value( QgsPalLayerSettings::PositionY, context.expressionContext() ).isNull() )
2386+
&& mDataDefinedProperties.isActive( QgsPalLayerSettings::PositionY ) )
23892387
{
2390-
ddPosition = true;
2388+
const QVariant xPosProperty = mDataDefinedProperties.value( QgsPalLayerSettings::PositionX, context.expressionContext() );
2389+
const QVariant yPosProperty = mDataDefinedProperties.value( QgsPalLayerSettings::PositionY, context.expressionContext() );
2390+
if ( !xPosProperty.isNull()
2391+
&& !yPosProperty.isNull() )
2392+
{
2393+
ddPosition = true;
23912394

2392-
bool ddXPos = false, ddYPos = false;
2393-
xPos = mDataDefinedProperties.value( QgsPalLayerSettings::PositionX, context.expressionContext() ).toDouble( &ddXPos );
2394-
yPos = mDataDefinedProperties.value( QgsPalLayerSettings::PositionY, context.expressionContext() ).toDouble( &ddYPos );
2395-
if ( ddXPos && ddYPos )
2396-
hasDataDefinedPosition = true;
2395+
bool ddXPos = false, ddYPos = false;
2396+
xPos = xPosProperty.toDouble( &ddXPos );
2397+
yPos = yPosProperty.toDouble( &ddYPos );
2398+
if ( ddXPos && ddYPos )
2399+
hasDataDefinedPosition = true;
2400+
}
23972401
}
2398-
else if ( mDataDefinedProperties.isActive( QgsPalLayerSettings::PositionPoint )
2399-
&& !mDataDefinedProperties.value( QgsPalLayerSettings::PositionPoint, context.expressionContext() ).isNull() )
2402+
else if ( mDataDefinedProperties.isActive( QgsPalLayerSettings::PositionPoint ) )
24002403
{
2401-
ddPosition = true;
2402-
2403-
QVariant pointAsVariant = mDataDefinedProperties.value( QgsPalLayerSettings::PositionPoint, context.expressionContext() );
2404-
QgsPoint point;
2405-
if ( pointAsVariant.canConvert<QgsReferencedGeometry>() )
2404+
const QVariant pointPosProperty = mDataDefinedProperties.value( QgsPalLayerSettings::PositionPoint, context.expressionContext() );
2405+
if ( !pointPosProperty.isNull() )
24062406
{
2407-
point = QgsPoint( pointAsVariant.value<QgsReferencedGeometry>().asPoint() );
2408-
}
2409-
else if ( pointAsVariant.canConvert<QgsGeometry>() )
2410-
{
2411-
point = QgsPoint( pointAsVariant.value<QgsGeometry>().asPoint() );
2412-
}
2413-
else if ( !pointAsVariant.toString().isEmpty() )
2414-
{
2415-
point.fromWkt( pointAsVariant.toString() );
2416-
}
2407+
ddPosition = true;
24172408

2418-
if ( !point.isEmpty() )
2419-
{
2420-
hasDataDefinedPosition = true;
2409+
QgsPoint point;
2410+
if ( pointPosProperty.canConvert<QgsReferencedGeometry>() )
2411+
{
2412+
QgsReferencedGeometry referencedGeometryPoint = pointPosProperty.value<QgsReferencedGeometry>();
2413+
point = QgsPoint( referencedGeometryPoint.asPoint() );
2414+
2415+
if ( !referencedGeometryPoint.isNull()
2416+
&& ct.sourceCrs() != referencedGeometryPoint.crs() )
2417+
QgsMessageLog::logMessage( QObject::tr( "Label position geometry is not in layer coordinates reference system. Layer CRS: '%1', Geometry CRS: '%2'" ).arg( ct.sourceCrs().userFriendlyIdentifier(), referencedGeometryPoint.crs().userFriendlyIdentifier() ), QObject::tr( "Labeling" ), Qgis::Warning );
2418+
}
2419+
else if ( pointPosProperty.canConvert<QgsGeometry>() )
2420+
{
2421+
point = QgsPoint( pointPosProperty.value<QgsGeometry>().asPoint() );
2422+
}
24212423

2422-
xPos = point.x();
2423-
yPos = point.y();
2424+
if ( !point.isEmpty() )
2425+
{
2426+
hasDataDefinedPosition = true;
2427+
2428+
xPos = point.x();
2429+
yPos = point.y();
2430+
}
24242431
}
24252432
}
24262433

2427-
24282434
if ( ddPosition )
24292435
{
24302436
//data defined position. But field values could be NULL -> positions will be generated by PAL
@@ -2525,12 +2531,6 @@ std::unique_ptr<QgsLabelFeature> QgsPalLayerSettings::registerFeatureWithDetails
25252531
else
25262532
{
25272533
anchorPosition = QgsPointXY( xPos, yPos );
2528-
2529-
// only rotate non-pinned OverPoint placements until other placements are supported in pal::Feature
2530-
if ( dataDefinedRotation && placement != QgsPalLayerSettings::OverPoint )
2531-
{
2532-
angle = 0.0;
2533-
}
25342534
}
25352535
}
25362536
}

‎src/gui/labeling/qgslabelinggui.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,8 +482,6 @@ void QgsLabelingGui::setLayer( QgsMapLayer *mapLayer )
482482
// do this after other widgets are configured, so they can be enabled/disabled
483483
populateDataDefinedButtons();
484484

485-
updateDataDefinedAlignment();
486-
487485
updateUi(); // should come after data defined button setup
488486
}
489487

‎src/gui/qgspropertyoverridebutton.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,9 +685,12 @@ void QgsPropertyOverrideButton::showExpressionDialog()
685685
if ( d.exec() == QDialog::Accepted )
686686
{
687687
mExpressionString = d.expressionText().trimmed();
688+
bool active = mProperty.isActive();
688689
mProperty.setExpressionString( mExpressionString );
689690
mProperty.setTransformer( nullptr );
690-
setActivePrivate( !mExpressionString.isEmpty() );
691+
mProperty.setActive( !mExpressionString.isEmpty() );
692+
if ( mProperty.isActive() != active )
693+
emit activated( mProperty.isActive() );
691694
updateSiblingWidgets( isActive() );
692695
updateGui();
693696
emit changed();

‎src/gui/qgstextformatwidget.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,8 @@ void QgsTextFormatWidget::populateDataDefinedButtons()
813813
registerDataDefinedButton( mCoordAlignmentVDDBtn, QgsPalLayerSettings::Vali );
814814
registerDataDefinedButton( mCoordRotationDDBtn, QgsPalLayerSettings::LabelRotation );
815815

816+
updateDataDefinedAlignment();
817+
816818
// rendering
817819
const QString ddScaleVisInfo = tr( "Value &lt; 0 represents a scale closer than 1:1, e.g. -10 = 10:1<br>"
818820
"Value of 0 disables the specific limit." );
@@ -1862,6 +1864,13 @@ void QgsTextFormatWidget::updateCalloutFrameStatus()
18621864
mCalloutFrame->setEnabled( mCalloutDrawDDBtn->isActive() || mCalloutsDrawCheckBox->isChecked() );
18631865
}
18641866

1867+
void QgsTextFormatWidget::updateDataDefinedAlignment()
1868+
{
1869+
// no data defined alignment without data defined position
1870+
mCoordAlignmentFrame->setEnabled( ( mCoordXDDBtn->isActive() && mCoordYDDBtn->isActive() )
1871+
|| mCoordPointDDBtn->isActive() );
1872+
}
1873+
18651874
void QgsTextFormatWidget::setFormatFromStyle( const QString &name, QgsStyle::StyleEntity type )
18661875
{
18671876
switch ( type )
@@ -2056,13 +2065,6 @@ void QgsTextFormatWidget::showBackgroundRadius( bool show )
20562065
mShapeRadiusUnitsDDBtn->setVisible( show );
20572066
}
20582067

2059-
void QgsTextFormatWidget::updateDataDefinedAlignment()
2060-
{
2061-
// no data defined alignment without data defined position
2062-
mCoordAlignmentFrame->setEnabled( ( mCoordXDDBtn->isActive() && mCoordYDDBtn->isActive() )
2063-
|| mCoordPointDDBtn->isActive() );
2064-
}
2065-
20662068
QgsExpressionContext QgsTextFormatWidget::createExpressionContext() const
20672069
{
20682070
if ( auto *lExpressionContext = mContext.expressionContext() )

0 commit comments

Comments
 (0)
Please sign in to comment.