Merge pull request 'fix(M00): release GUI capture render resources' (#15) from work/sindo-main-codex/m00-gui-rid-shutdown into master
Reviewed-on: #15
This commit was merged in pull request #15.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
# M00-QAR-GUI-RID-SHUTDOWN-001 — GUI capture RID shutdown
|
||||
|
||||
<!-- OPENWC_CLAIM:M00-QAR-GUI-RID-SHUTDOWN-001:sindo-main-codex:2026-07-15 -->
|
||||
|
||||
## 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
|
||||
@@ -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.
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -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,17 +2814,29 @@ 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()
|
||||
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:
|
||||
if _terrain_root == null:
|
||||
return
|
||||
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
@@ -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")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user