Port webgpu_app to render graph - #251
Conversation
487d552 to
e341435
Compare
|
Failing pipeline are unrelated unit tests. Review will be for Gerald :) |
a8cac59 to
36aab61
Compare
split the width, height and depht/layer count in TextureDesc into stand alone fields reduced code dup. added a missing unit test added common case factories for TextureDesc
ada13c6 to
6ca2849
Compare
|
ready to merge upstream. |
|
Qt Code Review Report Recommendation: Request changes All current CI checks pass. I did not submit a GitHub review or modify the checkout. Lint findings [L-001] Release builds accept a null allocator File: webgpu/base/RenderGraph.cpp:909 Rule: LCY-4 Finding: start_recording() uses Q_ASSERT(allocator) as its only null guard, then immediately dereferences it. The guard disappears in release builds. Mitigation: Make null handling part of the runtime contract or express non-nullness through the API. The deterministic scan produced 78 raw diagnostics. The remaining hits were primarily formatting/style rules or unchanged-line matches and are not proposed as review comments. Deep analysis findings [D-001] Spontaneous profiling callback has data races File: webgpu/base/RenderGraph.cpp:2921 Category: Thread Safety Confidence: 97/100 Finding: WGPUCallbackMode_AllowSpontaneous may execute on an arbitrary thread, but the callback reads and writes ordinary GpuProfiler and Slot fields also accessed by the render/UI thread. Trace: The callback changes results and slot->pending; execute(), free_slot(), sample_history(), and RenderGraphPanel access the same fields without synchronization. Mitigation: Use AllowProcessEvents with the existing main-thread event pump, or synchronize publication and slot reuse. [D-002] Pending callback can outlive its allocator File: webgpu/base/RenderGraph.cpp:2922 Category: Ownership & Lifecycle Confidence: 91/100 Finding: Callback userdata points inside GraphAllocator, but destroy_allocator() neither cancels nor drains pending mappings before deleting it. Trace: The callback dereferences both GpuProfiler* and Slot* after asynchronous completion. Mitigation: Drain/cancel pending callbacks before teardown or put callback state in independently owned, reference-counted storage. [D-003] GPU profiler resources are leaked File: webgpu/base/RenderGraph.cpp:508 Category: Ownership & Lifecycle Confidence: 98/100 Finding: Profiling creates a query set, resolve buffer, and three readback buffers, but none are released. Trace: destroy_allocator() tears down the resource pools and arenas only; no corresponding query-set or buffer releases exist. Mitigation: Add explicit GpuProfiler teardown after pending operations are settled. [D-004] Integer overflow bypasses buffer bounds validation File: webgpu/base/RenderGraph.cpp:2180 Category: Error Handling Confidence: 95/100 Finding: bufOffset + bufSize > bufferSize can wrap, allowing an invalid range through compilation. Trace: Public buffer ranges accept uint64_t; execution later forwards the unchecked range to WebGPU. Mitigation: Check offset > bufferSize || size > bufferSize - offset. Add overflow-boundary tests. [D-005] Texture subresource indices are silently truncated File: webgpu/base/RenderGraph.cpp:3067 Category: Error Handling Confidence: 96/100 Finding: Public mip/layer fields are uint32_t, but accesses store them as uint8_t/uint16_t before validation. Trace: Mip 256 or layer 65536 becomes zero, so validation can accept and execute a different subresource. Mitigation: Preserve uint32_t values or reject unrepresentable inputs before conversion. [D-006] RenderGraph GPU timing UI can never collect data File: apps/webgpu_app/App.cpp:245 Category: Performance & Quality Confidence: 100/100 Finding: The only RenderGraph::execute() call hardcodes profiling to false. Trace: No queries are resolved, pendingSlot remains unset, and collect_gpu_timings() immediately returns. The panel’s timing history therefore stays empty. Mitigation: Feed an actual profiling request from the panel, gated on timestamp-query support. [D-007] RenderGraph panel becomes inactive on idle frames File: apps/webgpu_app/ui/RenderGraphPanel.cpp:2739 Category: Performance & Quality Confidence: 98/100 Finding: The panel clears g_render_graph every GUI frame, while App only publishes a graph when the scene redraws. Trace: On a static scene, the next UI frame has no replacement and displays “Render graph inactive.” Mitigation: Retain a durable diagnostic snapshot, or keep the last graph until begin_frame() invalidates it. [D-008] Surface acquisition failure leaves CPU timing unbalanced File: apps/webgpu_app/App.cpp:182 Category: Error Handling Confidence: 88/100 Finding: The PR moved m_cputimer->start() before surface acquisition, but the failure branch returns without calling stop(). Mitigation: Start after successful acquisition or use scoped cleanup. [D-009] std::format lacks its required include File: webgpu/engine/overlay/OverlayRenderer.cpp:106 Category: API & C++ Correctness Confidence: 99/100 Finding: The new code uses std::format without including , relying on build-order or transitive-header behavior. Mitigation: Include directly. [D-010] Resolve accesses display as unknown File: apps/webgpu_app/ui/RenderGraphPanel.cpp:57 Category: API & C++ Correctness Confidence: 96/100 Finding: rg_access_name() omits AccessType::ResolveAttachment, so resolve accesses appear as "?". Mitigation: Add the missing case and preferably keep the enum switch exhaustive. Investigation targets (human verification needed) [I-001] Allocator ownership depends on manual teardown File: apps/webgpu_app/RenderingContext.h:81 Category: Ownership & Lifecycle Confidence: 74/100 Finding: RenderingContext owns a raw GraphAllocator* without destructor or repeated-initialization protection. How to verify: Confirm that every native/Wasm lifecycle calls destroy() exactly once and initialize() cannot repeat. Prefer RAII ownership. [I-002] Profiler resource creation is unchecked File: webgpu/base/RenderGraph.cpp:508 Category: Error Handling Confidence: 72/100 Finding: Initialization is marked complete without checking query-set or buffer creation results. How to verify: Confirm failure-object semantics for every supported WebGPU implementation and disable profiling cleanly when creation fails. [I-003] Headers depend on transitive declarations File: webgpu/engine/Window.h:57 Category: API & C++ Correctness Confidence: 72/100 Finding: TextureHandle currently reaches Window.h through unrelated renderer headers. How to verify: Run standalone-header compilation; add a direct include or forward declaration if independence is required. Summary Category Lint Deep Investigate Total Ownership & Lifecycle 1 2 1 4 The checkout remains clean: ## main...origin/main. |
2 participants
Ports the webgpu app onto the render graph added in #250. The frame is now
built by declaring resources and passes each frame instead of the previous hand-ordered
rendering.
Changes
webgpu/engine/Windowbuilds the graph each frame and drivesexecute().atmosphere, clouds, track, and the height-lines, screen-space-snow, texture, and
tile-debug overlays, plus
OverlayRendererand the sharedOverlayinterface.RenderGraphPanelis a debug UI panel for inspecting the compiled graph (passes,resources, ordering, culling), plumbed in through
ImGuiManagerandRenderingContext.App.cppandapps/webgpu_app/CMakeLists.txtregister the panel.webgpu/base/gpu_utilsused by the integration.Files
App.cpp,ImGuiManager.{h,cpp},RenderingContext.{h,cpp},ui/RenderGraphPanel.{h,cpp},CMakeLists.txtWindow.{h,cpp}, the atmosphere/cloud/track renderers, the overlays andOverlayRenderer,webgpu/base/gpu_utils.{h,cpp}