From 1c7ba0211b4de162a83eed5dcbd68cd6e0538ed1 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 20 Apr 2018 15:03:40 -0700 Subject: [PATCH] Only run placement for first layer per SymbolBucket. Fixes issue #6548. Restores behavior from before PR #5150 -- all layers that share the same bucket (and thus same layout properties) share the same placement. Before this fix, two layers with the same layout properties could collide against each other, and because they shared CrossTileIDs, _both_ layers could end up hidden. --- src/symbol/cross_tile_symbol_index.js | 3 +- src/symbol/placement.js | 4 +- .../mapbox-gl-js#6548/expected.png | Bin 0 -> 1727 bytes .../regressions/mapbox-gl-js#6548/style.json | 62 ++++++++++++++++++ test/unit/symbol/cross_tile_symbol_index.js | 3 +- 5 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6548/expected.png create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6548/style.json diff --git a/src/symbol/cross_tile_symbol_index.js b/src/symbol/cross_tile_symbol_index.js index 609edcf70bb..b0bc86b56c3 100644 --- a/src/symbol/cross_tile_symbol_index.js +++ b/src/symbol/cross_tile_symbol_index.js @@ -259,7 +259,8 @@ class CrossTileSymbolIndex { for (const tile of tiles) { const symbolBucket = ((tile.getBucket(styleLayer): any): SymbolBucket); - if (!symbolBucket) continue; + if (!symbolBucket || styleLayer.id !== symbolBucket.layerIds[0]) + continue; if (!symbolBucket.bucketInstanceId) { symbolBucket.bucketInstanceId = ++this.maxBucketInstanceId; diff --git a/src/symbol/placement.js b/src/symbol/placement.js index 557401e4c25..c0e798c3af2 100644 --- a/src/symbol/placement.js +++ b/src/symbol/placement.js @@ -107,7 +107,7 @@ export class Placement { placeLayerTile(styleLayer: StyleLayer, tile: Tile, showCollisionBoxes: boolean, seenCrossTileIDs: { [string | number]: boolean }) { const symbolBucket = ((tile.getBucket(styleLayer): any): SymbolBucket); const bucketFeatureIndex = tile.latestFeatureIndex; - if (!symbolBucket || !bucketFeatureIndex) + if (!symbolBucket || !bucketFeatureIndex || styleLayer.id !== symbolBucket.layerIds[0]) return; const collisionBoxArray = tile.collisionBoxArray; @@ -306,7 +306,7 @@ export class Placement { for (const tile of tiles) { const symbolBucket = ((tile.getBucket(styleLayer): any): SymbolBucket); - if (symbolBucket && tile.latestFeatureIndex) { + if (symbolBucket && tile.latestFeatureIndex && styleLayer.id === symbolBucket.layerIds[0]) { this.updateBucketOpacities(symbolBucket, seenCrossTileIDs, tile.collisionBoxArray); } } diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6548/expected.png b/test/integration/render-tests/regressions/mapbox-gl-js#6548/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..0aa9204c050d1b05d762f1b352532d816300a233 GIT binary patch literal 1727 zcmah~ZB!D58ukN7L3AK~q0!VvVEBR84@?qC0X09sn9{W3Bkx=@73Y+!{D1^Sgn43# z-87V`cuYq}yHaCbP*F3dwoT`nC8DKk>GUj59+%too^$X0yK~;>ea`cq_dLIz^S-t8 zR2meF1l!oyKob+<)AzY@AF&Rg{r>G-Ma4eZCB~DP0^3)}@^5WyK!X3Mb<Ero>N*HT_} zcem8(9OdQC9rL;XeDsO~e9yb~C?Ea+(5|;FZ3PwV=B|9UBY={-o)u|LSYaqRGMx;` zAN@)`jZyMsE{R-r&f2K`PUO@`xa!&=4YeQ&;e;b&rMsDOzar3$IFm7{L9Oj9D)yTl z0ASOLf61~fWfLa&ZZCKqbC4&Sy~}Un4B~IfPST}qgU{(9H6xbv?>XISw}GYPCgKJS zO0|TbhMx-G*P~plM4XXssE(!|=|gmfIMcSmjx~(0@JSa!>?B*B8}4_8w)F>Q_26~F z0UF;vf0WR#K@G5PvqMp%xMEK9#Oq%jo2DVo?62AA5)E+Gmx8JoVJ*~c3k!JfwZ!=k z%CY*CkOLp%A&zp6e|{Y5jRxdj=&9iKlpmI#^TUjPzhu|XA*Gtncq%?u$!b|5qP*4a z!{tP##<9@?TbYqD^swRAObKL0yhm^=@tStjo|Wzb8$l&R=rP&l^eDk zvrrRcN}n8q;f&o{GYB`eRW@@PGk-Qy@#i-#OaB1t#IWdh~X|mUQB>D7pr8 ztNM}7HM7n8qDl_)6h2%V0bQY`z2SzPHRTYlT`<(Ps}%mM%1lNblqHVlk6%6v$`rmY z1T+iai*7io7m`u;!3rZ5wK^VV@iLvv9XT(Hi|T0gc3DAv zhD(q~4d5U%27yO)1AgE1%B~?MZNkfg=dYbnEsK5 zOA$@N5Fy!gtg0$J)KRMa>=LAF5};&b@*A*ZDTi=cmzkE5I*DG8exy>I(zym(vTUBfbg*w5PO`QM# zxbXhFv)|##e;cf}FDrw(IEsJ9u8!o5ab%;7bm1b$HQ!9^*Fe0t=ftTi^wxn0^6|OW zIJ6mkHNik?PT;$iSg%}hb!kIE81)wknCox;2-i6J^MU0@cYMT}EV`mvI~w7meCR!Y z`>}k9P`NnHRD1ntq^LwbV6BmPq{B6uE;6qf`{t|mb`TF93j@nOHpe8JXXkOJq-D7H z@P96;lh2aiW1Y67u^v`El%@DwJK~&2kzP2gTHU*hEM)`H>VvCoh-Tq7t2XJCG2;s@ zRd{K4z+;*RYcaYTL|ZCxyTB<@4Xzko#JhG9`2V6p(UyaT8brlVk$c-{QwM-*M?$mfPUpHI=<$rS_1wnO z^~X#e3{i#E4FODH2|-M*SFah@4+dk}v;$vSPAfUEL2)&P=3TOc>SrTwSPth=A6p*K zi|?DvPCg+$J|CzZ@-$RsGzG{^C`3+79aiAItsP<+?sVQ)T|->fWznng0{fob z66D|g6$4v-;Lv5f$H5(urTCTEckdDBm^0_gXB|8Ya!Wqxq+|~yyNy~u?Vn(BQB>=> m`QHDz{N<6~UwHlJ^NI}lphU#TTiE|dHi?wf_}&w2;lBZRf$hHl literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6548/style.json b/test/integration/render-tests/regressions/mapbox-gl-js#6548/style.json new file mode 100644 index 00000000000..1f80d158870 --- /dev/null +++ b/test/integration/render-tests/regressions/mapbox-gl-js#6548/style.json @@ -0,0 +1,62 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 64, + "width": 128 + } + }, + "center": [ + 0, + 0 + ], + "zoom": 0, + "sources": { + "point": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {}, + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + } + } + ] + } + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "layers": [ + { + "id": "text1", + "type": "symbol", + "source": "point", + "layout": { + "text-field": "Rendered Twice", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + } + }, + { + "id": "text2", + "type": "symbol", + "source": "point", + "layout": { + "text-field": "Rendered Twice", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + } + } + ] +} diff --git a/test/unit/symbol/cross_tile_symbol_index.js b/test/unit/symbol/cross_tile_symbol_index.js index 298e2437113..0e42dabb76b 100644 --- a/test/unit/symbol/cross_tile_symbol_index.js +++ b/test/unit/symbol/cross_tile_symbol_index.js @@ -16,7 +16,8 @@ function makeSymbolInstance(x, y, key) { function makeTile(tileID, symbolInstances) { const bucket = { - symbolInstances: symbolInstances + symbolInstances: symbolInstances, + layerIds: ['test'] }; return { tileID: tileID,