Skip to content

Reinitialize water_plane compute resources on scene re-entry#1321

Open
adamcodeworks wants to merge 2 commits into
godotengine:masterfrom
adamcodeworks:fix/water-plane-scene-reload
Open

Reinitialize water_plane compute resources on scene re-entry#1321
adamcodeworks wants to merge 2 commits into
godotengine:masterfrom
adamcodeworks:fix/water-plane-scene-reload

Conversation

@adamcodeworks
Copy link
Copy Markdown

@adamcodeworks adamcodeworks commented May 4, 2026

Fixes #1152.

What's wrong

Switching the scene tab away from the water plane and back (or reloading the scene at runtime) left the plane blank and printed errors. Three things were going wrong:

  1. Init only ran once. The compute resources and the Texture2DRD binding were set up in _ready, which only fires the first time a node enters the tree. _exit_tree was already clearing texture_rd_rid and freeing the compute resources, but on re-entry nothing rebuilt them.
  2. The scene's Texture2DRD SubResource doesn't survive an unload reliably. Picking it back up via material.get_shader_parameter(&"effect_texture") on re-entry returned a Texture2DRD whose underlying RD texture had been torn down.
  3. Main-thread / render-thread race. _initialize_compute_code is queued onto the render thread via call_on_render_thread. Between _enter_tree returning and the render thread actually running, _process kept running on the main thread and bound texture_rds[next_texture] — still holding the previous lifecycle's now-freed RIDs — onto the texture. With 9 uniform sets backed by those RIDs you get exactly the 9 errors @Calinou reported on 4.7.beta2.

Fix

In water_plane.gd:

  • Move per-instance setup from _ready into _enter_tree so it runs every tree entry (matches what _exit_tree was already pairing with, and the approach @BastiaanOlij suggested in the issue).
  • Create a fresh Texture2DRD in _enter_tree and bind it onto the material with set_shader_parameter, instead of reading whatever the scene hands back. The script owns the Texture2DRD's lifetime now, so reloads can't leave it stranded.
  • Add a ready_for_render flag set on the render thread at the end of _initialize_compute_code and cleared on the main thread in _exit_tree. _process bails out before binding/dispatching while it's false, so we no longer bind freed RIDs during the init window.
  • Reset texture_rds / texture_sets / shader / pipeline at the end of _free_compute_resources so a stale read from the main thread can never observe freed RIDs.

No changes to the GLSL, .tscn, or any other file.

Verification

I wrote a small standalone scene mirroring the lifecycle pattern (init in _ready vs. init in _enter_tree) and exercised it through add_child / remove_child:

=== buggy variant (init in _ready) ===
after first add: initialized=1
after re-add:    initialized=1   <-- bug

=== fixed variant (init in _enter_tree) ===
after first add: initialized=1
after re-add:    initialized=2   <-- fixed

I haven't loaded the patched scene in the editor to confirm the 9 errors are gone and the ripples come back visually after a tab switch on macOS — please re-test before merging.

The compute resources and Texture2DRD binding were initialized in _ready,
which only fires once per node lifetime. After the first _exit_tree (for
example when switching scene tabs in the editor, or reloading the scene
at runtime), the resources were freed and never recreated, leaving the
plane with a blank texture. Moving the setup to _enter_tree restores the
resources on every entry. Closes godotengine#1152.
@Calinou Calinou added the bug label May 4, 2026
@BastiaanOlij
Copy link
Copy Markdown
Contributor

Haven't tested this but the fix seems sound. I'm a little surprised by the problem with switching tabs, I guess there was a recent change where we cache the node tree of each tab loaded? Before I'm pretty sure the tree was reconstructed.

All in all, this feels like a safe fix to do.

Calinou

This comment was marked as outdated.

@Calinou Calinou self-requested a review May 12, 2026 18:12
@Calinou
Copy link
Copy Markdown
Member

Calinou commented May 12, 2026

Tested locally on macOS (both Metal and Vulkan), this doesn't appear to fix the issue on 4.7.beta2. Switching scenes back and forth will still cause 9 errors to be printed and the water texture will no longer update in the editor.

The previous patch moved init from _ready to _enter_tree, but switching
scene tabs in the editor still printed 9 errors and left the water
texture frozen on macOS 4.7.beta2 (reported by Calinou).

Two remaining issues:

* The Texture2DRD declared as a SubResource on the material doesn't
  survive a scene unload reliably. Create a fresh Texture2DRD in
  _enter_tree and bind it onto the material instead of reading whatever
  the scene hands back.
* _initialize_compute_code is queued onto the render thread, but
  _process keeps running on the main thread in the meantime. Between
  re-entry and the render thread completing, _process would bind
  texture_rds[next_texture] - which still holds the previous
  lifecycle's now-freed RIDs - to the texture. Add a ready_for_render
  flag that the render thread sets after init completes; _process bails
  out until then. Also reset texture_rds/texture_sets/shader/pipeline at
  the end of _free_compute_resources so nothing stale lingers.
@adamcodeworks
Copy link
Copy Markdown
Author

Thanks for re-testing @Calinou — pushed a follow-up that should address the 9 errors and the frozen texture on 4.7.beta2.

Two extra things on top of the original _enter_tree rename:

  • The Texture2DRD SubResource from the scene file wasn't surviving the unload reliably, so the script now creates a fresh Texture2DRD in _enter_tree and binds it onto the material itself instead of reading whatever the scene hands back.
  • _initialize_compute_code runs on the render thread, but _process keeps running on the main thread in the meantime — so it was binding texture_rds[next_texture] from the previous lifecycle (already-freed RIDs) for a few frames after re-entry. Added a ready_for_render flag that the render thread sets after init and cleared on _exit_tree; _process returns early until then. Also reset the RID arrays at the end of _free_compute_resources for good measure.

PR body updated with the full description. I haven't been able to retest on macOS — could you give it another spin when you have a moment?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compute Texture Demo is Brittle + Proposed Fix

3 participants