Skip to content

Commit

Permalink
The nodetool only needs to consider spatial layers
Browse files Browse the repository at this point in the history
  • Loading branch information
@m-kuhn
m-kuhn committed Jun 16, 2017
1 parent f237c6c commit a77f791
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/app/nodetool/qgsnodetool.cpp
View file
Expand Up @@ -421,7 +421,7 @@ void QgsNodeTool::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
Q_FOREACH ( QgsMapLayer *layer, canvas()->layers() )
{
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
if ( !vlayer || !vlayer->isEditable() )
if ( !vlayer || !vlayer->isEditable() || !vlayer->isSpatial() )
continue;

QgsRectangle layerRect = toLayerCoordinates( vlayer, map_rect );
Expand Down

4 comments on commit a77f791

@m-kuhn
Copy link
Member Author

@m-kuhn m-kuhn commented on a77f791 Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wonder-sk this fixes a crash for me when selecting nodes on the canvas and having non-spatial (postgres) layers in the project.
I wonder why they even return features on line 429 a77f791#diff-cd44035d76fa6e1430345db6a8a0d57aL429 at all. Opinions?

@wonder-sk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @m-kuhn - thanks for the fix!

Looking into QgsPostgresFeatureIterator constructor it seems that filter rect is simply ignored when the layer does not have geometries. Not sure how consistent is it with other providers, but ideally that should return closed iterator as such request does not make much sense for non-spatial layer...

@m-kuhn
Copy link
Member Author

@m-kuhn m-kuhn commented on a77f791 Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how consistent is it with other providers, but ideally that should return closed iterator as such request does not make much sense for non-spatial layer...

Yes, that's what I would have expected as well. @jef-n @nyalldawson any further considerations from your side?

@3nids
Copy link
Member

@3nids 3nids commented on a77f791 Jun 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for a closed iterator

Please sign in to comment.