From 7b2d4a9db7f4bbef8800a0acbc0f80f466eba7d1 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 24 Oct 2023 14:26:49 +1000 Subject: [PATCH] When a vector tile style rule is set to "(all layers)", ensure that we correctly fetch the attributes required for the style's filter and labeling for all layers Fixes broken rendering of vector tile style rules set to (all layers) which use attribute filters --- .../vectortile/qgsvectortilemvtdecoder.cpp | 15 ++++++- tests/src/core/testqgsvectortilelayer.cpp | 42 ++++++++++++++++++ ...expected_render_test_filter_all_layers.png | Bin 0 -> 160537 bytes 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/testdata/control_images/vector_tile/expected_render_test_filter_all_layers/expected_render_test_filter_all_layers.png diff --git a/src/core/vectortile/qgsvectortilemvtdecoder.cpp b/src/core/vectortile/qgsvectortilemvtdecoder.cpp index f4847601ddc4..86af9d017df2 100644 --- a/src/core/vectortile/qgsvectortilemvtdecoder.cpp +++ b/src/core/vectortile/qgsvectortilemvtdecoder.cpp @@ -110,7 +110,20 @@ QgsVectorTileFeatures QgsVectorTileMVTDecoder::layerFeatures( const QMap layerFeatures; - const QgsFields layerFields = perLayerFields[layerName]; + QgsFields layerFields = perLayerFields[layerName]; + + const auto allLayerFields = perLayerFields.find( QString() ); + if ( allLayerFields != perLayerFields.end() ) + { + // need to add the fields from any "all layer" rules to every layer + for ( const QgsField &field : allLayerFields.value() ) + { + if ( layerFields.lookupField( field.name() ) == -1 ) + { + layerFields.append( field ); + } + } + } // figure out how field indexes in MVT encoding map to field indexes in QgsFields (we may not use all available fields) QHash tagKeyIndexToFieldIndex; diff --git a/tests/src/core/testqgsvectortilelayer.cpp b/tests/src/core/testqgsvectortilelayer.cpp index b12cb5bcb6a3..1683ffb25d8d 100644 --- a/tests/src/core/testqgsvectortilelayer.cpp +++ b/tests/src/core/testqgsvectortilelayer.cpp @@ -77,6 +77,7 @@ class TestQgsVectorTileLayer : public QgsTest void test_polygonWithMarker(); void test_styleMinZoomBeyondTileMaxZoom(); + void test_filterRuleAllLayers(); }; @@ -637,6 +638,47 @@ void TestQgsVectorTileLayer::test_styleMinZoomBeyondTileMaxZoom() QStringLiteral( "render_test_style_min_zoom" ), *mMapSettings, 0, 15 ) ); } +void TestQgsVectorTileLayer::test_filterRuleAllLayers() +{ + // test using a filter with field access for an "all layers" rule + QgsDataSourceUri ds; + ds.setParam( "type", "mbtiles" ); + ds.setParam( "url", QString( "/%1/mbtiles_vt.mbtiles" ).arg( mDataDir ) ); + std::unique_ptr< QgsVectorTileLayer > layer = std::make_unique< QgsVectorTileLayer >( ds.encodedUri(), "Vector Tiles Test" ); + QVERIFY( layer->isValid() ); + + mMapSettings->setLayers( QList() << layer.get() ); + + const QColor lineStrokeColor = Qt::blue; + const double lineStrokeWidth = DEFAULT_LINE_WIDTH * 4; + + QgsSimpleLineSymbolLayer *lineSymbolLayer = new QgsSimpleLineSymbolLayer; + lineSymbolLayer->setColor( lineStrokeColor ); + lineSymbolLayer->setWidth( lineStrokeWidth ); + QgsLineSymbol *lineSymbol = new QgsLineSymbol( QgsSymbolLayerList() << lineSymbolLayer ); + + QgsVectorTileBasicRendererStyle st( QStringLiteral( "Lines" ), QString(), Qgis::GeometryType::Line ); + st.setFilterExpression( QStringLiteral( "\"Name\"='Highway'" ) ); + st.setSymbol( lineSymbol ); + + QgsSimpleFillSymbolLayer *fillSymbolLayer = new QgsSimpleFillSymbolLayer; + fillSymbolLayer->setColor( Qt::white ); + fillSymbolLayer->setStrokeStyle( Qt::NoPen ); + QgsFillSymbol *fillSymbol = new QgsFillSymbol( QgsSymbolLayerList() << fillSymbolLayer ); + + QgsVectorTileBasicRendererStyle bgst( QStringLiteral( "background" ), QStringLiteral( "background" ), Qgis::GeometryType::Polygon ); + bgst.setSymbol( fillSymbol ); + + QgsVectorTileBasicRenderer *rend = new QgsVectorTileBasicRenderer; + rend->setStyles( QList() << bgst << st ); + layer->setRenderer( rend ); // takes ownership + + mMapSettings->setExtent( layer->extent() ); + mMapSettings->setDestinationCrs( layer->crs() ); + QVERIFY( renderMapSettingsCheck( QStringLiteral( "render_test_filter_all_layers" ), + QStringLiteral( "render_test_filter_all_layers" ), *mMapSettings, 0, 15 ) ); +} + QGSTEST_MAIN( TestQgsVectorTileLayer ) #include "testqgsvectortilelayer.moc" diff --git a/tests/testdata/control_images/vector_tile/expected_render_test_filter_all_layers/expected_render_test_filter_all_layers.png b/tests/testdata/control_images/vector_tile/expected_render_test_filter_all_layers/expected_render_test_filter_all_layers.png new file mode 100644 index 0000000000000000000000000000000000000000..f71919a57cdba937fd8c16cb575934f7c26e7315 GIT binary patch literal 160537 zcmeHQ3)odt8vdfBt1?X`N+}*QU0jlhN`K8?`9-9Rlo*#0lX006BPvFQ86snZ$51kc zicwJ{6%&orL=qC+rV*x#Zo2W!dE1V?*WTy)|7V}Q_c`zPJpX_8y6p9>^?v)^>$2D7 z?sIzes8zjDbrGpmc6#Z#vEP5;@8E-C|DWsk{Nu5oYS*29!4Q$!^YK^dFD+{yF49!W zO1t#Esgjh9nlZAH#v~SzLIj9F0t607;72baKm@)=fCS;AKm<}CK!T(Ihe1Su1mQD4 z1X3VCf}{Y4K}3KA;WI!4QXoKrqyUFOM1Tb0Ge87VAV7ko0EatoLhj?V?4 z@rXcv5r`OB@(T{N5dji}&j1lffdC1T0vrYr0TP7I01-%m011);90m~q5`@nH5lDdm z36cUF1`&bEgdi~jd-jMdS|l=Wp7qzcv&it_iR03@a1r40+a$J~IaB0}Gwi?)9Yp5G zRxwkG3jq=&iM_XO6{%ZSz2vw?xY}q1G9s2=fDfg|Yv6*uLvIH%J(b*qKVwBpo zMcTHt1NZI~S+Kx%^Qt%yAVE^txunDlMBju`TnLaLDeUbg$hdJSA~UEU5g2<} zVS-5a?xsIog@XWVkPK{zZz9vCiGUDJ*|0&RM-P$fuNV2|8)pcw3k3lZBtuI!Z!WTA ziA4zaE%)Cq(xi#V%P+e_qU`Qhcg-x$1#nuBWO1UA<{Guhj2R+h$C^XIxD9=ivNDlA zeMHu-O_LCVBZC0@Bjq&JvSlLu`-{B)zBd8PCDS#p-b z8I$xC0oEWs!aHV+$dDnnf&*f&qxI{(U+3)@yb2ovB#4g|!8-Q?4>T3vLnA&YQv5;S z*+aI)2GyHNS!tKPIoD7&YQ{*mxpEGxogk`^p+jvqu2!!$-E@T;0TRS#`!sE8Dj-2j zJi_iGL43B5n;@Tk=CeX3pHl=#ketHlP0US@Rja&l=o4N9*lYLMHamBECugBIkFSZ8 z@FTz)#Ah2}s&Au4rV@<(QlfzX36cjpoph3^WZ^>7O;>mkAVKnAn|AF?B_xQ6N0?nC zNFJ=?CJ4@7phN=!5+o0HI_@}A$%+-Go38L8;QJ`nF)g1z)3><*;l(v+&afLD&RqER zTPq4W@jG@Tjn04&A;8)s55~aou5*tjuPqF(^U4% zGp3ua@FBn&B#$|`Go*$5WkR3knWkW!Eg5tb2r|YAR@g& zg8&JVM^m7tz+RE!&^}R4Za{;h3cAGYKCa?np4kf{tcS^VrU~?+~d$ewC4*z00%9=F+WP~cyB0z$q zg~*=>yLX%E@{mLmkFdB%kbo@FvSmPusWvMFNRX`HV&q~&fCMQvi?h60AwYs;1y}AO zJ9ofFs{j3O?$TyTxkG>i$sLwirPQpM8U3rTOgCNOK!5}Z$PVrqy|>&FP)4XWB?2T! zN_hMWbZ_v0VcyA;{nN*k%ZC67Qa;=&eAme*i~RJbW-`C{g~+gB<`GtAjMT2__tdyQ zKXU<6=c7ti}mXdJtR`Ks+oa5{Yj*AXMSZjQ3=jPf&_>Cej>c^f|n%o=Zl

ehAo@H!s|kRbU$ zIZGYf86``=e2mT}mXF|;H)-sQ;udaU>P93#ZW1IvIA^70`*v^inl-(C^zk1762yOu zv(|9gW#%o%9AmCVG()4~!tr!blgd%caG~cHL%q|cnWg;oucLeJ;gTgH79K>5A* zdNb7mU%S+T!dh0DOR%6D5yA+!?sa|r4H{VWDRI8?X;f1QNy{7u@ zx2o&ctInOPdiL3>cn+L#-PcvBs9t%c>Sv$%g9BdFQ>LgkZR*WO|NfTF3KIkm_0vzQ z_UWTqx2{)_yU?lJ0CT_q)uBUGCr?&=`|aYXZAOgnVz+y@KMzJOQk^zU^{~Udd5+t+ zeS6hMA64D5CAqdQzf>JOSQRy8{JvzrUAm}_AFqnHeJ+g`m?moLvQ@2FW8 zEmFPhHWPzHI%?KbZPP~e&wsXRB9JT-L2ZHN{imO*cJHowzyXPM9eJecq)GORIctKT zT{v){YNbkv6(lND^X97YKZIAVocX=+^qMqLoiHI2D54q1-G*PeGJNGgGq_>H#I`&2 zRMn3@s(7C9Aft9V@x;V>;?INgw|;%qu3c3-b+Vf5efwHX{I+ea=kCx$lM^DI4qo)P z->$lIr&)%K34+ez%{QlSH-ZRPTw$^BtFKIDaTneg8#bseUab1~l?c0ph=n z8mXc=vVD6zwZiyyzcj)$kb%LLtFB7?z@kGvcC0G;pScv2AB4n%AGf_a6D!Ho_uY3@Z@ESFvdb)8$vXJ}s&Ql01q(7wIsEbB zBPj8QAYUUm>5xIQq(_g$wl691uLJGO7e1t5#n8Yt&GE z=pk!V%8u>-6>2iP6w&#=<{GPZLGvmTVxaE=f}u}~rWju6NH3ivAjrSIJa1ooE`WXe zs8Pu?^8E95r^dd+pQ{xstigd~pa2LIKFlLP>5zHyNfT>y95XRyx^&AMjv?qoBTX$@ zs4iVv-cd@bF6`#XP!}g9x{LV<@}cd{R({3ird@yi&3|&Tl1f%AT=f= z$h>)~_+aA%2R;6OQ_`<)^U=$4>a^pBUf6-wNvPSc-)TvXB4cr0{Tb3;o!AUS~dG%EhEUXTTU>*>* z1o{2%MKGTxm7H>lNaM!Vshc=?69)_)a)?Mti3q~tlhjhB;aAwUZ6cVDG-XO=8jx3< z`*zk@A~;F3RjZ2Ue?x4lEhY}W_@c8-e!=UtBPO!_{Sm-KP+%s^FYvJeXK~ga>oyPV4tS|FPQqAAjuKdnbspHg9hA z0Z@aZkA#{4M2ia^*g=CV7s>*Afh-_gu3iAAoo4lgQ2QVclphfAwm&fiB5Y&bezb!z zbsN*mvzY^wsI6XJKa3d7oar5DbEk(7bcqC`O0U0e&4fPsXs;dJ>A2mP!h`-Bz86pu zBM_^=0jw!^-YN3C-S+e;lni!Y=@x9hK1q-2a0s>9_sm9o+^^FpmSWM ziq%Soe_>*+;NAQi<@pr-$^&`pvw;gScCY1YL;of z|Gr3@Hs(z@OWM76mKlStd?3IYBp)cJ(XnNVHv-Aw#UuzH5+n%JT{=gNGCg|sG~ILs zhX4r@9QOM)<4D{&bIeR&$`2(R2(S+lpdJ4GZ;^KG>`98q3bsjq=N)T?ae!H)>hBR? z4HDp^aKjCEH2&y#doFu0wha7cEEIYcQb43SZjV(Kv|;N1PHLX zl>n4{cy;Y6GIy@63Cn6RwVe_!1W1qo?14#-&P7?s0@kx&+6g6G2(XD2fIToR-7O@C z8k(vYY+z*`M$)C(r>+GYQ^q# zW5$?%bOnb1Ymnfu|3Ndh+n+r72j~8?4wfvjPSLS_c@+c#tU-c6-K7(K4;+;h7mQ}z zbywWQ-yjiS4H6{!4&6BPWWWI11H--8z!&RtDZwGY8YDRE4bA=gn@3<_E((?$Qi4T* zHAt}d8`^PB!#U^JUKr6@x6b<+-j2bmj1V~KhEB2XvK2lffMm%C?fgW-lwTYu^2sN* zz7tLm!8R$BU=Uz)D;V4pYQ=W%Lxz|^7}{Y9%m;|@{0fol3%xKKz}`^$R1|>#2{L-voqx^2 zT!4xq5YhV|eN^O{YeaVMHVc3Q*~gFP`QIigxp4(ZmfRGe*iyk1l(ucXr5--qd%UhU z7JafsfCR}Fu@J;xzuww1e%m%PGpJqexksc*71N)t@*_Zklpni5@7%CKqcL?2?Aix?V6O_Q(xHP0Hl@R8mQ!}@5IOf;5p1SS$s7R^By+6x$9&|xuCWOq z72X#hOReRPj+)~i{}355!kfmRK_aic=JlsfVg%USN{mav_@cAkx35_oEa`pwZPSmg z#0ZceiE$|?-`TT8x^**)v~;OR>(-_pU0wv(-11^la6VYt>pr}F&YXfPrY{8|L41KV ze2Kbi5bUZ#Nsj;tk{+Xih;hUb1yPK=7lH)IJLJJA95yT%y;0Xnf<)bd8RUQYa_>q+ zY<)^ej{rxp(qk0q7}zldhl#X?&PpU@XA zva?>jx(HarAubz}bXRP1r9&v9;VV{{ zijo}yBuI96MJ6^5XLlafj*N4uVfN-DL9)jzGIwj+E30kRYk?DTq+)7gVd3S(L_&MLzjN1T_vN zIRb2MCC8>KYjuCM$IUGIOB2Jj|9dzE< z#0uI1dDM?x9NM%IS-H|yigOm`&GR0=YsV<=E7l-IWdCZ_L>_s>E-;3FFTU7z^U8|= zYY;Cc1@CjoCDs8WW5$RyXi)IQtxy3;kP5*U&71e_^X~8v&64<^YZAnNjHw~Q2(S+l zVdF9D{v(hTK?W}VW@hN-0{D*+H4uUQ2xLVTVnzgt4FM9Q*euTSW`zI=k`-KxTxUFiA|Ok}0OH-njQ|N!-1cYb(;z^CqydP*#f<<7 pQrz}u>C+%Uf}{b6!NrZhLp8Q-eCfDRR|AEzZoNvM@A}Kz{|}y5Prv{G literal 0 HcmV?d00001