Skip to content

Conversation

thclark
Copy link
Contributor

@thclark thclark commented Sep 9, 2025

Closes #9764

Change List

{ 
  lighting: {
    enabled: false
  },
  picking: { 
    isActive: 1
    isAttribute: false
  }
}

The picker was therefore incorrectly activated; it was running but gave an erroneous index 36094.

The penny dropped when I realised that 36094 (the index being erroneously picked) decodes to RGB [255,140,0] which was the fill colour of the polygon... so I guess the algorithm falls back to the color of the object, instead of the colour in the picking layer, which then gets decoded wrongly.

And for the first time ever...

ChatGPT actually did help a bit by steering me toward looking at this method.

@thclark thclark force-pushed the fix-terrain-picking-issue-9764 branch from a621eaa to 5cf39ed Compare September 9, 2025 13:35
@thclark thclark marked this pull request as ready for review September 9, 2025 13:35
@thclark thclark changed the title Correctly activate picking module in terrain picking pass fix[extensions]: Correctly activate picking module in terrain picking pass Sep 9, 2025
@coveralls
Copy link

coveralls commented Sep 9, 2025

Coverage Status

coverage: 91.132%. remained the same
when pulling aeb119e on thclark:fix-terrain-picking-issue-9764
into 107fee4 on visgl:master.

@thclark
Copy link
Contributor Author

thclark commented Sep 10, 2025

Note: This fixes my fundamental problem but I still feel like there's something broken, because although now I can successfully pick items, if I try to get more accurate pick coordinates (c.f. #7831) by doing:

const getPickCoordinates = (
  info: PickingInfo<Marker>,
  event,
): Coordinates | undefined => {
    // info is now a valid pick with this PR
    const pick = deckRef.current.pickObject({
      x: event.offsetCenter.x,
      y: event.offsetCenter.y,
      // layerIds,
      unproject3D: true,
      radius: 100,
    })
    return pick && [pick.x, pick.y]
}

My resulting pick is always undefined.

So @Pessimistress I'd be keen to hear if you think that's an unrelated problem, or whether I might have approached this fix in the wrong way and a better approach might have fixed both problems.

@chrisgervang
Copy link
Collaborator

Thanks for digging! There is an internal _getShaderModuleProps responsible for merge props from effects onto the shader props, but this is concerning a PickingLayerPass.

Terrain appears to be the only extension extending a PickLayerPass, which implements the props you mentioned. The other extensions that use getShaderModuleProps are doing so in EffectPass or LayerPass (Mask, CollisionFilter). These passes do not implement shader props, so calling super isn't necessary.

This leads me to believe this a well formed fix for the terrain picking pass's shader module props.

I'm not sure about the remaining issue with unproject3D, but I'm comfortable applying this patch and revisiting.

@chrisgervang chrisgervang changed the title fix[extensions]: Correctly activate picking module in terrain picking pass fix(extensions): Correctly activate picking module in terrain picking pass Sep 26, 2025
@chrisgervang chrisgervang merged commit e0287b1 into visgl:master Sep 26, 2025
3 checks passed
@thclark
Copy link
Contributor Author

thclark commented Sep 26, 2025

Woop! Thanks @chrisgervang

@chrisgervang
Copy link
Collaborator

I just released for testing in 9.2.0-beta.3. Lmk if you need it in a 9.1 patch, otherwise we're releasing 9.2 pretty soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Pick invalid / Corrupt pick index when using SolidPolygonLayer with a TerrainExtension
3 participants