Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

Commit

Permalink
HARP-14257: Fix geojson stress example (#2122)
Browse files Browse the repository at this point in the history
* HARP-14257: Disable label limit by default.

Also, remove unused option 'SecondChanceLabels'.

* HARP-14257: GeoJsonTiler generates feature ids if needed.

GeoJsonTiler will generate feature ids if input data doesn't have them.
They're useful for picking, text element deduplication and replacement.
  • Loading branch information
atomicsulfate committed Feb 22, 2021
1 parent 1ec0df4 commit e3a9ec3
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 37 deletions.
12 changes: 12 additions & 0 deletions @here/harp-datasource-protocol/lib/GeoJsonDataType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ export type FeatureGeometry =
| Polygon
| MultiPolygon;

export function isFeatureGeometry(object: any): object is FeatureGeometry {
const t = object.type;
return (
t === "Point" ||
t === "MultiPoint" ||
t === "LineString" ||
t === "MultiLineString" ||
t === "Polygon" ||
t === "MultiPolygon"
);
}

/**
* Represents "GeometryCollection" GeoJSON geometry object.
*/
Expand Down
14 changes: 11 additions & 3 deletions @here/harp-mapview-decoder/lib/GeoJsonTiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { GeoJson, ITiler } from "@here/harp-datasource-protocol";
import { GeoJson, isFeatureGeometry, ITiler } from "@here/harp-datasource-protocol";
import { TileKey } from "@here/harp-geoutils";
// @ts-ignore
import * as geojsonvtExport from "geojson-vt";
Expand Down Expand Up @@ -55,11 +55,19 @@ export class GeoJsonTiler implements ITiler {
`GeoJsonTiler: Unable to fetch ${input.href}: ${response.statusText}`
);
}
input = await response.json();
input = (await response.json()) as GeoJson;
} else {
input = input as GeoJson;
}

// Generate ids only if input doesn't have them.
const generateId =
isFeatureGeometry(input) ||
input.type === "GeometryCollection" ||
(input.type === "Feature" && input.id === undefined) ||
(input.type === "FeatureCollection" &&
input.features.length > 0 &&
input.features[0].id === undefined);
const index = geojsonvt(input, {
maxZoom: 20, // max zoom to preserve detail on
indexMaxZoom: 5, // max zoom in the tile index
Expand All @@ -69,7 +77,7 @@ export class GeoJsonTiler implements ITiler {
buffer: BUFFER, // tile buffer on each side
lineMetrics: false, // whether to calculate line metrics
promoteId: null, // name of a feature property to be promoted to feature.id
generateId: false, // whether to generate feature ids. Cannot be used with promoteId
generateId, // whether to generate feature ids. Cannot be used with promoteId
debug: 0 // logging level (0, 1 or 2)
});
index.geojson = input;
Expand Down
61 changes: 58 additions & 3 deletions @here/harp-mapview-decoder/test/GeoJsonTilerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { expect } from "chai";

import { GeoJsonTiler } from "../lib/GeoJsonTiler";

const featureCollection: FeatureCollection = {
const featureCollectionWithIds: FeatureCollection = {
type: "FeatureCollection",
features: [
{
Expand Down Expand Up @@ -55,6 +55,48 @@ const featureCollection: FeatureCollection = {
]
};

const featureCollectionWithoutIds: FeatureCollection = {
type: "FeatureCollection",
features: [
{
type: "Feature",
properties: {},

geometry: {
type: "Point",
coordinates: [1, 2]
}
},
{
type: "Feature",
properties: {},

geometry: {
type: "LineString",
coordinates: [
[1, 2],
[3, 4]
]
}
},
{
type: "Feature",
properties: {},

geometry: {
type: "Polygon",
coordinates: [
[
[1, 2],
[3, 4],
[5, 6]
]
]
}
}
]
};

describe("GeoJsonTiler", function () {
let tiler: GeoJsonTiler;

Expand All @@ -64,15 +106,28 @@ describe("GeoJsonTiler", function () {

it("returns features with their original geojson ids", async function () {
const indexId = "dummy";
await tiler.registerIndex(indexId, featureCollection);
await tiler.registerIndex(indexId, featureCollectionWithIds);

const tile = (await tiler.getTile(indexId, new TileKey(0, 0, 1))) as any;

expect(tile.features).has.lengthOf(3);
const expectedFeatureIds = featureCollection.features.map(feature => feature.id);
const expectedFeatureIds = featureCollectionWithIds.features.map(feature => feature.id);
const actualFeatureIds: string[] = tile.features.map(
(feature: { id: string }) => feature.id
);
expect(actualFeatureIds).has.members(expectedFeatureIds);
});

it("generates feature ids if input geojson doesn't have them", async function () {
const indexId = "dummy";
await tiler.registerIndex(indexId, featureCollectionWithoutIds);

const tile = (await tiler.getTile(indexId, new TileKey(0, 0, 1))) as any;

expect(tile.features).has.lengthOf(3);
const actualFeatureIds: Array<number | undefined> = tile.features.map(
(feature: { id: number }) => feature.id
);
expect(actualFeatureIds).to.not.include(undefined);
});
});
8 changes: 4 additions & 4 deletions @here/harp-mapview/lib/text/TextElementsRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ export class TextElementsRenderer {
private placeTextElementGroup(
groupState: TextElementGroupState,
renderParams: RenderParams,
maxNumPlacedLabels: number,
maxNumPlacedLabels: number | undefined,
pass: Pass
): boolean {
// Unvisited text elements are never placed.
Expand All @@ -858,7 +858,7 @@ export class TextElementsRenderer {
}
// Limit labels only in new labels pass (Pass.NewLabels).
else if (
maxNumPlacedLabels >= 0 &&
maxNumPlacedLabels !== undefined &&
renderParams.numRenderedTextElements >= maxNumPlacedLabels
) {
logger.debug("Placement label limit exceeded.");
Expand Down Expand Up @@ -1455,7 +1455,7 @@ export class TextElementsRenderer {
if (this.m_forceNewLabelsPass) {
this.m_forceNewLabelsPass = false;
}
const maxNumPlacedTextElements = this.m_options.maxNumVisibleLabels!;
const maxNumPlacedTextElements = this.m_options.maxNumVisibleLabels;

// TODO: HARP-7648. Potential performance improvement. Place persistent labels + rejected
// candidates from previous frame if there's been no placement in this one.
Expand Down Expand Up @@ -1522,7 +1522,7 @@ export class TextElementsRenderer {
!this.placeTextElementGroup(
groupStates[i],
renderParams,
this.m_options.maxNumVisibleLabels!,
this.m_options.maxNumVisibleLabels,
Pass.NewLabels
)
) {
Expand Down
28 changes: 1 addition & 27 deletions @here/harp-mapview/lib/text/TextElementsRendererOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Default number of labels/POIs rendered in the scene
*/
const DEFAULT_MAX_NUM_RENDERED_TEXT_ELEMENTS = 500;

/**
* Number of elements that are put into second queue. This second chance queue is used to render
* TextElements that have not been on screen before. This is a quick source for elements that can
* appear when the camera moves a bit, before new elements are placed.
*/
const DEFAULT_MAX_NUM_SECOND_CHANCE_ELEMENTS = 300;

/**
* Maximum distance for text labels expressed as a ratio of distance to from the camera (0) to the
* far plane (1.0). May be synchronized with fog value ?
Expand Down Expand Up @@ -65,16 +53,9 @@ export interface TextElementsRendererOptions {
/**
* Limits the number of {@link DataSource} labels visible, such as road names and POIs.
* On small devices, you can reduce this number to to increase performance.
* @default [[DEFAULT_MAX_NUM_RENDERED_TEXT_ELEMENTS]].
* @default `undefined` (no limit).
*/
maxNumVisibleLabels?: number;
/**
* The number of {@link TextElement}s that the {@link TextElementsRenderer} tries to render even
* if they were not visible during placement. This property only applies to {@link TextElement}s
* that were culled by the frustum; useful for map movements and animations.
* @default [[DEFAULT_MAX_NUM_SECOND_CHANCE_ELEMENTS]].
*/
numSecondChanceLabels?: number;
/**
* The maximum distance for {@link TextElement} to be rendered, expressed as a fraction of
* the distance between the near and far plane [0, 1.0].
Expand Down Expand Up @@ -142,13 +123,6 @@ export function initializeDefaultOptions(options: TextElementsRendererOptions) {
if (options.maxNumGlyphs === undefined) {
options.maxNumGlyphs = MAX_GLYPH_COUNT;
}
if (options.maxNumVisibleLabels === undefined) {
options.maxNumVisibleLabels = DEFAULT_MAX_NUM_RENDERED_TEXT_ELEMENTS;
}
// TODO: Unused so far.
if (options.numSecondChanceLabels === undefined) {
options.numSecondChanceLabels = DEFAULT_MAX_NUM_SECOND_CHANCE_ELEMENTS;
}
if (options.labelDistanceScaleMin === undefined) {
options.labelDistanceScaleMin = DEFAULT_LABEL_DISTANCE_SCALE_MIN;
}
Expand Down

0 comments on commit e3a9ec3

Please sign in to comment.