Skip to content

Commit

Permalink
Abort symbol rendering early if render job is cancelled
Browse files Browse the repository at this point in the history
Some symbol rendering operations take a long time, especially
if settings are accidentally ridiculous (e.g. changing a marker
line with interval 1 mm to map units on a small scale map can
result in millions+ of markers being rendered for a single
feature). If we don't abort these operations responsively,
then the render job can become effectively "stuck" and sit
burning away CPU for no good reason (or in some cases lock the
QGIS ui as a result).

Instead, for possibly length symbol rendering operations we
check at reasonable places for the QgsRenderContext::renderingStopped()
flag and if it's set, abort the rendering quickly and gracefully.

(cherry picked from commit bb0d449)
  • Loading branch information
nyalldawson committed May 23, 2019
1 parent 1171533 commit fa2ae2f
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 29 deletions.
6 changes: 6 additions & 0 deletions src/core/symbology/qgsarrowsymbollayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,9 @@ void QgsArrowSymbolLayer::renderPolyline( const QPolygonF &points, QgsSymbolRend
{
for ( int pIdx = 0; pIdx < points.size() - 1; pIdx += 2 )
{
if ( context.renderContext().renderingStopped() )
break;

mExpressionScope->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_POINT_NUM, pIdx + 1, true ) );
_resolveDataDefined( context );

Expand Down Expand Up @@ -778,6 +781,9 @@ void QgsArrowSymbolLayer::renderPolyline( const QPolygonF &points, QgsSymbolRend
// only straight arrows
for ( int pIdx = 0; pIdx < points.size() - 1; pIdx++ )
{
if ( context.renderContext().renderingStopped() )
break;

mExpressionScope->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_POINT_NUM, pIdx + 1, true ) );
_resolveDataDefined( context );

Expand Down
24 changes: 18 additions & 6 deletions src/core/symbology/qgsfillsymbollayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1237,11 +1237,11 @@ void QgsShapeburstFillSymbolLayer::renderPolygon( const QPolygonF &points, QList
imgPainter.end();

//apply distance transform to image, uses the current color ramp to calculate final pixel colors
double *dtArray = distanceTransform( fillImage.get() );
double *dtArray = distanceTransform( fillImage.get(), context.renderContext() );

//copy distance transform values back to QImage, shading by appropriate color ramp
dtArrayToQImage( dtArray, fillImage.get(), mColorType == QgsShapeburstFillSymbolLayer::SimpleTwoColor ? twoColorGradientRamp.get() : mGradientRamp.get(),
context.opacity(), useWholeShape, outputPixelMaxDist );
context.renderContext(), context.opacity(), useWholeShape, outputPixelMaxDist );

//clean up some variables
delete [] dtArray;
Expand Down Expand Up @@ -1314,7 +1314,7 @@ void QgsShapeburstFillSymbolLayer::distanceTransform1d( double *f, int n, int *v
}

/* distance transform of 2d function using squared distance */
void QgsShapeburstFillSymbolLayer::distanceTransform2d( double *im, int width, int height )
void QgsShapeburstFillSymbolLayer::distanceTransform2d( double *im, int width, int height, QgsRenderContext &context )
{
int maxDimension = std::max( width, height );
double *f = new double[ maxDimension ];
Expand All @@ -1325,6 +1325,9 @@ void QgsShapeburstFillSymbolLayer::distanceTransform2d( double *im, int width, i
// transform along columns
for ( int x = 0; x < width; x++ )
{
if ( context.renderingStopped() )
break;

for ( int y = 0; y < height; y++ )
{
f[y] = im[ x + y * width ];
Expand All @@ -1339,6 +1342,9 @@ void QgsShapeburstFillSymbolLayer::distanceTransform2d( double *im, int width, i
// transform along rows
for ( int y = 0; y < height; y++ )
{
if ( context.renderingStopped() )
break;

for ( int x = 0; x < width; x++ )
{
f[x] = im[ x + y * width ];
Expand All @@ -1357,7 +1363,7 @@ void QgsShapeburstFillSymbolLayer::distanceTransform2d( double *im, int width, i
}

/* distance transform of a binary QImage */
double *QgsShapeburstFillSymbolLayer::distanceTransform( QImage *im )
double *QgsShapeburstFillSymbolLayer::distanceTransform( QImage *im, QgsRenderContext &context )
{
int width = im->width();
int height = im->height();
Expand All @@ -1369,6 +1375,9 @@ double *QgsShapeburstFillSymbolLayer::distanceTransform( QImage *im )
int idx = 0;
for ( int heightIndex = 0; heightIndex < height; ++heightIndex )
{
if ( context.renderingStopped() )
break;

const QRgb *scanLine = reinterpret_cast< const QRgb * >( im->constScanLine( heightIndex ) );
for ( int widthIndex = 0; widthIndex < width; ++widthIndex )
{
Expand All @@ -1388,12 +1397,12 @@ double *QgsShapeburstFillSymbolLayer::distanceTransform( QImage *im )
}

//calculate squared distance transform
distanceTransform2d( dtArray, width, height );
distanceTransform2d( dtArray, width, height, context );

return dtArray;
}

void QgsShapeburstFillSymbolLayer::dtArrayToQImage( double *array, QImage *im, QgsColorRamp *ramp, double layerAlpha, bool useWholeShape, int maxPixelDistance )
void QgsShapeburstFillSymbolLayer::dtArrayToQImage( double *array, QImage *im, QgsColorRamp *ramp, QgsRenderContext &context, double layerAlpha, bool useWholeShape, int maxPixelDistance )
{
int width = im->width();
int height = im->height();
Expand Down Expand Up @@ -1431,6 +1440,9 @@ void QgsShapeburstFillSymbolLayer::dtArrayToQImage( double *array, QImage *im, Q

for ( int heightIndex = 0; heightIndex < height; ++heightIndex )
{
if ( context.renderingStopped() )
break;

QRgb *scanLine = reinterpret_cast< QRgb * >( im->scanLine( heightIndex ) );
for ( int widthIndex = 0; widthIndex < width; ++widthIndex )
{
Expand Down
6 changes: 3 additions & 3 deletions src/core/symbology/qgsfillsymbollayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -611,12 +611,12 @@ class CORE_EXPORT QgsShapeburstFillSymbolLayer : public QgsFillSymbolLayer
/* distance transform of a 1d function using squared distance */
void distanceTransform1d( double *f, int n, int *v, double *z, double *d );
/* distance transform of 2d function using squared distance */
void distanceTransform2d( double *im, int width, int height );
void distanceTransform2d( double *im, int width, int height, QgsRenderContext &context );
/* distance transform of a binary QImage */
double *distanceTransform( QImage *im );
double *distanceTransform( QImage *im, QgsRenderContext &context );

/* fills a QImage with values from an array of doubles containing squared distance transform values */
void dtArrayToQImage( double *array, QImage *im, QgsColorRamp *ramp, double layerAlpha = 1, bool useWholeShape = true, int maxPixelDistance = 0 );
void dtArrayToQImage( double *array, QImage *im, QgsColorRamp *ramp, QgsRenderContext &context, double layerAlpha = 1, bool useWholeShape = true, int maxPixelDistance = 0 );

#ifdef SIP_RUN
QgsShapeburstFillSymbolLayer( const QgsShapeburstFillSymbolLayer &other );
Expand Down
8 changes: 7 additions & 1 deletion src/core/symbology/qgsheatmaprenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ bool QgsHeatmapRenderer::renderFeature( const QgsFeature &feature, QgsRenderCont
int pointY = pixel.y() / mRenderQuality;
for ( int x = std::max( pointX - mRadiusPixels, 0 ); x < std::min( pointX + mRadiusPixels, width ); ++x )
{
if ( context.renderingStopped() )
break;

for ( int y = std::max( pointY - mRadiusPixels, 0 ); y < std::min( pointY + mRadiusPixels, height ); ++y )
{
int index = y * width + x;
Expand Down Expand Up @@ -221,7 +224,7 @@ void QgsHeatmapRenderer::stopRender( QgsRenderContext &context )

void QgsHeatmapRenderer::renderImage( QgsRenderContext &context )
{
if ( !context.painter() || !mGradientRamp )
if ( !context.painter() || !mGradientRamp || context.renderingStopped() )
{
return;
}
Expand All @@ -238,6 +241,9 @@ void QgsHeatmapRenderer::renderImage( QgsRenderContext &context )
QColor pixColor;
for ( int heightIndex = 0; heightIndex < image.height(); ++heightIndex )
{
if ( context.renderingStopped() )
break;

QRgb *scanLine = reinterpret_cast< QRgb * >( image.scanLine( heightIndex ) );
for ( int widthIndex = 0; widthIndex < image.width(); ++widthIndex )
{
Expand Down
5 changes: 5 additions & 0 deletions src/core/symbology/qgsinvertedpolygonrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ bool QgsInvertedPolygonRenderer::renderFeature( const QgsFeature &feature, QgsRe
void QgsInvertedPolygonRenderer::stopRender( QgsRenderContext &context )
{
QgsFeatureRenderer::stopRender( context );
if ( context.renderingStopped() )
{
mSubRenderer->stopRender( mContext );
return;
}

if ( !mSubRenderer )
{
Expand Down
9 changes: 9 additions & 0 deletions src/core/symbology/qgslinesymbollayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,9 @@ void QgsMarkerLineSymbolLayer::renderPolylineInterval( const QPolygonF &points,
int pointNum = 0;
for ( int i = 1; i < points.count(); ++i )
{
if ( context.renderContext().renderingStopped() )
break;

const QPointF &pt = points[i];

if ( lastPt == pt ) // must not be equal!
Expand All @@ -1044,6 +1047,9 @@ void QgsMarkerLineSymbolLayer::renderPolylineInterval( const QPolygonF &points,
// while we're not at the end of line segment, draw!
while ( lengthLeft > painterUnitInterval )
{
if ( context.renderContext().renderingStopped() )
break;

// "c" is 1 for regular point or in interval (0,1] for begin of line segment
lastPt += c * diff;
lengthLeft -= painterUnitInterval;
Expand Down Expand Up @@ -1108,6 +1114,9 @@ void QgsMarkerLineSymbolLayer::renderPolylineVertex( const QPolygonF &points, Qg
int pointNum = 0;
while ( context.renderContext().geometry()->nextVertex( vId, vPoint ) )
{
if ( context.renderContext().renderingStopped() )
break;

scope->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_POINT_NUM, ++pointNum, true ) );

if ( ( placement == Vertex && vId.type == QgsVertexId::SegmentVertex )
Expand Down
7 changes: 5 additions & 2 deletions src/core/symbology/qgspointdistancerenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,12 @@ void QgsPointDistanceRenderer::stopRender( QgsRenderContext &context )

//printInfoDisplacementGroups(); //just for debugging

Q_FOREACH ( const ClusteredGroup &group, mClusteredGroups )
if ( !context.renderingStopped() )
{
drawGroup( group, context );
Q_FOREACH ( const ClusteredGroup &group, mClusteredGroups )
{
drawGroup( group, context );
}
}

mClusteredGroups.clear();
Expand Down
35 changes: 19 additions & 16 deletions src/core/symbology/qgsrulebasedrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -929,26 +929,29 @@ void QgsRuleBasedRenderer::stopRender( QgsRenderContext &context )
//

// go through all levels
Q_FOREACH ( const RenderLevel &level, mRenderQueue )
if ( !context.renderingStopped() )
{
//QgsDebugMsg(QString("level %1").arg(level.zIndex));
// go through all jobs at the level
Q_FOREACH ( const RenderJob *job, level.jobs )
Q_FOREACH ( const RenderLevel &level, mRenderQueue )
{
context.expressionContext().setFeature( job->ftr.feat );
//QgsDebugMsg(QString("job fid %1").arg(job->f->id()));
// render feature - but only with symbol layers with specified zIndex
QgsSymbol *s = job->symbol;
int count = s->symbolLayerCount();
for ( int i = 0; i < count; i++ )
//QgsDebugMsg(QString("level %1").arg(level.zIndex));
// go through all jobs at the level
Q_FOREACH ( const RenderJob *job, level.jobs )
{
// TODO: better solution for this
// renderFeatureWithSymbol asks which symbol layer to draw
// but there are multiple transforms going on!
if ( s->symbolLayer( i )->renderingPass() == level.zIndex )
context.expressionContext().setFeature( job->ftr.feat );
//QgsDebugMsg(QString("job fid %1").arg(job->f->id()));
// render feature - but only with symbol layers with specified zIndex
QgsSymbol *s = job->symbol;
int count = s->symbolLayerCount();
for ( int i = 0; i < count; i++ )
{
int flags = job->ftr.flags;
renderFeatureWithSymbol( job->ftr.feat, job->symbol, context, i, flags & FeatIsSelected, flags & FeatDrawMarkers );
// TODO: better solution for this
// renderFeatureWithSymbol asks which symbol layer to draw
// but there are multiple transforms going on!
if ( s->symbolLayer( i )->renderingPass() == level.zIndex )
{
int flags = job->ftr.flags;
renderFeatureWithSymbol( job->ftr.feat, job->symbol, context, i, flags & FeatIsSelected, flags & FeatDrawMarkers );
}
}
}
}
Expand Down
23 changes: 22 additions & 1 deletion src/core/symbology/qgssymbol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@ class GeometryRestorer

void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &context, int layer, bool selected, bool drawVertexMarker, int currentVertexMarkerType, double currentVertexMarkerSize )
{
if ( context.renderingStopped() )
return;

const QgsGeometry geom = feature.geometry();
if ( geom.isNull() )
{
Expand Down Expand Up @@ -886,6 +889,9 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont

for ( int i = 0; i < mp.numGeometries(); ++i )
{
if ( context.renderingStopped() )
break;

mSymbolRenderContext->setGeometryPartNum( i + 1 );
if ( needsExpressionContext )
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_NUM, i + 1, true ) );
Expand Down Expand Up @@ -916,6 +922,9 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
const unsigned int num = geomCollection.numGeometries();
for ( unsigned int i = 0; i < num; ++i )
{
if ( context.renderingStopped() )
break;

mSymbolRenderContext->setGeometryPartNum( i + 1 );
if ( needsExpressionContext )
mSymbolRenderContext->expressionContextScope()->addVariable( QgsExpressionContextScope::StaticVariable( QgsExpressionContext::EXPR_GEOMETRY_PART_NUM, i + 1, true ) );
Expand Down Expand Up @@ -967,6 +976,9 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont
const QList<unsigned int> &listPartIndex = iter->second;
for ( int idx = 0; idx < listPartIndex.size(); ++idx )
{
if ( context.renderingStopped() )
break;

const unsigned i = listPartIndex[idx];
mSymbolRenderContext->setGeometryPartNum( i + 1 );
if ( needsExpressionContext )
Expand Down Expand Up @@ -1013,7 +1025,7 @@ void QgsSymbol::renderFeature( const QgsFeature &feature, QgsRenderContext &cont

if ( drawVertexMarker )
{
if ( !markers.isEmpty() )
if ( !markers.isEmpty() && !context.renderingStopped() )
{
Q_FOREACH ( QPointF marker, markers )
{
Expand Down Expand Up @@ -1540,6 +1552,9 @@ void QgsMarkerSymbol::renderPoint( QPointF point, const QgsFeature *f, QgsRender

Q_FOREACH ( QgsSymbolLayer *symbolLayer, mLayers )
{
if ( context.renderingStopped() )
break;

if ( !symbolLayer->enabled() )
continue;

Expand Down Expand Up @@ -1770,6 +1785,9 @@ void QgsLineSymbol::renderPolyline( const QPolygonF &points, const QgsFeature *f

Q_FOREACH ( QgsSymbolLayer *symbolLayer, mLayers )
{
if ( context.renderingStopped() )
break;;

if ( !symbolLayer->enabled() )
continue;

Expand Down Expand Up @@ -1851,6 +1869,9 @@ void QgsFillSymbol::renderPolygon( const QPolygonF &points, QList<QPolygonF> *ri

Q_FOREACH ( QgsSymbolLayer *symbolLayer, mLayers )
{
if ( context.renderingStopped() )
break;

if ( !symbolLayer->enabled() )
continue;

Expand Down

0 comments on commit fa2ae2f

Please sign in to comment.