From 09b3e87a8dd6ff574d44c6b91b6a27dce4a9a40a Mon Sep 17 00:00:00 2001 From: sindoring Date: Mon, 13 Jul 2026 11:08:30 +0400 Subject: [PATCH] fix(M00): release GUI capture render resources Work-Package: M00-QAR-GUI-RID-SHUTDOWN-001 Agent: sindo-main-codex Tests: render baseline dry-run; runtime cache shutdown; comparator self-test; coordination and documentation gates Fidelity: shutdown-only ownership fix; no visual or Blizzlike335 behavior change --- RENDER.md | 8 ++ addons/mpq_extractor/loaders/adt_builder.gd | 5 +- .../claims/M00-QAR-GUI-RID-SHUTDOWN-001.md | 78 +++++++++++++++++++ docs/M00_FINAL_PAIRED_EVIDENCE.md | 6 +- docs/modules/world-renderer.md | 10 ++- .../streaming/streaming_world_loader.gd | 59 ++++++++++++-- .../verify_render_runtime_cache_shutdown.gd | 77 ++++++++++++++++++ tools/run_render_baseline.ps1 | 1 + 8 files changed, 231 insertions(+), 13 deletions(-) create mode 100644 coordination/claims/M00-QAR-GUI-RID-SHUTDOWN-001.md create mode 100644 src/tools/verify_render_runtime_cache_shutdown.gd diff --git a/RENDER.md b/RENDER.md index 8d0e039..fc31a78 100644 --- a/RENDER.md +++ b/RENDER.md @@ -822,6 +822,14 @@ $exe = Join-Path $env:TEMP 'godot-4.6.1-openwc\Godot_v4.6.1-stable_win64.exe' - Added `src/tools/capture_render_checkpoints.gd` to load the Eastern Kingdoms streaming scene, move a dedicated camera to fixed regression viewpoints and save PNGs under `user://render_checkpoints`. - Current checkpoints cover the Elwynn waterfall from front/side and Goldshire Inn/Blacksmith windows. Headless runs are dry-run only because the headless display server cannot produce viewport PNGs. +## 2026-07-13 Renderer Shutdown Ownership + +- `StreamingWorldLoader` drains its asynchronous work before teardown, leaves parented streamed nodes to SceneTree ownership, then explicitly frees detached M2/WMO prototype nodes and clears resource caches. +- `ADTBuilder` explicitly frees a temporary `Water` root when an ADT liquid payload produces no child geometry. Dry tiles must not leave empty nodes in ObjectDB. +- `src/tools/verify_render_runtime_cache_shutdown.gd` regression-checks detached prototype and resource-cache release plus empty-liquid-root ownership. The unified render-baseline runner executes it before capture. +- A focused 8-second GUI checkpoint and a seven-checkpoint single-pass GUI run finish without Node, renderer resource or RID leak diagnostics. A timing-dependent anonymous zero-reference `RefCounted` ObjectDB warning can remain at engine teardown; it is tracked separately and is not suppressed. +- This is shutdown-only ownership behavior and does not alter checkpoint rendering, cache formats or the `Blizzlike335` compatibility profile. + ## Practical Rule For Future Work If something improves quality but creates visible hitch, it is not done. Move it to bake/cache/background work, split finalization over frames, or prewarm it before the player can see it. diff --git a/addons/mpq_extractor/loaders/adt_builder.gd b/addons/mpq_extractor/loaders/adt_builder.gd index 11a6afd..81ee1c9 100644 --- a/addons/mpq_extractor/loaders/adt_builder.gd +++ b/addons/mpq_extractor/loaders/adt_builder.gd @@ -788,7 +788,10 @@ static func _build_tile_water_root(data: Dictionary, origin_offset: Vector3) -> mi.cast_shadow = GeometryInstance3D.SHADOW_CASTING_SETTING_OFF root.add_child(mi) - return root if root.get_child_count() > 0 else null + if root.get_child_count() > 0: + return root + root.free() + return null static func _append_liquid_geometry( diff --git a/coordination/claims/M00-QAR-GUI-RID-SHUTDOWN-001.md b/coordination/claims/M00-QAR-GUI-RID-SHUTDOWN-001.md new file mode 100644 index 0000000..62ffe62 --- /dev/null +++ b/coordination/claims/M00-QAR-GUI-RID-SHUTDOWN-001.md @@ -0,0 +1,78 @@ +# M00-QAR-GUI-RID-SHUTDOWN-001 — GUI capture RID shutdown + + + +## Ownership + +- Target: M00 +- Program: QAR +- Owner/Agent ID: sindo-main-codex +- Branch: `work/sindo-main-codex/m00-gui-rid-shutdown` +- Lease expires UTC: 2026-07-15 +- Integrator: milestone integrator + +## Outcome + +Identify ownership of GUI render-baseline shutdown leaks and apply the smallest regression-covered fix that lets queued world resources release before engine teardown. + +## Non-goals + +- Renderer architecture decomposition. +- Hiding or filtering Godot leak diagnostics. +- Changing M00 status, Evidence or `OPENWC_TARGET_DONE`. + +## Paths + +- Exclusive: capture shutdown sequencing and its regression diagnostic +- Shared/hotspots: `src/tools/capture_render_checkpoints.gd`, renderer module shutdown documentation +- Generated/ignored: GUI capture output and verbose logs under `user://` + +## Contracts and data + +- Public API/events: unchanged +- Schema/format version: unchanged +- Migration/compatibility: capture images and report schema remain unchanged +- Consumers: M00 baseline runner and milestone integrator + +## Dependencies + +- Requires: merged seven-reference evidence audit +- Blocks: integrator disposition of the final GUI renderer leak risk +- External state: proprietary references are not required for the focused shutdown regression + +## Verification + +- Commands: focused GUI single-checkpoint capture with verbose shutdown, headless project load, manifest/self-tests, repository gates +- Fixtures: existing non-proprietary render-baseline manifest and local extracted corpus +- Fidelity evidence: no visual behavior change; focused PNG hash/report remains valid +- Performance budget: shutdown-only change + +## Documentation deliverables + +- Inline public API docs: shutdown method semantics if introduced +- Module specification: shutdown ownership and observed leak status +- Data-flow diagram: unchanged +- Sequence/state/dependency diagrams: update shutdown sequence only if behavior changes +- Source map/status updates: capture tool if changed + +## Simplicity and naming + +- Important names introduced: `_release_runtime_caches_for_shutdown`, `_free_detached_node_cache`, `_queue_free_streamed_node` +- Simplest considered solution: explicitly release capture-owned scene references and drain deferred deletion +- Rejected complexity/abstractions: global RID registry or suppressed engine diagnostics +- Unavoidable complexity and justification: async streaming jobs must finish before resource ownership can drain +- Measured optimization evidence: not applicable + +## Status + +- State: ready-for-review +- Done: reproduced the focused leak; assigned detached prototype/resource caches and empty liquid roots to explicit owners; added a headless regression; verified zero Node/resource/RID leak lines in focused and seven-checkpoint GUI captures +- Next: integrator review and disposition of the residual engine-level zero-reference `RefCounted` diagnostic +- Blocked by: + +## Handoff + +- Commit: this work-package commit +- Results: before the fix the focused capture retained 6 mesh, 6 material, 4 shader, 6 texture, 6 instance and RenderingDevice RIDs (30 resources); after the fix focused and seven-checkpoint captures retained none +- Remaining risks: D3D12 descriptor fallback remains unrelated; Godot can report a timing-dependent anonymous `RefCounted` whose reference count is already zero, without retained Node/resource/RID ownership +- Documentation updated: `RENDER.md`, `docs/modules/world-renderer.md`, `docs/M00_FINAL_PAIRED_EVIDENCE.md`; existing renderer shutdown sequence updated, no new diagram required diff --git a/docs/M00_FINAL_PAIRED_EVIDENCE.md b/docs/M00_FINAL_PAIRED_EVIDENCE.md index e753fd2..60a749c 100644 --- a/docs/M00_FINAL_PAIRED_EVIDENCE.md +++ b/docs/M00_FINAL_PAIRED_EVIDENCE.md @@ -62,7 +62,9 @@ The reports demonstrate current differences and do not claim `1:1`. Camera calib ## Operational risks -The full GUI visual capture completed and wrote its report, but Godot still emitted renderer RID/resource leak diagnostics at exit, including leaked material, shader, texture, instance and rendering-device RIDs. The earlier anonymous headless `RefCounted` leak is fixed; these GUI renderer allocations are a distinct known risk. They do not invalidate the written images or metrics, but must remain recorded rather than hidden. +A follow-up shutdown ownership fix removed the GUI renderer leaks found by the final visual capture. Before the fix, the focused animated-M2 checkpoint retained 6 mesh, 6 material, 4 shader, 6 texture, 6 instance and associated RenderingDevice RIDs, for 30 resources in use. After the fix, both the focused 8-second capture and a seven-checkpoint single-pass capture completed without Node, resource or RID leak diagnostics. + +Godot can still emit a timing-dependent ObjectDB warning for one or more anonymous `RefCounted` objects whose reported reference count is already zero. The warning has no retained Node, renderer resource or RID ownership in the verified captures. It remains recorded as an engine-teardown diagnostic rather than being hidden, but it is distinct from the resolved GUI renderer leak. ## Integrator decision checklist @@ -70,5 +72,5 @@ The full GUI visual capture completed and wrote its report, but Godot still emit - Animated M2: build-12340 native motion observed in two phases; synthetic/real placement mismatch explicitly classified. - Sky transition: build-12340 19:00 evidence present with approximate paired camera and human-approved composition. - Renderer visual differences: registered as gaps, not parity claims. -- GUI renderer RID/resource diagnostics: accepted as a tracked risk or assigned a follow-up blocker. +- GUI renderer RID/resource diagnostics: resolved by explicit shutdown ownership and covered by a headless regression verifier; the residual zero-reference `RefCounted` engine-teardown warning remains tracked separately. - Target status, target Evidence and `OPENWC_TARGET_DONE` marker: integrator-only actions. diff --git a/docs/modules/world-renderer.md b/docs/modules/world-renderer.md index 78c9574..fe26d89 100644 --- a/docs/modules/world-renderer.md +++ b/docs/modules/world-renderer.md @@ -143,7 +143,8 @@ sequenceDiagram Stream->>Render: evict outside retention range Stream->>Worker: shutdown: wait for WorkerThreadPool tasks Stream->>Stream: shutdown: finish registered ResourceLoader requests - Stream->>Render: clear queues, nodes, caches and RIDs + Stream->>Render: clear queues and owned tree nodes/RIDs + Stream->>Stream: free detached prototypes and release resource caches ``` ## Ownership, threading and resources @@ -153,6 +154,8 @@ sequenceDiagram - Parsed/grouped results передаются обратно через guarded result queues. - Mesh/material/node/RID finalization выполняется main thread и ограничивается exported budgets. - Shutdown/map switch обязан дождаться WorkerThreadPool jobs и зарегистрированных threaded ResourceLoader requests до очистки очередей, nodes, caches и RIDs. +- Parented streamed nodes остаются owned by SceneTree during shutdown; detached M2/WMO prototype caches are explicitly `free()`d after asynchronous work drains. +- A temporary liquid root with no generated child geometry is explicitly freed by `ADTBuilder` instead of being returned or abandoned. - Cache resources считаются immutable после публикации. ## Errors, cancellation and recovery @@ -165,7 +168,7 @@ sequenceDiagram | Main-thread hitch | Named section timing | Frame spike, work remains queued | `HITCH` log | Lower budget/fix finalize path | | D3D12 descriptor exhaustion | Rendering backend error | Render failure/fallback backend | Godot error + baseline report | Dedup resources/fix settings | | Teleport/map change | Focus/session transition | Old jobs become stale | Target/session generation | Cancel/drop stale results | -| Shutdown leak | Godot leak/RID diagnostics | Resource retained | Verbose shutdown report | Drain owned worker and resource requests before clearing registries | +| Shutdown leak | Godot leak/RID diagnostics | Resource retained | Verbose shutdown report + cache shutdown verifier | Drain requests, preserve tree ownership, then free detached prototypes and clear resource caches | ## Configuration and capabilities @@ -235,7 +238,7 @@ Exact exported settings and cache versions remain documented in [`../../RENDER.m - Joint Goldshire pitch/FOV refinement had no human-approved optimum: full-frame error preferred grass-heavy FOV 62/pitch -25, while FOV 38/pitch -10 better matched building scale. Landmark/region scoring is required before calibration. - Common-region scoring `[300,100,1400,650]` moved Goldshire's numeric optimum to FOV 38/yaw 10/pitch -15, but human inspection still found an oversized/cropped facade; semantic landmarks or masks remain required. - Final paired audit compared fourteen cold/warm images for seven private build 12340 references with zero missing pairs; mean error ranges `0.066893..0.176955`. Animated-M2 native motion is observed across two original-client phases, but its real Lakeshire placement differs from the synthetic Goldshire probe. Sky-transition evidence uses the checkpoint map position/time with manually approved approximate framing. -- D3D12 descriptor issues remain; the capture-path anonymous `RefCounted` shutdown leak is regression-covered by a clean verbose dry-run, while other RID/resource diagnostics still require independent evidence. +- D3D12 descriptor issues remain. Explicit shutdown ownership removes the GUI capture's Node/resource/RID leaks in focused and seven-checkpoint runs; a timing-dependent ObjectDB warning for anonymous zero-reference `RefCounted` objects remains as a separate engine-teardown diagnostic. - M2/WMO/material/particle/ribbon/portal parity incomplete. - Public API is mostly exported configuration rather than stable contracts. @@ -253,6 +256,7 @@ Exact exported settings and cache versions remain documented in [`../../RENDER.m | `src/tools/build_*cache.gd`, `src/tools/bake_*cache.gd` | Offline cache generation | | `tools/run_render_baseline.ps1` | Unified M00 baseline runner | | `src/tools/compare_render_checkpoints.gd` | Offline JPG/PNG paired-image perceptual metrics and JSON pass/fail report | +| `src/tools/verify_render_runtime_cache_shutdown.gd` | Headless ownership regression for detached runtime prototypes, resource caches and empty liquid roots | | `src/tools/capture_render_checkpoints.gd` | Deterministic no-roll checkpoint camera, performance and visual capture | | `tools/sweep_render_checkpoint_camera_pose.ps1` | Offline yaw/pitch capture grid and paired-error ranking | | `src/tools/verify_render_coordinate_calibration.gd` | Build 12340 camera-coordinate golden point round-trip diagnostic | diff --git a/src/scenes/streaming/streaming_world_loader.gd b/src/scenes/streaming/streaming_world_loader.gd index 22d2c7c..6cdd77a 100644 --- a/src/scenes/streaming/streaming_world_loader.gd +++ b/src/scenes/streaming/streaming_world_loader.gd @@ -374,6 +374,39 @@ func _exit_tree() -> void: _shutting_down = true _wait_for_tile_tasks() _clear_streamed_world() + _release_runtime_caches_for_shutdown() + + +## Frees detached prototype nodes and releases render resources owned by this +## loader after all asynchronous work has completed. Map refreshes retain these +## caches; only scene shutdown destroys them. +func _release_runtime_caches_for_shutdown() -> void: + _free_detached_node_cache(_m2_scene_cache) + _free_detached_node_cache(_m2_animated_scene_cache) + _free_detached_node_cache(_wmo_prototype_cache) + _m2_mesh_cache.clear() + _m2_static_animation_cache.clear() + _m2_missing_cache.clear() + _wmo_render_cache.clear() + _wmo_scene_resource_cache.clear() + _wmo_render_missing_cache.clear() + _wmo_scene_cache_missing.clear() + _wmo_missing_cache.clear() + _shared_tex_cache.clear() + + +func _free_detached_node_cache(node_cache: Dictionary) -> void: + for cached_value in node_cache.values(): + if not (cached_value is Node): + continue + var cached_node := cached_value as Node + if not is_instance_valid(cached_node): + continue + if cached_node.is_inside_tree(): + cached_node.queue_free() + else: + cached_node.free() + node_cache.clear() func _apply_quality_preset() -> void: @@ -2781,15 +2814,27 @@ func _clear_streamed_world() -> void: _free_render_instance(chunk_rid) var tile_root: Node = state.get("root", null) if tile_root: - tile_root.queue_free() + _queue_free_streamed_node(tile_root) _tile_states.clear() _clear_tile_mesh_cache() _clear_quality_terrain_mesh_cache() _clear_wmo_registry() - _ensure_world_wmo_root() - if enable_wmo: - _place_global_wmo() + if not _shutting_down: + _ensure_world_wmo_root() + if enable_wmo: + _place_global_wmo() + + +## During world shutdown, parented nodes are recursively owned by the world +## root. Queueing them separately from `_exit_tree()` can detach them from that +## deletion and leave empty Node objects alive at engine teardown. +func _queue_free_streamed_node(node: Node) -> void: + if node == null or not is_instance_valid(node): + return + if _shutting_down and node.is_inside_tree(): + return + node.queue_free() func _reset_terrain_root_children() -> void: @@ -3889,10 +3934,10 @@ func _clear_wmo_registry() -> void: var entry: Dictionary = entry_variant var node: Node = entry.get("node", null) if node and is_instance_valid(node): - node.queue_free() + _queue_free_streamed_node(node) _wmo_registry.clear() if _world_wmo_root and is_instance_valid(_world_wmo_root): - _world_wmo_root.queue_free() + _queue_free_streamed_node(_world_wmo_root) _world_wmo_root = null @@ -4458,7 +4503,7 @@ func _cancel_m2_build_job(key: String) -> void: var job: Dictionary = _m2_build_jobs[key] var root: Node = job.get("root", null) if root != null and is_instance_valid(root): - root.queue_free() + _queue_free_streamed_node(root) _m2_build_jobs.erase(key) diff --git a/src/tools/verify_render_runtime_cache_shutdown.gd b/src/tools/verify_render_runtime_cache_shutdown.gd new file mode 100644 index 0000000..735e954 --- /dev/null +++ b/src/tools/verify_render_runtime_cache_shutdown.gd @@ -0,0 +1,77 @@ +extends SceneTree + +const STREAMING_WORLD_LOADER := preload("res://src/scenes/streaming/streaming_world_loader.gd") +const ADT_BUILDER := preload("res://addons/mpq_extractor/loaders/adt_builder.gd") + +var _failures := 0 + + +func _initialize() -> void: + _verify_empty_water_root_ownership() + _verify_runtime_cache_ownership() + + if _failures > 0: + push_error("Render runtime cache shutdown verification failed: %d issue(s)" % _failures) + quit(1) + return + print("Render runtime cache shutdown verification passed.") + quit(0) + + +func _verify_empty_water_root_ownership() -> void: + var node_count_before := int(Performance.get_monitor(Performance.OBJECT_NODE_COUNT)) + var empty_water_root: Node3D = ADT_BUILDER.build_tile_water_scene({"chunks": []}, Vector3.ZERO) + var node_count_after := int(Performance.get_monitor(Performance.OBJECT_NODE_COUNT)) + _expect(empty_water_root == null, "empty ADT liquid payload should not produce a water root") + _expect(node_count_after == node_count_before, "empty ADT liquid payload should release its temporary water root") + + +func _verify_runtime_cache_ownership() -> void: + var loader: Node = STREAMING_WORLD_LOADER.new() + var static_m2_prototype := Node3D.new() + var animated_m2_prototype := Node3D.new() + var wmo_prototype := Node3D.new() + + loader.set("_m2_scene_cache", {"static": static_m2_prototype}) + loader.set("_m2_animated_scene_cache", {"animated": animated_m2_prototype}) + loader.set("_wmo_prototype_cache", {"wmo": wmo_prototype}) + loader.set("_m2_mesh_cache", {"mesh": ArrayMesh.new()}) + loader.set("_m2_static_animation_cache", {"static": true}) + loader.set("_m2_missing_cache", {"missing": true}) + loader.set("_wmo_render_cache", {"render": Resource.new()}) + loader.set("_wmo_scene_resource_cache", {"scene": PackedScene.new()}) + loader.set("_wmo_render_missing_cache", {"missing": true}) + loader.set("_wmo_scene_cache_missing", {"missing": true}) + loader.set("_wmo_missing_cache", {"missing": true}) + loader.set("_shared_tex_cache", {"texture": ImageTexture.new()}) + + loader.call("_release_runtime_caches_for_shutdown") + + _expect(not is_instance_valid(static_m2_prototype), "static M2 prototype should be freed") + _expect(not is_instance_valid(animated_m2_prototype), "animated M2 prototype should be freed") + _expect(not is_instance_valid(wmo_prototype), "WMO prototype should be freed") + for cache_name in [ + "_m2_scene_cache", + "_m2_animated_scene_cache", + "_wmo_prototype_cache", + "_m2_mesh_cache", + "_m2_static_animation_cache", + "_m2_missing_cache", + "_wmo_render_cache", + "_wmo_scene_resource_cache", + "_wmo_render_missing_cache", + "_wmo_scene_cache_missing", + "_wmo_missing_cache", + "_shared_tex_cache", + ]: + var cache: Dictionary = loader.get(cache_name) + _expect(cache.is_empty(), "%s should be empty after shutdown" % cache_name) + + loader.free() + + +func _expect(condition: bool, message: String) -> void: + if condition: + return + _failures += 1 + push_error(message) diff --git a/tools/run_render_baseline.ps1 b/tools/run_render_baseline.ps1 index a2b8417..64b2f7e 100644 --- a/tools/run_render_baseline.ps1 +++ b/tools/run_render_baseline.ps1 @@ -54,6 +54,7 @@ try { Invoke-GodotStep "project-load" @("--headless", "--path", ".", "--quit") Invoke-GodotStep "render-materials" @("--headless", "--path", ".", "--script", "res://src/tools/verify_render_materials.gd") Invoke-GodotStep "m2-unique-dedupe" @("--headless", "--path", ".", "--script", "res://src/tools/verify_m2_unique_dedupe.gd") + Invoke-GodotStep "runtime-cache-shutdown" @("--headless", "--path", ".", "--script", "res://src/tools/verify_render_runtime_cache_shutdown.gd") Invoke-GodotStep "baseline-manifest" @("--headless", "--path", ".", "--script", "res://src/tools/verify_render_baseline_manifest.gd") Invoke-GodotStep "coordinate-calibration" @("--headless", "--path", ".", "--script", "res://src/tools/verify_render_coordinate_calibration.gd") -- 2.52.0