Skip to content

Commit

Permalink
do not hide icons if text is an empty string (#6164)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ansis authored Feb 14, 2018
1 parent 87bec2a commit eb90e09
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 14 deletions.
24 changes: 12 additions & 12 deletions src/data/bucket/symbol_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ export type CollisionArrays = {
textCircles?: Array<number>;
};

export type SymbolFeature = {|
text: string | void,
icon: string | void,
index: number,
sourceLayerIndex: number,
geometry: Array<Array<Point>>,
properties: Object,
type: 'Point' | 'LineString' | 'Polygon',
id?: any
|};

export type SymbolInstance = {
key: string,
textBoxStartIndex: number,
Expand All @@ -76,7 +87,7 @@ export type SymbolInstance = {
anchor: Anchor,
line: Array<Point>,
featureIndex: number,
feature: ExpressionFeature,
feature: SymbolFeature,
textCollisionFeature?: {boxStartIndex: number, boxEndIndex: number},
iconCollisionFeature?: {boxStartIndex: number, boxEndIndex: number},
placedTextSymbolIndices: Array<number>;
Expand All @@ -92,17 +103,6 @@ export type SymbolInstance = {
hidden?: boolean;
};

export type SymbolFeature = {|
text: string | void,
icon: string | void,
index: number,
sourceLayerIndex: number,
geometry: Array<Array<Point>>,
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
Expand Down
4 changes: 2 additions & 2 deletions src/symbol/placement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -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
]
}
}
]
}

0 comments on commit eb90e09

Please sign in to comment.