Reset reverse depth between native validation scenes - #1895
bkaradzic-microsoft wants to merge 3 commits into
Conversation
Extract the focused cleanup fix from the shotgun worktree. Restore forward depth before releasing effects on the reused engine and cover repeated cleanup with a Node regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 723c9021-b79c-4089-9ff7-0d8ad63f1e98
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 3
Open (4)
validation_native.jsto locate… · NewcleanupAfterTest(per the provided context) callsengine._releaseTexture(...)for leaked… · Newvalidation_native.jsto locate… · New
What changed in this PR
Extracts a targeted fix for the native validation runner to prevent a scene that enables reverse depth from affecting subsequent scenes when the engine is reused.
Changes:
- Reset
engine.useReverseDepthBuffertofalseduring validation cleanup before releasing cached effects. - Add a Node-based regression test that directly exercises
cleanupAfterTestviavm. - Document the engine-reuse cleanup behavior and how to run the regression test.
| File | Description |
|---|---|
| Documentation/ContinuousIntegration.md | Documents engine reuse side effects and adds the command to run the new cleanup regression test. |
| Apps/Playground/Tests/validation_cleanup.test.cjs | Adds a focused Node test ensuring cleanup is idempotent and restores forward depth before effect release. |
| Apps/Playground/Scripts/validation_native.js | Restores forward depth (useReverseDepthBuffer = false) as part of cleanup to prevent cross-scene state leakage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const start = source.indexOf(" function cleanupAfterTest("); | ||
| const end = source.indexOf(" function makeTestDone(", start); | ||
| assert.ok(start >= 0 && end > start); |
There was a problem hiding this comment.
Addressed in acf43ff9: the test now locates the function with a whitespace-insensitive regex and uses brace-aware extraction that skips strings and comments, so it no longer depends on indentation or the following function name.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (7)
vm.runInNewContextevaluates the function declaration but does not return it, and function… · NewRegExpfrom an unescapedname. Ifnameever contains regex metacharacters, the… · Newvalidation_native.jsto locate…cleanupAfterTest(per the provided context) callsengine._releaseTexture(...)for leaked…validation_native.jsto locate…useReverseDepthBuffer, but the code change isengine.useReverseDepthBuffer(a… · New
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d35d0a8b-b073-4f2a-bbd3-a0b1d3584305
Summary
Extracts the one-line reverse-depth cleanup fix from the BabylonNative shotgun worktree onto official master. The validation runner reuses its engine, so a scene that enables reverse depth can affect later scenes unless cleanup restores the default.
Validation
node --test Apps/Playground/Tests/validation_cleanup.test.cjspasses on this isolated branch. This regression exercises the actual cleanup function without requiring a native host.Independently based on official
b8c93d24; no unrelated shotgun commits.