Skip to content

Commit

Permalink
Make QgsMapRendererStagedRenderJob thread safe
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Aug 17, 2019
1 parent 84f086b commit b1e3d04
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 50 deletions.
2 changes: 1 addition & 1 deletion python/core/auto_generated/qgsmaprenderercustompainterjob.sip.in
View file
Expand Up @@ -117,7 +117,7 @@ and an alternative to renderSynchronously() (which should only ever be called fr
%Docstring
Render a pre-prepared job. Can be safely called in a background thread.

Must be preceeded by a call to prepare()
Must be preceded by a call to prepare()

This is an alternative to ordinary API (using start() + waiting for finished() signal),
and an alternative to renderSynchronously() (which should only ever be called from the main thread).
Expand Down
46 changes: 46 additions & 0 deletions src/core/qgslabelingengine.cpp
View file
Expand Up @@ -139,6 +139,52 @@ QList< QgsMapLayer * > QgsLabelingEngine::participatingLayers() const
return layers;
}

QStringList QgsLabelingEngine::participatingLayerIds() const
{
QStringList layers;

// try to return layers sorted in the desired z order for rendering
QList< QgsAbstractLabelProvider * > providersByZ = mProviders;
std::sort( providersByZ.begin(), providersByZ.end(),
[]( const QgsAbstractLabelProvider * a, const QgsAbstractLabelProvider * b ) -> bool
{
const QgsVectorLayerLabelProvider *providerA = dynamic_cast<const QgsVectorLayerLabelProvider *>( a );
const QgsVectorLayerLabelProvider *providerB = dynamic_cast<const QgsVectorLayerLabelProvider *>( b );

if ( providerA && providerB )
{
return providerA->settings().zIndex < providerB->settings().zIndex ;
}
return false;
} );

QList< QgsAbstractLabelProvider * > subProvidersByZ = mSubProviders;
std::sort( subProvidersByZ.begin(), subProvidersByZ.end(),
[]( const QgsAbstractLabelProvider * a, const QgsAbstractLabelProvider * b ) -> bool
{
const QgsVectorLayerLabelProvider *providerA = dynamic_cast<const QgsVectorLayerLabelProvider *>( a );
const QgsVectorLayerLabelProvider *providerB = dynamic_cast<const QgsVectorLayerLabelProvider *>( b );

if ( providerA && providerB )
{
return providerA->settings().zIndex < providerB->settings().zIndex ;
}
return false;
} );

for ( QgsAbstractLabelProvider *provider : qgis::as_const( providersByZ ) )
{
if ( !layers.contains( provider->layerId() ) )
layers << provider->layerId();
}
for ( QgsAbstractLabelProvider *provider : qgis::as_const( subProvidersByZ ) )
{
if ( !layers.contains( provider->layerId() ) )
layers << provider->layerId();
}
return layers;
}

void QgsLabelingEngine::addProvider( QgsAbstractLabelProvider *provider )
{
provider->setEngine( this );
Expand Down
6 changes: 6 additions & 0 deletions src/core/qgslabelingengine.h
View file
Expand Up @@ -234,6 +234,12 @@ class CORE_EXPORT QgsLabelingEngine
*/
QList< QgsMapLayer * > participatingLayers() const;

/**
* Returns a list of layer IDs for layers with providers in the engine.
* \since QGIS 3.10
*/
QStringList participatingLayerIds() const;

//! Add provider of label features. Takes ownership of the provider
void addProvider( QgsAbstractLabelProvider *provider );

Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsmaprenderercustompainterjob.h
View file
Expand Up @@ -119,7 +119,7 @@ class CORE_EXPORT QgsMapRendererCustomPainterJob : public QgsMapRendererAbstract
/**
* Render a pre-prepared job. Can be safely called in a background thread.
*
* Must be preceeded by a call to prepare()
* Must be preceded by a call to prepare()
*
* This is an alternative to ordinary API (using start() + waiting for finished() signal),
* and an alternative to renderSynchronously() (which should only ever be called from the main thread).
Expand Down
21 changes: 3 additions & 18 deletions src/core/qgsmaprendererstagedrenderjob.cpp
View file
Expand Up @@ -143,7 +143,7 @@ bool QgsMapRendererStagedRenderJob::renderCurrentPart( QPainter *painter )
painter->setCompositionMode( QPainter::CompositionMode_SourceOver );

// render just the current layer's labels
static_cast< QgsStagedRenderLabelingEngine * >( mLabelingEngineV2.get() )->renderLabelsForLayer( mLabelJob.context, ( *mLabelLayerIt )->id() );
static_cast< QgsStagedRenderLabelingEngine * >( mLabelingEngineV2.get() )->renderLabelsForLayer( mLabelJob.context, *mLabelLayerIt );
}
else
{
Expand Down Expand Up @@ -176,7 +176,7 @@ bool QgsMapRendererStagedRenderJob::nextPart()
{
mLabelingEngineV2->run( mLabelJob.context );
mPreparedStagedLabelJob = true;
mLabelingLayers = mLabelingEngineV2->participatingLayers();
mLabelingLayers = mLabelingEngineV2->participatingLayerIds();
mLabelLayerIt = mLabelingLayers.begin();
if ( mLabelLayerIt == mLabelingLayers.end() )
{
Expand Down Expand Up @@ -218,21 +218,6 @@ bool QgsMapRendererStagedRenderJob::isFinished()
return currentStage() == Finished;
}

const QgsMapLayer *QgsMapRendererStagedRenderJob::currentLayer()
{
if ( mJobIt != mLayerJobs.end() )
{
LayerRenderJob &job = *mJobIt;
return job.layer;
}
else if ( mFlags & RenderLabelsByMapLayer && mPreparedStagedLabelJob )
{
if ( mLabelLayerIt != mLabelingLayers.end() )
return *mLabelLayerIt;
}
return nullptr;
}

QString QgsMapRendererStagedRenderJob::currentLayerId()
{
if ( mJobIt != mLayerJobs.end() )
Expand All @@ -243,7 +228,7 @@ QString QgsMapRendererStagedRenderJob::currentLayerId()
else if ( mFlags & RenderLabelsByMapLayer && mPreparedStagedLabelJob )
{
if ( mLabelLayerIt != mLabelingLayers.end() )
return ( *mLabelLayerIt )->id();
return *mLabelLayerIt;
}
return QString();
}
Expand Down
10 changes: 2 additions & 8 deletions src/core/qgsmaprendererstagedrenderjob.h
View file
Expand Up @@ -92,12 +92,6 @@ class CORE_EXPORT QgsMapRendererStagedRenderJob : public QgsMapRendererAbstractC
*/
bool isFinished();

/**
* Returns a pointer to the current layer about to be rendered in the next render operation.
* \see currentLayerId()
*/
const QgsMapLayer *currentLayer();

/**
* Returns the ID of the current layer about to be rendered in the next render operation.
* \see currentLayer()
Expand All @@ -121,8 +115,8 @@ class CORE_EXPORT QgsMapRendererStagedRenderJob : public QgsMapRendererAbstractC
bool mExportedLabels = false;
Flags mFlags = nullptr;
bool mPreparedStagedLabelJob = false;
QList< QgsMapLayer * > mLabelingLayers;
QList< QgsMapLayer * >::iterator mLabelLayerIt;
QStringList mLabelingLayers;
QStringList::iterator mLabelLayerIt;
};

#endif // QGSMAPRENDERERSTAGEDRENDERJOB_H
22 changes: 0 additions & 22 deletions tests/src/core/testqgsmaprendererjob.cpp
View file
Expand Up @@ -510,7 +510,6 @@ void TestQgsMapRendererJob::stagedRenderer()
// nothing to render
QVERIFY( job->isFinished() );
QVERIFY( !job->renderCurrentPart( nullptr ) );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );

Expand All @@ -519,7 +518,6 @@ void TestQgsMapRendererJob::stagedRenderer()
job = qgis::make_unique< QgsMapRendererStagedRenderJob >( mapSettings );
job->start();
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), polygonsLayer.get() );
QCOMPARE( job->currentLayerId(), polygonsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -532,7 +530,6 @@ void TestQgsMapRendererJob::stagedRenderer()
QVERIFY( imageCheck( QStringLiteral( "staged_render1" ), im ) );
QVERIFY( !job->isFinished() );
QVERIFY( job->nextPart() );
QCOMPARE( job->currentLayer(), linesLayer.get() );
QCOMPARE( job->currentLayerId(), linesLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -545,7 +542,6 @@ void TestQgsMapRendererJob::stagedRenderer()
QVERIFY( imageCheck( QStringLiteral( "staged_render2" ), im ) );
QVERIFY( !job->isFinished() );
QVERIFY( job->nextPart() );
QCOMPARE( job->currentLayer(), pointsLayer.get() );
QCOMPARE( job->currentLayerId(), pointsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -559,7 +555,6 @@ void TestQgsMapRendererJob::stagedRenderer()

// nothing left!
QVERIFY( !job->nextPart() );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );
QVERIFY( job->isFinished() );
Expand All @@ -585,7 +580,6 @@ void TestQgsMapRendererJob::stagedRenderer()
job = qgis::make_unique< QgsMapRendererStagedRenderJob >( mapSettings );
job->start();
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), polygonsLayer.get() );
QCOMPARE( job->currentLayerId(), polygonsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -598,7 +592,6 @@ void TestQgsMapRendererJob::stagedRenderer()
QVERIFY( imageCheck( QStringLiteral( "staged_render1" ), im ) );
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), linesLayer.get() );
QCOMPARE( job->currentLayerId(), linesLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -611,7 +604,6 @@ void TestQgsMapRendererJob::stagedRenderer()
QVERIFY( imageCheck( QStringLiteral( "staged_render2" ), im ) );
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), pointsLayer.get() );
QCOMPARE( job->currentLayerId(), pointsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -624,7 +616,6 @@ void TestQgsMapRendererJob::stagedRenderer()
QVERIFY( imageCheck( QStringLiteral( "staged_render3" ), im ) );
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Labels );
// labels
Expand All @@ -637,7 +628,6 @@ void TestQgsMapRendererJob::stagedRenderer()

// nothing left!
QVERIFY( !job->nextPart() );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );
QVERIFY( job->isFinished() );
Expand Down Expand Up @@ -672,7 +662,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
// nothing to render
QVERIFY( job->isFinished() );
QVERIFY( !job->renderCurrentPart( nullptr ) );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );

Expand All @@ -681,7 +670,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
job = qgis::make_unique< QgsMapRendererStagedRenderJob >( mapSettings, QgsMapRendererStagedRenderJob::RenderLabelsByMapLayer );
job->start();
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), polygonsLayer.get() );
QCOMPARE( job->currentLayerId(), polygonsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -694,7 +682,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
QVERIFY( imageCheck( QStringLiteral( "staged_render1" ), im ) );
QVERIFY( !job->isFinished() );
QVERIFY( job->nextPart() );
QCOMPARE( job->currentLayer(), linesLayer.get() );
QCOMPARE( job->currentLayerId(), linesLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -707,7 +694,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
QVERIFY( imageCheck( QStringLiteral( "staged_render2" ), im ) );
QVERIFY( !job->isFinished() );
QVERIFY( job->nextPart() );
QCOMPARE( job->currentLayer(), pointsLayer.get() );
QCOMPARE( job->currentLayerId(), pointsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -721,7 +707,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()

// nothing left!
QVERIFY( !job->nextPart() );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );
QVERIFY( job->isFinished() );
Expand Down Expand Up @@ -760,7 +745,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
job = qgis::make_unique< QgsMapRendererStagedRenderJob >( mapSettings, QgsMapRendererStagedRenderJob::RenderLabelsByMapLayer );
job->start();
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), polygonsLayer.get() );
QCOMPARE( job->currentLayerId(), polygonsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -773,7 +757,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
QVERIFY( imageCheck( QStringLiteral( "staged_render1" ), im ) );
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), linesLayer.get() );
QCOMPARE( job->currentLayerId(), linesLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -786,7 +769,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
QVERIFY( imageCheck( QStringLiteral( "staged_render2" ), im ) );
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), pointsLayer.get() );
QCOMPARE( job->currentLayerId(), pointsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Symbology );

Expand All @@ -801,7 +783,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
// points labels (these must be in z-order!)
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), pointsLayer.get() );
QCOMPARE( job->currentLayerId(), pointsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Labels );
im = QImage( 512, 512, QImage::Format_ARGB32_Premultiplied );
Expand All @@ -814,7 +795,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
// polygon labels
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), polygonsLayer.get() );
QCOMPARE( job->currentLayerId(), polygonsLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Labels );
// labels
Expand All @@ -828,7 +808,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()
// line labels
QVERIFY( job->nextPart() );
QVERIFY( !job->isFinished() );
QCOMPARE( job->currentLayer(), linesLayer.get() );
QCOMPARE( job->currentLayerId(), linesLayer->id() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Labels );
// labels
Expand All @@ -841,7 +820,6 @@ void TestQgsMapRendererJob::stagedRendererWithStagedLabeling()

// nothing left!
QVERIFY( !job->nextPart() );
QVERIFY( !job->currentLayer() );
QVERIFY( job->currentLayerId().isEmpty() );
QCOMPARE( job->currentStage(), QgsMapRendererStagedRenderJob::Finished );
QVERIFY( job->isFinished() );
Expand Down

0 comments on commit b1e3d04

Please sign in to comment.