Skip to content

Commit

Permalink
Check for correct layerName when highlighting in MVTLayer (visgl#6062)
Browse files Browse the repository at this point in the history
  • Loading branch information
felixpalmer authored Aug 5, 2021
1 parent 1f6ab32 commit a364a48
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 19 deletions.
41 changes: 27 additions & 14 deletions modules/geo-layers/src/mvt-layer/mvt-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,24 @@ export default class MVTLayer extends TileLayer {
_updateAutoHighlight(info) {
const {uniqueIdProperty} = this.props;

const {hoveredFeatureId} = this.state;
const {hoveredFeatureId, hoveredFeatureLayerName} = this.state;
const hoveredFeature = info.object;
let newHoveredFeatureId;
let newHoveredFeatureLayerName;

if (hoveredFeature) {
newHoveredFeatureId = getFeatureUniqueId(hoveredFeature, uniqueIdProperty);
newHoveredFeatureLayerName = getFeatureLayerName(hoveredFeature);
}

if (hoveredFeatureId !== newHoveredFeatureId && newHoveredFeatureId !== -1) {
this.setState({hoveredFeatureId: newHoveredFeatureId});
if (
hoveredFeatureId !== newHoveredFeatureId ||
hoveredFeatureLayerName !== newHoveredFeatureLayerName
) {
this.setState({
hoveredFeatureId: newHoveredFeatureId,
hoveredFeatureLayerName: newHoveredFeatureLayerName
});
}
}

Expand All @@ -188,26 +196,27 @@ export default class MVTLayer extends TileLayer {
}

getHighlightedObjectIndex(tile) {
const {hoveredFeatureId} = this.state;
const {hoveredFeatureId, hoveredFeatureLayerName} = this.state;
const {uniqueIdProperty, highlightedFeatureId, binary} = this.props;
const {data} = tile;

const isFeatureIdPresent =
isFeatureIdDefined(hoveredFeatureId) || isFeatureIdDefined(highlightedFeatureId);
const isHighlighted = isFeatureIdDefined(highlightedFeatureId);
const isFeatureIdPresent = isFeatureIdDefined(hoveredFeatureId) || isHighlighted;

if (!isFeatureIdPresent) {
return -1;
}

const featureIdToHighlight = isFeatureIdDefined(highlightedFeatureId)
? highlightedFeatureId
: hoveredFeatureId;
const featureIdToHighlight = isHighlighted ? highlightedFeatureId : hoveredFeatureId;

// Iterable data
if (Array.isArray(data)) {
return data.findIndex(
feature => getFeatureUniqueId(feature, uniqueIdProperty) === featureIdToHighlight
);
return data.findIndex(feature => {
const isMatchingId = getFeatureUniqueId(feature, uniqueIdProperty) === featureIdToHighlight;
const isMatchingLayer =
isHighlighted || getFeatureLayerName(feature) === hoveredFeatureLayerName;
return isMatchingId && isMatchingLayer;
});

// Non-iterable data
} else if (data && binary) {
Expand Down Expand Up @@ -242,7 +251,7 @@ export default class MVTLayer extends TileLayer {
for (const f of features) {
const featureId = getFeatureUniqueId(f.object, this.props.uniqueIdProperty);

if (featureId === -1) {
if (featureId === undefined) {
// we have no id for the feature, we just add to the list
renderedFeatures.push(f.object);
} else if (!featureCache.has(featureId)) {
Expand Down Expand Up @@ -299,7 +308,11 @@ function getFeatureUniqueId(feature, uniqueIdProperty) {
return feature.id;
}

return -1;
return undefined;
}

function getFeatureLayerName(feature) {
return feature.properties?.layerName || null;
}

function isFeatureIdDefined(value) {
Expand Down
7 changes: 6 additions & 1 deletion modules/test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ export {toLowPrecision} from './utils/precision';
export {default as gl} from './utils/setup-gl';

// Utilities for update tests (lifecycle tests)
export {testLayer, testLayerAsync, testInitializeLayer} from './lifecycle-test';
export {
testLayer,
testLayerAsync,
testInitializeLayer,
testInitializeLayerAsync
} from './lifecycle-test';
export {generateLayerTests} from './generate-layer-tests';

// Basic utility for rendering multiple scenes (could go into "deck.gl/core")
Expand Down
16 changes: 15 additions & 1 deletion modules/test-utils/src/lifecycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,29 @@ function defaultOnError(error, title) {
}
}

export function testInitializeLayer({layer, viewport = testViewport, onError = defaultOnError}) {
function initializeLayerManager({layer, viewport = testViewport, onError = defaultOnError}) {
const layerManager = new LayerManager(gl, {viewport});
layerManager.setProps({
onError: error => onError(error, `initializing ${layer.id}`)
});

layerManager.setLayers([layer]);
return layerManager;
}

export function testInitializeLayer(opts) {
const layerManager = initializeLayerManager(opts);
layerManager.finalize();
return null;
}

export async function testInitializeLayerAsync(opts) {
const layerManager = initializeLayerManager(opts);
const deckRenderer = new DeckRenderer(gl);
while (!opts.layer.isLoaded) {
await update({layerManager, deckRenderer});
}
layerManager.finalize();
return null;
}

Expand Down
86 changes: 83 additions & 3 deletions test/modules/core/lib/picking.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import test from 'tape-catch';
import {processPickInfo} from '@deck.gl/core/lib/picking/pick-info';
import {WebMercatorViewport} from '@deck.gl/core';
import {ScatterplotLayer, GeoJsonLayer} from '@deck.gl/layers';
import {testInitializeLayer} from '@deck.gl/test-utils';
import {MVTLayer} from '@deck.gl/geo-layers';
import {testInitializeLayer, testInitializeLayerAsync} from '@deck.gl/test-utils';

import {equals} from 'math.gl';

Expand All @@ -46,6 +47,33 @@ const testCompositeLayer = new GeoJsonLayer({
data: [{type: 'Feature', geometry: {type: 'Point', coordinates: [0, 0]}}]
});

class TestMVTLayer extends MVTLayer {
getTileData() {
return this.props.data;
}
}

TestMVTLayer.componentName = 'TestMVTLayer';

const testMVTLayer = new TestMVTLayer({
id: 'test-mvt-layer',
autoHighlight: true,
data: [
{
id: 12,
type: 'Feature',
geometry: {type: 'Point', coordinates: [0, 0]},
properties: {layerName: 'layerA'}
},
{
id: 12,
type: 'Feature',
geometry: {type: 'Point', coordinates: [0, 0]},
properties: {layerName: 'layerB'}
}
]
});

const parameters = {
mode: 'hover',
viewports: [
Expand Down Expand Up @@ -78,7 +106,7 @@ const parameters = {
height: 100
})
],
layers: [testLayer, testLayerWithCallback, testCompositeLayer],
layers: [testLayer, testLayerWithCallback, testCompositeLayer, testMVTLayer],
layerFilter: ({layer, viewport}) => {
if (viewport.id === 'minimap') {
return layer.id !== 'test-layer-with-callback';
Expand All @@ -98,10 +126,11 @@ function validateUniforms(actual, expected) {
}

/* eslint-disable max-statements */
test('processPickInfo', t => {
test('processPickInfo', async t => {
testInitializeLayer({layer: testLayer});
testInitializeLayer({layer: testLayerWithCallback});
testInitializeLayer({layer: testCompositeLayer});
await testInitializeLayerAsync({layer: testMVTLayer});

const TEST_CASES = [
{
Expand Down Expand Up @@ -202,6 +231,50 @@ test('processPickInfo', t => {
lastPickedInfo: {layerId: 'test-composite-layer-points-circle', index: 0},
testLayerUniforms: {picking_uSelectedColorValid: 0}
},
{
pickInfo: {
pickedColor: [1, 0, 0, 0],
pickedLayer: testMVTLayer.getSubLayers()[0].getSubLayers()[0],
pickedObjectIndex: 0
},
x: 100,
y: 100,
size: 2,
info: {
layer: testMVTLayer,
object: {
id: 12,
type: 'Feature',
geometry: {type: 'Point'},
properties: {layerName: 'layerA'}
}
},
highlightedObjectIndex: 0,
lastPickedInfo: {layerId: 'test-mvt-layer-0-0-1-points-circle', index: 0},
testLayerUniforms: {picking_uSelectedColorValid: 0}
},
{
pickInfo: {
pickedColor: [2, 0, 0, 0],
pickedLayer: testMVTLayer.getSubLayers()[0].getSubLayers()[0],
pickedObjectIndex: 1
},
x: 100,
y: 100,
size: 2,
info: {
layer: testMVTLayer,
object: {
id: 12,
type: 'Feature',
geometry: {type: 'Point'},
properties: {layerName: 'layerB'}
}
},
highlightedObjectIndex: 1,
lastPickedInfo: {layerId: 'test-mvt-layer-0-0-1-points-circle', index: 1},
testLayerUniforms: {picking_uSelectedColorValid: 0}
},
{
pickInfo: {
pickedColor: [1, 0, 0, 0],
Expand Down Expand Up @@ -277,11 +350,18 @@ test('processPickInfo', t => {
'currentLayerUniforms'
);
}

if (testCase.highlightedObjectIndex !== undefined) {
const renderedLayer = info.layer.renderLayers()[0][0];
const {highlightedObjectIndex} = renderedLayer.props;
t.deepEqual(highlightedObjectIndex, testCase.highlightedObjectIndex, 'highlightObjectIndex');
}
}

testLayer._finalize();
testLayerWithCallback._finalize();
testCompositeLayer._finalize();
testMVTLayer._finalize();

t.end();
});

0 comments on commit a364a48

Please sign in to comment.