From b20f388cd5e5d794c35b89bcd2be49c04df51cd0 Mon Sep 17 00:00:00 2001 From: Frauke Fritz <42568257+FraukeF@users.noreply.github.com> Date: Thu, 28 Jan 2021 13:28:09 +0100 Subject: [PATCH] =?UTF-8?q?HARP-13136:=20Adds=20forcePickable=20to=20fill?= =?UTF-8?q?=20technique,=20to=20force=20invisible=20=E2=80=A6=20(#2064)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * HARP-13785: Adds forcePickable to fill technique, to force invisible polygons being pickable Signed-off-by: Frauke Fritz * HARP-13785: Use Pikckability enum instead of conflicting booleans to catch all pickability options Signed-off-by: Frauke Fritz * HARP-13136: Fix wrong commentts Signed-off-by: Frauke Fritz --- .../lib/TechniqueParams.ts | 48 ++++++++++++++++++- @here/harp-datasource-protocol/lib/Theme.ts | 14 ++++-- @here/harp-mapview/lib/MapObjectAdapter.ts | 22 +++++---- @here/harp-mapview/lib/TileObjectsRenderer.ts | 8 +++- .../lib/geometry/AddGroundPlane.ts | 4 +- .../lib/geometry/TileGeometryCreator.ts | 13 +++-- .../harp-mapview/test/PickingRaycasterTest.ts | 21 ++++++-- .../lib/VectorTileDataEmitter.ts | 10 +--- 8 files changed, 105 insertions(+), 35 deletions(-) diff --git a/@here/harp-datasource-protocol/lib/TechniqueParams.ts b/@here/harp-datasource-protocol/lib/TechniqueParams.ts index 627f3465cb..6ff55ae4d3 100644 --- a/@here/harp-datasource-protocol/lib/TechniqueParams.ts +++ b/@here/harp-datasource-protocol/lib/TechniqueParams.ts @@ -209,9 +209,17 @@ export interface BaseTechniqueParams { category?: DynamicProperty; /** - * Optional. If `true`, no IDs will be saved for the geometry this technique creates. + * Optional. If `true` or `Pickability.transient`, no IDs will be saved for the geometry + * this style creates. Default is `Pickability.onlyVisible`, which allows all pickable and + * visible objects to be picked, Pickability.all, will also allow invisible objects to be + * picked. + * @defaultValue `Pickability.onlyVisible` + * The boolean option is for backwardscompatibilty, please use the Pickability. + * + * + * TODO: deprecate and rename to something that makes more sense */ - transient?: boolean; + transient?: boolean | Pickability; /** * Distance to the camera `(0.0 = camera position, 1.0 = farPlane) at which the object start @@ -484,6 +492,42 @@ export enum PoiStackMode { ShowParent = "show-parent" } +/** + * Define the pickability of an object. + */ +export enum Pickability { + /** + * Pickable if visible. + */ + onlyVisible = "only-visible", + /** + * Not Pickable at all. + */ + transient = "transient", + /** + * All objects of this type pickable. + */ + all = "all" +} + +/** + * Converts backwards compatible transient property to pure {@type Pickabilty} object + * + * @param transient The transient property from the style + */ +export function transientToPickability(transient?: boolean | Pickability): Pickability { + let pickability: Pickability = Pickability.onlyVisible; + if (transient !== undefined && transient !== null) { + pickability = + typeof transient === "string" + ? transient + : transient === true + ? Pickability.transient + : Pickability.onlyVisible; + } + return pickability; +} + /** * Defines options (tokens) supported for text placements defined via [[placements]] attribute. * diff --git a/@here/harp-datasource-protocol/lib/Theme.ts b/@here/harp-datasource-protocol/lib/Theme.ts index 53c0ba8cd6..d172e4330b 100644 --- a/@here/harp-datasource-protocol/lib/Theme.ts +++ b/@here/harp-datasource-protocol/lib/Theme.ts @@ -14,6 +14,7 @@ import { FillTechniqueParams, LineTechniqueParams, MarkerTechniqueParams, + Pickability, PointTechniqueParams, SegmentsTechniqueParams, ShaderTechniqueParams, @@ -350,10 +351,17 @@ export interface StyleAttributes { maxZoomLevel?: number | JsonExpr; /** - * Optional. If `true`, no IDs will be saved for the geometry this style creates. Default is - * `false`. + * Optional. If `true` or `Pickability.transient`, no IDs will be saved for the geometry + * this style creates. Default is `Pickability.onlyVisible`, which allows all pickable and visible + * objects to be picked, Pickability.all, will also allow invisible objects to be + * picked. + * @defaultValue `Pickability.onlyVisible` + * The boolean option is for backwardscompatibilty, please use the Pickability. + * + * + * TODO: deprecate and rename to something that makes more sense */ - transient?: boolean; + transient?: boolean | Pickability; /** * Optional: If `true`, the objects with matching `when` statement will be printed to the diff --git a/@here/harp-mapview/lib/MapObjectAdapter.ts b/@here/harp-mapview/lib/MapObjectAdapter.ts index b32bf2ce88..8136542444 100644 --- a/@here/harp-mapview/lib/MapObjectAdapter.ts +++ b/@here/harp-mapview/lib/MapObjectAdapter.ts @@ -3,8 +3,7 @@ * Licensed under Apache 2.0, see full license in LICENSE * SPDX-License-Identifier: Apache-2.0 */ - -import { GeometryKind, getPropertyValue, MapEnv, Technique } from "@here/harp-datasource-protocol"; +import { GeometryKind, MapEnv, Pickability, Technique } from "@here/harp-datasource-protocol"; import * as THREE from "three"; import { DataSource } from "./DataSource"; @@ -19,8 +18,8 @@ export interface MapObjectAdapterParams { dataSource?: DataSource; technique?: Technique; kind?: GeometryKind[]; - pickable?: boolean; level?: number; + pickability?: Pickability; // TODO: Move here in following refactor. //featureData?: TileFeatureData; @@ -80,7 +79,7 @@ export class MapObjectAdapter { */ readonly level: number | undefined; - private readonly m_pickable: boolean; + private readonly m_pickability: Pickability; private m_lastUpdateFrameNumber = -1; private m_notCompletlyTransparent = true; @@ -89,7 +88,7 @@ export class MapObjectAdapter { this.technique = params.technique; this.kind = params.kind; this.dataSource = params.dataSource; - this.m_pickable = params.pickable ?? true; + this.m_pickability = params.pickability ?? Pickability.onlyVisible; this.m_notCompletlyTransparent = this.getObjectMaterials().some( material => material.opacity > 0 ); @@ -139,15 +138,18 @@ export class MapObjectAdapter { * @param env - Property lookup environment. */ isPickable(env: MapEnv) { - // An object is pickable only if it's visible, not transient and it's not explicitely marked - // as non-pickable. + // An object is pickable only if it's visible and Pickabilty.onlyVisible or + // Pickabililty.all set. return ( - this.m_pickable && - this.isVisible() && - getPropertyValue(this.technique?.transient, env) !== true + (this.pickability === Pickability.onlyVisible && this.isVisible()) || + this.m_pickability === Pickability.all ); } + get pickability(): Pickability { + return this.m_pickability; + } + private updateMaterials(context: MapAdapterUpdateEnv) { let somethingChanged: boolean = false; const materials = this.getObjectMaterials(); diff --git a/@here/harp-mapview/lib/TileObjectsRenderer.ts b/@here/harp-mapview/lib/TileObjectsRenderer.ts index 7e37ef17d1..f75933c697 100644 --- a/@here/harp-mapview/lib/TileObjectsRenderer.ts +++ b/@here/harp-mapview/lib/TileObjectsRenderer.ts @@ -8,7 +8,8 @@ import { getFeatureId, getPropertyValue, IndexedTechnique, - MapEnv + MapEnv, + Pickability } from "@here/harp-datasource-protocol"; import { Object3D, Vector3 } from "three"; @@ -207,7 +208,10 @@ export class TileObjectRenderer { if (mapObjectAdapter) { mapObjectAdapter.ensureUpdated(tile.mapView); - if (!mapObjectAdapter.isVisible()) { + if ( + !mapObjectAdapter.isVisible() && + !(mapObjectAdapter.pickability === Pickability.all) + ) { return false; } } diff --git a/@here/harp-mapview/lib/geometry/AddGroundPlane.ts b/@here/harp-mapview/lib/geometry/AddGroundPlane.ts index f84a12bfea..2fbdaec561 100644 --- a/@here/harp-mapview/lib/geometry/AddGroundPlane.ts +++ b/@here/harp-mapview/lib/geometry/AddGroundPlane.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { GeometryKind } from "@here/harp-datasource-protocol"; +import { GeometryKind, Pickability } from "@here/harp-datasource-protocol"; import { EdgeLengthGeometrySubdivisionModifier, SubdivisionMode @@ -48,7 +48,7 @@ export function addGroundPlane( ); mesh.receiveShadow = receiveShadow; mesh.renderOrder = renderOrder; - registerTileObject(tile, mesh, GeometryKind.Background, { pickable: false }); + registerTileObject(tile, mesh, GeometryKind.Background, { pickability: Pickability.transient }); tile.objects.push(mesh); } diff --git a/@here/harp-mapview/lib/geometry/TileGeometryCreator.ts b/@here/harp-mapview/lib/geometry/TileGeometryCreator.ts index 3145ecdeeb..8ed965c27d 100644 --- a/@here/harp-mapview/lib/geometry/TileGeometryCreator.ts +++ b/@here/harp-mapview/lib/geometry/TileGeometryCreator.ts @@ -36,11 +36,13 @@ import { MakeTechniqueAttrs, MapEnv, needsVertexNormals, + Pickability, SolidLineTechnique, StandardExtrudedLineTechnique, Technique, TerrainTechnique, TextPathGeometry, + transientToPickability, Value } from "@here/harp-datasource-protocol"; import { @@ -789,7 +791,7 @@ export class TileGeometryCreator { // for elevation overlay. registerTileObject(tile, depthPassMesh, techniqueKind, { technique, - pickable: false + pickability: Pickability.transient }); objects.push(depthPassMesh); @@ -804,7 +806,9 @@ export class TileGeometryCreator { // it's enough to make outlines pickable. registerTileObject(tile, object, techniqueKind, { technique, - pickable: !hasSolidLinesOutlines + pickability: hasSolidLinesOutlines + ? Pickability.transient + : transientToPickability(getPropertyValue(technique.transient, mapView.env)) }); objects.push(object); @@ -891,8 +895,9 @@ export class TileGeometryCreator { registerTileObject(tile, edgeObj, techniqueKind, { technique, - pickable: false + pickability: Pickability.transient }); + MapMaterialAdapter.create(edgeMaterial, { color: buildingTechnique.lineColor, objectColor: buildingTechnique.color, @@ -970,7 +975,7 @@ export class TileGeometryCreator { registerTileObject(tile, outlineObj, techniqueKind, { technique, - pickable: false + pickability: Pickability.transient }); MapMaterialAdapter.create(outlineMaterial, { color: fillTechnique.lineColor, diff --git a/@here/harp-mapview/test/PickingRaycasterTest.ts b/@here/harp-mapview/test/PickingRaycasterTest.ts index 2dde5b8b43..e7f8cf302c 100644 --- a/@here/harp-mapview/test/PickingRaycasterTest.ts +++ b/@here/harp-mapview/test/PickingRaycasterTest.ts @@ -7,7 +7,7 @@ // Mocha discourages using arrow functions, see https://mochajs.org/#arrow-functions // Chai uses properties instead of functions for some expect checks. -import { MapEnv } from "@here/harp-datasource-protocol"; +import { MapEnv, Pickability } from "@here/harp-datasource-protocol"; import { expect } from "chai"; import * as sinon from "sinon"; import * as THREE from "three"; @@ -60,7 +60,7 @@ describe("PickingRaycaster", function () { it("skips non-pickable objects", function () { const object = createFakeObject(THREE.Object3D); - MapObjectAdapter.create(object, { pickable: false }); + MapObjectAdapter.create(object, { pickability: Pickability.transient }); { const intersections = raycaster.intersectObject(object); expect(intersections).to.be.empty; @@ -71,12 +71,27 @@ describe("PickingRaycaster", function () { } }); + it("picks invisible but force-pickable objects", function () { + const object = createFakeObject(THREE.Mesh); + object.material = new THREE.Material(); + object.material.opacity = 0; + MapObjectAdapter.create(object, { pickability: Pickability.all }); + { + const intersections = raycaster.intersectObject(object); + expect(intersections).to.have.length(1); + } + { + const intersections = raycaster.intersectObjects([object, object]); + expect(intersections).to.have.length(2); + } + }); + it("tests pickable objects", function () { const object = createFakeObject(THREE.Object3D); const mesh = createFakeObject(THREE.Mesh); mesh.material = new THREE.Material(); mesh.material.opacity = 1; - MapObjectAdapter.create(mesh, { pickable: true }); + MapObjectAdapter.create(mesh, { pickability: Pickability.onlyVisible }); { const intersections = raycaster.intersectObject(object); diff --git a/@here/harp-vectortile-datasource/lib/VectorTileDataEmitter.ts b/@here/harp-vectortile-datasource/lib/VectorTileDataEmitter.ts index afec80673f..9255b28927 100644 --- a/@here/harp-vectortile-datasource/lib/VectorTileDataEmitter.ts +++ b/@here/harp-vectortile-datasource/lib/VectorTileDataEmitter.ts @@ -858,14 +858,7 @@ export class VectorTileDataEmitter { const isLine = isSolidLineTechnique(technique) || isLineTechnique(technique); if (isPolygon) { - this.applyPolygonTechnique( - polygons, - technique, - techniqueIndex, - featureId, - context, - extents - ); + this.applyPolygonTechnique(polygons, technique, techniqueIndex, context, extents); } else if (isLine) { const lineGeometry = technique.name === "line" ? this.m_simpleLines : this.m_solidLines; @@ -1250,7 +1243,6 @@ export class VectorTileDataEmitter { polygons: Ring[][], technique: Technique, techniqueIndex: number, - featureId: number | undefined, context: AttrEvaluationContext, extents: number ): void {