Skip to content

Commit

Permalink
Flip task manager to use weak layer pointers instead of layer ids
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Feb 12, 2017
1 parent c42f1ec commit 35bb5c6
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 59 deletions.
28 changes: 4 additions & 24 deletions python/core/qgstaskmanager.sip
View file
Expand Up @@ -126,19 +126,9 @@ class QgsTask : QObject
void addSubTask( QgsTask* subTask /Transfer/, const QgsTaskList& dependencies = QgsTaskList(),
SubTaskDependency subTaskDependency = SubTaskIndependent );

/**
* Sets a list of layer IDs on which the task depends. The task will automatically
* be canceled if any of these layers are about to be removed.
* @see dependentLayerIds()
*/
void setDependentLayers( const QStringList& dependentLayerIds );
void setDependentLayers( const QList<QgsMapLayer*>& dependentLayers );

/**
* Returns the list of layer IDs on which the task depends. The task will automatically
* be canceled if any of these layers are about to be removed.
* @see setDependentLayers()
*/
QStringList dependentLayerIds() const;
QList< QgsMapLayer* > dependentLayers() const;

signals:

Expand Down Expand Up @@ -317,19 +307,9 @@ class QgsTaskManager : QObject
//! @note not available in Python bindings
//QSet< long > dependencies( long taskId ) const;

/** Returns a list of layers on which as task is dependent. The task will automatically
* be canceled if any of these layers are above to be removed.
* @param taskId task ID
* @returns list of layer IDs
* @see tasksDependentOnLayer()
*/
QStringList dependentLayers( long taskId ) const;
QList< QgsMapLayer* > dependentLayers( long taskId ) const;

/**
* Returns a list of tasks which depend on a layer.
* @see dependentLayers()
*/
QList< QgsTask* > tasksDependentOnLayer( const QString& layerId ) const;
QList< QgsTask* > tasksDependentOnLayer( QgsMapLayer* layer ) const;

/** Returns a list of the active (queued or running) tasks.
* @see countActiveTasks()
Expand Down
4 changes: 2 additions & 2 deletions src/app/qgisapp.cpp
View file
Expand Up @@ -8444,7 +8444,7 @@ void QgisApp::removeLayer()
QStringList activeTaskDescriptions;
Q_FOREACH ( QgsMapLayer * layer, mLayerTreeView->selectedLayers() )
{
QList< QgsTask* > tasks = QgsApplication::taskManager()->tasksDependentOnLayer( layer->id() );
QList< QgsTask* > tasks = QgsApplication::taskManager()->tasksDependentOnLayer( layer );
if ( !tasks.isEmpty() )
{
Q_FOREACH ( QgsTask* task, tasks )
Expand Down Expand Up @@ -9649,7 +9649,7 @@ bool QgisApp::checkTasksDependOnProject()

for ( ; layerIt != layers.constEnd(); ++layerIt )
{
QList< QgsTask* > tasks = QgsApplication::taskManager()->tasksDependentOnLayer( layerIt.key() );
QList< QgsTask* > tasks = QgsApplication::taskManager()->tasksDependentOnLayer( layerIt.value() );
if ( !tasks.isEmpty() )
{
Q_FOREACH ( QgsTask* task, tasks )
Expand Down
38 changes: 22 additions & 16 deletions src/core/qgstaskmanager.cpp
View file
Expand Up @@ -17,6 +17,7 @@

#include "qgstaskmanager.h"
#include "qgsproject.h"
#include "qgsmaplayerlistutils.h"
#include <QtConcurrentRun>


Expand Down Expand Up @@ -136,9 +137,14 @@ void QgsTask::addSubTask( QgsTask* subTask, const QgsTaskList& dependencies,
connect( subTask, &QgsTask::statusChanged, this, &QgsTask::subTaskStatusChanged );
}

void QgsTask::setDependentLayers( const QStringList& dependentLayerIds )
QList<QgsMapLayer*> QgsTask::dependentLayers() const
{
mDependentLayerIds = dependentLayerIds;
return _qgis_listQPointerToRaw( mDependentLayers );
}

void QgsTask::setDependentLayers( const QList< QgsMapLayer* >& dependentLayers )
{
mDependentLayers = _qgis_listRawToQPointer( dependentLayers );
}

void QgsTask::subTaskStatusChanged( int status )
Expand Down Expand Up @@ -325,8 +331,8 @@ QgsTaskManager::QgsTaskManager( QObject* parent )
, mTaskMutex( new QMutex( QMutex::Recursive ) )
, mNextTaskId( 0 )
{
connect( QgsProject::instance(), SIGNAL( layersWillBeRemoved( QStringList ) ),
this, SLOT( layersWillBeRemoved( QStringList ) ) );
connect( QgsProject::instance(), static_cast < void ( QgsProject::* )( const QList< QgsMapLayer* >& ) > ( &QgsProject::layersWillBeRemoved ),
this, &QgsTaskManager::layersWillBeRemoved );
}

QgsTaskManager::~QgsTaskManager()
Expand Down Expand Up @@ -376,8 +382,8 @@ long QgsTaskManager::addTaskPrivate( QgsTask* task, QgsTaskList dependencies, bo
{
mParentTasks << task;
}
if ( !task->dependentLayerIds().isEmpty() )
mLayerDependencies.insert( taskId, task->dependentLayerIds() );
if ( !task->dependentLayers().isEmpty() )
mLayerDependencies.insert( taskId, _qgis_listRawToQPointer( task->dependentLayers() ) );
mTaskMutex->unlock();

connect( task, &QgsTask::statusChanged, this, &QgsTaskManager::taskStatusChanged );
Expand Down Expand Up @@ -546,20 +552,20 @@ bool QgsTaskManager::hasCircularDependencies( long taskId ) const
return !resolveDependencies( taskId, taskId, d );
}

QStringList QgsTaskManager::dependentLayers( long taskId ) const
QList<QgsMapLayer*> QgsTaskManager::dependentLayers( long taskId ) const
{
QMutexLocker ml( mTaskMutex );
return mLayerDependencies.value( taskId, QStringList() );
return _qgis_listQPointerToRaw( mLayerDependencies.value( taskId, QgsWeakMapLayerPointerList() ) );
}

QList<QgsTask*> QgsTaskManager::tasksDependentOnLayer( const QString& layerId ) const
QList<QgsTask*> QgsTaskManager::tasksDependentOnLayer( QgsMapLayer* layer ) const
{
QMutexLocker ml( mTaskMutex );
QList< QgsTask* > tasks;
QMap< long, QStringList >::const_iterator layerIt = mLayerDependencies.constBegin();
QMap< long, QgsWeakMapLayerPointerList >::const_iterator layerIt = mLayerDependencies.constBegin();
for ( ; layerIt != mLayerDependencies.constEnd(); ++layerIt )
{
if ( layerIt.value().contains( layerId ) )
if ( _qgis_listQPointerToRaw( layerIt.value() ).contains( layer ) )
{
QgsTask* layerTask = task( layerIt.key() );
if ( layerTask )
Expand Down Expand Up @@ -648,21 +654,21 @@ void QgsTaskManager::taskStatusChanged( int status )

}

void QgsTaskManager::layersWillBeRemoved( const QStringList& layerIds )
void QgsTaskManager::layersWillBeRemoved( const QList< QgsMapLayer* >& layers )
{
mTaskMutex->lock();
// scan through layers to be removed
QMap< long, QStringList > layerDependencies = mLayerDependencies;
QMap< long, QgsWeakMapLayerPointerList > layerDependencies = mLayerDependencies;
layerDependencies.detach();
mTaskMutex->unlock();

Q_FOREACH ( const QString& layerId, layerIds )
Q_FOREACH ( QgsMapLayer* layer, layers )
{
// scan through tasks with layer dependencies
for ( QMap< long, QStringList >::const_iterator it = layerDependencies.constBegin();
for ( QMap< long, QgsWeakMapLayerPointerList >::const_iterator it = layerDependencies.constBegin();
it != layerDependencies.constEnd(); ++it )
{
if ( !it.value().contains( layerId ) )
if ( !( _qgis_listQPointerToRaw( it.value() ).contains( layer ) ) )
{
//task not dependent on this layer
continue;
Expand Down
21 changes: 11 additions & 10 deletions src/core/qgstaskmanager.h
View file
Expand Up @@ -24,6 +24,7 @@
#include <QReadWriteLock>

#include "qgis_core.h"
#include "qgsmaplayer.h"

class QgsTask;
class QgsTaskRunnableWrapper;
Expand Down Expand Up @@ -170,18 +171,18 @@ class CORE_EXPORT QgsTask : public QObject
SubTaskDependency subTaskDependency = SubTaskIndependent );

/**
* Sets a list of layer IDs on which the task depends. The task will automatically
* Sets a list of layers on which the task depends. The task will automatically
* be canceled if any of these layers are about to be removed.
* @see dependentLayerIds()
*/
void setDependentLayers( const QStringList& dependentLayerIds );
void setDependentLayers( const QList<QgsMapLayer*>& dependentLayers );

/**
* Returns the list of layer IDs on which the task depends. The task will automatically
* Returns the list of layers on which the task depends. The task will automatically
* be canceled if any of these layers are about to be removed.
* @see setDependentLayers()
*/
QStringList dependentLayerIds() const { return mDependentLayerIds; }
QList< QgsMapLayer* > dependentLayers() const;

signals:

Expand Down Expand Up @@ -296,7 +297,7 @@ class CORE_EXPORT QgsTask : public QObject
};
QList< SubTask > mSubTasks;

QStringList mDependentLayerIds;
QgsWeakMapLayerPointerList mDependentLayers;


/**
Expand Down Expand Up @@ -430,16 +431,16 @@ class CORE_EXPORT QgsTaskManager : public QObject
/** Returns a list of layers on which as task is dependent. The task will automatically
* be canceled if any of these layers are above to be removed.
* @param taskId task ID
* @returns list of layer IDs
* @returns list of layers
* @see tasksDependentOnLayer()
*/
QStringList dependentLayers( long taskId ) const;
QList< QgsMapLayer* > dependentLayers( long taskId ) const;

/**
* Returns a list of tasks which depend on a layer.
* @see dependentLayers()
*/
QList< QgsTask* > tasksDependentOnLayer( const QString& layerId ) const;
QList< QgsTask* > tasksDependentOnLayer( QgsMapLayer* layer ) const;

/** Returns a list of the active (queued or running) tasks.
* @see countActiveTasks()
Expand Down Expand Up @@ -489,7 +490,7 @@ class CORE_EXPORT QgsTaskManager : public QObject

void taskProgressChanged( double progress );
void taskStatusChanged( int status );
void layersWillBeRemoved( const QStringList& layerIds );
void layersWillBeRemoved( const QList<QgsMapLayer*>& layers );

private:

Expand All @@ -506,7 +507,7 @@ class CORE_EXPORT QgsTaskManager : public QObject

QMap< long, TaskInfo > mTasks;
QMap< long, QgsTaskList > mTaskDependencies;
QMap< long, QStringList > mLayerDependencies;
QMap< long, QgsWeakMapLayerPointerList > mLayerDependencies;

//! Tracks the next unique task ID
long mNextTaskId;
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsvectorfilewritertask.cpp
View file
Expand Up @@ -30,7 +30,7 @@ QgsVectorFileWriterTask::QgsVectorFileWriterTask( QgsVectorLayer* layer, const Q
mOptions.feedback = mOwnedFeedback.get();
}
if ( mLayer )
setDependentLayers( QStringList() << mLayer->id() );
setDependentLayers( QList< QgsMapLayer* >() << mLayer );
}

void QgsVectorFileWriterTask::cancel()
Expand Down
12 changes: 6 additions & 6 deletions tests/src/core/testqgstaskmanager.cpp
View file
Expand Up @@ -1026,13 +1026,13 @@ void TestQgsTaskManager::layerDependencies()
//test that remove layers cancels all tasks which are dependent on them
TestTask* task = new TestTask();
task->hold();
task->setDependentLayers( QStringList() << layer2->id() << layer3->id() );
QCOMPARE( task->dependentLayerIds(), QStringList() << layer2->id() << layer3->id() );
task->setDependentLayers( QList< QgsMapLayer* >() << layer2 << layer3 );
QCOMPARE( task->dependentLayers(), QList< QgsMapLayer* >() << layer2 << layer3 );
long taskId = manager.addTask( task );
QCOMPARE( manager.dependentLayers( taskId ), QStringList() << layer2->id() << layer3->id() );
QVERIFY( manager.tasksDependentOnLayer( "xxx" ).isEmpty() );
QCOMPARE( manager.tasksDependentOnLayer( layer2->id() ), QList< QgsTask* >() << task );
QCOMPARE( manager.tasksDependentOnLayer( layer3->id() ), QList< QgsTask* >() << task );
QCOMPARE( manager.dependentLayers( taskId ), QList< QgsMapLayer* >() << layer2 << layer3 );
QVERIFY( manager.tasksDependentOnLayer( nullptr ).isEmpty() );
QCOMPARE( manager.tasksDependentOnLayer( layer2 ), QList< QgsTask* >() << task );
QCOMPARE( manager.tasksDependentOnLayer( layer3 ), QList< QgsTask* >() << task );

QCOMPARE( task->status(), QgsTask::OnHold );
//removing layer1 should have no effect
Expand Down

0 comments on commit 35bb5c6

Please sign in to comment.