Skip to content
Permalink
Browse files

[symbology] When rendering features, split the geometry fetching

and preparation stage from the symbol layer rendering stage, and
ensure that QgsSymbolLayer::startFeatureRender and ::stopFeatureRender
is correctly called in the right sequence when rendering multi-layer
symbols

This fixes issues with symbol layers which rely on startFeatureRender
and stopFeatureRender to correctly render, e.g. the Random Marker Fill
symbol layer.

Before this fix, the logic looked like:

- for every symbol layer in the symbol, call startFeatureRender
- for each part in polygon, prepare the part geometry and then render each symbol layer
- for every symbol layer in the symbol, call stopFeatureRender

The issue with this approach is that symbol layers which defer
rendering to the stopFeatureRender stage are always rendered
after ALL other symbol layers in the symbol, regardless of the actual
order of the symbol layers. Ultimately this causes Random Marker Fill
layers to always render on the top of symbols.

The new logic is:
- for each part in polygon, prepare the geometry and store the result
- for each symbol layer in the symbol:
   - call startFeatureRender
   - render the layer using each of the previously prepared parts
   - call stopFeatureRender

This results in correct stacking of the random marker fill in multi
layer symbols, because the stopFeatureRender call is correctly called
before the next layer's startFeatureRender and renderPolygon calls

Also, use QVector instead of QList for rings for improved efficiency
  • Loading branch information
nyalldawson committed May 18, 2020
1 parent ea4f2bb commit ef97e8c6fca72e63d39478ca563991806ee9cc7b
@@ -45,7 +45,7 @@ Caller takes ownership of the returned symbol layer.
virtual void stopRender( QgsSymbolRenderContext &context );


virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsStringMap properties() const;
@@ -214,7 +214,7 @@ Caller takes ownership of the returned symbol layer.
virtual void stopRender( QgsSymbolRenderContext &context );


virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsStringMap properties() const;
@@ -374,7 +374,7 @@ Caller takes ownership of the returned symbol layer.
virtual void stopRender( QgsSymbolRenderContext &context );


virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsStringMap properties() const;
@@ -686,7 +686,7 @@ Base class for polygon renderers generating texture images*
public:

QgsImageFillSymbolLayer();
virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsSymbol *subSymbol();
@@ -806,7 +806,7 @@ Used internally when reading/writing symbols.

virtual QString layerType() const;

virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );

virtual void startRender( QgsSymbolRenderContext &context );

@@ -1952,7 +1952,7 @@ Caller takes ownership of the returned symbol layer.

virtual void stopRender( QgsSymbolRenderContext &context );

virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );

virtual QgsStringMap properties() const;

@@ -2139,7 +2139,7 @@ Caller takes ownership of the returned symbol layer.
virtual void stopRender( QgsSymbolRenderContext &context );


virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );


virtual QgsStringMap properties() const;
@@ -52,7 +52,7 @@ Creates a new QgsSimpleLineSymbolLayer from an SLD XML DOM ``element``.

virtual void renderPolyline( const QPolygonF &points, QgsSymbolRenderContext &context );

virtual void renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );

virtual QgsStringMap properties() const;

@@ -506,7 +506,7 @@ calculating individual symbol angles.

virtual void renderPolyline( const QPolygonF &points, QgsSymbolRenderContext &context ) ${SIP_FINAL};

virtual void renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) ${SIP_FINAL};
virtual void renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) ${SIP_FINAL};

virtual QgsUnitTypes::RenderUnit outputUnit() const ${SIP_FINAL};

@@ -548,7 +548,7 @@ If ``correctRingOrientation`` is ``True`` then the ring will be oriented to matc
clockwise for exterior rings and counter-clockwise for interior rings.
%End

static void _getPolygon( QPolygonF &pts, QList<QPolygonF> &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true, bool correctRingOrientation = false );
static void _getPolygon( QPolygonF &pts, QVector<QPolygonF> &holes, QgsRenderContext &context, const QgsPolygon &polygon, bool clipToExtent = true, bool correctRingOrientation = false );
%Docstring
Creates a polygon in screen coordinates from a QgsPolygonXYin map coordinates

@@ -1206,7 +1206,7 @@ Ownership of the ``layers`` are transferred to the symbol.
%End
void setAngle( double angle );

void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, const QgsFeature *f, QgsRenderContext &context, int layer = -1, bool selected = false );
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, const QgsFeature *f, QgsRenderContext &context, int layer = -1, bool selected = false );
%Docstring
Renders the symbol using the given render ``context``.

@@ -979,7 +979,7 @@ Renders the line symbol layer along the line joining ``points``, using the given
.. seealso:: :py:func:`renderPolygonStroke`
%End

virtual void renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
virtual void renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );
%Docstring
Renders the line symbol layer along the outline of polygon, using the given render ``context``.

@@ -1167,9 +1167,11 @@ class QgsFillSymbolLayer : QgsSymbolLayer



virtual void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) = 0;
virtual void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) = 0;
%Docstring
QgsFillSymbolLayer cannot be copied
Renders the fill symbol layer for the polygon whose outer ring is defined by ``points``, using the given render ``context``.

The ``rings`` argument optionally specifies a list of polygon rings to render as holes.
%End

virtual void drawPreviewIcon( QgsSymbolRenderContext &context, QSize size );
@@ -1180,7 +1182,7 @@ QgsFillSymbolLayer cannot be copied

protected:
QgsFillSymbolLayer( bool locked = false );
void _renderPolygon( QPainter *p, const QPolygonF &points, const QList<QPolygonF> *rings, QgsSymbolRenderContext &context );
void _renderPolygon( QPainter *p, const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context );
%Docstring
Default method to render polygon
%End
@@ -713,7 +713,7 @@ Determines an SVG symbol's name from its ``path``.
Calculate the centroid point of a QPolygonF
%End

static QPointF polygonPointOnSurface( const QPolygonF &points, QList<QPolygonF> *rings = 0 );
static QPointF polygonPointOnSurface( const QPolygonF &points, const QVector<QPolygonF> *rings = 0 );
%Docstring
Calculate a point on the surface of a QPolygonF
%End
@@ -358,7 +358,7 @@ void QgsAnnotation::drawFrame( QgsRenderContext &context ) const

QPolygonF poly;
poly.reserve( 9 + ( mHasFixedMapPosition ? 3 : 0 ) );
QList<QPolygonF> rings; //empty list
QVector<QPolygonF> rings; //empty list
for ( int i = 0; i < 4; ++i )
{
QLineF currentSegment = segment( i, &context );
@@ -127,7 +127,7 @@ void QgsLayoutItemMapOverview::draw( QPainter *painter )
QPolygonF intersectPolygon;
intersectPolygon = mapTransform.map( intersectExtent );

QList<QPolygonF> rings; //empty list
QVector<QPolygonF> rings; //empty list
if ( !mInverted )
{
//Render the intersecting map extent
@@ -280,7 +280,7 @@ void QgsLayoutItemPage::draw( QgsLayoutItemRenderContext &context )
// round up
QPolygonF pagePolygon = QPolygonF( QRectF( maxBleedPixels, maxBleedPixels,
std::ceil( rect().width() * scale ) - 2 * maxBleedPixels, std::ceil( rect().height() * scale ) - 2 * maxBleedPixels ) );
QList<QPolygonF> rings; //empty list
QVector<QPolygonF> rings; //empty list

symbol->renderPolygon( pagePolygon, &rings, nullptr, context.renderContext() );
symbol->stopRender( context.renderContext() );
@@ -117,7 +117,7 @@ void QgsLayoutItemPolygon::_draw( QgsLayoutItemRenderContext &context, const QSt
double scale = context.renderContext().convertToPainterUnits( 1, QgsUnitTypes::RenderMillimeters );
QTransform t = QTransform::fromScale( scale, scale );

QList<QPolygonF> rings; //empty
QVector<QPolygonF> rings; //empty
QPainterPath polygonPath;
polygonPath.addPolygon( mPolygon );

@@ -217,7 +217,7 @@ void QgsLayoutItemShape::draw( QgsLayoutItemRenderContext &context )
}
}

QList<QPolygonF> rings; //empty list
QVector<QPolygonF> rings; //empty list

symbol()->startRender( context.renderContext() );
symbol()->renderPolygon( shapePolygon, &rings, nullptr, context.renderContext() );
@@ -263,7 +263,7 @@ void QgsSimpleFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
Q_UNUSED( context )
}

void QgsSimpleFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsSimpleFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -842,7 +842,7 @@ void QgsGradientFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
Q_UNUSED( context )
}

void QgsGradientFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsGradientFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -1129,7 +1129,7 @@ void QgsShapeburstFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
Q_UNUSED( context )
}

void QgsShapeburstFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsShapeburstFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -1560,7 +1560,7 @@ QgsImageFillSymbolLayer::QgsImageFillSymbolLayer()
setSubSymbol( new QgsLineSymbol() );
}

void QgsImageFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsImageFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -1606,8 +1606,7 @@ void QgsImageFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPol
mStroke->renderPolyline( points, context.feature(), context.renderContext(), -1, SELECT_FILL_BORDER && context.selected() );
if ( rings )
{
QList<QPolygonF>::const_iterator ringIt = rings->constBegin();
for ( ; ringIt != rings->constEnd(); ++ringIt )
for ( auto ringIt = rings->constBegin(); ringIt != rings->constEnd(); ++ringIt )
{
mStroke->renderPolyline( *ringIt, context.feature(), context.renderContext(), -1, SELECT_FILL_BORDER && context.selected() );
}
@@ -3507,7 +3506,7 @@ void QgsCentroidFillSymbolLayer::stopRender( QgsSymbolRenderContext &context )
mMarker->stopRender( context.renderContext() );
}

void QgsCentroidFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsCentroidFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
if ( !mPointOnAllParts )
{
@@ -3758,7 +3757,7 @@ QString QgsRasterFillSymbolLayer::layerType() const
return QStringLiteral( "RasterFill" );
}

void QgsRasterFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsRasterFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -4009,7 +4008,7 @@ void QgsRandomMarkerFillSymbolLayer::stopRender( QgsSymbolRenderContext &context
mMarker->stopRender( context.renderContext() );
}

void QgsRandomMarkerFillSymbolLayer::renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsRandomMarkerFillSymbolLayer::renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
Part part;
part.exterior = points;
@@ -65,7 +65,7 @@ class CORE_EXPORT QgsSimpleFillSymbolLayer : public QgsFillSymbolLayer

void stopRender( QgsSymbolRenderContext &context ) override;

void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsStringMap properties() const override;

@@ -229,7 +229,7 @@ class CORE_EXPORT QgsGradientFillSymbolLayer : public QgsFillSymbolLayer

void stopRender( QgsSymbolRenderContext &context ) override;

void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsStringMap properties() const override;

@@ -391,7 +391,7 @@ class CORE_EXPORT QgsShapeburstFillSymbolLayer : public QgsFillSymbolLayer

void stopRender( QgsSymbolRenderContext &context ) override;

void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsStringMap properties() const override;

@@ -648,7 +648,7 @@ class CORE_EXPORT QgsImageFillSymbolLayer: public QgsFillSymbolLayer
public:

QgsImageFillSymbolLayer();
void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsSymbol *subSymbol() override { return mStroke.get(); }
bool setSubSymbol( QgsSymbol *symbol SIP_TRANSFER ) override;
@@ -754,7 +754,7 @@ class CORE_EXPORT QgsRasterFillSymbolLayer: public QgsImageFillSymbolLayer

// implemented from base classes
QString layerType() const override;
void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void startRender( QgsSymbolRenderContext &context ) override;
void stopRender( QgsSymbolRenderContext &context ) override;
QgsStringMap properties() const override;
@@ -1759,7 +1759,7 @@ class CORE_EXPORT QgsRandomMarkerFillSymbolLayer : public QgsFillSymbolLayer
QString layerType() const override;
void startRender( QgsSymbolRenderContext &context ) override;
void stopRender( QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
QgsStringMap properties() const override;
QgsRandomMarkerFillSymbolLayer *clone() const override SIP_FACTORY;

@@ -1895,7 +1895,7 @@ class CORE_EXPORT QgsRandomMarkerFillSymbolLayer : public QgsFillSymbolLayer
struct Part
{
QPolygonF exterior;
QList<QPolygonF> rings;
QVector<QPolygonF> rings;
};

QVector< Part > mCurrentParts;
@@ -1942,7 +1942,7 @@ class CORE_EXPORT QgsCentroidFillSymbolLayer : public QgsFillSymbolLayer

void stopRender( QgsSymbolRenderContext &context ) override;

void renderPolygon( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygon( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;

QgsStringMap properties() const override;

@@ -238,7 +238,7 @@ void QgsSimpleLineSymbolLayer::stopRender( QgsSymbolRenderContext &context )
Q_UNUSED( context )
}

void QgsSimpleLineSymbolLayer::renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsSimpleLineSymbolLayer::renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
QPainter *p = context.renderContext().painter();
if ( !p )
@@ -263,8 +263,7 @@ void QgsSimpleLineSymbolLayer::renderPolygonStroke( const QPolygonF &points, QLi
if ( rings )
{
//add polygon rings
QList<QPolygonF>::const_iterator it = rings->constBegin();
for ( ; it != rings->constEnd(); ++it )
for ( auto it = rings->constBegin(); it != rings->constEnd(); ++it )
{
QPolygonF ring = *it;
clipPath.addPolygon( ring );
@@ -867,7 +866,7 @@ void QgsTemplatedLineSymbolLayerBase::renderPolyline( const QPolygonF &points, Q
context.renderContext().painter()->restore();
}

void QgsTemplatedLineSymbolLayerBase::renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context )
void QgsTemplatedLineSymbolLayerBase::renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context )
{
const QgsCurvePolygon *curvePolygon = dynamic_cast<const QgsCurvePolygon *>( context.renderContext().geometry() );

@@ -68,7 +68,7 @@ class CORE_EXPORT QgsSimpleLineSymbolLayer : public QgsLineSymbolLayer
void stopRender( QgsSymbolRenderContext &context ) override;
void renderPolyline( const QPolygonF &points, QgsSymbolRenderContext &context ) override;
//overridden so that clip path can be set when using draw inside polygon option
void renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
void renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) override;
QgsStringMap properties() const override;
QgsSimpleLineSymbolLayer *clone() const override SIP_FACTORY;
void toSld( QDomDocument &doc, QDomElement &element, const QgsStringMap &props ) const override;
@@ -467,7 +467,7 @@ class CORE_EXPORT QgsTemplatedLineSymbolLayerBase : public QgsLineSymbolLayer
const QgsMapUnitScale &averageAngleMapUnitScale() const { return mAverageAngleLengthMapUnitScale; }

void renderPolyline( const QPolygonF &points, QgsSymbolRenderContext &context ) FINAL;
void renderPolygonStroke( const QPolygonF &points, QList<QPolygonF> *rings, QgsSymbolRenderContext &context ) FINAL;
void renderPolygonStroke( const QPolygonF &points, const QVector<QPolygonF> *rings, QgsSymbolRenderContext &context ) FINAL;
QgsUnitTypes::RenderUnit outputUnit() const FINAL;
void setMapUnitScale( const QgsMapUnitScale &scale ) FINAL;
QgsMapUnitScale mapUnitScale() const FINAL;

0 comments on commit ef97e8c

Please sign in to comment.
You can’t perform that action at this time.