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

Commit

Permalink
HARP-11672: Add TileGeometryLoader's finished promise (#1875)
Browse files Browse the repository at this point in the history
* HARP-11672: Add finished promise to TileGeometryLoader.

* HARP-11672: Make Tile's removeDecodedTile and loadingFinished non-public.

Call them from within Tile's code using TileGeometryLoader's finished promise
to know when the geometry is loaded.

* HARP-11672: Add remarks to reset method.

reset does not settle the finishing promise.

* HARP-11672: Add unit tests to cover TileGeometryLoader.reset.
  • Loading branch information
atomicsulfate committed Sep 28, 2020
1 parent b3398a3 commit 9cda429
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 52 deletions.
124 changes: 112 additions & 12 deletions @here/harp-mapview-decoder/test/TileGeometryLoaderTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { TileGeometryCreator } from "@here/harp-mapview/lib/geometry/TileGeometr
import { TileGeometryLoader } from "@here/harp-mapview/lib/geometry/TileGeometryLoader";
import { TileTaskGroups } from "@here/harp-mapview/lib/MapView";
import { ITileLoader, TileLoaderState } from "@here/harp-mapview/lib/Tile";
import { willEventually } from "@here/harp-test-utils";
import { TaskQueue } from "@here/harp-utils";
import * as chai from "chai";
import * as chaiAsPromised from "chai-as-promised";
Expand All @@ -25,6 +24,8 @@ import * as sinon from "sinon";

chai.use(chaiAsPromised);
const { expect } = chai;
chai.should();

class FakeVisibleTileSet {
disposeTile(tile: Tile) {}
}
Expand Down Expand Up @@ -155,7 +156,7 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(1);
mapView.taskQueue.processNext(TileTaskGroups.CREATE);

await willEventually(() => {
geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
});
Expand All @@ -170,7 +171,7 @@ describe("TileGeometryLoader", function() {
expect(geometryLoader.hasDecodedTile).to.be.false;
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(0);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(geometryLoader.isFinished).to.be.false;
});
});
Expand All @@ -185,7 +186,7 @@ describe("TileGeometryLoader", function() {
expect(geometryLoader.hasDecodedTile).to.be.false;
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(0);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(geometryLoader.isFinished).to.be.false;
});
});
Expand All @@ -197,7 +198,7 @@ describe("TileGeometryLoader", function() {
expect(geometryLoader.hasDecodedTile).to.be.false;
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).to.equal(0);

await willEventually(() => {
geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
});
Expand Down Expand Up @@ -268,7 +269,7 @@ describe("TileGeometryLoader", function() {
expect(spySetDecodedTile.callCount).equal(1);
expect(spyProcessTechniques.callCount).equal(1);

await willEventually(() => {
geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(geometryLoader.isFinished).to.be.true;
});
});
Expand All @@ -294,7 +295,7 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

await willEventually(() => {
geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
Expand All @@ -317,7 +318,7 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
Expand All @@ -341,7 +342,7 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;
Expand All @@ -366,7 +367,7 @@ describe("TileGeometryLoader", function() {

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
// The geometry loader doesn't finish, because we expect the task to expire
Expand All @@ -391,7 +392,7 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(0);

await willEventually(() => {
geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
// The geometry loader doesn't finish, because we expect the task to expire
Expand All @@ -405,11 +406,110 @@ describe("TileGeometryLoader", function() {
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

await willEventually(() => {
geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1, "should create geometry now");
expect(geometryLoader.isFinished).to.be.true;
});
});

it("should reload geometry for already loaded tile that was reset", async function() {
tile.decodedTile = createFakeDecodedTile();

const geometryCreator = TileGeometryCreator.instance;
const spyProcessTechniques = sandbox.spy(geometryCreator, "processTechniques") as any;
const spyCreateGeometries = sandbox.spy(geometryCreator, "createAllGeometries") as any;
expect(spyCreateGeometries.callCount).equal(0);
expect(spyProcessTechniques.callCount).equal(0);

geometryLoader!.update();

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;

geometryLoader.reset();
expect(geometryLoader.isFinished).to.be.false;

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(2);
expect(geometryLoader.isFinished).to.be.true;
});
});
});

it("should load geometry for canceled tile that was reset", async function() {
tile.decodedTile = createFakeDecodedTile();

const geometryCreator = TileGeometryCreator.instance;
const spyProcessTechniques = sandbox.spy(geometryCreator, "processTechniques") as any;
const spyCreateGeometries = sandbox.spy(geometryCreator, "createAllGeometries") as any;
expect(spyCreateGeometries.callCount).equal(0);
expect(spyProcessTechniques.callCount).equal(0);

geometryLoader!.update();
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);

tile.isVisible = false;

expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;

geometryLoader.reset();

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});
});
});

it("should load geometry for disposed tile that was reset", async function() {
tile.decodedTile = createFakeDecodedTile();

const geometryCreator = TileGeometryCreator.instance;
const spyProcessTechniques = sandbox.spy(geometryCreator, "processTechniques") as any;
const spyCreateGeometries = sandbox.spy(geometryCreator, "createAllGeometries") as any;
expect(spyCreateGeometries.callCount).equal(0);
expect(spyProcessTechniques.callCount).equal(0);

geometryLoader!.update();
expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);

tile.dispose();

geometryLoader.waitFinished().should.be.rejected.then(() => {
expect(spyProcessTechniques.callCount).equal(1);
expect(spyCreateGeometries.callCount).equal(0, "should not create geometry");
expect(geometryLoader.isFinished).to.be.false;

geometryLoader.reset();

expect(mapView.taskQueue.numItemsLeft(TileTaskGroups.CREATE)).equal(1);
expect(mapView.taskQueue.processNext(TileTaskGroups.CREATE)).equal(true);

geometryLoader.waitFinished().should.be.fulfilled.then(() => {
expect(spyProcessTechniques.callCount).equal(2);
expect(spyCreateGeometries.callCount).equal(1);
expect(geometryLoader.isFinished).to.be.true;
});
});
});
});
});
1 change: 0 additions & 1 deletion @here/harp-mapview/lib/BackgroundDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export class BackgroundDataSource extends DataSource {
getTile(tileKey: TileKey): Tile | undefined {
const tile = new Tile(this, tileKey);
tile.forceHasGeometry(true);
tile.removeDecodedTile(); // Skip geometry loading.
TileGeometryCreator.instance.addGroundPlane(tile, Number.MIN_SAFE_INTEGER);

return tile;
Expand Down
51 changes: 31 additions & 20 deletions @here/harp-mapview/lib/Tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,16 @@ export class Tile implements CachedResource {
this.forceHasGeometry(true);
}

this.m_tileGeometryLoader
?.waitFinished()
.then(() => {
this.loadingFinished();
this.removeDecodedTile();
})
.catch(() => {
/* ignore */
});

// If the decoder provides a more accurate bounding box than the one we computed from
// the flat geo box we take it instead. Otherwise, if an elevation range was set, elevate
// bounding box to match the elevated geometry.
Expand All @@ -772,26 +782,6 @@ export class Tile implements CachedResource {
this.dataSource.requestUpdate();
}

/**
* Remove the decodedTile when no longer needed.
*/
removeDecodedTile() {
this.m_decodedTile = undefined;
this.invalidateResourceInfo();
}

/**
* Called by the {@link @here/harp-mapview-decoder#TileLoader}
*
* @remarks
* after the `Tile` has finished loading its map data. Can be used
* to add content to the `Tile`.
* The {@link @here/harp-datasource-protocol#DecodedTile} should still be available.
*/
loadingFinished() {
// To be used in subclasses.
}

/**
* Called when the default implementation of `dispose()` needs
* to free the geometry of a `Tile` object.
Expand Down Expand Up @@ -834,6 +824,7 @@ export class Tile implements CachedResource {

/**
* Gets the [[TileGeometryLoader]] that manages this tile.
* @internal
*/
get tileGeometryLoader(): TileGeometryLoader | undefined {
return this.m_tileGeometryLoader;
Expand All @@ -844,6 +835,7 @@ export class Tile implements CachedResource {
*
* @param tileGeometryLoader - A [[TileGeometryLoader]] instance to manage the geometry creation
* for this tile.
* @internal
*/
set tileGeometryLoader(tileGeometryLoader: TileGeometryLoader | undefined) {
this.m_tileGeometryLoader = tileGeometryLoader;
Expand Down Expand Up @@ -1089,6 +1081,25 @@ export class Tile implements CachedResource {
return this.m_boundingBox;
}

/**
* Called when {@link TileGeometryLoader} is finished.
*
* @remarks
* It may be used to add content to the `Tile`.
* The {@link @here/harp-datasource-protocol#DecodedTile} is still available.
*/
protected loadingFinished() {
// To be used in subclasses.
}

/**
* Remove the decodedTile when no longer needed.
*/
private removeDecodedTile() {
this.m_decodedTile = undefined;
this.invalidateResourceInfo();
}

/**
* Updates the tile's world bounding box.
* @param newBoundingBox - The new bounding box to set. If undefined, the bounding box will be
Expand Down
Loading

0 comments on commit 9cda429

Please sign in to comment.