Merge bf68f54 into f043e62

modules/core/src/lib/deck-picker.js

@@ -176,6 +176,7 @@ export default class DeckPicker {
         deviceRadius,
         deviceRect
       });
+
       // Only exclude if we need to run picking again.
       // We need to run picking again if an object is detected AND
       // we have not exhausted the requested depth.
@@ -318,19 +319,17 @@ export default class DeckPicker {
   // Calculate a picking rect centered on deviceX and deviceY and clipped to device
   // Returns null if pixel is outside of device
   getPickingRect({deviceX, deviceY, deviceRadius, deviceWidth, deviceHeight}) {
-    const valid = deviceX >= 0 && deviceY >= 0 && deviceX < deviceWidth && deviceY < deviceHeight;
+    // Create a box of size `radius * 2 + 1` centered at [deviceX, deviceY]
+    const x = Math.max(0, deviceX - deviceRadius);
+    const y = Math.max(0, deviceY - deviceRadius - 1);
+    const width = Math.min(deviceWidth, deviceX + deviceRadius + 1) - x;
+    const height = Math.min(deviceHeight, deviceY + deviceRadius) - y;
 
     // x, y out of bounds.
-    if (!valid) {
+    if (width <= 0 || height <= 0) {
       return null;
     }
 
-    // Create a box of size `radius * 2 + 1` centered at [deviceX, deviceY]
-    const x = Math.max(0, deviceX - deviceRadius);
-    const y = Math.max(0, deviceY - deviceRadius);
-    const width = Math.min(deviceWidth, deviceX + deviceRadius) - x + 1;
-    const height = Math.min(deviceHeight, deviceY + deviceRadius) - y + 1;
-
     return {x, y, width, height};
   }
 

modules/core/src/lib/picking/query-object.js

@@ -48,7 +48,7 @@ export function getClosestObject({
     let i = 0;
 
     for (let row = 0; row < height; row++) {
-      const dy = row + y - deviceY;
+      const dy = row + y + 1 - deviceY;
       const dy2 = dy * dy;
 
       if (dy2 > minSquareDistanceToCenter) {

test/modules/core/lib/deck-picker.spec.js

@@ -0,0 +1,45 @@
+import test from 'tape-catch';
+import DeckPicker from '@deck.gl/core/lib/deck-picker';
+import {gl} from '@deck.gl/test-utils';
+
+const DEVICE_RECT_TEST_CASES = [
+  {
+    title: 'at center with radius',
+    input: {deviceX: 5, deviceY: 5, deviceRadius: 1, deviceWidth: 10, deviceHeight: 10},
+    output: {x: 4, y: 3, width: 3, height: 3}
+  },
+  {
+    title: 'at center without radius',
+    input: {deviceX: 5, deviceY: 5, deviceRadius: 0, deviceWidth: 10, deviceHeight: 10},
+    output: {x: 5, y: 4, width: 1, height: 1}
+  },
+  {
+    title: 'clipped by bounds',
+    input: {deviceX: 0, deviceY: 10, deviceRadius: 1, deviceWidth: 10, deviceHeight: 10},
+    output: {x: 0, y: 8, width: 2, height: 2}
+  },
+  {
+    title: 'x out of bounds',
+    input: {deviceX: -1, deviceY: 1, deviceRadius: 0, deviceWidth: 1, deviceHeight: 1},
+    output: null
+  },
+  {
+    title: 'y out of bounds',
+    input: {deviceX: 0, deviceY: 2, deviceRadius: 0, deviceWidth: 1, deviceHeight: 1},
+    output: null
+  }
+];
+
+test('DeckPicker#getPickingRect', t => {
+  const deckPicker = new DeckPicker(gl);
+
+  for (const testCase of DEVICE_RECT_TEST_CASES) {
+    t.deepEqual(
+      deckPicker.getPickingRect(testCase.input),
+      testCase.output,
+      `${testCase.title}: returns correct result`
+    );
+  }
+
+  t.end();
+});

test/modules/core/lib/deck.spec.js

@@ -1,5 +1,6 @@
 import test from 'tape-catch';
 import {Deck} from '@deck.gl/core';
+import {ScatterplotLayer} from '@deck.gl/layers';
 import {gl} from '@deck.gl/test-utils';
 
 test('Deck#constructor', t => {
@@ -53,3 +54,41 @@ test('Deck#constructor', t => {
 
   t.pass('Deck constructor did not throw');
 });
+
+test('Deck#picking', t => {
+  const deck = new Deck({
+    gl,
+    width: 1,
+    height: 1,
+    // This is required because the jsdom canvas does not have client width/height
+    autoResizeDrawingBuffer: gl.canvas.clientWidth > 0,
+
+    viewState: {
+      longitude: 0,
+      latitude: 0,
+      zoom: 12
+    },
+
+    layers: [
+      new ScatterplotLayer({
+        data: [{position: [0, 0]}, {position: [0, 0]}],
+        radiusMinPixels: 100,
+        pickable: true
+      })
+    ],
+
+    onLoad: () => {
+      const info = deck.pickObject({x: 0, y: 0});
+      t.is(info && info.index, 1, 'Picked object');
+
+      let infos = deck.pickMultipleObjects({x: 0, y: 0});
+      t.is(infos.length, 2, 'Picked multiple objects');
+
+      infos = deck.pickObjects({x: 0, y: 0, width: 1, height: 1});
+      t.is(infos.length, 1, 'Picked objects');
+
+      deck.finalize();
+      t.end();
+    }
+  });
+});

test/modules/core/lib/index.js

@@ -23,6 +23,7 @@ import './attribute.spec';
 import './attribute-manager.spec';
 import './attribute-transition-manager.spec';
 import './deck.spec';
+import './deck-picker.spec';
 import './layer.spec';
 import './composite-layer.spec';
 import './layer-manager.spec';

test/modules/core/lib/pick-layers.spec.js

@@ -54,12 +54,12 @@ const NEW_GRID_LAYER_PICK_METHODS = {
     {
       parameters: {
         x: 300,
-        y: 300
+        y: 209
       },
       results: {
         count: 1,
         // point count in the aggregated cell for each pickInfo object
-        cellCounts: [1]
+        cellCounts: [8]
       }
     }
   ],
@@ -92,7 +92,7 @@ const NEW_GRID_LAYER_PICK_METHODS = {
     {
       parameters: {
         x: 86,
-        y: 216
+        y: 215
       },
       results: {
         count: 4,