Skip to content

FrameGraph: Fix undo/redo in NRGE#17999

Open
Popov72 wants to merge 1 commit intoBabylonJS:masterfrom
Popov72:fix-framegraph-undo-redo
Open

FrameGraph: Fix undo/redo in NRGE#17999
Popov72 wants to merge 1 commit intoBabylonJS:masterfrom
Popov72:fix-framegraph-undo-redo

Conversation

@Popov72
Copy link
Contributor

@Popov72 Popov72 commented Mar 1, 2026

See https://forum.babylonjs.com/t/ctrl-z-in-frame-graph-brokes-playground-demo/62620

It also fixes the GUI button disappearing in the example PG when we undo a change (though, for this to work, the GUI button must be created in nrg.onBeforeBuildObservable or in nrg.onBuildObservable, see http://playground.babylonjs.com/#JWKDME#213).

Copilot AI review requested due to automatic review settings March 1, 2026 16:35
@Popov72 Popov72 added bug nrge node render graph editor labels Mar 1, 2026
@bjsplat
Copy link
Collaborator

bjsplat commented Mar 1, 2026

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

Copy link
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

Fixes undo/redo behavior in the Node Render Graph Editor (NRGE) by preserving host-provided external input values across graph re-parses, and addresses related preview/GUI lifecycle issues that caused the GUI layer to disappear after undo.

Changes:

  • Preserve and restore external NodeRenderGraphInputBlock values during history undo/redo application (values aren’t part of the serialized graph).
  • Ensure the preview scene detaches its frameGraph on dispose to restore the scene render path cleanly.
  • Create standalone fullscreen GUI ADTs with an explicit scene so the GUI layer is associated with the correct scene.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/tools/nodeRenderGraphEditor/src/graphEditor.tsx Saves/restores external input values around parseSerializedObject during undo/redo.
packages/tools/nodeRenderGraphEditor/src/components/preview/previewManager.ts Clears scene.frameGraph before disposing to avoid leaving the scene in frame-graph render mode.
packages/dev/gui/src/2D/FrameGraph/renderGraphGUIBlock.ts Passes scene into ADT fullscreen creation options for correct scene association.
packages/dev/gui/src/2D/FrameGraph/guiTask.ts When creating an internal ADT, passes scene: frameGraph.scene in options.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 1, 2026

Snapshot stored with reference name:
refs/pull/17999/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/17999/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/17999/merge/index.html#WGZLGJ#4600

Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves):

https://playground.babylonjs.com/?snapshot=refs/pull/17999/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/17999/merge
https://gui.babylonjs.com/?snapshot=refs/pull/17999/merge
https://nme.babylonjs.com/?snapshot=refs/pull/17999/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/17999/merge#BCU1XR#0

If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools.

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 1, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 1, 2026

@bjsplat
Copy link
Collaborator

bjsplat commented Mar 1, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug nrge node render graph editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants