Inspector v2: Back compat for DebugLayer.openedPanes and DebugLayer.setAsActiveScene #17499
Conversation
Implement Inspector._SetNewScene Make show/hide calls robust against React's hidden asynchrony
|
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/17499/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/17499/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/17499/merge#BCU1XR#0 |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/17499/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/17499/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
|
Devhost visualization test reporter: |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
2 participants
This PR implements
Inspector._OpenedPaneandInspector._SetNewSceneto makeDebugLayer.openedPanesandDebugLayer.setAsActiveScenework.The short term implementation for
_SetNewSceneis to just hide and reshow Inspector. This is because we already know we want to have multi-scene support, so we'll need to do more thinking about what_SetNewScenewould mean in that context and whether there is something better than hiding/showing Inspector in this scenario.However, doing this revealed some subtle timing bugs when hiding/showing Inspector back to back synchronously. Ultimately the problem is that React render/unmount both do hidden async work, and it must be sequentialized for everything to work correctly. So the biggest change in this PR is really just about managing all this internal asynchrony in a robust way that hides the complexity from the user. The new
ShowInspectorAPI still just synchronously returns a dispose token to keep things simple. Calling dispose on that token internally manages the async sequentialization. I like this because it makes the API simple, but I'm also not 100% sure hiding this asynchrony from the user will be the long term best solution, so I may still revisit the actual API before the 9.0 release (e.g. it may become async), but all the logic for managing the asynchrony (the hard part) should not need to change.I'm creating this PR in draft as I will be OOF next week. If we don't want to leave it in draft for a week, we can also close it and re-open when I'm back.