diff --git a/@here/harp-mapview/lib/poi/PoiRenderer.ts b/@here/harp-mapview/lib/poi/PoiRenderer.ts index 62673920a5..ae72acc40a 100644 --- a/@here/harp-mapview/lib/poi/PoiRenderer.ts +++ b/@here/harp-mapview/lib/poi/PoiRenderer.ts @@ -261,8 +261,7 @@ export class PoiBatchRegistry { const { imageItem, imageTexture } = poiInfo; if (!imageItem) { - // No image -> invisible -> ignore - poiInfo.isValid = false; + // No image found, therefore just return undefined. It will probably come in soon? return undefined; } @@ -373,8 +372,10 @@ export class PoiBatchRegistry { } } -// keep track of the missing textures, but only warn once -const missingTextureName: Map = new Map(); +// keep track of the missing textures, we throw an error if the number of attempts goes over some +// threshold. +const missingTextureName: Map = new Map(); +const SEARCH_CACHE_ATTEMPTS = 5; function findImageItem( poiInfo: PoiInfo, @@ -382,28 +383,25 @@ function findImageItem( imageTexture?: ImageTexture ): ImageItem | undefined { assert(poiInfo.imageTextureName !== undefined); - const imageTextureName = poiInfo.imageTextureName!; + const imageTextureName = imageTexture ? imageTexture.image : poiInfo.imageTextureName!; - let imageItem: ImageItem | undefined; for (const imageCache of imageCaches) { - imageItem = imageTexture - ? imageCache.findImageByName(imageTexture.image) - : imageCache.findImageByName(imageTextureName); + const imageItem = imageCache.findImageByName(imageTextureName); if (imageItem) { - break; + missingTextureName.delete(imageTextureName); + return imageItem; } } - if (!imageItem) { - logger.error(`init: No imageItem found with name - '${imageTexture?.image ?? imageTextureName}'`); - poiInfo.isValid = false; - if (missingTextureName.get(imageTextureName) === undefined) { - missingTextureName.set(imageTextureName, true); - logger.error(`preparePoi: No imageTexture with name '${imageTextureName}' found`); - } + // There is a texture missing in the cache, we attempt again, and then error out. + const missingTextureCount = missingTextureName.get(imageTextureName); + missingTextureName.set(imageTextureName, missingTextureCount ? missingTextureCount + 1 : 0); + if (missingTextureName.get(imageTextureName)! === SEARCH_CACHE_ATTEMPTS) { + logger.error(`PoiRenderer::findImageItem: No imageItem found with name: + '${imageTexture?.image ?? imageTextureName}' + after ${SEARCH_CACHE_ATTEMPTS} attempts.`); } - return imageItem; + return undefined; } /** diff --git a/@here/harp-mapview/test/PoiBatchRegistryTest.ts b/@here/harp-mapview/test/PoiBatchRegistryTest.ts index 9639185263..124868d8af 100644 --- a/@here/harp-mapview/test/PoiBatchRegistryTest.ts +++ b/@here/harp-mapview/test/PoiBatchRegistryTest.ts @@ -25,12 +25,12 @@ describe("PoiBatchRegistry", () => { registry = new PoiBatchRegistry(renderer.capabilities); }); describe("registerPoi", function () { - it("marks PoiInfo as invalid if it has no image item", () => { + it("return undefined if poiInfo has no image item", () => { const poiInfo = new PoiInfoBuilder().build(textElement); const buffer = registry.registerPoi(poiInfo, defaultPoiLayer); expect(buffer).to.be.undefined; - expect((poiInfo as any).isValid).to.be.false; + expect(poiInfo.isValid).true; }); it("uses PoiInfo's imageTexture's image by default as batch key", () => { diff --git a/@here/harp-mapview/test/PoiRendererTest.ts b/@here/harp-mapview/test/PoiRendererTest.ts index d9efe0bd33..c5e60ed33d 100644 --- a/@here/harp-mapview/test/PoiRendererTest.ts +++ b/@here/harp-mapview/test/PoiRendererTest.ts @@ -13,9 +13,15 @@ import { Env } from "@here/harp-datasource-protocol"; import { Math2D } from "@here/harp-utils"; import { expect } from "chai"; -import * as THREE from "three"; +import { MapViewImageCache } from "../lib/image/MapViewImageCache"; +import { MapView } from "../lib/MapView"; +import { PoiManager } from "../lib/poi/PoiManager"; import { PoiBuffer, PoiRenderer } from "../lib/poi/PoiRenderer"; +import { TextElement } from "../lib/text/TextElement"; + +import sinon = require("sinon"); +import * as THREE from "three"; describe("PoiRenderer", function () { describe("computeIconScreenBox", function () { @@ -97,4 +103,90 @@ describe("PoiRenderer", function () { expect(screenBox.h).to.equal(16); }); }); + + describe("search for cached images", function () { + const webGLRenderer = { + capabilities: { + isWebGL2: false + } + } as THREE.WebGLRenderer; + const mapViewStub = sinon.createStubInstance(MapView); + const testImageName = "testImage"; + const createPointLabel = (name: string) => { + return { + poiInfo: { + imageTextureName: name, + isValid: true, + buffer: undefined, + technique: {} + }, + visible: true + } as TextElement; + }; + + const addRandomImageToCache = ( + testCache: MapViewImageCache, + name: string, + load: boolean + ) => { + return testCache.addImage( + name, + `/@here/harp-mapview/test/resources/headshot.jpg`, + load + ); + }; + let poiManager: PoiManager; + let mapEnv: Env; + let testCache: MapViewImageCache; + beforeEach(() => { + poiManager = new PoiManager((mapViewStub as any) as MapView); + mapEnv = new Env(); + testCache = new MapViewImageCache(); + }); + + it("poi is valid when in cache and loading started", async function () { + this.timeout(5000); + + const testImageItem = addRandomImageToCache(testCache, testImageName, true); + const caches = [testCache]; + const poiRenderer = new PoiRenderer(webGLRenderer, poiManager, caches); + const pointLabel = createPointLabel(testImageName); + const waitingForLoad = poiRenderer.prepareRender(pointLabel, mapEnv); + // This is false because the image is still being loaded in the background, notice the + // true flag passed to the call to testCache.addImage. + expect(waitingForLoad).false; + + // Promise.resolve used to convert the type `ImageItem | Promise` to + // `Promise`. + await Promise.resolve(testImageItem); + + const imageLoaded = poiRenderer.prepareRender(pointLabel, mapEnv); + expect(imageLoaded).true; + + // Check that a second attempt to prepareRender succeeds. + const imageLoadedAgain = poiRenderer.prepareRender(pointLabel, mapEnv); + expect(imageLoadedAgain).true; + }); + + it("poi is invalid when not in cache, adding to cache means it will load", async function () { + this.timeout(5000); + + // Empty cache for now + const caches = [testCache]; + const poiRenderer = new PoiRenderer(webGLRenderer, poiManager, caches); + const pointLabel = createPointLabel(testImageName); + const waitingForLoad = poiRenderer.prepareRender(pointLabel, mapEnv); + expect(waitingForLoad).false; + + const testImageItem = addRandomImageToCache(testCache, testImageName, true); + + // Promise.resolve used to convert the type `ImageItem | Promise` to + // `Promise`. + await Promise.resolve(testImageItem); + + const imageLoaded = poiRenderer.prepareRender(pointLabel, mapEnv); + // Check that adding to the cache means it will now load. + expect(imageLoaded).true; + }); + }); }); diff --git a/karma.options.js b/karma.options.js index 2f5e88f4a9..8e879cd537 100644 --- a/karma.options.js +++ b/karma.options.js @@ -170,6 +170,7 @@ const options = function (isCoverage, isMapSdk, prefixDirectory) { allowJs: true }, coverageOptions: { + instrumentation: isCoverage ? true : false, // This is needed otherwise the tests are included in the code coverage %. exclude: [ /test\/+/,