Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure labeling map tools only try to affect labels from vector layers #45311

Merged
merged 3 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions python/gui/auto_generated/qgsmapcanvas.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,18 @@ Emits signal scaleChanged to update scale in main window
QgsMapLayer *layer( int index );
%Docstring
Returns the map layer at position index in the layer stack
%End

QgsMapLayer *layer( const QString &id );
%Docstring
Returns the map layer with the matching ID, or ``None`` if no layers could be found.

This method searches both layers associated with the map canvas (see :py:func:`~QgsMapCanvas.layers`)
and layers from the :py:class:`QgsProject` associated with the canvas
(which is current the :py:func:`QgsProject.instance()`). It can be used to resolve layer IDs to
layers which may be visible in the canvas, but not associated with a :py:class:`QgsProject`.

.. versionadded:: 3.22
%End

int layerCount() const;
Expand Down
12 changes: 12 additions & 0 deletions python/gui/auto_generated/qgsmaptool.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,17 @@ Transforms a ``rect`` from map coordinates to ``layer`` coordinates.
QPoint toCanvasCoordinates( const QgsPointXY &point ) const;
%Docstring
Transforms a ``point`` from map coordinates to screen coordinates.
%End

QgsMapLayer *layer( const QString &id );
%Docstring
Returns the map layer with the matching ID, or ``None`` if no layers could be found.

This method searches both layers associated with the map canvas (see :py:func:`QgsMapCanvas.layers()`)
and layers from the :py:class:`QgsProject` associated with the canvas. It can be used to resolve layer IDs to
layers which may be visible in the canvas, but not associated with a :py:class:`QgsProject`.

.. versionadded:: 3.22
%End

void setToolName( const QString &name );
Expand All @@ -338,6 +349,7 @@ Sets the tool's ``name``.




};

QFlags<QgsMapTool::Flag> operator|(QgsMapTool::Flag f1, QFlags<QgsMapTool::Flag> f2);
Expand Down
2 changes: 1 addition & 1 deletion src/app/labeling/qgslabelpropertydialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void QgsLabelPropertyDialog::buttonBox_clicked( QAbstractButton *button )
void QgsLabelPropertyDialog::init( const QString &layerId, const QString &providerId, QgsFeatureId featureId, const QString &labelText )
{
//get feature attributes
QgsVectorLayer *vlayer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( layerId );
QgsVectorLayer *vlayer = qobject_cast< QgsVectorLayer * >( mCanvas->layer( layerId ) );
if ( !vlayer )
{
return;
Expand Down
2 changes: 1 addition & 1 deletion src/app/labeling/qgsmaptoolchangelabelproperties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void QgsMapToolChangeLabelProperties::canvasPressEvent( QgsMapMouseEvent *e )
return;
}

mCurrentLabel = LabelDetails( labelPos );
mCurrentLabel = LabelDetails( labelPos, canvas() );
if ( !mCurrentLabel.valid || !mCurrentLabel.layer )
{
return;
Expand Down
61 changes: 57 additions & 4 deletions src/app/labeling/qgsmaptoollabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,33 @@ bool QgsMapToolLabel::labelAtPosition( QMouseEvent *e, QgsLabelPosition &p )
return false;

QList<QgsLabelPosition> labelPosList = labelingResults->labelsAtPosition( pt );
labelPosList.erase( std::remove_if( labelPosList.begin(), labelPosList.end(), [this]( const QgsLabelPosition & position )
{
if ( position.layerID.isEmpty() )
return true;

if ( QgsMapLayer *layer = QgsMapTool::layer( position.layerID ) )
{
// strip out any labels from non vector layers (e.g. those from vector tile layers). Only vector layer labels
// are supported by the map tools.
switch ( layer->type() )
{
case QgsMapLayerType::VectorLayer:
return false;

case QgsMapLayerType::RasterLayer:
case QgsMapLayerType::PluginLayer:
case QgsMapLayerType::MeshLayer:
case QgsMapLayerType::VectorTileLayer:
case QgsMapLayerType::AnnotationLayer:
case QgsMapLayerType::PointCloudLayer:
return true;
}
}

return true;
} ), labelPosList.end() );

if ( labelPosList.empty() )
return false;

Expand Down Expand Up @@ -131,6 +158,32 @@ bool QgsMapToolLabel::calloutAtPosition( QMouseEvent *e, QgsCalloutPosition &p,
const double tol = QgsTolerance::vertexSearchRadius( canvas()->mapSettings() );

QList<QgsCalloutPosition> calloutPosList = labelingResults->calloutsWithinRectangle( QgsRectangle::fromCenterAndSize( pt, tol * 2, tol * 2 ) );
calloutPosList.erase( std::remove_if( calloutPosList.begin(), calloutPosList.end(), [ this ]( const QgsCalloutPosition & position )
{
if ( position.layerID.isEmpty() )
return true;

if ( QgsMapLayer *layer = QgsMapTool::layer( position.layerID ) )
{
// strip out any callouts from non vector layers (e.g. those from vector tile layers). Only vector layer callouts
// are supported by the map tools.
switch ( layer->type() )
{
case QgsMapLayerType::VectorLayer:
return false;

case QgsMapLayerType::RasterLayer:
case QgsMapLayerType::PluginLayer:
case QgsMapLayerType::MeshLayer:
case QgsMapLayerType::VectorTileLayer:
case QgsMapLayerType::AnnotationLayer:
case QgsMapLayerType::PointCloudLayer:
return true;
}
}

return true;
} ), calloutPosList.end() );
if ( calloutPosList.empty() )
return false;

Expand Down Expand Up @@ -827,10 +880,10 @@ bool QgsMapToolLabel::diagramCanShowHide( QgsVectorLayer *vlayer, int &showCol )

//

QgsMapToolLabel::LabelDetails::LabelDetails( const QgsLabelPosition &p )
QgsMapToolLabel::LabelDetails::LabelDetails( const QgsLabelPosition &p, QgsMapCanvas *canvas )
: pos( p )
{
layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( pos.layerID );
layer = qobject_cast< QgsVectorLayer * >( canvas->layer( pos.layerID ) );
if ( layer && layer->labelsEnabled() && !p.isDiagram )
{
settings = layer->labeling()->settings( pos.providerID );
Expand Down Expand Up @@ -957,7 +1010,7 @@ bool QgsMapToolLabel::createAuxiliaryFields( QgsCalloutIndexes &calloutIndexes )
bool QgsMapToolLabel::createAuxiliaryFields( QgsCalloutPosition &details, QgsCalloutIndexes &calloutIndexes )
{
bool newAuxiliaryLayer = false;
QgsVectorLayer *vlayer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( details.layerID );
QgsVectorLayer *vlayer = qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( details.layerID ) );

if ( !vlayer )
return newAuxiliaryLayer;
Expand Down Expand Up @@ -1059,7 +1112,7 @@ void QgsMapToolLabel::updateHoveredLabel( QgsMapMouseEvent *e )
return;
}

LabelDetails newHoverLabel( labelPos );
LabelDetails newHoverLabel( labelPos, canvas() );

if ( mCurrentHoverLabel.valid &&
newHoverLabel.layer == mCurrentHoverLabel.layer &&
Expand Down
2 changes: 1 addition & 1 deletion src/app/labeling/qgsmaptoollabel.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapToolAdvancedDigitizing
struct APP_EXPORT LabelDetails
{
LabelDetails() = default;
explicit LabelDetails( const QgsLabelPosition &p );
explicit LabelDetails( const QgsLabelPosition &p, QgsMapCanvas *canvas );
bool valid = false;
QgsLabelPosition pos;
QgsVectorLayer *layer = nullptr;
Expand Down
12 changes: 6 additions & 6 deletions src/app/labeling/qgsmaptoolmovelabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )

clearHoveredLabel();

QgsVectorLayer *vlayer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( mCurrentCallout.layerID );
QgsVectorLayer *vlayer = qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( mCurrentCallout.layerID ) );
if ( !vlayer || xCol < 0 || yCol < 0 )
{
return;
Expand Down Expand Up @@ -174,7 +174,7 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )

clearHoveredLabel();

mCurrentLabel = LabelDetails( labelPos );
mCurrentLabel = LabelDetails( labelPos, canvas() );

QgsVectorLayer *vlayer = mCurrentLabel.layer;
if ( !vlayer )
Expand Down Expand Up @@ -275,7 +275,7 @@ void QgsMapToolMoveLabel::cadCanvasPressEvent( QgsMapMouseEvent *e )

deleteRubberBands();

QgsVectorLayer *vlayer = !isCalloutMove ? mCurrentLabel.layer : QgsProject::instance()->mapLayer<QgsVectorLayer *>( mCurrentCallout.layerID );
QgsVectorLayer *vlayer = !isCalloutMove ? mCurrentLabel.layer : qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( mCurrentCallout.layerID ) );
if ( !vlayer )
{
return;
Expand Down Expand Up @@ -452,7 +452,7 @@ void QgsMapToolMoveLabel::keyReleaseEvent( QKeyEvent *e )

// delete the stored label/callout position
const bool isCalloutMove = !mCurrentCallout.layerID.isEmpty();
QgsVectorLayer *vlayer = !isCalloutMove ? mCurrentLabel.layer : QgsProject::instance()->mapLayer<QgsVectorLayer *>( mCurrentCallout.layerID );
QgsVectorLayer *vlayer = !isCalloutMove ? mCurrentLabel.layer : qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( mCurrentCallout.layerID ) );
const QgsFeatureId featureId = !isCalloutMove ? mCurrentLabel.pos.featureId : mCurrentCallout.featureId;
if ( vlayer )
{
Expand Down Expand Up @@ -514,7 +514,7 @@ void QgsMapToolMoveLabel::keyReleaseEvent( QKeyEvent *e )

bool QgsMapToolMoveLabel::canModifyCallout( const QgsCalloutPosition &pos, bool isOrigin, int &xCol, int &yCol )
{
QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( pos.layerID );
QgsVectorLayer *layer = qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( pos.layerID ) );
QgsPalLayerSettings settings;
if ( layer && layer->labelsEnabled() )
{
Expand Down Expand Up @@ -558,7 +558,7 @@ bool QgsMapToolMoveLabel::canModifyCallout( const QgsCalloutPosition &pos, bool

bool QgsMapToolMoveLabel::currentCalloutDataDefinedPosition( double &x, bool &xSuccess, double &y, bool &ySuccess, int &xCol, int &yCol )
{
QgsVectorLayer *vlayer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( mCurrentCallout.layerID );
QgsVectorLayer *vlayer = qobject_cast< QgsVectorLayer * >( QgsMapTool::layer( mCurrentCallout.layerID ) );
const QgsFeatureId featureId = mCurrentCallout.featureId;

xSuccess = false;
Expand Down
6 changes: 3 additions & 3 deletions src/app/labeling/qgsmaptoolpinlabels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ void QgsMapToolPinLabels::highlightPinnedLabels()
QList<QgsLabelPosition>::const_iterator it;
for ( const QgsLabelPosition &pos : labelPosList )
{
mCurrentLabel = LabelDetails( pos );
mCurrentLabel = LabelDetails( pos, canvas() );

if ( isPinned() )
{
Expand All @@ -209,7 +209,7 @@ void QgsMapToolPinLabels::highlightPinnedLabels()
}

QColor lblcolor = QColor( 54, 129, 255, 63 );
QgsMapLayer *layer = QgsProject::instance()->mapLayer( pos.layerID );
QgsMapLayer *layer = QgsMapTool::layer( pos.layerID );
if ( !layer )
{
continue;
Expand Down Expand Up @@ -287,7 +287,7 @@ void QgsMapToolPinLabels::pinUnpinLabels( const QgsRectangle &ext, QMouseEvent *
{
const QgsLabelPosition &pos = *it;

mCurrentLabel = LabelDetails( pos );
mCurrentLabel = LabelDetails( pos, canvas() );

if ( !mCurrentLabel.valid )
{
Expand Down
2 changes: 1 addition & 1 deletion src/app/labeling/qgsmaptoolrotatelabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void QgsMapToolRotateLabel::canvasPressEvent( QgsMapMouseEvent *e )
return;
}

mCurrentLabel = LabelDetails( labelPos );
mCurrentLabel = LabelDetails( labelPos, canvas() );

if ( !mCurrentLabel.valid )
return;
Expand Down
2 changes: 1 addition & 1 deletion src/app/labeling/qgsmaptoolshowhidelabels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ bool QgsMapToolShowHideLabels::selectedLabelFeatures( QgsVectorLayer *vlayer,

bool QgsMapToolShowHideLabels::showHide( const QgsLabelPosition &pos, bool show )
{
LabelDetails details = LabelDetails( pos );
LabelDetails details = LabelDetails( pos, canvas() );

if ( !details.valid )
return false;
Expand Down
15 changes: 15 additions & 0 deletions src/gui/qgsmapcanvas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,21 @@ QgsMapLayer *QgsMapCanvas::layer( int index )
return nullptr;
}

QgsMapLayer *QgsMapCanvas::layer( const QString &id )
{
// first check for layers from canvas map settings
const QList<QgsMapLayer *> layers = mapSettings().layers();
for ( QgsMapLayer *layer : layers )
{
if ( layer && layer->id() == id )
return layer;
}

// else fallback to searching project layers
// TODO: allow a specific project to be associated with a canvas!
return QgsProject::instance()->mapLayer( id );
}

void QgsMapCanvas::setCurrentLayer( QgsMapLayer *layer )
{
if ( mCurrentLayer == layer )
Expand Down
12 changes: 12 additions & 0 deletions src/gui/qgsmapcanvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,18 @@ class GUI_EXPORT QgsMapCanvas : public QGraphicsView, public QgsExpressionContex
//! Returns the map layer at position index in the layer stack
QgsMapLayer *layer( int index );

/**
* Returns the map layer with the matching ID, or NULLPTR if no layers could be found.
*
* This method searches both layers associated with the map canvas (see layers())
* and layers from the QgsProject associated with the canvas
* (which is current the QgsProject::instance()). It can be used to resolve layer IDs to
* layers which may be visible in the canvas, but not associated with a QgsProject.
*
* \since QGIS 3.22
*/
QgsMapLayer *layer( const QString &id );

//! Returns number of layers on the map
int layerCount() const;

Expand Down
5 changes: 5 additions & 0 deletions src/gui/qgsmaptool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ QPoint QgsMapTool::toCanvasCoordinates( const QgsPointXY &point ) const
return QPoint( std::round( x ), std::round( y ) );
}

QgsMapLayer *QgsMapTool::layer( const QString &id )
{
return mCanvas->layer( id );
}

void QgsMapTool::setToolName( const QString &name )
{
mToolName = name;
Expand Down
13 changes: 13 additions & 0 deletions src/gui/qgsmaptool.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,17 @@ class GUI_EXPORT QgsMapTool : public QObject
//! Transforms a \a point from map coordinates to screen coordinates.
QPoint toCanvasCoordinates( const QgsPointXY &point ) const;

/**
* Returns the map layer with the matching ID, or NULLPTR if no layers could be found.
*
* This method searches both layers associated with the map canvas (see QgsMapCanvas::layers())
* and layers from the QgsProject associated with the canvas. It can be used to resolve layer IDs to
* layers which may be visible in the canvas, but not associated with a QgsProject.
*
* \since QGIS 3.22
*/
QgsMapLayer *layer( const QString &id );

/**
* Sets the tool's \a name.
*
Expand Down Expand Up @@ -327,6 +338,8 @@ class GUI_EXPORT QgsMapTool : public QObject
//! The translated name of the map tool
QString mToolName;

friend class TestQgsMapToolEdit;

};

Q_DECLARE_OPERATORS_FOR_FLAGS( QgsMapTool::Flags )
Expand Down
10 changes: 5 additions & 5 deletions tests/src/app/testqgsmaptoollabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class TestQgsMapToolLabel : public QObject
QVERIFY( tool->labelAtPosition( event.get(), pos ) );
QCOMPARE( pos.layerID, vl1->id() );
QCOMPARE( pos.labelText, QStringLiteral( "label" ) );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos, canvas.get() );

// defaults to bottom left
QString hali, vali;
Expand All @@ -315,7 +315,7 @@ class TestQgsMapToolLabel : public QObject
QVERIFY( tool->labelAtPosition( event.get(), pos ) );
QCOMPARE( pos.layerID, vl1->id() );
QCOMPARE( pos.labelText, QStringLiteral( "label" ) );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos, canvas.get() );

tool->currentAlignment( hali, vali );
QCOMPARE( hali, QStringLiteral( "right" ) );
Expand All @@ -330,7 +330,7 @@ class TestQgsMapToolLabel : public QObject
QVERIFY( tool->labelAtPosition( event.get(), pos ) );
QCOMPARE( pos.layerID, vl1->id() );
QCOMPARE( pos.labelText, QStringLiteral( "label" ) );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos, canvas.get() );

tool->currentAlignment( hali, vali );
QCOMPARE( hali, QStringLiteral( "center" ) );
Expand All @@ -354,7 +354,7 @@ class TestQgsMapToolLabel : public QObject
QVERIFY( tool->labelAtPosition( event.get(), pos ) );
QCOMPARE( pos.layerID, vl1->id() );
QCOMPARE( pos.labelText, QStringLiteral( "label" ) );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos, canvas.get() );

tool->currentAlignment( hali, vali );
QCOMPARE( hali, QStringLiteral( "left" ) );
Expand All @@ -369,7 +369,7 @@ class TestQgsMapToolLabel : public QObject
QVERIFY( tool->labelAtPosition( event.get(), pos ) );
QCOMPARE( pos.layerID, vl1->id() );
QCOMPARE( pos.labelText, QStringLiteral( "label" ) );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos );
tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos, canvas.get() );

tool->currentAlignment( hali, vali );
QCOMPARE( hali, QStringLiteral( "right" ) );
Expand Down
Loading