From eb90e09f7148d56c636412912e5de6214934b329 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 14 Feb 2018 13:53:10 -0500 Subject: [PATCH] do not hide icons if text is an empty string (#6164) Empty text strings do not have any collision boxes so the collision index check was never done and the text was not counted as "placed". This fixes that by making "placed" the default when the feature has text. fix #6160 --- src/data/bucket/symbol_bucket.js | 24 +++--- src/symbol/placement.js | 4 +- .../mapbox-gl-js#6160/expected.png | Bin 0 -> 1701 bytes .../regressions/mapbox-gl-js#6160/style.json | 73 ++++++++++++++++++ 4 files changed, 87 insertions(+), 14 deletions(-) create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6160/expected.png create mode 100644 test/integration/render-tests/regressions/mapbox-gl-js#6160/style.json diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index cadf77f1d00..00c0ebe44d7 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -65,6 +65,17 @@ export type CollisionArrays = { textCircles?: Array; }; +export type SymbolFeature = {| + text: string | void, + icon: string | void, + index: number, + sourceLayerIndex: number, + geometry: Array>, + properties: Object, + type: 'Point' | 'LineString' | 'Polygon', + id?: any +|}; + export type SymbolInstance = { key: string, textBoxStartIndex: number, @@ -76,7 +87,7 @@ export type SymbolInstance = { anchor: Anchor, line: Array, featureIndex: number, - feature: ExpressionFeature, + feature: SymbolFeature, textCollisionFeature?: {boxStartIndex: number, boxEndIndex: number}, iconCollisionFeature?: {boxStartIndex: number, boxEndIndex: number}, placedTextSymbolIndices: Array; @@ -92,17 +103,6 @@ export type SymbolInstance = { hidden?: boolean; }; -export type SymbolFeature = {| - text: string | void, - icon: string | void, - index: number, - sourceLayerIndex: number, - geometry: Array>, - properties: Object, - type: 'Point' | 'LineString' | 'Polygon', - id?: any -|}; - // Opacity arrays are frequently updated but don't contain a lot of information, so we pack them // tight. Each Uint32 is actually four duplicate Uint8s for the four corners of a glyph // 7 bits are for the current opacity, and the lowest bit is the target opacity diff --git a/src/symbol/placement.js b/src/symbol/placement.js index eef09402770..7de69bbabb0 100644 --- a/src/symbol/placement.js +++ b/src/symbol/placement.js @@ -118,8 +118,8 @@ class Placement { for (const symbolInstance of bucket.symbolInstances) { if (!seenCrossTileIDs[symbolInstance.crossTileID]) { - let placeText = false; - let placeIcon = false; + let placeText = symbolInstance.feature.text !== undefined; + let placeIcon = symbolInstance.feature.icon !== undefined; let offscreen = true; let placedGlyphBoxes = null; diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6160/expected.png b/test/integration/render-tests/regressions/mapbox-gl-js#6160/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..be74a15a32ffb7990dcebfc4e0936044573c361a GIT binary patch literal 1701 zcmY*adpHwn7~f{Ijk$Xym${sY+~Qa!kzrHQ$efT{$i1+!j8n{-OQl1P<0QGJ6RDV$ z>qxB>=akDHVk=gQ z*R`zPnmT3g^{_D21=raSdFoo-UCl%{bG`Z;5yOG{fyq$J0zjX`y(YZ>3&mh{%qA$g(CrD|rK9ydz3Z4$be`Z9XxFFZ>9GS0Z@$bZ5iS2V2@dBwCb z7vFVA zj>)6Z&Eg=-fQnlx@&-a{(JFVZh|2wmp&SCyG+=uu-buCif=8hdeL?^*huvDW$YS;9 zm46QPa0=v~bp`;zs#qI!_cv@|!^MFOrHv?;wpeT11I{WSm>~lIf?b-l27g zM6b9gu>(a@s^A0qhDeyVHI0v@WwmFnZ`xALs6t}F$r=06zd9Nfvwp5kYfizoPrdSR zO7&__Xw%R|mM^fZ_jzhd(qYb@P$SKK-tnCg4+_|=u56CHXHBqu+ALzpMw}UrkRmA{ z2jvP7UGb4}hAyvxSd+sFG=7N%9TNP$JL$v!kH`?t+;r zkCX7D!!lmH0x}I~(MRp*xRz_pTEe-_TRPj$~ zVZbi!xEWs1K$Y;29=`BGcMVh`L&hf#m>;DK9dp6kT2#+4iB>1Dl|_RD6uJBFX#e_CQB(b-Q@*s*-x z_-&`EAF2t>eus+doT}VoZIB|MJ2roD8lWLP#yfF8<+^|~x&Hd~h|8!|7wLS~G{>{{ z$ETT}V!zaUO+-mtWjg2FeBfWJ;u_hQzb&%*#w@*j{YGX&EIr1+F*{Js8W1QVe5+P= zg3NMH@frfufLgvoQ~*# zWhhE_8~D)I*hs2!?=a`gm`}_oC4`6(j#+i-x7qiSPCwsW-VxOq|DFB9yvEr=_Fw~2brz7 zl+CC98rM18c;d=e$rw2#jJkjWlpBp+chC>OV2Gtg2zMrn)coMX<_DdsK4^d#NaYO%lP zdgk}}CVmNO)_%P-uCB5rui0e8YZnm{YLkG`r3JU%Y$YiL>H7WuQQyhdKT7vWcX5rf To|*n8`*#2Yyf=>J7S8+!3^519 literal 0 HcmV?d00001 diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#6160/style.json b/test/integration/render-tests/regressions/mapbox-gl-js#6160/style.json new file mode 100644 index 00000000000..693bec2bd7f --- /dev/null +++ b/test/integration/render-tests/regressions/mapbox-gl-js#6160/style.json @@ -0,0 +1,73 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 64, + "description": "Checks that icons are not hidden when the text is an empty string" + } + }, + "glyphs": "local://glyphs/{fontstack}/{range}.pbf", + "sources": { + "geojson": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + }, + "properties": { + "text": "OK" + } + }, + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 20, + 0 + ] + }, + "properties": { + "text": "" + } + } + ] + } + } + }, + "sprite": "local://sprites/sprite", + "layers": [ + { + "id": "symbol", + "type": "symbol", + "source": "geojson", + "layout": { + "symbol-placement": "point", + "text-allow-overlap": true, + "icon-allow-overlap": true, + "icon-image": "triangle-12", + "text-field": "{text}", + "text-font": [ + "Open Sans Semibold", + "Arial Unicode MS Bold" + ] + }, + "paint": { + "icon-opacity": 0.5, + "text-translate": [ + -10, + 0 + ] + } + } + ] +}