Skip to content

Commit

Permalink
Avoid unnecessary handling of active layer changed when we are
Browse files Browse the repository at this point in the history
adding many layers at once, or removing many (e.g. due to project
clear)

Saves a lot of unnecessary and potentially expensive processing
  • Loading branch information
nyalldawson committed Aug 17, 2018
1 parent 19a4357 commit 59dbe0e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/app/gps/qgsgpsinformationwidget.cpp
Expand Up @@ -260,7 +260,7 @@ QgsGpsInformationWidget::QgsGpsInformationWidget( QgsMapCanvas *thepCanvas, QWid
//SLM - added functionality
mLogFile = nullptr;

connect( QgisApp::instance()->layerTreeView(), &QgsLayerTreeView::currentLayerChanged,
connect( QgisApp::instance(), &QgisApp::activeLayerChanged,
this, &QgsGpsInformationWidget::updateCloseFeatureButton );

mStackedWidget->setCurrentIndex( 3 ); // force to Options
Expand Down
31 changes: 25 additions & 6 deletions src/app/qgisapp.cpp
Expand Up @@ -567,9 +567,13 @@ void QgisApp::layerTreeViewDoubleClicked( const QModelIndex &index )
}
}

void QgisApp::activeLayerChanged( QgsMapLayer *layer )
void QgisApp::onActiveLayerChanged( QgsMapLayer *layer )
{
Q_FOREACH ( QgsMapCanvas *canvas, mapCanvases() )
if ( mBlockActiveLayerChanged )
return;

const QList< QgsMapCanvas * > canvases = mapCanvases();
for ( QgsMapCanvas *canvas : canvases )
canvas->setCurrentLayer( layer );

if ( mUndoWidget )
Expand All @@ -584,6 +588,8 @@ void QgisApp::activeLayerChanged( QgsMapLayer *layer )
}
updateUndoActions();
}

emit activeLayerChanged( layer );
}

/*
Expand Down Expand Up @@ -1702,6 +1708,10 @@ QVector<QPointer<QgsLayoutCustomDropHandler> > QgisApp::customLayoutDropHandlers

void QgisApp::handleDropUriList( const QgsMimeDataUtils::UriList &lst )
{
// avoid unnecessary work when adding lots of layers at once - defer emitting the active layer changed signal until we've
// added all layers, and only emit the signal once for the final layer added
mBlockActiveLayerChanged = true;

// insert items in reverse order as each one is inserted on top of previous one
for ( int i = lst.size() - 1 ; i >= 0 ; i-- )
{
Expand Down Expand Up @@ -1742,8 +1752,10 @@ void QgisApp::handleDropUriList( const QgsMimeDataUtils::UriList &lst )
openFile( u.uri, QStringLiteral( "project" ) );
}
}
}

mBlockActiveLayerChanged = false;
emit activeLayerChanged( activeLayer() );
}

bool QgisApp::event( QEvent *event )
{
Expand Down Expand Up @@ -3339,9 +3351,9 @@ void QgisApp::setupConnections()
} );

// connect legend signals
connect( mLayerTreeView, &QgsLayerTreeView::currentLayerChanged,
connect( this, &QgisApp::activeLayerChanged,
this, &QgisApp::activateDeactivateLayerRelatedActions );
connect( mLayerTreeView, &QgsLayerTreeView::currentLayerChanged,
connect( this, &QgisApp::activeLayerChanged,
this, &QgisApp::setMapStyleDockLayer );

connect( mLayerTreeView->selectionModel(), &QItemSelectionModel::selectionChanged,
Expand Down Expand Up @@ -3837,7 +3849,7 @@ void QgisApp::initLayerTreeView()
setupLayerTreeViewFromSettings();

connect( mLayerTreeView, &QAbstractItemView::doubleClicked, this, &QgisApp::layerTreeViewDoubleClicked );
connect( mLayerTreeView, &QgsLayerTreeView::currentLayerChanged, this, &QgisApp::activeLayerChanged );
connect( mLayerTreeView, &QgsLayerTreeView::currentLayerChanged, this, &QgisApp::onActiveLayerChanged );
connect( mLayerTreeView->selectionModel(), &QItemSelectionModel::currentChanged, this, &QgisApp::updateNewLayerInsertionPoint );
connect( QgsProject::instance()->layerTreeRegistryBridge(), &QgsLayerTreeRegistryBridge::addedLayersToLayerTree,
this, &QgisApp::autoSelectAddedLayer );
Expand Down Expand Up @@ -11172,7 +11184,14 @@ void QgisApp::closeProject()
mMapCanvas->clearCache();
mOverviewCanvas->setLayers( QList<QgsMapLayer *>() );
mMapCanvas->freeze( false );

// Avoid unnecessary layer changed handling for each layer removed - instead,
// defer the handling until we've removed all layers
mBlockActiveLayerChanged = true;
QgsProject::instance()->clear();
mBlockActiveLayerChanged = false;

emit activeLayerChanged( activeLayer() );
}


Expand Down
12 changes: 11 additions & 1 deletion src/app/qgisapp.h
Expand Up @@ -719,7 +719,7 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
void updateNewLayerInsertionPoint();
//! connected to layer tree registry bridge, selects first of the newly added map layers
void autoSelectAddedLayer( QList<QgsMapLayer *> layers );
void activeLayerChanged( QgsMapLayer *layer );

//! Zoom to full extent
void zoomFull();
//! Zoom to the previous extent
Expand Down Expand Up @@ -1664,6 +1664,8 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
//! handle project crs changes
void projectCrsChanged();

void onActiveLayerChanged( QgsMapLayer *layer );

signals:

/**
Expand Down Expand Up @@ -1737,6 +1739,11 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow
*/
void layerSavedAs( QgsMapLayer *l, const QString &path );

/**
* Emitted when the active layer is changed.
*/
void activeLayerChanged( QgsMapLayer *layer );

private:
void startProfile( const QString &name );
void endProfile();
Expand Down Expand Up @@ -2285,6 +2292,9 @@ class APP_EXPORT QgisApp : public QMainWindow, private Ui::MainWindow

QgsProxyProgressTask *mProjectLoadingProxyTask = nullptr;

//! True if we are blocking the activeLayerChanged signal from being emitted
bool mBlockActiveLayerChanged = false;

friend class TestQgisAppPython;
};

Expand Down
2 changes: 1 addition & 1 deletion src/app/qgisappinterface.cpp
Expand Up @@ -53,7 +53,7 @@ QgisAppInterface::QgisAppInterface( QgisApp *_qgis )
, pluginManagerIface( _qgis->pluginManager() )
{
// connect signals
connect( qgis->layerTreeView(), &QgsLayerTreeView::currentLayerChanged,
connect( qgis, &QgisApp::activeLayerChanged,
this, &QgisInterface::currentLayerChanged );
connect( qgis, &QgisApp::currentThemeChanged,
this, &QgisAppInterface::currentThemeChanged );
Expand Down

1 comment on commit 59dbe0e

@nirvn
Copy link
Contributor

@nirvn nirvn commented on 59dbe0e Aug 20, 2018

Choose a reason for hiding this comment

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

@nyalldawson , I'm pretty sure this caused a regression, whereas most map canvas tools (identify tool, selection tools) aren't aware of layer selection changed when a single layer is added via browser panel.

Steps to reproduce

  • Create a project, add a single vector layer
  • Switch to identify tool, make sure its mode is set to "current layer"
  • Open the browser panel, add a new vector layer (note the new layer is selected)
  • Use the identify tool to try and identify features from the new layer, it won't work, you'll notice it actually still considers the previously selected layer as current layer

Please sign in to comment.