Skip to content

feat(core): Remove redundant instance picking colors#10275

Merged
ibgreen merged 13 commits into
masterfrom
ib/remove-instance-picking-colors
May 14, 2026
Merged

feat(core): Remove redundant instance picking colors#10275
ibgreen merged 13 commits into
masterfrom
ib/remove-instance-picking-colors

Conversation

@ibgreen-openai
Copy link
Copy Markdown
Collaborator

@ibgreen-openai ibgreen-openai commented May 8, 2026

Summary

WebGL2 and WebGPU shaders can derive picking colors directly from builtin gl_InstanceID / instance_index variables, so no need to create and initialize instancePickingColors for most layers.

This removes the default instancePickingColors buffer for layers where the value is just the standard encoded instance index. while layers that need non-instance logical ids keep explicit picking color attributes.

First of 3 planned PRs

    1. Remove instancePickingColors
    1. switch from pickingColors to pickingIndexes buffers
    1. Adopt luma.gl multi-render-target picking module (removes the current 256 layer picking limit)
image

pickMultipleObjects keeps the old buffer-mutation behavior for explicit picking color buffers. Builtin-index layers instead pass the already-picked object ids as a small disabled-index uniform list during repeated picking passes.

Changes

  • Adds picking shader helpers for deriving standard picking colors from builtin instance ids.
  • Removes core auto-registration of instancePickingColors.
  • Migrates 1:1 instanced layer shaders to builtin instance picking.
  • Preserves explicit instancePickingColors/pickingColors paths for logical feature ids, grouped text/path ids, and tessellated per-vertex picking.
  • Updates custom layer docs and core tests for the new default behavior.
Layer Name Type instance_index Comment
ScatterplotLayer instanced Falls back to explicit instancePickingColors only when binary data supplies it.
IconLayer instanced Falls back to explicit instancePickingColors only when binary data supplies it.
PointCloudLayer instanced Standard 1:1 instance picking.
LineLayer instanced Standard 1:1 instance picking.
ArcLayer instanced Standard 1:1 instance picking.
ColumnLayer instanced Standard 1:1 instance picking.
GridCellLayer instanced Standard 1:1 instance picking.
HexagonCellLayer instanced Standard 1:1 instance picking.
ScreenGridCellLayer instanced Standard 1:1 instance picking.
SimpleMeshLayer instanced Standard 1:1 instance picking.
ScenegraphLayer instanced Standard 1:1 instance picking.
TextBackgroundLayer instanced One background per text object.
CARTO RasterColumnLayer instanced Already lays cells out from gl_InstanceID.
MeshLayer instanced Uses instance_index unless pickFeatureIds is enabled.
PathLayer instanced Keeps instancePickingColors; path segments can map to source object ids.
MultiIconLayer / text characters instanced Keeps instancePickingColors; glyph instances map back to text labels.
GeoJSON binary point sublayers instanced Keep supplied colors for global feature ids.
GeoJSON binary line / outline sublayers instanced Keep supplied colors for global feature ids.
SolidPolygonLayer primitive N/A Keeps pickingColors; tessellated vertices need source-object colors.
BitmapLayer primitive N/A Does not need default instancePickingColors.

Validation

  • npx tsc -p modules/core/tsconfig.json --noEmit
  • npx tsc -p modules/layers/tsconfig.json --noEmit
  • npx tsc -p modules/aggregation-layers/tsconfig.json --noEmit
  • npx tsc -p modules/mesh-layers/tsconfig.json --noEmit
  • npx tsc -p modules/geo-layers/tsconfig.json --noEmit
  • npx tsc -p modules/carto/tsconfig.json --noEmit
  • yarn -s lint
  • yarn -s test --project node test/modules/core/shaderlib/picking.spec.ts test/modules/core/lib/layer.spec.ts test/modules/core/lib/layer-extension.spec.ts
  • yarn -s test --project node test/modules/core/lib/pick-layers.spec.ts test/modules/core/lib/picking.spec.ts test/modules/layers/geojson-layer.spec.ts test/modules/layers/bitmap-layer.spec.ts

@ibgreen-openai ibgreen-openai marked this pull request as ready for review May 8, 2026 01:49
@coveralls
Copy link
Copy Markdown

coveralls commented May 8, 2026

Coverage Status

coverage: 83.67% (-0.08%) from 83.746% — ib/remove-instance-picking-colors into master

@ibgreen-openai ibgreen-openai changed the title [codex] Remove redundant instance picking colors feat(core): Remove redundant instance picking colors May 8, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3b2c86d40

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread modules/layers/src/icon-layer/icon-layer.wgsl.ts Outdated
Comment thread docs/developer-guide/custom-layers/primitive-layers.md Outdated

const pickingUniformsGLSL = /* glsl */ `\
float disabledPickingIndexCount;
vec4 disabledPickingIndices0;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could do mat4 disabledPickingIndices instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, perhaps, that would give us a few more. They could probably be an open ended array at the end of the buffer if we really wanted to generalized.
I'll consider it as I keep working on this.

@ibgreen ibgreen merged commit 0c4b7d1 into master May 14, 2026
5 checks passed
@ibgreen ibgreen deleted the ib/remove-instance-picking-colors branch May 14, 2026 21:44
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.

5 participants