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

Commit

Permalink
MINOR: Fix text canvas picking.
Browse files Browse the repository at this point in the history
Old picking data was still used after clearing the geometry.

Signed-off-by: Andres Mandado <andres.mandado-almajano@here.com>
  • Loading branch information
atomicsulfate authored and AndriiHeonia committed Mar 24, 2021
1 parent 7a64bd3 commit 5fe2612
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 10 deletions.
15 changes: 5 additions & 10 deletions @here/harp-text-canvas/lib/rendering/TextGeometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ export class TextGeometry {
private m_mesh: THREE.Mesh;
private m_bgMesh: THREE.Mesh;

private m_pickingCount: number;
private m_pickingDataArray: PickingData[];
private m_pickingDataArray: PickingData[] = [];

/**
* Creates a new `TextGeometry`.
Expand All @@ -107,7 +106,6 @@ export class TextGeometry {
this.m_currentCapacity = Math.min(initialSize, capacity);
this.m_drawCount = 0;
this.m_updateOffset = 0;
this.m_pickingCount = 0;

this.m_vertexBuffer = new THREE.InterleavedBuffer(
new Float32Array(this.m_currentCapacity * QUAD_VERTEX_MEMORY_FOOTPRINT),
Expand All @@ -132,8 +130,6 @@ export class TextGeometry {
this.m_geometry.setAttribute("bgColor", this.m_bgColorAttribute);
this.m_geometry.setIndex(this.m_indexBuffer);

this.m_pickingDataArray = new Array(this.m_currentCapacity);

this.m_mesh = new THREE.Mesh(this.m_geometry, material);
this.m_bgMesh = new THREE.Mesh(this.m_geometry, backgroundMaterial);
this.m_mesh.renderOrder = Number.MAX_SAFE_INTEGER;
Expand All @@ -157,7 +153,7 @@ export class TextGeometry {
clear() {
this.m_drawCount = 0;
this.m_updateOffset = 0;
this.m_pickingCount = 0;
this.m_pickingDataArray.length = 0;
}

/**
Expand Down Expand Up @@ -440,17 +436,16 @@ export class TextGeometry {
* @param pickingData - Picking data to be added.
*/
addPickingData(startIdx: number, endIdx: number, pickingData: any): boolean {
if (this.m_pickingCount >= this.m_currentCapacity) {
if (this.m_pickingDataArray.length >= this.m_currentCapacity) {
return false;
}

this.m_pickingDataArray[this.m_pickingCount] = {
this.m_pickingDataArray.push({
start: Math.min(startIdx, this.capacity),
end: Math.min(endIdx, this.capacity),
data: pickingData
};
});

++this.m_pickingCount;
return true;
}

Expand Down
88 changes: 88 additions & 0 deletions @here/harp-text-canvas/test/TextGeometryTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright (C) 2021 HERE Europe B.V.
* Licensed under Apache 2.0, see full license in LICENSE
* SPDX-License-Identifier: Apache-2.0
*/

import { expect } from "chai";
import * as sinon from "sinon";
import * as THREE from "three";

import { Font, FontMetrics } from "../lib/rendering/FontCatalog";
import { GlyphData } from "../lib/rendering/GlyphData";
import { TextGeometry } from "../lib/rendering/TextGeometry";
import { TextRenderStyle } from "../lib/rendering/TextStyle";

describe("TextGeometry", () => {
let style: TextRenderStyle;
let textGeometry: TextGeometry;
beforeEach(() => {
style = new TextRenderStyle();
textGeometry = new TextGeometry(
new THREE.Scene(),
new THREE.Material(),
new THREE.Material(),
10,
10
);
});

function createGlyph(): GlyphData {
const metrics: FontMetrics = {
size: 10,
distanceRange: 0,
base: 0,
lineHeight: 5,
lineGap: 0,
capHeight: 10,
xHeight: 5
};
const font: Font = { name: "", metrics, charset: "" };
return new GlyphData(0, "", 5, 5, 0, 0, 0, 0, 0, 1, 1, new THREE.Texture(), font);
}

function addGlyph(
corners = [
new THREE.Vector3(0, 0),
new THREE.Vector3(10, 0),
new THREE.Vector3(0, 10),
new THREE.Vector3(10, 10)
]
) {
const glyphData = createGlyph();
expect(textGeometry.add(glyphData, corners, 1, 1, false, style)).equals(true);
}

describe("pick", () => {
it("pick within glyph boundaries returns glyph's pick data", () => {
addGlyph();
const pickData = {};
expect(textGeometry.addPickingData(0, 1, pickData)).equals(true);

const callbackSpy = sinon.spy();
textGeometry.pick(new THREE.Vector2(5, 5), callbackSpy);
expect(callbackSpy.calledWith(pickData)).equals(true);
});

it("pick out of glyph boundaries returns no data", () => {
addGlyph();
const pickData = {};
expect(textGeometry.addPickingData(0, 1, pickData)).equals(true);

const callbackSpy = sinon.spy();
textGeometry.pick(new THREE.Vector2(11, 10), callbackSpy);
expect(callbackSpy.called).equals(false);
});

it("pick after clear returns no data", () => {
addGlyph();
const pickData = {};
expect(textGeometry.addPickingData(0, 1, pickData)).equals(true);
textGeometry.clear();

const callbackSpy = sinon.spy();
textGeometry.pick(new THREE.Vector2(5, 5), callbackSpy);
expect(callbackSpy.called).equals(false);
});
});
});

0 comments on commit 5fe2612

Please sign in to comment.