Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[vector tile] Fix issue with disappearing features (fixes #36982)
The issue was that renderer/labeling were only keeping "their" required fields,
but the final fields may have been extended by the other, making the field indices
cached in expressions invalid.

The fix is to keep the final QgsFields around and avoid creating those QgsFields on-the-fly
from previously requested fields.
  • Loading branch information
wonder-sk committed Jun 11, 2020
1 parent 60fe81a commit b2da49b
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 12 deletions.
Expand Up @@ -42,6 +42,15 @@ Sets polygon of the tile
QPolygon tilePolygon() const;
%Docstring
Returns polygon (made out of four corners of the tile) in screen coordinates calculated from render context
%End

void setFields( const QMap<QString, QgsFields> &fields );
%Docstring
Sets per-layer fields
%End
QMap<QString, QgsFields> fields() const;
%Docstring
Returns per-layer fields
%End

QStringList layers() const;
Expand Down
8 changes: 4 additions & 4 deletions src/core/vectortile/qgsvectortilebasiclabeling.cpp
Expand Up @@ -143,9 +143,9 @@ QMap<QString, QSet<QString> > QgsVectorTileBasicLabelProvider::usedAttributes( c
return requiredFields;
}

void QgsVectorTileBasicLabelProvider::setFields( const QMap<QString, QSet<QString> > &requiredFields )
void QgsVectorTileBasicLabelProvider::setFields( const QMap<QString, QgsFields> &perLayerFields )
{
mRequiredFields = requiredFields;
mPerLayerFields = perLayerFields;
}

QList<QgsAbstractLabelProvider *> QgsVectorTileBasicLabelProvider::subProviders()
Expand All @@ -167,7 +167,7 @@ bool QgsVectorTileBasicLabelProvider::prepare( QgsRenderContext &context, QSet<Q
// populate sub-providers
for ( int i = 0; i < mSubProviders.count(); ++i )
{
QgsFields fields = QgsVectorTileUtils::makeQgisFields( mRequiredFields[mStyles[i].layerName()] );
QgsFields fields = mPerLayerFields[mStyles[i].layerName()];

QgsExpressionContextScope *scope = new QgsExpressionContextScope( QObject::tr( "Layer" ) ); // will be deleted by popper
scope->setFields( fields );
Expand All @@ -194,7 +194,7 @@ void QgsVectorTileBasicLabelProvider::registerTileFeatures( const QgsVectorTileR
if ( !layerStyle.isActive( zoomLevel ) )
continue;

QgsFields fields = QgsVectorTileUtils::makeQgisFields( mRequiredFields[layerStyle.layerName()] );
QgsFields fields = mPerLayerFields[layerStyle.layerName()];

QgsExpressionContextScope *scope = new QgsExpressionContextScope( QObject::tr( "Layer" ) ); // will be deleted by popper
scope->setFields( fields );
Expand Down
4 changes: 2 additions & 2 deletions src/core/vectortile/qgsvectortilebasiclabeling.h
Expand Up @@ -150,7 +150,7 @@ class QgsVectorTileBasicLabelProvider : public QgsVectorTileLabelProvider
// virtual functions from QgsVectorTileLabelProvider
void registerTileFeatures( const QgsVectorTileRendererData &tile, QgsRenderContext &context ) override;
QMap<QString, QSet<QString> > usedAttributes( const QgsRenderContext &context, int tileZoom ) const override;
void setFields( const QMap<QString, QSet<QString>> &requiredFields ) override;
void setFields( const QMap<QString, QgsFields> &perLayerFields ) override;

private:
QList<QgsVectorTileBasicLabelingStyle> mStyles;
Expand All @@ -160,7 +160,7 @@ class QgsVectorTileBasicLabelProvider : public QgsVectorTileLabelProvider

public:
//! Names of required fields for each sub-layer (only valid between startRender/stopRender calls)
QMap<QString, QSet<QString> > mRequiredFields;
QMap<QString, QgsFields> mPerLayerFields;
};

/// @endcond
Expand Down
4 changes: 1 addition & 3 deletions src/core/vectortile/qgsvectortilebasicrenderer.cpp
Expand Up @@ -151,10 +151,8 @@ void QgsVectorTileBasicRenderer::renderTile( const QgsVectorTileRendererData &ti
if ( !layerStyle.isActive( zoomLevel ) )
continue;

QgsFields fields = QgsVectorTileUtils::makeQgisFields( mRequiredFields[layerStyle.layerName()] );

QgsExpressionContextScope *scope = new QgsExpressionContextScope( QObject::tr( "Layer" ) ); // will be deleted by popper
scope->setFields( fields );
scope->setFields( tile.fields()[layerStyle.layerName()] );
QgsExpressionContextScopePopper popper( context.expressionContext(), scope );

QgsExpression filterExpression( layerStyle.filterExpression() );
Expand Down
4 changes: 2 additions & 2 deletions src/core/vectortile/qgsvectortilelabeling.h
Expand Up @@ -39,8 +39,8 @@ class QgsVectorTileLabelProvider : public QgsVectorLayerLabelProvider
//! Returns field names for each sub-layer that are required for labeling
virtual QMap<QString, QSet<QString> > usedAttributes( const QgsRenderContext &context, int tileZoom ) const = 0;

//! Sets required fields
virtual void setFields( const QMap<QString, QSet<QString>> &requiredFields ) = 0;
//! Sets fields for each sub-layer
virtual void setFields( const QMap<QString, QgsFields> &perLayerFields ) = 0;

//! Registers label features for given tile to the labeling engine
virtual void registerTileFeatures( const QgsVectorTileRendererData &tile, QgsRenderContext &context ) = 0;
Expand Down
3 changes: 2 additions & 1 deletion src/core/vectortile/qgsvectortilelayerrenderer.cpp
Expand Up @@ -137,7 +137,7 @@ bool QgsVectorTileLayerRenderer::render()

if ( mLabelProvider )
{
mLabelProvider->setFields( requiredFields );
mLabelProvider->setFields( mPerLayerFields );
QSet<QString> attributeNames; // we don't need this - already got referenced columns in provider constructor
if ( !mLabelProvider->prepare( ctx, attributeNames ) )
{
Expand Down Expand Up @@ -197,6 +197,7 @@ void QgsVectorTileLayerRenderer::decodeAndDrawTile( const QgsVectorTileRawData &
QgsCoordinateTransform ct = ctx.coordinateTransform();

QgsVectorTileRendererData tile( rawTile.id );
tile.setFields( mPerLayerFields );
tile.setFeatures( decoder.layerFeatures( mPerLayerFields, ct ) );
tile.setTilePolygon( QgsVectorTileUtils::tilePolygon( rawTile.id, ct, mTileMatrix, ctx.mapToPixel() ) );

Expand Down
7 changes: 7 additions & 0 deletions src/core/vectortile/qgsvectortilerenderer.h
Expand Up @@ -48,6 +48,11 @@ class CORE_EXPORT QgsVectorTileRendererData
//! Returns polygon (made out of four corners of the tile) in screen coordinates calculated from render context
QPolygon tilePolygon() const { return mTilePolygon; }

//! Sets per-layer fields
void setFields( const QMap<QString, QgsFields> &fields ) { mFields = fields; }
//! Returns per-layer fields
QMap<QString, QgsFields> fields() const { return mFields; }

//! Sets features of the tile
void setFeatures( const QgsVectorTileFeatures &features ) SIP_SKIP { mFeatures = features; }
//! Returns features of the tile grouped by sub-layer names
Expand All @@ -60,6 +65,8 @@ class CORE_EXPORT QgsVectorTileRendererData
private:
//! Position of the tile in the tile matrix set
QgsTileXYZ mId;
//! Per-layer fields
QMap<QString, QgsFields> mFields;
//! Features of the tile grouped into sub-layers
QgsVectorTileFeatures mFeatures;
//! Polygon (made out of four corners of the tile) in screen coordinates calculated from render context
Expand Down

0 comments on commit b2da49b

Please sign in to comment.