Fix gpu picking instances when instances are disposed #16528
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16528/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/16528/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/16528/merge#BCU1XR#0 |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
Its not good enough. :( I opened #16534 to fix the callback and some minor cleanup. Pls review, thanks. |
The fix in #16528 now correctly throws `Uncaught TypeError: Cannot read properties of undefined (reading 'r')` when disposing last instance from a mesh (https://playground.babylonjs.com/?snapshot=refs/pull/16528/merge#0E9WBX). This is identical to the thin instance case in forum (https://forum.babylonjs.com/t/removing-thin-instances-vs-gpupicker/57989). The correct sequence is for the user to update the picking list after dispose/matrix buffer. While this repo isn't the primary culprit, its still annoying when the error crashes user app during dev. This PR adds a stricter check to the callback and performs some minor cleanup missed in the previous fix.
From: https://forum.babylonjs.com/t/removing-thin-instances-vs-gpupicker/57989/5
Disposing an instance fouls up the gpu picking result. This is due to changes in the order of instances in
mesh.instances. Fixed correct mapping of color id of instances to pickable instances by checking against_pickableMeshesinstead.