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

Commit

Permalink
HARP-7916: Remove label sorting by camera distance.
Browse files Browse the repository at this point in the history
Sorting labels by camera distance is one of the bottlenecks in label placement.
Also, current implementation is broken, since sorting is done per tile
and not across tiles.
Furthermore, the improvement brought by sorting labels by distance is arguable.

Signed-off-by: Andres Mandado <andres.mandado-almajano@here.com>
  • Loading branch information
atomicsulfate committed Dec 2, 2019
1 parent 7d23554 commit bfae429
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 50 deletions.
42 changes: 3 additions & 39 deletions @here/harp-mapview/lib/text/TextElementGroupState.ts
Expand Up @@ -20,11 +20,8 @@ export type TextElementFilter = (textElementState: TextElementState) => number |
* they're being rendered.
*/
export class TextElementGroupState {
// Sorted list of element states belonging to the group state. It does NOT have the same order
// as their elements in the group.
private m_textElementStates: TextElementState[];
private m_visited: boolean = false;
private m_needsSorting: boolean = false;

/**
* Creates the state for specified group.
Expand Down Expand Up @@ -102,44 +99,11 @@ export class TextElementGroupState {
return this.m_textElementStates.length;
}

get needsSorting(): boolean {
return this.m_needsSorting;
}

invalidateSorting() {
this.m_needsSorting = true;
}

/**
* Returns text element states sorted by view distance.
* NOTE: The order does not match with the text elements in the group!.
* @param maxViewDistance Maximum distance from the view center that a visible element may have.
* @returns Array of sorted element states.
* Returns text element states.
* @returns Array of element states.
*/
sortedTextElementStates(maxViewDistance: number): TextElementState[] {
if (!this.m_needsSorting) {
return this.m_textElementStates;
}

// Do the actual sort based on view distance.
this.m_textElementStates.sort((a: TextElementState, b: TextElementState) => {
// Move elements with undefined view distance to the end.
if (a.viewDistance === b.viewDistance) {
return 0;
}
if (a.viewDistance === undefined) {
return 1;
}

if (b.viewDistance === undefined) {
return -1;
}

// Both elements have valid view distances.
return a.viewDistance - b.viewDistance;
});
this.m_needsSorting = false;

get textElementStates(): TextElementState[] {
return this.m_textElementStates;
}
}
3 changes: 0 additions & 3 deletions @here/harp-mapview/lib/text/TextElementState.ts
Expand Up @@ -121,9 +121,6 @@ export class TextElementState {
return;
}
this.m_viewDistance = viewDistance;
if (this.m_viewDistance !== undefined) {
groupState.invalidateSorting();
}
}

/**
Expand Down
9 changes: 1 addition & 8 deletions @here/harp-mapview/lib/text/TextElementsRenderer.ts
Expand Up @@ -622,10 +622,6 @@ export class TextElementsRenderer {
return false;
}

const textElementStates = groupState.sortedTextElementStates(
this.m_viewState.maxVisibilityDist
);

const shieldGroups: number[][] = [];

const temp: TempParams = {
Expand All @@ -636,7 +632,7 @@ export class TextElementsRenderer {
};
const hiddenKinds = this.m_viewState.hiddenGeometryKinds;

for (const textElementState of textElementStates) {
for (const textElementState of groupState.textElementStates) {
if (pass === Pass.PersistentLabels) {
if (placementStats) {
++placementStats.total;
Expand Down Expand Up @@ -1219,9 +1215,6 @@ export class TextElementsRenderer {
const textElementGroupState = groupStates[i];
if (placementStats) {
++placementStats.totalGroups;
if (textElementGroupState.needsSorting) {
++placementStats.resortedGroups;
}
}

const newPriority = textElementGroupState.priority;
Expand Down

0 comments on commit bfae429

Please sign in to comment.