Skip to content

Commit

Permalink
Rework QgsMapToolLabel to provide more fine tuned status results,
Browse files Browse the repository at this point in the history 
and show a descriptive error when interactive labeling cannot
be used because a label rotation is set to an invalid expression

Fixes #47091
  • Loading branch information
@nyalldawson
nyalldawson committed Feb 1, 2022
1 parent 30b81ce commit d2955fe
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 58 deletions.
47 changes: 34 additions & 13 deletions src/app/labeling/qgsmaptoollabel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,9 @@ bool QgsMapToolLabel::hasDataDefinedColumn( QgsPalLayerSettings::DataDefinedProp
}
#endif

QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer ) const
QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer, PropertyStatus &status ) const
{
status = PropertyStatus::DoesNotExist;
if ( !labelSettings.dataDefinedProperties().isActive( p ) )
return QString();

Expand All @@ -559,14 +560,20 @@ QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p,
switch ( property.propertyType() )
{
case QgsProperty::InvalidProperty:
break;

case QgsProperty::StaticProperty:
status = PropertyStatus::Valid;
break;

case QgsProperty::FieldBasedProperty:
status = PropertyStatus::Valid;
return property.field();

case QgsProperty::ExpressionBasedProperty:
{
status = PropertyStatus::Valid;

// an expression based property may still be a effectively a single field reference in the map canvas context.
// e.g. if it is a expression like '"some_field"', or 'case when @some_project_var = 'a' then "field_a" else "field_b" end'

Expand Down Expand Up @@ -608,6 +615,10 @@ QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p,
}

}
else
{
status = PropertyStatus::CurrentExpressionInvalid;
}
break;
}
}
Expand All @@ -617,7 +628,8 @@ QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p,

int QgsMapToolLabel::dataDefinedColumnIndex( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *vlayer ) const
{
QString fieldname = dataDefinedColumnName( p, labelSettings, vlayer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString fieldname = dataDefinedColumnName( p, labelSettings, vlayer, status );
if ( !fieldname.isEmpty() )
return vlayer->fields().lookupField( fieldname );
return -1;
Expand Down Expand Up @@ -698,11 +710,15 @@ bool QgsMapToolLabel::currentLabelDataDefinedPosition( double &x, bool &xSuccess
return true;
}

bool QgsMapToolLabel::labelIsRotatable( QgsVectorLayer *layer, const QgsPalLayerSettings &settings, int &rotationCol ) const
QgsMapToolLabel::PropertyStatus QgsMapToolLabel::labelRotatableStatus( QgsVectorLayer *layer, const QgsPalLayerSettings &settings, int &rotationCol ) const
{
QString rColName = dataDefinedColumnName( QgsPalLayerSettings::LabelRotation, settings, layer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString rColName = dataDefinedColumnName( QgsPalLayerSettings::LabelRotation, settings, layer, status );
if ( status == PropertyStatus::CurrentExpressionInvalid )
return status;

rotationCol = layer->fields().lookupField( rColName );
return rotationCol != -1;
return rotationCol != -1 ? PropertyStatus::Valid : PropertyStatus::DoesNotExist;
}

bool QgsMapToolLabel::currentLabelDataDefinedRotation( double &rotation, bool &rotationSuccess, int &rCol, bool ignoreXY ) const
Expand All @@ -716,7 +732,7 @@ bool QgsMapToolLabel::currentLabelDataDefinedRotation( double &rotation, bool &r
return false;
}

if ( !labelIsRotatable( vlayer, mCurrentLabel.settings, rCol ) )
if ( labelRotatableStatus( vlayer, mCurrentLabel.settings, rCol ) != PropertyStatus::Valid )
{
return false;
}
Expand Down Expand Up @@ -747,16 +763,18 @@ bool QgsMapToolLabel::changeCurrentLabelDataDefinedPosition( const QVariant &x,
{
if ( mCurrentLabel.settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionPoint ) )
{
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, mCurrentLabel.settings, mCurrentLabel.layer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, mCurrentLabel.settings, mCurrentLabel.layer, status );
int pointCol = mCurrentLabel.layer->fields().lookupField( pointColName );

if ( !mCurrentLabel.layer->changeAttributeValue( mCurrentLabel.pos.featureId, pointCol, QVariant::fromValue( QgsReferencedGeometry( QgsGeometry::fromPointXY( QgsPoint( x.toDouble(), y.toDouble() ) ), mCurrentLabel.layer->crs() ) ) ) )
return false;
}
else
{
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, mCurrentLabel.layer );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, mCurrentLabel.layer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, mCurrentLabel.layer, status );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, mCurrentLabel.layer, status );
int xCol = mCurrentLabel.layer->fields().lookupField( xColName );
int yCol = mCurrentLabel.layer->fields().lookupField( yColName );

Expand Down Expand Up @@ -835,10 +853,11 @@ bool QgsMapToolLabel::labelCanShowHide( QgsVectorLayer *vlayer, int &showCol ) c
}

const auto constSubProviders = vlayer->labeling()->subProviders();
PropertyStatus status = PropertyStatus::DoesNotExist;
for ( const QString &providerId : constSubProviders )
{
QString fieldname = dataDefinedColumnName( QgsPalLayerSettings::Show,
vlayer->labeling()->settings( providerId ), vlayer );
vlayer->labeling()->settings( providerId ), vlayer, status );
showCol = vlayer->fields().lookupField( fieldname );
if ( showCol != -1 )
return true;
Expand Down Expand Up @@ -878,7 +897,8 @@ bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, const QgsPalLayerSe

if ( settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionPoint ) )
{
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, settings, vlayer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString pointColName = dataDefinedColumnName( QgsPalLayerSettings::PositionPoint, settings, vlayer, status );
pointCol = vlayer->fields().lookupField( pointColName );
if ( pointCol >= 0 )
return true;
Expand All @@ -887,8 +907,9 @@ bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, const QgsPalLayerSe
if ( settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionX )
&& settings.dataDefinedProperties().isActive( QgsPalLayerSettings::PositionY ) )
{
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, settings, vlayer );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, settings, vlayer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, settings, vlayer, status );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, settings, vlayer, status );
xCol = vlayer->fields().lookupField( xColName );
yCol = vlayer->fields().lookupField( yColName );
if ( xCol >= 0 || yCol >= 0 )
Expand Down
11 changes: 9 additions & 2 deletions src/app/labeling/qgsmaptoollabel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,18 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapToolAdvancedDigitizing
*/
bool labelCanShowHide( QgsVectorLayer *vlayer, int &showCol ) const;

enum class PropertyStatus
{
Valid,
DoesNotExist,
CurrentExpressionInvalid
};

/**
* Checks if labels in a layer can be rotated
* \param rotationCol out: attribute column for data defined label rotation
*/
bool labelIsRotatable( QgsVectorLayer *layer, const QgsPalLayerSettings &settings, int &rotationCol ) const;
PropertyStatus labelRotatableStatus( QgsVectorLayer *layer, const QgsPalLayerSettings &settings, int &rotationCol ) const;

protected:
QgsRubberBand *mHoverRubberBand = nullptr;
Expand Down Expand Up @@ -146,7 +153,7 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapToolAdvancedDigitizing
QFont currentLabelFont();

//! Returns a data defined attribute column name for particular property or empty string if not defined
QString dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer ) const;
QString dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer, PropertyStatus &status ) const;

/**
* Returns a data defined attribute column index
Expand Down
5 changes: 3 additions & 2 deletions src/app/labeling/qgsmaptoolmovelabel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,9 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )

if ( !labelMoveable( vlayer, mCurrentLabel.settings, xCol, yCol, pointCol ) )
{
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, vlayer );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, vlayer );
PropertyStatus status = PropertyStatus::DoesNotExist;
QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, mCurrentLabel.settings, vlayer, status );
QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, mCurrentLabel.settings, vlayer, status );
if ( xCol < 0 && yCol < 0 )
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 ) );
else if ( xCol < 0 )
Expand Down
53 changes: 33 additions & 20 deletions src/app/labeling/qgsmaptoolrotatelabel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,29 +114,42 @@ void QgsMapToolRotateLabel::canvasPressEvent( QgsMapMouseEvent *e )
bool hasRotationValue;
int rotationCol;

if ( !labelIsRotatable( mCurrentLabel.layer, mCurrentLabel.settings, rotationCol ) )
const PropertyStatus status = labelRotatableStatus( mCurrentLabel.layer, mCurrentLabel.settings, rotationCol );
switch ( status )
{
QgsPalIndexes indexes;
if ( createAuxiliaryFields( indexes ) )
return;
case PropertyStatus::DoesNotExist:
{
QgsPalIndexes indexes;
if ( createAuxiliaryFields( indexes ) )
return;

if ( !labelIsRotatable( mCurrentLabel.layer, mCurrentLabel.settings, rotationCol ) )
return;
}
else
{
const bool usesAuxField = mCurrentLabel.layer->fields().fieldOrigin( rotationCol ) == QgsFields::OriginJoin;
if ( !usesAuxField && !mCurrentLabel.layer->isEditable() )
if ( labelRotatableStatus( mCurrentLabel.layer, mCurrentLabel.settings, rotationCol ) != PropertyStatus::Valid )
return;
break;
}

case PropertyStatus::Valid:
{
if ( mCurrentLabel.layer->startEditing() )
const bool usesAuxField = mCurrentLabel.layer->fields().fieldOrigin( rotationCol ) == QgsFields::OriginJoin;
if ( !usesAuxField && !mCurrentLabel.layer->isEditable() )
{
QgisApp::instance()->messageBar()->pushInfo( tr( "Rotate Label" ), tr( "Layer “%1” was made editable" ).arg( mCurrentLabel.layer->name() ) );
}
else
{
QgisApp::instance()->messageBar()->pushWarning( tr( "Rotate Label" ), tr( "Cannot rotate “%1” — the layer “%2” could not be made editable" ).arg( mCurrentLabel.pos.labelText, mCurrentLabel.layer->name() ) );
return;
if ( mCurrentLabel.layer->startEditing() )
{
QgisApp::instance()->messageBar()->pushInfo( tr( "Rotate Label" ), tr( "Layer “%1” was made editable" ).arg( mCurrentLabel.layer->name() ) );
}
else
{
QgisApp::instance()->messageBar()->pushWarning( tr( "Rotate Label" ), tr( "Cannot rotate “%1” — the layer “%2” could not be made editable" ).arg( mCurrentLabel.pos.labelText, mCurrentLabel.layer->name() ) );
return;
}
}
break;
}

case PropertyStatus::CurrentExpressionInvalid:
{
QgisApp::instance()->messageBar()->pushWarning( tr( "Rotate Label" ), tr( "Cannot rotate “%1” — the layer “%2” has an invalid expression set for label rotation" ).arg( mCurrentLabel.pos.labelText, mCurrentLabel.layer->name() ) );
return;
}
}

Expand Down Expand Up @@ -194,7 +207,7 @@ void QgsMapToolRotateLabel::canvasPressEvent( QgsMapMouseEvent *e )
}

int rotationCol;
if ( !labelIsRotatable( vlayer, mCurrentLabel.settings, rotationCol ) )
if ( labelRotatableStatus( vlayer, mCurrentLabel.settings, rotationCol ) != PropertyStatus::Valid )
{
return;
}
Expand Down Expand Up @@ -245,7 +258,7 @@ void QgsMapToolRotateLabel::keyReleaseEvent( QKeyEvent *e )
if ( vlayer )
{
int rotationCol;
if ( labelIsRotatable( vlayer, mCurrentLabel.settings, rotationCol ) )
if ( labelRotatableStatus( vlayer, mCurrentLabel.settings, rotationCol ) == PropertyStatus::Valid )
{
vlayer->beginEditCommand( tr( "Delete Label Rotation" ) + QStringLiteral( " '%1'" ).arg( currentLabelText( 24 ) ) );
if ( !vlayer->changeAttributeValue( mCurrentLabel.pos.featureId, rotationCol, QVariant() ) )
Expand Down

0 comments on commit d2955fe

Please sign in to comment.