diff --git a/src/symbol/collision_index.js b/src/symbol/collision_index.js index c1ea7aaf8eb..587f5a27a94 100644 --- a/src/symbol/collision_index.js +++ b/src/symbol/collision_index.js @@ -297,17 +297,17 @@ class CollisionIndex { return result; } - insertCollisionBox(collisionBox: Array, ignorePlacement: boolean, bucketInstanceId: number, featureIndex: number, collisionGroup: number) { + insertCollisionBox(collisionBox: Array, ignorePlacement: boolean, bucketInstanceId: number, featureIndex: number, collisionGroupID: number) { const grid = ignorePlacement ? this.ignoredGrid : this.grid; - const key = { bucketInstanceId: bucketInstanceId, featureIndex: featureIndex, collisionGroup: collisionGroup }; + const key = { bucketInstanceId: bucketInstanceId, featureIndex: featureIndex, collisionGroupID: collisionGroupID }; grid.insert(key, collisionBox[0], collisionBox[1], collisionBox[2], collisionBox[3]); } - insertCollisionCircles(collisionCircles: Array, ignorePlacement: boolean, bucketInstanceId: number, featureIndex: number, collisionGroup: number) { + insertCollisionCircles(collisionCircles: Array, ignorePlacement: boolean, bucketInstanceId: number, featureIndex: number, collisionGroupID: number) { const grid = ignorePlacement ? this.ignoredGrid : this.grid; - const key = { bucketInstanceId: bucketInstanceId, featureIndex: featureIndex, collisionGroup: collisionGroup }; + const key = { bucketInstanceId: bucketInstanceId, featureIndex: featureIndex, collisionGroupID: collisionGroupID }; for (let k = 0; k < collisionCircles.length; k += 4) { grid.insertCircle(key, collisionCircles[k], collisionCircles[k + 1], collisionCircles[k + 2]); } diff --git a/src/symbol/grid_index.js b/src/symbol/grid_index.js index 3f2e21a9964..174d71fa0dd 100644 --- a/src/symbol/grid_index.js +++ b/src/symbol/grid_index.js @@ -117,17 +117,15 @@ class GridIndex { y2: y + radius }); } - if (predicate) { - return result.filter(predicate); - } + return predicate ? result.filter(predicate) : result; } else { const queryArgs = { hitTest, seenUids: { box: {}, circle: {} } }; this._forEachCell(x1, y1, x2, y2, this._queryCell, result, queryArgs, predicate); + return hitTest ? result.length > 0 : result; } - return hitTest ? result.length > 0 : result; } _queryCircle(x: number, y: number, radius: number, hitTest: boolean, predicate?: any) { diff --git a/src/symbol/placement.js b/src/symbol/placement.js index 0a3671d06d8..76ccba9de80 100644 --- a/src/symbol/placement.js +++ b/src/symbol/placement.js @@ -83,40 +83,34 @@ export class RetainedQueryData { } class CollisionGroups { - collisionGroups: { [layerId: ?string]: { ID: number, predicate?: any }}; + collisionGroups: { [groupName: string]: { ID: number, predicate?: any }}; maxGroupID: number; + crossSourceCollisions: boolean; - constructor() { + constructor(crossSourceCollisions: boolean) { + this.crossSourceCollisions = crossSourceCollisions; this.maxGroupID = 0; - this.collisionGroups = { - undefined: { - ID: 0, - predicate: null - } - }; + this.collisionGroups = {}; } - get(groupName?: string) { - if (groupName && this.maxGroupID === 0) { - // Keep the predicate null until the first collision - // group gets added to avoid overhead in the case - // everything's in the default group. - this.collisionGroups[undefined].predicate = - (key) => { - return key.collisionGroup === 0; + get(sourceID: string) { + // The predicate/groupID mechanism allows for arbitrary grouping, + // but the current interface defines one source == one group when + // crossSourceCollisions == true. + if (!this.crossSourceCollisions) { + if (!this.collisionGroups[sourceID]) { + const nextGroupID = ++this.maxGroupID; + this.collisionGroups[sourceID] = { + ID: nextGroupID, + predicate: (key) => { + return key.collisionGroupID === nextGroupID; + } }; + } + return this.collisionGroups[sourceID]; + } else { + return { ID: 0, predicate: null }; } - - if (!this.collisionGroups[groupName]) { - const nextGroupID = ++this.maxGroupID; - this.collisionGroups[groupName] = { - ID: nextGroupID, - predicate: (key) => { - return key.collisionGroup === nextGroupID; - } - }; - } - return this.collisionGroups[groupName]; } } @@ -132,7 +126,6 @@ export class Placement { fadeDuration: number; retainedQueryData: {[number]: RetainedQueryData}; collisionGroups: CollisionGroups; - crossSourceCollisions: boolean; constructor(transform: Transform, fadeDuration: number, crossSourceCollisions: boolean) { this.transform = transform.clone(); @@ -142,8 +135,7 @@ export class Placement { this.stale = false; this.fadeDuration = fadeDuration; this.retainedQueryData = {}; - this.collisionGroups = new CollisionGroups(); - this.crossSourceCollisions = crossSourceCollisions; + this.collisionGroups = new CollisionGroups(crossSourceCollisions); } placeLayerTile(styleLayer: StyleLayer, tile: Tile, showCollisionBoxes: boolean, seenCrossTileIDs: { [string | number]: boolean }) { @@ -197,9 +189,7 @@ export class Placement { const iconWithoutText = !bucket.hasTextData() || layout.get('text-optional'); const textWithoutIcon = !bucket.hasIconData() || layout.get('icon-optional'); - const collisionGroup = this.crossSourceCollisions ? - this.collisionGroups.get() : - this.collisionGroups.get(bucket.sourceID); + const collisionGroup = this.collisionGroups.get(bucket.sourceID); for (const symbolInstance of bucket.symbolInstances) { if (!seenCrossTileIDs[symbolInstance.crossTileID]) { diff --git a/test/integration/render-tests/icon-collision-group/default/expected.png b/test/integration/render-tests/icon-no-cross-source-collision/default/expected.png similarity index 100% rename from test/integration/render-tests/icon-collision-group/default/expected.png rename to test/integration/render-tests/icon-no-cross-source-collision/default/expected.png diff --git a/test/integration/render-tests/icon-collision-group/default/style.json b/test/integration/render-tests/icon-no-cross-source-collision/default/style.json similarity index 94% rename from test/integration/render-tests/icon-collision-group/default/style.json rename to test/integration/render-tests/icon-no-cross-source-collision/default/style.json index adf78059c81..b88ab61d71b 100644 --- a/test/integration/render-tests/icon-collision-group/default/style.json +++ b/test/integration/render-tests/icon-no-cross-source-collision/default/style.json @@ -72,7 +72,7 @@ "sprite": "local://sprites/sprite", "layers": [ { - "id": "defaultGroup1", + "id": "source1Group1", "type": "symbol", "source": "source1", "layout": { @@ -81,7 +81,7 @@ } }, { - "id": "defaultGroup2", + "id": "source1Group2", "type": "symbol", "source": "source1", "layout": { @@ -90,7 +90,7 @@ } }, { - "id": "firstGroup1", + "id": "source2Group1", "type": "symbol", "source": "source2", "layout": { @@ -103,7 +103,7 @@ } }, { - "id": "firstGroup2", + "id": "source2Group2", "type": "symbol", "source": "source2", "layout": { @@ -116,7 +116,7 @@ } }, { - "id": "secondGroup1", + "id": "source3Group1", "type": "symbol", "source": "source3", "layout": { @@ -129,7 +129,7 @@ } }, { - "id": "secondGroup2", + "id": "source3Group2", "type": "symbol", "source": "source3", "layout": { diff --git a/test/integration/render-tests/text-collision-group/default/expected.png b/test/integration/render-tests/text-collision-group/default/expected.png deleted file mode 100644 index a84431f4cd5..00000000000 Binary files a/test/integration/render-tests/text-collision-group/default/expected.png and /dev/null differ diff --git a/test/integration/render-tests/text-no-cross-source-collision/default/expected.png b/test/integration/render-tests/text-no-cross-source-collision/default/expected.png new file mode 100644 index 00000000000..84fe9793f47 Binary files /dev/null and b/test/integration/render-tests/text-no-cross-source-collision/default/expected.png differ diff --git a/test/integration/render-tests/text-collision-group/default/style.json b/test/integration/render-tests/text-no-cross-source-collision/default/style.json similarity index 91% rename from test/integration/render-tests/text-collision-group/default/style.json rename to test/integration/render-tests/text-no-cross-source-collision/default/style.json index 8415c3258ac..2a3db9fd462 100644 --- a/test/integration/render-tests/text-collision-group/default/style.json +++ b/test/integration/render-tests/text-no-cross-source-collision/default/style.json @@ -120,11 +120,11 @@ "glyphs": "local://glyphs/{fontstack}/{range}.pbf", "layers": [ { - "id": "defaultGroup1", + "id": "source1Group1", "type": "symbol", "source": "source1", "layout": { - "text-field": "Default Group {name}", + "text-field": "Source1 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold", @@ -133,11 +133,11 @@ } }, { - "id": "defaultGroup2", + "id": "source1Group2", "type": "symbol", "source": "source1", "layout": { - "text-field": "2nd Layer Default Group {name}", + "text-field": "2nd Layer Source1 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold", @@ -146,11 +146,11 @@ } }, { - "id": "firstGroup1", + "id": "source2Group1", "type": "symbol", "source": "source2", "layout": { - "text-field": "First Group {name}", + "text-field": "Source2 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold", @@ -163,11 +163,11 @@ } }, { - "id": "firstGroup2", + "id": "source2Group2", "type": "symbol", "source": "source2", "layout": { - "text-field": "2nd Layer First Group {name}", + "text-field": "2nd Layer Source2 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold", @@ -180,11 +180,11 @@ } }, { - "id": "secondGroup1", + "id": "source3Group1", "type": "symbol", "source": "source3", "layout": { - "text-field": "Second Group {name}", + "text-field": "Source3 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold", @@ -197,11 +197,11 @@ } }, { - "id": "secondGroup2", + "id": "ssource3Group2", "type": "symbol", "source": "source3", "layout": { - "text-field": "2nd Layer Second Group {name}", + "text-field": "2nd Layer Source3 Group {name}", "text-max-width": 30, "text-font": [ "Open Sans Semibold",