Picking bug

[dcposch]

Version: 4.0.0-rc1

Steps to reproduce

• Create a ScatterplotLayer
• Zoom and pan

Observed result

Once per minute or so, it crashes with the following error:

Uncaught TypeError: Cannot read property 'model' of null
    at ScatterplotLayer.setUniforms (webpack:///./~/deck.gl/dist/lib/layer.js?:767:21)
    at ScatterplotLayer.getPickingInfo (webpack:///./~/deck.gl/dist/lib/layer.js?:201:14)
    at ScatterplotLayer.pickLayer (webpack:///./~/deck.gl/dist/lib/layer.js?:590:19)
    at eval (webpack:///./~/deck.gl/dist/lib/draw-and-pick.js?:152:20)
    at Array.forEach (native)
    at eval (webpack:///./~/deck.gl/dist/lib/draw-and-pick.js?:141:20)
    at glContextWithState (webpack:///./~/luma.gl/dist-es6/webgl/context.js?:188:13)
    at pickLayers (webpack:///./~/deck.gl/dist/lib/draw-and-pick.js?:63:32)
    at LayerManager.pickLayer (webpack:///./~/deck.gl/dist/lib/layer-manager.js?:188:42)
    at DeckGL._onMouseMove (webpack:///./~/deck.gl/dist/react/deckgl.js?:215:45)
    at EventsProxy.mousemove (webpack:///./~/luma.gl/dist-es6/core/event.js?:352:24)
    at HTMLCanvasElement.eval (webpack:///./~/luma.gl/dist-es6/core/event.js?:160:26)

I couldn't find a reliable way to reproduce this, but it happens frequently.

[ibgreen]

@dc Thanks for reporting, we haven't seen this in our testing.

• Can this be reproduced in any of the bundled examples or is this in your app?
• stacktrace shows that this is happening during picking (which was refactored in R4) so that is a starting point...
• Looks like layer.state is null, this typically happens only when a discarded layer from a previous render cycle is accessed.
• Do you have multiple Scatterplot layers? (perhaps there are some issues with layer id prop?).

[howtimeflies0]

From my experience, this is usually due to two Scatterplot layer having the same "id".

[dcposch]

@shaojingli @ibgreen thanks!

I just reproduced it with IconLayer as well. I have just two layers, IconLayer and PathLayer, and they have two different ids.

[howtimeflies0]

@dcposch are you reproducing the bug using the layer-browser example shipped along with deck.gl repo? or you have your own app? can you provide us a simple app that reproduces if the latter:?

[ibgreen]

@dcposch - Do you see anything else in the console?

The main thing I can think of at this point is that there is some other exception during layer matching which causes things to get out of sync.

[dcposch]

@ibgreen nope, just that one

It's all good ~ I've disabled pickable and am doing a totally different approach to picking now: unproject the mouse coordinates, look up which object is closest to that point on the map.

This lets me give objects bigger, more robust click targets. The way deck.gl currently does picking has some deeper issues beyond this bug. For example, say you draw location markers via an IconLayer:

When the mouse hits the edge of the marker, that location is picked. When the mouse is in the whitespace inside the marker, that location marker is no longer being picked. That's because the picking render pass is rendered exactly the same as the drawing render pass.

Here's what I recommend to making picking more robust:

• The picking pass could be rendered with different geometry than the drawing pass.
• For IconLayer, the picking pass could render bounding polygons bigger than the icon.
• For ScatterplotLayer, the picking pass could simply render bigger circles.
• For PathLayer, the picking pass could render wider paths. For example, you could let users define getPickingWidth, which would default to something like getWidth() * 2.
• In all cases, the picking pass could have the depth test enabled, and the geometry could have depth that increases from the center of the object (or centerline of the path) to the edge. That way, if two objects overlap, deck.gl would do the right thing: whichever one your mouse is closer to will win. (Currently, I think whichever object happens to be drawn last wins, covering up its neighbors.)

[ibgreen]

@dcposch - Yes this is a very interesting idea, we have actually considered this. As an example we have played with the idea of using separate invisible support layers (with the same data) to provide wider hit areas for picking (e.g. we have a z coordinate based Voronoi layer that we have experimented with for point picking - it works exactly as you describe by rendering a "cone" facing away from the viewer for each point, so the z buffer automatically determines which data point is nearest any screen pixel).

Improving picking could be a good theme for deck.gl v4.1 / v5. It needs some thought though - for really small objects it makes sense to have a wider target area but for bigger objects I think being pixel precise is desirable, at least in some apps.

In some cases there might be other good solutions that can provide good results (e.g. for the icon layer one could consider supporting a second bit mask used during picking to avoid not being able to pick inside transparent parts of icons?).

[howtimeflies0]

The original bug can be reproduced internally right now. We should fix it before we ship v4.0

[ibgreen]

@shaojingli @Pessimistress Looks like exactly the same bug that we had in v3 - calling the picking callback activates the React app, which typically calls the React components setState to store the picked id.

The gotcha here is that before it returns, React's setState (sometimes) causes a React rerender, which triggers an (immediate) deck.gl rerender. New layers are generated and the old ones are invalidated. When this is done, setState returns, the react callback returns to deck.gl's picking implementation, but surprisingly the deck.gl layers being picked are no longer good.

This is subtle behavior from React and easy to fall prey to. I actually did add a comment warning maintainers about this when I fixed it in the 3.0 code but even so it didn't prevent the bug from coming back.

[Pessimistress]

@ibgreen @dcposch Regarding the picking tolerance proposal, here is an alternative idea: we could do a 'fat pointer' - instead of querying only one pixel, we get N * N pixels centered at the pointer. Starting at the center, we traverse these pixels in a spiral, and stop at the first hit. This way no change to existing layers is required. We could make N a base layer prop called pickingRadius.

[howtimeflies0]

@Pessimistress Regarding the picking arbitration, this is a very interesting issue and we might need to discuss in details offline and make sure our solution covers as many situation as possible.

[dcposch]

@Pessimistress yeah, that's a cool idea. dunno if you have to "traverse in a spiral", but regardless of the exact way it's done, once you've already rendered the whole picking buffer, it should be negligible extra computation to:

• check a neighborhood around a given pixel coordinate
• see if there are any objects within some small radius
• if so, return the closest object