Skip to content

Picker Enhancements, add scissor and normals#8762

Open
MAG-AdrianMeredith wants to merge 6 commits into
playcanvas:mainfrom
MAG-AdrianMeredith:feat/depth-picking
Open

Picker Enhancements, add scissor and normals#8762
MAG-AdrianMeredith wants to merge 6 commits into
playcanvas:mainfrom
MAG-AdrianMeredith:feat/depth-picking

Conversation

@MAG-AdrianMeredith
Copy link
Copy Markdown
Contributor

Description

  • Enhance performance by adding support for scissor command on prepare
  • Added new function to return the normal of the pick point derived from depth
  • picker.getWorldPointAndNormalAsync(x, y);
  • Updated examples to use scissor command
  • Added visual to show pick point + normal in editor example
picking-feat.mp4

Fixes #

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the engine’s picking pipeline by adding optional scissoring to reduce picker render cost, and introduces a new async API to reconstruct a picked world-space normal from the depth buffer. It also updates multiple examples to use the new scissor-based prepare flow and visualizes the pick hit/normal in the editor example.

Changes:

  • Add optional scissor rect support to the picker render pass to narrow rasterization during prepare().
  • Add picker.getWorldPointAndNormalAsync(x, y) to return the picked world point plus a derived per-pixel normal from a 2×2 depth neighborhood.
  • Update examples to scissor picker renders and add editor visualization + UI toggle for pick axes.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/framework/graphics/render-pass-picker.js Adds scissor rect plumbing into the picker render pass and applies it during execution.
src/framework/graphics/picker.js Adds normal reconstruction API, refactors unprojection helpers, and adds scissor option handling in prepare().
examples/src/examples/misc/editor.selector.mjs Enables depth picking and uses scissored prepare(); queries selection + normal in parallel.
examples/src/examples/misc/editor.example.mjs Adds pick-hit axes visualization and integrates events from the selector.
examples/src/examples/misc/editor.controls.mjs Adds UI toggle to show/hide pick point axes.
examples/src/examples/graphics/area-picker.example.mjs Computes union scissor rect for all queries to reduce rasterization cost.
examples/src/examples/gaussian-splatting/picking.example.mjs Uses scissored picker prepare around the click point.
examples/src/examples/gaussian-splatting/paint.example.mjs Re-prepares picker per brush position with a 1×1 scissor for painting.
examples/src/examples/gaussian-splatting-legacy/picking.example.mjs Uses scissored picker prepare around the click point.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/framework/graphics/picker.js Outdated
Comment thread src/framework/graphics/picker.js
@mvaligursky
Copy link
Copy Markdown
Contributor

Fantastic PR! Keep to get this in. Please check the copilot comments.

@MAG-AdrianMeredith
Copy link
Copy Markdown
Contributor Author

Fantastic PR! Keep to get this in. Please check the copilot comments.

Sorry, I've been pretty lazy, either making changes to our fork or our own wrapper library so I'm trying to push more stuff upstream from now on

@mvaligursky
Copy link
Copy Markdown
Contributor

any updates on those copilot comments?

@MAG-AdrianMeredith
Copy link
Copy Markdown
Contributor Author

Been on other tickets, back now and it looks like its only working in webgpu, not webgl. Taking a look now....

…added support for getting normals from pick point; updated examples; added pick point to editor example
@MAG-AdrianMeredith
Copy link
Copy Markdown
Contributor Author

@mvaligursky All done, had to rewrite. My previous technique, which I thought would be easier, turned out not to be. Now use MRT instead to grab the vertex normals, webgl and webgpu now confirmed working

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/framework/graphics/picker.js:298

  • In _readTexture(), x/y are floored before applying the WebGL2 Y-flip to keep reads aligned with prepare()'s scissor. However width/height can still be fractional (they're only floored inside sanitizeRect()), so the Y-flip can differ by up to 1 row when height is non-integer (e.g. height=2.2), causing scissored rasterization and readback to sample different pixels. Floor width/height before the Y-flip as well so the flip uses the same integer dimensions as sanitizeRect()/prepare().
    _readTexture(texture, x, y, width, height, renderTarget) {
        // Floor before the WebGL2 Y-flip so the flipped row matches prepare()'s scissor, which
        // floors (via sanitizeRect) before flipping. With fractional pick coordinates, flipping
        // first and flooring after lands the read one row off the rasterized pixel.
        x = Math.floor(x);
        y = Math.floor(y);
        if (this.device?.isWebGL2) {
            y = renderTarget.height - (y + height);
        }
        const rect = this.sanitizeRect(x, y, width, height);

Comment thread src/framework/script/script-create.js
Comment thread examples/src/examples/graphics/area-picker.example.mjs Outdated
MAG-AdrianMeredith and others added 2 commits June 3, 2026 14:22
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

3 participants