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

Commit

Permalink
HARP-8717: Support mutable user text elements. (#1293)
Browse files Browse the repository at this point in the history 
* HARP-8717: Support mutable user text elements.

Signed-off-by: Andres Mandado <andres.mandado-almajano@here.com>

* HARP-8717: Address review comments.

Signed-off-by: Andres Mandado <andres.mandado-almajano@here.com>
  • Loading branch information
@atomicsulfate
atomicsulfate committed Feb 19, 2020
1 parent 947f1c1 commit a6201a4
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 27 deletions.
37 changes: 28 additions & 9 deletions @here/harp-mapview/lib/Tile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ export class Tile implements CachedResource {

// Used for [[TextElement]]s which the developer defines. Group created with maximum priority
// so that user text elements are placed before others.
private readonly m_userTextElements = new TextElementGroup(Number.MAX_SAFE_INTEGER);
private m_userTextElements = new TextElementGroup(Number.MAX_SAFE_INTEGER);

// Used for [[TextElement]]s that are stored in the data, and that are placed explicitly,
// fading in and out.
Expand All @@ -369,8 +369,9 @@ export class Tile implements CachedResource {
// Blocks other labels from showing.
private readonly m_pathBlockingElements: PathBlockingElement[] = [];

// If `true`, the text content of the [[Tile]] changed.
private m_textElementsChanged: boolean = false;
// If `true`, the text content of the [[Tile]] changed after the last time it was rendered.
// It's `Undefined` when no text content has been added yet.
private m_textElementsChanged: boolean | undefined;

private m_visibleArea: number = 0;
private m_minElevation: number = 0;
Expand Down Expand Up @@ -505,6 +506,15 @@ export class Tile implements CachedResource {
* @param textElement The Text element to add.
*/
addUserTextElement(textElement: TextElement) {
if (this.m_textElementsChanged === false) {
// HARP-8733: Text content in the tile is about to change, but it has already been
// rendered at least once (otherwise m_textElementsChanged would be undefined). Clone
// the text element group so that it's handled as a new group by TextElementsRenderer
// and it doesn't reuse the same state stored for the old one.
// TODO: HARP-8910 Deprecate user text elements.
this.m_userTextElements = this.m_userTextElements.clone();
}

this.m_userTextElements.elements.push(textElement);
this.textElementsChanged = true;
}
Expand All @@ -517,12 +527,21 @@ export class Tile implements CachedResource {
*/
removeUserTextElement(textElement: TextElement): boolean {
const foundIndex = this.m_userTextElements.elements.indexOf(textElement);
if (foundIndex >= 0) {
this.m_userTextElements.elements.splice(foundIndex, 1);
this.textElementsChanged = true;
return true;
if (foundIndex === -1) {
return false;
}
return false;

if (this.m_textElementsChanged === false) {
// HARP-8733: Text content in the tile is about to change, but it has already been
// rendered at least once (otherwise m_textElementsChanged would be undefined). Clone
// the text element group so that it's handled as a new group by TextElementsRenderer
// and it doesn't reuse the same state stored for the old one.
// TODO: HARP-8910 Deprecate user text elements.
this.m_userTextElements = this.m_userTextElements.clone();
}
this.m_userTextElements.elements.splice(foundIndex, 1);
this.textElementsChanged = true;
return true;
}

/**
Expand Down Expand Up @@ -579,7 +598,7 @@ export class Tile implements CachedResource {
* value is `true` the TextElement is placed for rendering during the next frame.
*/
get textElementsChanged(): boolean {
return this.m_textElementsChanged;
return this.m_textElementsChanged ?? false;
}

set textElementsChanged(changed: boolean) {
Expand Down
6 changes: 3 additions & 3 deletions @here/harp-mapview/lib/text/TextElement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ export class TextElement {
* Creates a new `TextElement`.
*
* @param text The text to display.
* @param points The position or a list of points for a curved text, both in local tile space.
* @param points The position or a list of points for a curved text, both in world space.
* @param renderParams `TextElement` text rendering parameters.
* @param layoutParams `TextElement` text layout parameters.
* @param priority The priority of the `TextElement. Elements with the highest priority get
Expand Down Expand Up @@ -376,7 +376,7 @@ export class TextElement {

/**
* The text element position or the first point of the path used to render a curved text, both
* in local tile space.
* in world space.
*/
get position(): THREE.Vector3 {
if (this.points instanceof Array) {
Expand All @@ -387,7 +387,7 @@ export class TextElement {
}

/**
* The list of points in local tile space used to render the text along a path or `undefined`.
* The list of points in world space used to render the text along a path or `undefined`.
*/
get path(): THREE.Vector3[] | undefined {
if (this.points instanceof Array) {
Expand Down
5 changes: 5 additions & 0 deletions @here/harp-mapview/test/PoiInfoBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ export class PoiInfoBuilder {
return this;
}

withMayOverlap(mayOverlap: boolean): PoiInfoBuilder {
this.m_mayOverlap = mayOverlap;
return this;
}

build(textElement: TextElement): PoiInfo {
return {
technique: this.m_technique,
Expand Down
8 changes: 8 additions & 0 deletions @here/harp-mapview/test/TextElementBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export const DEF_PRIORITY: number = 0;
export const DEF_POSITION = new THREE.Vector3(0, 0, 0);
export const DEF_PATH = [new THREE.Vector3(0, 0, 0), new THREE.Vector3(0.1, 0.1, 0)];
export const DEF_IGNORE_DISTANCE: boolean = true;
export const DEF_MAY_OVERLAP: boolean = false;
const DEF_TILE_CENTER = new THREE.Vector3(0, 0, 0.1);

export function pointTextBuilder(text: string = DEF_TEXT): TextElementBuilder {
Expand Down Expand Up @@ -57,6 +58,7 @@ export class TextElementBuilder {
private m_featureId: number | undefined;
private m_pathLengthSqr: number | undefined;
private m_userData: any;
private m_mayOverlap: boolean = DEF_MAY_OVERLAP;

withPoiInfo(poiInfoBuilder: PoiInfoBuilder): TextElementBuilder {
this.m_poiInfoBuilder = poiInfoBuilder;
Expand Down Expand Up @@ -117,6 +119,10 @@ export class TextElementBuilder {
return this;
}

withMayOverlap(mayOverlap: boolean): TextElementBuilder {
this.m_mayOverlap = mayOverlap;
return this;
}
build(sandbox: sinon.SinonSandbox): TextElement {
const textElement = new TextElement(
this.m_text,
Expand All @@ -134,6 +140,8 @@ export class TextElementBuilder {
}
textElement.userData = this.m_userData;
textElement.pathLengthSqr = this.m_pathLengthSqr;
textElement.mayOverlap = this.m_mayOverlap;

// Stub render style setter, so that a spy is installed on the style opacity
// whenever it's called.
const renderStyleProperty = Object.getOwnPropertyDescriptor(
Expand Down
140 changes: 128 additions & 12 deletions @here/harp-mapview/test/TextElementsRendererTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
fadeInAndFadedOut,
FadeState,
firstNFrames,
framesEnabled,
frameStates,
INITIAL_TIME,
InputTextElement,
Expand Down Expand Up @@ -125,6 +126,93 @@ const tests: TestCase[] = [
],
frameTimes: FADE_2_CYCLES
},
// USER LABELS
{
name: "Newly visited, visible user poi fades in",
tiles: [{ userLabels: [[poiBuilder(), FADE_IN]] }],
frameTimes: FADE_CYCLE
},
{
name: "User poi added after first frames fades in",
tiles: [
{
userLabels: [
[
poiBuilder(),
fadedOut(FADE_IN.length).concat(
fadeIn(FADE_2_CYCLES.length - FADE_IN.length)
),
not(firstNFrames(FADE_2_CYCLES, FADE_IN.length))
]
]
}
],
frameTimes: FADE_2_CYCLES
},
{
name: "Two user poi added in different frames to same tile fade in",
tiles: [
{
userLabels: [
[
poiBuilder("Marker 1")
.withMayOverlap(true)
.withPoiInfo(new PoiInfoBuilder().withMayOverlap(true)),
fadedOut(2).concat(fadeIn(FADE_2_CYCLES.length - 2)),
not(firstNFrames(FADE_2_CYCLES, 2))
],
[
poiBuilder("Marker 2")
.withMayOverlap(true)
.withPoiInfo(new PoiInfoBuilder().withMayOverlap(true)),
fadedOut(FADE_IN.length).concat(
fadeIn(FADE_2_CYCLES.length - FADE_IN.length)
),
not(firstNFrames(FADE_2_CYCLES, FADE_IN.length))
]
]
}
],
frameTimes: FADE_2_CYCLES
},
{
name: "Removed user poi fades out",
tiles: [
{
userLabels: [
[
poiBuilder(),
FADE_IN.concat(fadedOut(FADE_2_CYCLES.length - FADE_IN.length)),
firstNFrames(FADE_2_CYCLES, FADE_IN.length)
]
]
}
],
frameTimes: FADE_2_CYCLES
},
{
name: "From two user pois in same tile, removed poi fades out, remaining poi is faded in",
tiles: [
{
userLabels: [
[
poiBuilder("Marker 1")
.withMayOverlap(true)
.withPoiInfo(new PoiInfoBuilder().withMayOverlap(true)),
fadeIn(FADE_2_CYCLES.length)
],
[
poiBuilder("Marker 2")
.withMayOverlap(true)
.withPoiInfo(new PoiInfoBuilder().withMayOverlap(true)),
FADE_IN.concat(fadedOut(FADE_2_CYCLES.length - FADE_IN.length)),
firstNFrames(FADE_2_CYCLES, FADE_IN.length)
]
]
}
],
frameTimes: FADE_2_CYCLES
},
// LABEL COLLISIONS
{
name: "Least prioritized from two colliding persistent point texts fades out",
Expand Down Expand Up @@ -520,6 +608,25 @@ describe("TextElementsRenderer", function() {
}
});

function buildLabels(
inputElements: InputTextElement[] | undefined,
elementFrameStates: Array<[TextElement, FadeState[]]>,
frameCount: number
): TextElement[] {
if (!inputElements) {
return [];
}
return inputElements.map((inputElement: InputTextElement) => {
expect(frameStates(inputElement).length).equal(frameCount);
// Only used to identify some text elements for testing purposes.
const dummyUserData = {};
const element = builder(inputElement)
.withUserData(dummyUserData)
.build(sandbox);
elementFrameStates.push([element, frameStates(inputElement)]);
return element;
});
}
async function initTest(
test: TestCase
): Promise<{
Expand All @@ -541,18 +648,14 @@ describe("TextElementsRenderer", function() {
expect(tile.terrainFrames.length).equal(test.frameTimes.length);
enableElevation = true;
}
const elements = tile.labels.map((inputElement: InputTextElement) => {
expect(frameStates(inputElement).length).equal(test.frameTimes.length);
// Only used to identify some text elements for testing purposes.
const dummyUserData = {};
const element = builder(inputElement)
.withUserData(dummyUserData)
.build(sandbox);
elementFrameStates.push([element, frameStates(inputElement)]);
return element;
});
const labels = buildLabels(tile.labels, elementFrameStates, test.frameTimes.length);
const userLabels = buildLabels(
tile.userLabels,
elementFrameStates,
test.frameTimes.length
);
allTileIndices.push(tileIndex);
fixture.addTile(elements);
fixture.addTile(labels, userLabels);
});

// Keeps track of the opacity that text elements had in the previous frame.
Expand All @@ -562,7 +665,6 @@ describe("TextElementsRenderer", function() {
// to initialized.
await fixture.renderFrame(INITIAL_TIME, allTileIndices, []);
fixture.setElevationProvider(enableElevation);

return { elementFrameStates, prevOpacities };
}

Expand All @@ -586,6 +688,19 @@ describe("TextElementsRenderer", function() {
return { tileIdcs, terrainTileIdcs };
}

function prepareUserLabels(frameIdx: number, tiles: InputTile[], tileIndices: number[]) {
for (const tileIndex of tileIndices) {
const tile = tiles[tileIndex];
if (!tile.userLabels) {
continue;
}
const elementsEnabled = tile.userLabels.map(
label => framesEnabled(label)?.[frameIdx] ?? true
);
fixture.setTileUserTextElements(tileIndex, elementsEnabled);
}
}

for (const test of tests) {
it(test.name, async function() {
const { elementFrameStates, prevOpacities } = await initTest(test);
Expand All @@ -595,6 +710,7 @@ describe("TextElementsRenderer", function() {
const frameTime = test.frameTimes[frameIdx];
const collisionEnabled =
test.collisionFrames === undefined ? true : test.collisionFrames[frameIdx];
prepareUserLabels(frameIdx, test.tiles, tileIdcs);
await fixture.renderFrame(frameTime, tileIdcs, terrainTileIdcs, collisionEnabled);

let elementIdx = 0;
Expand Down
25 changes: 24 additions & 1 deletion @here/harp-mapview/test/TextElementsRendererTestFixture.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export class TestFixture {
private m_textRenderer: TextElementsRenderer | undefined;
private m_defaultTile: Tile | undefined;
private m_allTiles: Tile[] = [];
private m_allUserTextElements: TextElement[][] = [];

constructor(readonly sandbox: sinon.SinonSandbox) {
this.m_screenCollisions = new ScreenCollisions();
Expand Down Expand Up @@ -219,7 +220,7 @@ export class TestFixture {
* can later be referenced by index when rendering a frame. See [[renderFrame]].
* @param elements The text elements the new tile will contain.
*/
addTile(elements: TextElement[]) {
addTile(elements: TextElement[], userElements: TextElement[]) {
const tile =
this.m_allTiles.length > 0
? this.m_dataSource.getTile(
Expand All @@ -233,7 +234,29 @@ export class TestFixture {
for (const element of elements) {
tile.addTextElement(element);
}
for (const element of userElements) {
tile.addUserTextElement(element);
}
this.m_allTiles.push(tile);
this.m_allUserTextElements.push(userElements);
}

setTileUserTextElements(tileIndex: number, elementEnabled: boolean[]) {
const tile = this.m_allTiles[tileIndex];
const allElements = this.m_allUserTextElements[tileIndex];
const tileElements = tile.userTextElements;
assert(allElements.length === elementEnabled.length);

for (let i = 0; i < allElements.length; ++i) {
const element = allElements[i];
const addElement = elementEnabled[i];
const elementPresent = tileElements.elements.indexOf(element) !== -1;
if (addElement && !elementPresent) {
tile.addUserTextElement(element);
} else if (!addElement && elementPresent) {
tile.removeUserTextElement(element);
}
}
}

/**
Expand Down
Loading

0 comments on commit a6201a4

Please sign in to comment.