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

Commit

Permalink
HARP-13785: Mark rejected new labels as rejected instead of invisible.
Browse files Browse the repository at this point in the history 
If text is marked as invisible, it's icon is still rendered.
  • Loading branch information
@atomicsulfate
atomicsulfate committed Jan 19, 2021
1 parent 82d1858 commit 4d1c814
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 35 deletions.
12 changes: 6 additions & 6 deletions @here/harp-mapview/lib/text/Placement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,11 +565,8 @@ function placePointLabelChoosingAnchor(
return allInvisible
? // All text's placements out of the screen.
PlacementResult.Invisible
: persistent
? // All placements are either colliding or out of screen for persistent label.
PlacementResult.Rejected
: // No placement found for the new label.
PlacementResult.Invisible;
: // All placements are either colliding or out of screen .
PlacementResult.Rejected;
}

/**
Expand Down Expand Up @@ -683,7 +680,10 @@ function placePointLabelAtAnchor(
// Apply additional persistent margin, keep in mind that text bounds just calculated
// are not (0, 0, w, h) based, so their coords usually are also non-zero.
// TODO: Make the margin configurable
tmpBox.copy(labelBounds).expandByVector(persistentPointLabelTextMargin).translate(textOffset);
tmpBox
.copy(labelBounds)
.expandByVector(persistentPointLabelTextMargin)
.translate(textOffset);
tmpBox.getCenter(tmpCenter);
tmpBox.getSize(tmpSize);

Expand Down
100 changes: 71 additions & 29 deletions @here/harp-mapview/test/PlacementTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ function createTextPlacements(
return result;
}

describe("Placement", function () {
describe("Placement", function() {
const sandbox = sinon.createSandbox();
let textCanvas: TextCanvas;
const screenCollisions: ScreenCollisions = new ScreenCollisions();
Expand All @@ -184,7 +184,7 @@ describe("Placement", function () {
// Canvas padding - padding applied in bounds calculated on TextCanvas.measureText()
const cPadding = new THREE.Vector2(2, 2);

before(async function () {
before(async function() {
if (inNodeContext) {
(global as any).window = {
location: {
Expand All @@ -203,20 +203,20 @@ describe("Placement", function () {
textCanvas = await createTextCanvas();
});

after(function () {
after(function() {
if (inNodeContext) {
delete (global as any).window;
delete (global as any).document;
}
sandbox.restore();
});

afterEach(function () {
afterEach(function() {
screenCollisions.reset();
});

describe("placePointLabel", function () {
context("single line text", function () {
describe("placePointLabel", function() {
context("single line text", function() {
interface SingleTextPlacementRun {
// Test run name.
it: string;
Expand Down Expand Up @@ -343,8 +343,8 @@ describe("Placement", function () {
outPosition: new THREE.Vector2(5 - cPadding.x / 2, 5 - cPadding.y)
}
];
runs.forEach(function (run) {
it(run.it, async function () {
runs.forEach(function(run) {
it(run.it, async function() {
const textElement = await createTextElement(
textCanvas,
"Test 123456",
Expand Down Expand Up @@ -384,8 +384,8 @@ describe("Placement", function () {
});
});

context("multi line text", function () {
it("places text below", async function () {
context("multi line text", function() {
it("places text below", async function() {
const textElement = await createTextElement(
textCanvas,
"Test Test Test Test Test Test Test Test Test",
Expand Down Expand Up @@ -418,7 +418,7 @@ describe("Placement", function () {
expect(position.y).to.equal(-cPadding.y);
});

it("places text center", async function () {
it("places text center", async function() {
const textElement = await createTextElement(
textCanvas,
"Test Test Test Test Test Test Test Test Test",
Expand Down Expand Up @@ -451,7 +451,7 @@ describe("Placement", function () {
expect(position.y).to.equal(76 + 0.25);
});

it("places text above", async function () {
it("places text above", async function() {
const textElement = await createTextElement(
textCanvas,
"Test Test Test Test Test Test Test Test Test",
Expand Down Expand Up @@ -484,7 +484,7 @@ describe("Placement", function () {
expect(position.y).to.equal(152 + 0.5 + cPadding.y);
});

it("places text with offset", async function () {
it("places text with offset", async function() {
const textElement = await createTextElement(
textCanvas,
"Test 123456",
Expand Down Expand Up @@ -520,7 +520,7 @@ describe("Placement", function () {
expect(position.y).to.equal(5 + 19 + 0.5 + cPadding.y);
});

it("scales offsets", async function () {
it("scales offsets", async function() {
const textElement = await createTextElement(
textCanvas,
"Test 123456",
Expand Down Expand Up @@ -557,7 +557,7 @@ describe("Placement", function () {
});
});

context("single text with and without alternative placement", function () {
context("single text with and without alternative placement", function() {
interface PlacementAlternativesRun {
// Test run name.
it: string;
Expand Down Expand Up @@ -711,8 +711,8 @@ describe("Placement", function () {
}
}
];
runs.forEach(function (run) {
it(run.it, async function () {
runs.forEach(function(run) {
it(run.it, async function() {
const textElement = await createTextElement(
textCanvas,
run.text,
Expand Down Expand Up @@ -775,7 +775,7 @@ describe("Placement", function () {
});
});

context("single text moving to the screen edge", function () {
context("single text moving to the screen edge", function() {
interface AlternativesWithMovementRun {
// Test run name.
it: string;
Expand Down Expand Up @@ -932,8 +932,8 @@ describe("Placement", function () {
);
// Finally process all runs.
const runs = runsNoFading.concat(runsWithFading);
runs.forEach(function (run) {
it(run.it, async function () {
runs.forEach(function(run) {
it(run.it, async function() {
const textElement = await createTextElement(
textCanvas,
run.text,
Expand Down Expand Up @@ -962,7 +962,7 @@ describe("Placement", function () {
state.update(1);

// Process each frame sequentially.
run.frames.forEach(function (frame) {
run.frames.forEach(function(frame) {
// Move label.
const textExtent = new THREE.Vector2(
state.element.bounds!.max.x - state.element.bounds!.min.x,
Expand Down Expand Up @@ -1006,8 +1006,8 @@ describe("Placement", function () {
});
});

context("texts colliding with alternative placements", function () {
it("place two text at almost the same position", async function () {
context("texts colliding with alternative placements", function() {
it("place two text at almost the same position", async function() {
const elements = await createPointTextElements(
textCanvas,
["Test 1", "Test 1"],
Expand Down Expand Up @@ -1090,7 +1090,7 @@ describe("Placement", function () {
expect(results[1]).to.equal(PlacementResult.Ok);
});

it("fail to place two centered texts at almost the same position", async function () {
it("fail to place two centered texts at almost the same position", async function() {
const elements = await createPointTextElements(
textCanvas,
["Test 1", "Test 2"],
Expand Down Expand Up @@ -1140,7 +1140,7 @@ describe("Placement", function () {
expect(results[1]).to.equal(PlacementResult.Rejected);
});

it("place two approaching texts", async function () {
it("place two approaching texts", async function() {
const elements = await createPointTextElements(
textCanvas,
["Test 1", "Test 1"],
Expand Down Expand Up @@ -1279,7 +1279,7 @@ describe("Placement", function () {
});
});

it("only sets label bounds on successful placement", async function () {
it("only sets label bounds on successful placement", async function() {
const collisionsStub = new ScreenCollisions();

const textElement = await createTextElement(
Expand Down Expand Up @@ -1325,10 +1325,52 @@ describe("Placement", function () {

expect(textElement.bounds).to.not.be.undefined;
});

it("returns rejected for visible new labels with no available space", async function() {
const collisionsStub = new ScreenCollisions();

const textElement = await createTextElement(
textCanvas,
"Text",
new THREE.Vector3(),
{},
{
horizontalAlignment: HorizontalAlignment.Right,
verticalAlignment: VerticalAlignment.Below,
placements: createTextPlacements(
HorizontalPlacement.Left,
VerticalPlacement.Bottom
)
}
);
const state = new TextElementState(textElement);
const screenPos = new THREE.Vector2(0, 0);
const scale = 1.0;
const env = new Env();
const outPos = new THREE.Vector3();
textCanvas.textRenderStyle = textElement.renderStyle!;
textCanvas.textLayoutStyle = textElement.layoutStyle!;

sandbox.stub(collisionsStub, "isVisible").returns(true);
sandbox.stub(collisionsStub, "isAllocated").returns(true);

const result = placePointLabel(
state,
screenPos,
scale,
textCanvas,
env,
collisionsStub,
outPos,
true
);

expect(result).equals(PlacementResult.Rejected);
});
});

describe("placeIcon", function () {
it("places icon without offset", function () {
describe("placeIcon", function() {
it("places icon without offset", function() {
const iconRenderState = new RenderState();
iconRenderState.updateFading(800, true);

Expand All @@ -1352,7 +1394,7 @@ describe("Placement", function () {
expect(result).to.equal(PlacementResult.Ok);
});

it("places icon with offset", function () {
it("places icon with offset", function() {
const iconRenderState = new RenderState();
iconRenderState.updateFading(800, true);

Expand Down

0 comments on commit 4d1c814

Please sign in to comment.