Renderer performance & memory pass — simplification, crash fixes, and editor polish#28
Merged
Merged
Conversation
Core's Dist filter overrode the workspace-level optimize "Full" down to "On"; align it with Editor/Lux-Runtime. Add LinkTimeOptimization (/GL + /LTCG) to the workspace Dist filter so it applies across the engine, apps, and vendored static libs. Release is left untouched to keep Tracy baseline comparability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
RT_DrawStaticMesh heap-allocated (and freed) a std::vector<uint8_t> for every draw command in every mesh pass on the render thread. Replace it with a persistent render-thread-only scratch member; assign() preserves the zero-fill semantics of the old value-initialized vector while retaining capacity across draws. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
… draw Every mesh-pass draw captured TransformMapData by value into its Renderer::Submit lambda, heap-copying the ObjectIndices vector once per draw per pass on the main thread. RT_DrawStaticMesh only reads four scalars from it, so snapshot those into a POD MeshDrawParams at submit time and capture that instead. Snapshot semantics are unchanged: the old closure copied the whole struct at submit time; the new one copies the four fields actually read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The upload lambda copied eight per-frame vectors into locals and then copied each local again into the closure — including the full GPUScene instance array. Use lambda init-captures: scratch-backed vectors are copied once (their capacity must stay with the member for next frame), and the frame-local instance/transient GPUScene vectors are moved. The emptiness guard evaluates before closure construction and nothing reads the moved-from locals afterwards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
SyncRenderScene rebuilt a std::vector<StaticMeshSyncItem> from scratch every frame (one proxy with several Ref<>s per static mesh). Keep it in a thread_local scratch so capacity is retained; clear it after the upsert loop so no Ref<>s survive past the call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
FlushDrawList copied the entire TextureScene handle table and MaterialScene GPU-material table into scratch every frame even when nothing changed. Give both scenes a monotonic version counter (bumped in MarkTextureDirty/MarkMaterialDirty and Clear, which every table mutation funnels through) and skip the copy when the same scene instance is submitted with an unchanged version. On unchanged frames the texture scratch is truncated back to its persistent prefix before this frame's transient handles are appended; the material scratch carries over untouched. The texture resolve loop is deliberately left running every frame — it is what picks up async texture-load completions, which do not mark the scene dirty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Add a status ledger to the optimization plan, correct A1 (the MeshDrawCommandCache is live; the remaining work is retained draw lists, with its blockers spelled out), add the A2-prime dirty-range GPUScene upload item, and update the priority order. Add Phase 2 hypotheses and an After-Phase-2 column to the baseline sheet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The vendored premake5 build rejects flags { "LinkTimeOptimization" }
(invalid value); the dedicated linktimeoptimization "On" API is the
current spelling and produces the same /GL + /LTCG output.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Changing the quality preset resized the screen-space-effect framebuffers immediately, before the BeginScene resize block resizes the main framebuffers. Passes whose framebuffers wrap the scene-color image via ExistingImages (DeferredLighting, composites, fog, sky) baked the old scene-color texture handle into their FramebufferDesc; when the resize block then recreated SceneColor, their Framebuffer::Resize early-outed (size already matched) and left them pointing at the orphaned texture. Lit geometry rendered into that orphan forever — draws still submitted, nothing on screen, unrecoverable without a renderer rebuild, and switching the preset back just repeated the cycle one generation behind. RefreshScreenSpaceEffectResources now defers to the BeginScene resize block, which already runs ResizeScreenSpaceEffectResources after the main framebuffers and with aliasing cleared. This covers all callers (SetQualityPreset, ApplyProjectSettings, the editor panel) including resolution-unchanged preset switches. Also harden ClearRenderTargetAliasing: always RT_Invalidate the cleared aliased images. ApplyRenderTargetAliasing excludes invalid images from the rebuilt alias graph, so a storage-stripped image was never re-aliased and never recovered, and fed a null handle into descriptor baking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
InvalidateAndUpdate added changed inputs to InvalidatedInputResources and called Bake() when non-empty, but never removed entries — the only clear lived in dead #if TODO code. One invalidation (guaranteed by the startup viewport resize) left the set non-empty forever, so every dynamic pass rebuilt all its binding sets for all frames in flight, every frame. Clear the set at the top of InvalidateAndUpdate: the handle-comparison loop re-populates anything actually stale, Bake() re-adds genuinely-null deferred resources for retry, and null-to-valid transitions still bake because the stored handles stay null until a successful bake. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The snapshot's validation loops (every instance, every material, object indexes twice, plus std::format diagnostics) ran unconditionally in FlushDrawList every frame, including in runtime and Dist builds, purely to populate the editor's Renderer Debugger panel. Gate it behind a per-frame request flag that the panel sets while its GPU Scene section is open; the snapshot is empty (and free) otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The pre-integration visibility pyramid is consumed solely by the SSR pass, but its per-mip compute dispatches ran every frame regardless of EnableSSR. Register the pass and its graph resource only when SSR is on; the SSR node's read-dependency append is already inside the same gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Both compute dispatches ran every frame even with zero point/spot lights. Now the build pass early-outs (its AABBs feed only the cull dispatch) and the cull pass zero-fills the per-cluster grids instead of dispatching, so lighting shaders read count=0 everywhere. Lights appearing re-run the full path the same frame since the grid is rebuilt per frame while lights exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The ~40-clamp UB rebuild (plus a 3-layer cloud loop and local-fog volume copies) and its upload ran every frame even with sky, clouds, and fog all disabled. When inactive, write a disabled-flags UB once per frame-in-flight buffer and go idle; active frames run the full path unchanged (the struct carries per-frame time fields, so per-frame upload is correct then) and re-arm the idle counter for the next transition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
A 4-layer 4096x4096 D24S8 cascade array costs ~268 MB of VRAM plus the bandwidth to render it — too heavy as the everyday default for a 60-80 FPS target. Default (options, project settings, and the High preset) is now Tier_2K with soft shadows; Ultra raises to 4K and Cinematic to 8K. Existing projects keep whatever their serialized settings say. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The per-frame timer query begin/end/poll machinery exists to feed the editor's Renderer Debugger panels; Release keeps it, shipping builds now skip it. All readers already early-return when queries are off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Framebuffers bake their attachment images' nvrhi texture handles into their FramebufferDesc at RT_Invalidate. Images are recreated in place (same Ref, new handle) on resize and aliasing changes, and Framebuffer::Resize early-outs purely on size — so a framebuffer that wraps a shared image via ExistingImages can be left rendering into the orphaned old texture with no path that ever repairs it. That is how the exported runtime lost all lit geometry in fullscreen: the deferred lighting framebuffer (which wraps SceneColor) went one generation stale on the fullscreen startup path, so every lit pixel landed in a dead texture while the sky wrote to the live one. Add Framebuffer::HasStaleAttachments (baked handles vs current) and a per-frame sweep in BeginScene that re-invalidates any stale framebuffer, owners before wrappers so repairs converge in one sweep. Cost when clean is a few pointer compares per framebuffer; a repair logs a warning naming the framebuffer so the triggering path can be identified from the field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Ref<> propagates constness to the pointee, so the const Ref<Framebuffer>& lambda parameter made the non-const Invalidate() call ill-formed (C2662). Take the smart pointers by value; GetTargetFramebuffer() is non-const too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Both are blurred HDR color chains; fp16 covers the range and halves the memory and bandwidth of every down/upsample step. Shader storage layouts updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Every mesh vertex/index buffer and texture upload created its own RenderCommandBuffer and called executeCommandList immediately under the global graphics-queue mutex - one vkQueueSubmit per resource, serialized against the render thread mid-frame. Loading a model with dozens of meshes meant dozens of queue submits in one frame: the classic streaming hitch. The per-buffer command lists were also retained for the resource's whole lifetime. Add Renderer::RecordResourceUpload/FlushResourceUploads: a mutex-guarded shared nvrhi command list accumulates uploads from any thread and is submitted once. Ordering is guaranteed structurally: RT_Submit flushes the pending batch before executing any command list, so uploads always reach the queue ahead of every possible consumer (frame rendering, readbacks, env-map bakes). nvrhi stages source data at record time, so caller-side data lifetimes are unchanged. The GPU-blocking readback path (CopyToHostBuffer) keeps its dedicated list. Also downgrade the per-creation TextureCube warning to a trace tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Replaces the stale "not much is implemented" intro with an audited feature list (renderer, physics, scripting, assets, editor, runtime), an explicit limitations section, active development notes, and a technology table. Getting-started and CI sections kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetStats reported Used from a VMA-side tracker that only the dead #if OLD allocation paths ever fed - all live allocations go through NVRHI, so the HUD showed ~0 used. TotalAvailable also summed every heap's budget, including host-visible. Query per-heap usage/budget with VkPhysicalDeviceMemoryBudgetPropertiesEXT (physical-device-level functionality: requires the extension to be supported, not enabled) and sum only DEVICE_LOCAL heaps. This is process-wide driver truth and includes NVRHI's allocations. Falls back to the previous behavior when the extension is unsupported. Both HUD consumers (editor Render Stats, runtime overlay) read Used/TotalAvailable and need no changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Two findings from the memory audit: 1. m_TriangleCache stored three full Vertex structs per triangle (~168 B/ triangle) and was built by both the Assimp importer and the runtime mesh deserializer - but GetTriangleCache had zero callers anywhere. Removed the member, accessor, and both build loops. 2. m_Vertices/m_Indices stayed in system RAM forever, doubling every mesh. The only CPU consumers are the two Jolt cooking paths, and they read vertex positions + indices exclusively. The standalone runtime now compacts each MeshSource after GPU upload: positions are kept in a positions-only array (12 B/vertex instead of the full ~56 B vertex), indices are kept for cooking, and the full vertex array is freed. Physics reads through new GetVertexCount/GetVertexPosition accessors that work in both modes. The editor retains full data (mesh export and serialization read it); behavior there is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
SelectedGeometry (RGBA32F + depth), the three JumpFlood outline targets (RGBA32F), AO-Debug (RGBA32F), GBufferDebug (RGBA16F), and the wireframe target were always allocated at full viewport size even in shipped games, where selection and debug views cannot occur. Add SceneRendererSpecification::EnableEditorRenderTargets (default true; the runtime sets it false) gating their creation. All references null-guard: graph registrations skip absent passes, the resize block and pass bodies check first, and the resize/repair/stats helpers already guarded. Editor behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
- Spot-shadow candidate scoring, sorting, atlas sizing, and per-light matrix building now skip entirely with zero spot lights (shaders read Count=0; the state hash resets so returning lights re-render). - The directional shadow UBO uploads only when the cascade matrices actually changed (memcmp gate), once per frame-in-flight buffer, then idles - it previously uploaded every frame while being mutated only on cascade recompute. - The UpdateStatistics draw-list re-walk compiles out of Dist builds; the functional tail (dynamic render resolution) stays in all builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
RenderPass::OnShaderReloaded and ComputePass::OnShaderReloaded ran the descriptor-set rebake synchronously on whatever thread triggered the reload (shader hot-reload runs off UpdateDirtyShaders, called directly from Renderer2D::BeginScene rather than through Renderer::Submit). The render thread can still have queued GPU work reading the same pass's binding sets, so mutating them immediately races with that work - exactly the pattern Pipeline::Invalidate and PipelineCompute::CreatePipeline already avoid by deferring their real work via Renderer::Submit (RT_Invalidate / RT_CreatePipeline). This explains why the crash changed shape after the previous fix: the Vulkan validation error went away (the binding/ordering bugs were fixed) but a raw access violation with no validation output took its place - consistent with a CPU-side data race corrupting DescriptorSetManager's state rather than a GPU-visible resource mismatch. Both OnShaderReloaded methods now capture a Ref to themselves and submit the actual m_DescriptorSetManager.OnShaderReloaded() call through Renderer::Submit, mirroring the two existing reload paths exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The frame-1 access violation has no Vulkan validation output, so it is a CPU-side fault inside a pass execute callback rather than a GPU resource mismatch. Pass names are intentionally not materialized on the per-frame executable graph path (allocation avoidance), so the render-graph warnings only show 'Pass N'. Add a zero-alloc const char* DebugName to PassDesc (pointer to the string-literal name passed to addPass) and log the pass sequence on the first executed frame only. The last 'Executing pass' line before the crash names the faulting pass; a trailing 'executed without crashing' line confirms if frame 1 survived. Temporary - to be removed once the crash is located. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
…ll compile
The diagnostic commit added DebugName after Name in PassDesc, which
shifted the positional fields in the render-graph self-test aggregate
initializers (graph.AddPass({ "Name", {reads}, {writes}, PassFlags }))
and broke the Core build with C2665. Move the field to the end of the
struct; the 4-element initializers map to Name/Reads/Writes/Flags again
and leave Execute + DebugName defaulted. The addPass lambda sets
DebugName via member assignment, so ordering does not affect it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The frame-1 crash is inside the GBuffer pass (pass [9]). Add budget-gated trace logs that bracket every sub-step of the render-pass begin (enter -> per-attachment clear -> depth clear -> commit state -> Prepare -> GetBindingSets -> RT_CommitGraphicsState -> done) and of the GBuffer draw (enter -> bind material -> commit -> drawIndexed[Indirect]). The last logged line before the crash names the exact faulting call. The budgets self-terminate (400 BRP lines, 16 GBuffer-draw lines) so later frames are not spammed; the draw logs are gated to the GBuffer_Static shader so shadow/pre-depth draws do not consume them. Also add the missing ImageFormatToString RG32UI case. Temporary - to be removed with the earlier pass logging once the crash is located. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
TAA was already off by default (m_Options.EnableTAA defaults false and is not persisted in project settings) and only reachable via the editor's TAA checkbox. Remove it: - Do not create m_TAAResolvePass or the two full-viewport RGBA16F history images at init (saves a compute pipeline + ~2x viewport of VRAM). The members stay null; every consumer null-guards (the resize sweep checks each image, TAAResolvePass early-returns on null pass/history). - Delete the editor TAA toggle (+ its history-blend/sharpness sub-options) so EnableTAA can no longer be set true. Everything else is already gated on m_Options.EnableTAA and goes inert with the flag pinned false: the render-graph TAA node, camera sub-pixel jitter (so the image is no longer jittered), the TAA texture mip-bias, and the resolve dispatch. The TAA shader and TAAResolvePass function are left in place for revertibility. Velocity is untouched (still consumed by the SSR/GTAO/cloud temporal passes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The frame-1 crash is fixed (AO-composite fold reverted). Strip the temporary tracing added to locate it: - RenderGraph::Execute first-frame per-pass logging - [BRP] bracket logs in Renderer::BeginRenderPass - [DRAW] bracket logs in RT_DrawStaticMesh Kept: PassDesc::DebugName (zero-cost, lets the render graph name passes on the executable path for future diagnostics) and the ImageFormatToString RG32UI case (correctness). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
These three share one creation block, so they're cut together. No longer created at init: the sky-atmosphere / cloud / cloud-composite / fog fullscreen passes, the cloud noise-bake passes + three 3D noise volumes (128^3 + 2x 32^3 RGBA16F), the cloud temporal pass + half-res history ping-pong, and all their pipelines/materials/framebuffers. Reclaims that VRAM and removes their per-frame GPU cost (cloud raymarch was the heaviest optional pass). The members stay null and every path is inert: the render-graph nodes gate on the pass existing (+ frame flags), the execute functions early-return on null, and the resize/repair sweeps null-check. Added the one missing guard in RecreateRenderTargetFramebuffers where the three composite framebuffers were resized unconditionally. Shaders and pass functions are kept on disk for revertibility; the skybox path is untouched, so scenes still get a sky. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
The Cinematic quality preset used an 8K directional shadow atlas; drop it to 2K to match the other presets' shadow budget (High=4K, Medium/Low=2K/1K). An 8K atlas is a heavy per-frame shadow-render + memory cost with little visible payoff at this scene scale. Resolution only; cascade count and shadow distance unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
…r texture Texture2D/TextureCube::GenerateMips built a fresh LinearSample / LinearSampleUInt PipelineCompute for every texture, so loading a project produced hundreds of redundant 'Creating compute pipeline' driver builds (the bulk of the first-launch/asset-load stutter and log spam). Add Renderer::GetOrCreateMipGenPipeline(shader): a process-wide, mutex-guarded cache keyed by shader hash that builds each mip-gen pipeline once and hands the same object to every GenerateMips call. The per-texture ComputePass + per-mip Material bindings are unchanged, so each texture still mips against its own image views. Cleared in Renderer::Shutdown before device teardown. Net: the compute mip generator is built twice total (float + uint) instead of once per texture -> hundreds fewer pipeline builds at load. (A cross-run on-disk cache for the ~50 real render pipelines is separate: nvrhi owns pipeline creation, so it needs submodule changes.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Foundation for scheduling independent compute passes on the async compute queue. No pass is wired yet, so runtime behavior is unchanged except that nvrhi is now given a compute queue. - Window.cpp: deviceParams.enableComputeQueue = true, so the device passes a compute VkQueue to nvrhi (previously null -> executeCommandList on the compute queue was impossible). Desktop NVIDIA always has a compute queue family, so device creation still succeeds. - RenderCommandBuffer: can now be created for a specific nvrhi::CommandQueue (Graphics default, so existing buffers are unchanged). Command lists are created with CommandListParameters().setQueueType(queue); RT_Submit runs executeCommandList(list, queue) and records the returned execution instance (GetLastExecutionInstance()). - Renderer::QueueWaitForCommandList(waitQueue, execQueue, instance): thin wrapper over IDevice::queueWaitForCommandList for cross-queue ordering (no manual semaphores). - SceneRendererOptions::EnableAsyncCompute (default false): the master switch the per-pass wiring will honor next. Next: route cluster light-culling onto a compute RenderCommandBuffer with one compute->graphics sync before deferred lighting, gated on the flag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
…ectness-first) With EnableAsyncCompute (still off by default), cluster build + light culling run on the compute queue instead of inline on graphics: - A dedicated m_ComputeCommandBuffer (compute queue) is created at init. - FlushDrawList records both cluster passes onto it after the uploads, submits it, and the graphics submit waits on that instance via Renderer::QueueWaitForCommandList so deferred lighting reads valid light grids/index lists. - The two cluster passes route their dispatches/barriers/clears to the compute buffer when async, and skip the graphics-buffer GPU perf markers (timer queries live on the graphics buffer). - BuildRenderGraph omits the two cluster nodes when async (they ran on compute); the graph never modeled their SSBOs anyway, so no graph edge is lost — cross-queue ordering is the queue wait. Correctness-first: this is the smallest cross-queue slice and validates that compute-written SSBOs are safely read on graphics. The graphics queue waits up front, so it does NOT overlap graphics work yet (perf-neutral); the graphics-submit split for real overlap is the next step. Off path is unchanged (cluster passes stay in the graph on the graphics buffer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Runtime toggle for EnableAsyncCompute so the compute-queue cluster light-culling path can be exercised without a rebuild. Flipping it re-registers the cluster nodes in/out of the render graph (structure hash change -> one recompile) and routes the dispatches to the compute vs graphics command buffer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Previously the graphics queue waited for the async cluster culling up front,
so the two queues never actually overlapped. Split FlushDrawList's graphics
recording at the first consumer of the compute output ("Deferred Lighting"):
the pre-lighting half submits with no wait (overlapping the compute cluster
work), then queueWaitForCommandList is inserted, then the deferred-onward half
submits and waits.
- RenderGraph::Execute gains a [begin,end) range overload.
- RenderCommandBuffer::Begin/End gain a recordFrameQueries flag so the second
half doesn't re-reset the per-frame-in-flight timer/pipeline-stat pool the
first half already bracketed (would trip Vulkan validation).
- All gated behind EnableAsyncCompute (off by default); the OFF path and the
no-Deferred-Lighting fallback keep the original single-submit behavior.
Foundation only: cluster culling is cheap, so the win is marginal until the
expensive passes (GTAO/SSR) move to compute and shadows reorder after GBuffer.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
…pute wait" This reverts commit 94e025d.
The per-pass CPU/GPU timing table rendered in registration order, so finding the frame's most expensive passes meant eyeballing ~20 rows. Add ImGui column sorting (tristate, so the default stays registration order): click "GPU ms" to surface the real bottlenecks at a glance. Sorting reorders a display-index list and leaves the underlying stats untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Two device-creation settings were leaving significant performance on the table: - maxFramesInFlight was 1, so the present loop blocked on full GPU completion every frame — the CPU and GPU ran in lockstep and the triple-buffered swapchain / FramesInFlight=3 resource sets went unused. Raise to 2 so the CPU can stay a frame ahead. Every per-frame resource is already sized for 3, so this is within the existing buffering. - enableDebugRuntime was true unconditionally, loading VK_LAYER_KHRONOS_validation (which validates every Vulkan call) even in Release/Dist. Gate it to LUX_DEBUG so shipping/perf builds don't pay the validation CPU tax; Debug still gets it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
Replace Hazel's orange/cyan palette with a distinct, cleaner look: - New Colors::Theme palette — cool-graphite neutral surfaces with a single luminous indigo-violet accent (#7C83F8, "Lux" = light), used sparingly for selection, active separators, checkmarks, focus and drag/drop. - Retint the theme's remaining hardcoded Hazel colors (warm tabs, blue separators) onto the accent so the UI is cohesive. - Softer rounding on controls (frames, grabs, tabs, scrollbars, popups) and pill scrollbars; spacing/padding left untouched so panels' hand-tuned cursor offsets still line up. - WindowBg now derives from the palette instead of a flat gray override. Pure colour/style values — no layout or API changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0138sgSFpVegqRsGVXVnjBa8
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR compacts mesh CPU geometry after GPU upload, batches resource uploads with multi-queue command buffer support, adds shader hot-reload descriptor rebaking, restructures SceneRenderer (packed GBuffer IDs, editor-target gating, TAA/atmosphere removal, async compute, GPUScene dirty-range uploads, MeshDrawParams refactor), updates the editor theme/panels, and revises docs and build configuration. ChangesMesh CPU Geometry Compaction
Renderer Resource Upload Batching and Multi-Queue Support
Shader Hot-Reload and Descriptor Rebaking
SceneRenderer Feature Overhaul
Editor Theme and Panels
Misc Engine Tweaks and Build Configuration
Documentation Updates
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR is a broad renderer-focused performance/memory/stability pass, paired with a small editor refresh and documentation updates. It introduces multi-queue plumbing (compute queue + optional async scheduling), reduces per-frame and per-resource CPU overhead, and simplifies the renderer by removing several major atmospheric/TAA features while null-guarding remaining consumers.
Changes:
- Reduce runtime CPU hitches and per-frame overhead via batched resource uploads, scratch reuse, draw-order caching, and descriptor-set rebake fixes.
- Cut VRAM/bandwidth costs via format reductions (RGBA32F→RGBA16F), GBuffer ID target merge, skipping editor-only RTs in runtime, and lower shadow defaults.
- Add experimental async-compute scaffolding (off by default), renderer debugger sorting/panel updates, and update optimization/docs/README.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Major README rewrite (feature/status summary, limitations, getting started, tech stack). |
| premake5.lua | Enables link-time optimization in Dist at workspace level. |
| Lux-Runtime/src/RuntimeLayer.cpp | Disables editor-only render targets in runtime renderer spec. |
| Lux-Runtime/src/RuntimeApplication.cpp | Disables full CPU mesh retention in runtime (memory compaction path). |
| Editor/Source/Panels/SceneRendererPanel.cpp | Removes TAA UI (feature removed) and adds async compute toggle (experimental). |
| Editor/Source/Panels/RendererDebuggerPanel.cpp | Makes pass-timing table sortable; requests GPUScene snapshot while panel open. |
| Editor/Resources/Shaders/PostProcessing/Pre-Convolution.glsl | Switches storage image format to rgba16f for bandwidth/VRAM reduction. |
| Editor/Resources/Shaders/PostProcessing/Bloom.glsl | Switches storage image format to rgba16f for bandwidth/VRAM reduction. |
| Editor/Resources/Shaders/Include/GLSL/LuxGBuffer.glslh | Packs material/object IDs into a single uvec2 (RG32UI target). |
| Editor/Resources/Shaders/GBufferDebug.glsl | Updates bindings and DecodeGBuffer call for packed material/object ID target. |
| Editor/Resources/Shaders/GBuffer_Static.glsl | Writes packed uvec2 material/object IDs and shifts velocity MRT location. |
| Editor/Resources/Shaders/DeferredLighting.glsl | Reads packed material/object ID target. |
| docs/RENDERER_PERF_BASELINE.md | Extends perf table and adds Phase 2/3 hypotheses and correctness gates. |
| docs/ENGINE_OPTIMIZATION_PLAN.md | Adds a detailed status ledger and extensive optimization notes/results. |
| Core/Source/Lux/Scene/Scene.cpp | Reuses thread-local scratch for scene sync items; clears at end to avoid shutdown races. |
| Core/Source/Lux/Renderer/VertexBuffer.cpp | Routes initial buffer uploads through shared batched upload path. |
| Core/Source/Lux/Renderer/TextureScene.h | Adds monotonic version counter for texture table change tracking. |
| Core/Source/Lux/Renderer/TextureScene.cpp | Increments texture-table version on clear/dirty operations. |
| Core/Source/Lux/Renderer/Texture.cpp | Caches mip-gen compute pipeline; downgrades noisy TextureCube log; uses batched uploads. |
| Core/Source/Lux/Renderer/SceneRenderer.h | Updates defaults (2K shadows), adds async-compute option, editor RT gating, debug snapshot request, draw-order caching, and multiple scratch/epoch trackers. |
| Core/Source/Lux/Renderer/SceneRenderer.cpp | Core renderer perf/memory/simplification changes (TAA/atmosphere removal, format changes, RT gating, dirty-range uploads, draw-order cache, async compute plumbing, framebuffer self-heal, etc.). |
| Core/Source/Lux/Renderer/RenderPass.h | Adds OnShaderReloaded hook for descriptor rebake after in-place shader recompiles. |
| Core/Source/Lux/Renderer/RenderPass.cpp | Registers shader dependencies; defers descriptor rebake to render thread on reload. |
| Core/Source/Lux/Renderer/RenderGraph.h | Adds DebugName to PassDesc for crash diagnostics without per-frame string allocation. |
| Core/Source/Lux/Renderer/Renderer.h | Adds batched upload API, shader dependency registration for passes, mip-gen pipeline cache, and cross-queue wait helper. |
| Core/Source/Lux/Renderer/Renderer.cpp | Implements upload batching, mip-gen pipeline cache, pass dependency reload handling, cross-queue wait, plus minor loop/scissor fixes. |
| Core/Source/Lux/Renderer/RenderCommandBuffer.h | Adds queue selection, execution-instance tracking for cross-queue synchronization. |
| Core/Source/Lux/Renderer/RenderCommandBuffer.cpp | Creates queue-typed command lists; stores execution instance; flushes upload batch before submits; disables queries in Dist. |
| Core/Source/Lux/Renderer/Mesh.h | Adds positions-only accessors + CPU geometry compaction controls; removes triangle cache API. |
| Core/Source/Lux/Renderer/Mesh.cpp | Implements CPU geometry compaction (positions-only) gated by runtime-set static flag. |
| Core/Source/Lux/Renderer/MaterialScene.h | Adds monotonic version counter for material table change tracking. |
| Core/Source/Lux/Renderer/MaterialScene.cpp | Increments material-table version on clear/dirty operations. |
| Core/Source/Lux/Renderer/IndexBuffer.cpp | Routes initial buffer uploads through shared batched upload path. |
| Core/Source/Lux/Renderer/Image.h | Adds RG32UI format support (for packed GBuffer IDs). |
| Core/Source/Lux/Renderer/Image.cpp | Restricts UAV usage to true compute-write cases; routes initial texture data through batched upload path. |
| Core/Source/Lux/Renderer/Framebuffer.h | Adds stale-attachment detection (ExistingImages baked-handle repair support). |
| Core/Source/Lux/Renderer/Framebuffer.cpp | Implements stale-attachment detection by comparing baked vs current texture handles. |
| Core/Source/Lux/Renderer/ComputePass.h | Adds OnShaderReloaded hook; adjusts binding set access signature. |
| Core/Source/Lux/Renderer/ComputePass.cpp | Registers shader dependency; defers descriptor rebake to render thread on reload. |
| Core/Source/Lux/Project/Project.h | Changes default shadow resolution tier in serialized project config (Tier_2K). |
| Core/Source/Lux/Platform/Vulkan/VulkanAllocator.cpp | Uses VK_EXT_memory_budget for driver-truth VRAM usage/budget when available. |
| Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.h | Adds granular BakeSet and OnShaderReloaded APIs. |
| Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp | Fixes perpetual re-bake bug, adds shader-reload rebake path, and rebakes only affected sets. |
| Core/Source/Lux/Physics/PhysicsScene.cpp | Uses new MeshSource position accessors (works after CPU compaction). |
| Core/Source/Lux/Physics/JoltPhysics/JoltShapes.cpp | Uses new MeshSource position accessors (works after CPU compaction). |
| Core/Source/Lux/ImGui/ImGuiLayer.cpp | Theme polish (background surfaces, accent feedback, rounding tweaks, more consistent color usage). |
| Core/Source/Lux/ImGui/Colors.h | Replaces palette with cool-graphite base and indigo-violet accent theme. |
| Core/Source/Lux/Core/Window.cpp | Enables compute queue, raises maxFramesInFlight to 2, disables validation layer outside Debug. |
| Core/Source/Lux/Asset/MeshRuntimeSerializer.cpp | Drops triangle-cache build; compacts CPU geometry after GPU upload in runtime. |
| Core/Source/Lux/Asset/AssimpMeshImporter.cpp | Removes triangle-cache construction during import (no consumers). |
| Core/premake5.lua | Dist optimization switched to Full (no longer overridden to On). |
Comments suppressed due to low confidence (2)
Core/Source/Lux/Renderer/SceneRenderer.cpp:6347
- When async compute is enabled, the cluster passes run on the compute queue but depend on camera/light UBOs written by the prior upload submission (graphics queue). There is currently no cross-queue wait from Compute→Graphics here, so the compute queue can read stale/partially-written UBO data.
Ref<Image2D> SceneRenderer::GetFinalPassImage()
{
if (Ref<Image2D> debugImage = GetDebugViewImage(m_DebugViewMode))
return debugImage;
Core/Source/Lux/Renderer/SceneRenderer.cpp:6806
- In the no-local-lights fast path, the buffers are cleared but no resource-state transition/barrier is issued afterwards. The normal dispatch path explicitly transitions these buffers to ShaderRead via BufferMemoryBarrier; without an equivalent here, deferred lighting may read the grids/counters in the wrong state on the same frame.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Core/Source/Lux/Renderer/SceneRenderer.cpp (1)
4681-4699: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
returnhere abortsFlushDrawList; usecontinueinstead.At
Core/Source/Lux/Renderer/SceneRenderer.cpp:4686-4687, this is inside the per-slot loop body, so a matching texture exits the whole function and skips the remaining uploads/render-graph work. That should only skip the current slot.Fix
- if (m_GPUMaterialTextures[textureIndex].Raw() == texture.Raw()) - return; + if (m_GPUMaterialTextures[textureIndex].Raw() == texture.Raw()) + continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Source/Lux/Renderer/SceneRenderer.cpp` around lines 4681 - 4699, The early return in SceneRenderer::FlushDrawList is aborting the entire texture upload loop when a slot already matches, which skips remaining slots and later render-graph updates. Change that guard in the per-texture loop to skip only the current iteration so the rest of the uploads still run, and keep the existing GPU material texture assignment and pass input updates for non-matching slots.
🧹 Nitpick comments (3)
Core/Source/Lux/ImGui/ImGuiLayer.cpp (1)
357-360: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded accent RGB duplicates
Colors::Theme::accent.
ImColor(124, 131, 248, 45)(Line 359),ImColor(124, 131, 248, 90)(Line 360), andImColor(124, 131, 248, 150)(Line 394) repeat the same RGB triple asColors::Theme::accentdefined inCore/Source/Lux/ImGui/Colors.h. If the accent hue is retuned inColors.h, these three sites won't follow, causing visual drift from the rest of the theme.♻️ Derive from the theme constant instead of duplicating the literal
- colors[ImGuiCol_TabHovered] = ImColor(124, 131, 248, 45); - colors[ImGuiCol_TabActive] = ImColor(124, 131, 248, 90); + colors[ImGuiCol_TabHovered] = ImColor(ImGui::ColorConvertU32ToFloat4(Colors::Theme::accent)).Value * ImVec4(1, 1, 1, 45.0f / 255.0f); + colors[ImGuiCol_TabActive] = ImColor(ImGui::ColorConvertU32ToFloat4(Colors::Theme::accent)).Value * ImVec4(1, 1, 1, 90.0f / 255.0f); ... - colors[ImGuiCol_SeparatorHovered] = ImColor(124, 131, 248, 150); + colors[ImGuiCol_SeparatorHovered] = ImColor(ImGui::ColorConvertU32ToFloat4(Colors::Theme::accent)).Value * ImVec4(1, 1, 1, 150.0f / 255.0f);(Or introduce a small helper, e.g.
WithAlpha(Colors::Theme::accent, alpha), and use it consistently.)Also applies to: 391-394
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Source/Lux/ImGui/ImGuiLayer.cpp` around lines 357 - 360, The tab and button accent colors in ImGuiLayer are hardcoding the same RGB values as Colors::Theme::accent instead of deriving from the theme constant. Update the color assignments in ImGuiLayer to use Colors::Theme::accent with varying alpha, or a small helper like WithAlpha, and apply the same fix to the other repeated accent uses in the same function so all accent-tinted ImGuiCol values stay in sync with the theme.Core/Source/Lux/Renderer/Mesh.h (2)
218-219: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo bounds checking inside
GetVertexPosition/GetVertexCount.Both accessors trust the caller entirely. Current call sites (
JoltShapes.cpp,PhysicsScene.cpp) bounds-check before calling, but as a public API on a widely-friended class, an out-of-rangeindexwill silently read out-of-bounds memory (UB) rather than fail safely.🛡️ Suggested defensive check
- glm::vec3 GetVertexPosition(size_t index) const { return m_Vertices.empty() ? m_CollisionPositions[index] : m_Vertices[index].Position; } + glm::vec3 GetVertexPosition(size_t index) const + { + LUX_CORE_ASSERT(index < GetVertexCount(), "MeshSource::GetVertexPosition index out of range"); + return m_Vertices.empty() ? m_CollisionPositions[index] : m_Vertices[index].Position; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Source/Lux/Renderer/Mesh.h` around lines 218 - 219, `Mesh::GetVertexPosition` and `Mesh::GetVertexCount` currently expose unchecked access to vertex data, so add a defensive bounds guard in `GetVertexPosition` before indexing `m_Vertices` or `m_CollisionPositions`, and make `GetVertexCount`/its callers rely on a safe validated index path. Keep the fix localized to `Mesh` so public callers like the ones in `JoltShapes.cpp` and `PhysicsScene.cpp` don’t depend on undefined behavior if an invalid index slips through.
264-268: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNon-atomic static toggle read across threads.
s_RetainFullCPUGeometryis a plainstatic bool, set once inRuntimeApplication's constructor before asset loading, then read fromCompactCPUGeometry()on whatever thread constructsMeshSourceinstances (potentially asset-loading worker threads). Given the single early write happens-before any mesh loading in practice, this is unlikely to cause real issues, but it's not formally synchronized.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Core/Source/Lux/Renderer/Mesh.h` around lines 264 - 268, The toggle used by CompactCPUGeometry is a shared static state accessed across threads, so make s_RetainFullCPUGeometry thread-safe rather than a plain bool. Update the declaration in Mesh to use an atomic or other synchronized mechanism, and adjust the RuntimeApplication constructor write and CompactCPUGeometry read sites to use the matching atomic load/store (or equivalent) so the flag is formally synchronized even when MeshSource instances are built on worker threads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp`:
- Around line 170-183: Reload logic in DescriptorSetManager is restoring the
entire saved RenderPassInput and overwriting the newly reflected resource type,
so update the merge in the savedInputs loop to preserve the current reflection’s
Type (and IsWriteable) from InputDeclarations/InputResources while copying only
the bound handles from the saved input, or skip restoring that entry when the
reflected type differs; use the RenderPassInput assignment in the reload path
and the BakeSet flow as the key places to adjust.
In `@Core/Source/Lux/Platform/Vulkan/VulkanAllocator.cpp`:
- Around line 44-102: QueryDeviceLocalMemoryBudget in VulkanAllocator.cpp is
checking for VK_EXT_memory_budget support but that does not guarantee the budget
properties are populated. Update the device creation path in VulkanDevice.cpp to
add VK_EXT_MEMORY_BUDGET_EXTENSION_NAME to deviceExtensions when supported, so
vkGetPhysicalDeviceMemoryProperties2 can actually return valid
VkPhysicalDeviceMemoryBudgetPropertiesEXT data for this query.
In `@Core/Source/Lux/Renderer/RenderCommandBuffer.cpp`:
- Around line 15-16: The RenderCommandBuffer constructor declaration and
definition are out of sync, since RenderCommandBuffer.cpp defines a 4-argument
constructor that takes nvrhi::CommandQueue while RenderCommandBuffer.h still
exposes only the 3-argument version. Update the RenderCommandBuffer class
declaration to include the queue parameter so it matches the existing
constructor implementation and the Create() factory call that forwards queue.
Make sure the signature in RenderCommandBuffer and any related constructor
references are consistent across declaration and definition.
In `@Core/Source/Lux/Renderer/RenderCommandBuffer.h`:
- Line 22: The RenderCommandBuffer::Create factory is forwarding a queue
argument that the class declaration does not accept, so update the private
RenderCommandBuffer constructor declaration to match the 4-argument definition
used by Create and the .cpp implementation. Make sure the constructor signature
in RenderCommandBuffer includes the nvrhi::CommandQueue parameter in addition to
count, enableQueries, and debugName so Ref<RenderCommandBuffer>::Create can
instantiate it correctly.
In `@Core/Source/Lux/Renderer/SceneRenderer.cpp`:
- Line 5098: The submit path is using dirty-range state inconsistently: the
pre-submit check references gpuSceneRangeRows, but the lambda passed to
Renderer::Submit reads sceneRangeRows and sceneRangeList without capturing the
same data. Update SceneRenderer::Renderer::Submit call so the lambda captures
and uses the exact dirty-range variables being checked, or rename the locals to
match consistently, ensuring the same range data is passed through.
In `@docs/ENGINE_OPTIMIZATION_PLAN.md`:
- Around line 126-131: The aliasing rejection note is stale because it still
mentions cloud history and TAA as active consumers. Update the text in the
optimization plan section to remove those obsolete references and keep only the
currently relevant exclusions, using the existing IsRenderGraphAliasCandidate
discussion as the anchor. Make sure the wording reflects the PR’s current state
so the note no longer implies those removed paths still block aliasing.
In `@docs/RENDERER_PERF_BASELINE.md`:
- Around line 208-209: The docs use inconsistent project-generation entrypoints,
so update the guidance in RENDERER_PERF_BASELINE to match the same script
entrypoint used elsewhere in the docs. Locate the references around the baseline
regeneration note and make them consistent with the shared project-generation
path referenced in README so setup instructions use one command only.
In `@README.md`:
- Around line 11-25: Update the “What works today” section so it no longer
claims removed renderer paths are available: remove the cloud, atmosphere/fog,
and TAA bullets or rewrite them to match the current renderer surface. Keep the
rest of the list aligned with the actual features still present, and use the
existing “Rendering (Vulkan 1.4)” subsection as the place to reflect the current
state.
- Around line 83-87: The Vulkan SDK setup instructions are inaccurate because
scripts/Setup.bat and scripts/Setup.py only detect whether the SDK is installed
and do not download or launch an installer. Update the dependency setup section
to tell users to install the Vulkan SDK manually, then rerun Setup.bat, and if
there is a real helper for bootstrap/install flow, reference that explicitly
instead of describing non-existent behavior.
---
Outside diff comments:
In `@Core/Source/Lux/Renderer/SceneRenderer.cpp`:
- Around line 4681-4699: The early return in SceneRenderer::FlushDrawList is
aborting the entire texture upload loop when a slot already matches, which skips
remaining slots and later render-graph updates. Change that guard in the
per-texture loop to skip only the current iteration so the rest of the uploads
still run, and keep the existing GPU material texture assignment and pass input
updates for non-matching slots.
---
Nitpick comments:
In `@Core/Source/Lux/ImGui/ImGuiLayer.cpp`:
- Around line 357-360: The tab and button accent colors in ImGuiLayer are
hardcoding the same RGB values as Colors::Theme::accent instead of deriving from
the theme constant. Update the color assignments in ImGuiLayer to use
Colors::Theme::accent with varying alpha, or a small helper like WithAlpha, and
apply the same fix to the other repeated accent uses in the same function so all
accent-tinted ImGuiCol values stay in sync with the theme.
In `@Core/Source/Lux/Renderer/Mesh.h`:
- Around line 218-219: `Mesh::GetVertexPosition` and `Mesh::GetVertexCount`
currently expose unchecked access to vertex data, so add a defensive bounds
guard in `GetVertexPosition` before indexing `m_Vertices` or
`m_CollisionPositions`, and make `GetVertexCount`/its callers rely on a safe
validated index path. Keep the fix localized to `Mesh` so public callers like
the ones in `JoltShapes.cpp` and `PhysicsScene.cpp` don’t depend on undefined
behavior if an invalid index slips through.
- Around line 264-268: The toggle used by CompactCPUGeometry is a shared static
state accessed across threads, so make s_RetainFullCPUGeometry thread-safe
rather than a plain bool. Update the declaration in Mesh to use an atomic or
other synchronized mechanism, and adjust the RuntimeApplication constructor
write and CompactCPUGeometry read sites to use the matching atomic load/store
(or equivalent) so the flag is formally synchronized even when MeshSource
instances are built on worker threads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cbacc57f-9fa9-4dc0-8102-45371c2e0bb2
⛔ Files ignored due to path filters (5)
Editor/Resources/Shaders/DeferredLighting.glslis excluded by!**/*.glslEditor/Resources/Shaders/GBufferDebug.glslis excluded by!**/*.glslEditor/Resources/Shaders/GBuffer_Static.glslis excluded by!**/*.glslEditor/Resources/Shaders/PostProcessing/Bloom.glslis excluded by!**/*.glslEditor/Resources/Shaders/PostProcessing/Pre-Convolution.glslis excluded by!**/*.glsl
📒 Files selected for processing (45)
Core/Source/Lux/Asset/AssimpMeshImporter.cppCore/Source/Lux/Asset/MeshRuntimeSerializer.cppCore/Source/Lux/ImGui/Colors.hCore/Source/Lux/ImGui/ImGuiLayer.cppCore/Source/Lux/Physics/JoltPhysics/JoltShapes.cppCore/Source/Lux/Physics/PhysicsScene.cppCore/Source/Lux/Platform/Vulkan/DescriptorSetManager.cppCore/Source/Lux/Platform/Vulkan/DescriptorSetManager.hCore/Source/Lux/Platform/Vulkan/VulkanAllocator.cppCore/Source/Lux/Project/Project.hCore/Source/Lux/Renderer/ComputePass.cppCore/Source/Lux/Renderer/ComputePass.hCore/Source/Lux/Renderer/Framebuffer.cppCore/Source/Lux/Renderer/Framebuffer.hCore/Source/Lux/Renderer/Image.cppCore/Source/Lux/Renderer/Image.hCore/Source/Lux/Renderer/IndexBuffer.cppCore/Source/Lux/Renderer/MaterialScene.cppCore/Source/Lux/Renderer/MaterialScene.hCore/Source/Lux/Renderer/Mesh.cppCore/Source/Lux/Renderer/Mesh.hCore/Source/Lux/Renderer/RenderCommandBuffer.cppCore/Source/Lux/Renderer/RenderCommandBuffer.hCore/Source/Lux/Renderer/RenderGraph.hCore/Source/Lux/Renderer/RenderPass.cppCore/Source/Lux/Renderer/RenderPass.hCore/Source/Lux/Renderer/Renderer.cppCore/Source/Lux/Renderer/Renderer.hCore/Source/Lux/Renderer/SceneRenderer.cppCore/Source/Lux/Renderer/SceneRenderer.hCore/Source/Lux/Renderer/Texture.cppCore/Source/Lux/Renderer/TextureScene.cppCore/Source/Lux/Renderer/TextureScene.hCore/Source/Lux/Renderer/VertexBuffer.cppCore/Source/Lux/Scene/Scene.cppCore/premake5.luaEditor/Resources/Shaders/Include/GLSL/LuxGBuffer.glslhEditor/Source/Panels/RendererDebuggerPanel.cppEditor/Source/Panels/SceneRendererPanel.cppLux-Runtime/src/RuntimeApplication.cppLux-Runtime/src/RuntimeLayer.cppREADME.mddocs/ENGINE_OPTIMIZATION_PLAN.mddocs/RENDERER_PERF_BASELINE.mdpremake5.lua
💤 Files with no reviewable changes (1)
- Core/Source/Lux/Asset/AssimpMeshImporter.cpp
Comment on lines
+170
to
+183
| for (auto& [name, input] : savedInputs) | ||
| { | ||
| auto declIt = InputDeclarations.find(name); | ||
| if (declIt == InputDeclarations.end()) | ||
| continue; | ||
| const RenderInputDeclaration& decl = declIt->second; | ||
| if (input.Input.size() != (size_t)decl.Count) | ||
| continue; | ||
|
|
||
| RenderPassInput& target = InputResources[decl.Set][decl.Binding]; | ||
| const bool isWriteable = target.IsWriteable; // from the new reflection | ||
| target = input; | ||
| target.IsWriteable = isWriteable; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and symbols first
ast-grep outline Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp --view expanded || true
# Read the relevant slice around the reported lines
sed -n '1,260p' Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp
# Find RenderPassInput definition and the code paths that consume Type / IsWriteable
rg -n "struct RenderPassInput|class RenderPassInput|RenderPassInput|IsCompatibleInput|Validate\\(|BakeSet\\(|Bake\\(" Core/Source/Lux/Platform/Vulkan -S
# Read the relevant declarations/implementations
for f in $(rg -l "struct RenderPassInput|class RenderPassInput|IsCompatibleInput|Validate\\(|BakeSet\\(" Core/Source/Lux/Platform/Vulkan -S); do
echo "===== $f ====="
wc -l "$f"
done
# Inspect the definition if found
for f in $(rg -l "struct RenderPassInput|class RenderPassInput" Core/Source/Lux/Platform/Vulkan -S); do
echo "===== $f ====="
sed -n '1,260p' "$f"
done
# Inspect BakeSet / validation slices if found in the same file(s)
for f in $(rg -l "IsCompatibleInput|Validate\\(|BakeSet\\(" Core/Source/Lux/Platform/Vulkan -S); do
echo "===== $f ====="
sed -n '1,260p' "$f"
doneRepository: starbounded-dev/LuxEngine
Length of output: 36501
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the validation and bake logic around the reported concern
sed -n '330,520p' Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp
# Also inspect any later update path that might use RenderPassInput::Type
sed -n '520,1040p' Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cppRepository: starbounded-dev/LuxEngine
Length of output: 25929
Keep the reflected descriptor type on reload.
target = input restores the old RenderResourceType as well as the bound handles. If a shader permutation changes a binding’s resource kind but keeps the same name/count, BakeSet() will switch on the stale type and can build the wrong descriptor kind even though validation still accepts the resource. Keep the new reflection’s Type/IsWriteable and copy only the bound inputs, or skip restoring when the type changed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Source/Lux/Platform/Vulkan/DescriptorSetManager.cpp` around lines 170 -
183, Reload logic in DescriptorSetManager is restoring the entire saved
RenderPassInput and overwriting the newly reflected resource type, so update the
merge in the savedInputs loop to preserve the current reflection’s Type (and
IsWriteable) from InputDeclarations/InputResources while copying only the bound
handles from the saved input, or skip restoring that entry when the reflected
type differs; use the RenderPassInput assignment in the reload path and the
BakeSet flow as the key places to adjust.
Comment on lines
+44
to
+102
|
|
||
| // Driver-reported per-heap usage/budget via VK_EXT_memory_budget. This is | ||
| // process-wide truth and therefore INCLUDES allocations made by NVRHI's | ||
| // internal allocator, which bypass this VMA instance entirely (the local | ||
| // tracking below only ever sees the legacy #if OLD paths, i.e. ~nothing). | ||
| // Physical-device-level functionality of a device extension only requires | ||
| // the extension to be *supported*, not enabled on the device. | ||
| bool QueryDeviceLocalMemoryBudget(uint64_t& outUsed, uint64_t& outBudget) | ||
| { | ||
| nvrhi::DeviceHandle device = Application::GetGraphicsDevice(); | ||
| if (!device) | ||
| return false; | ||
|
|
||
| VkPhysicalDevice physicalDevice = (VkPhysicalDevice)device->getNativeObject(nvrhi::ObjectTypes::VK_PhysicalDevice); | ||
| if (!physicalDevice) | ||
| return false; | ||
|
|
||
| static int s_MemoryBudgetSupport = -1; // -1 unknown, 0 no, 1 yes | ||
| if (s_MemoryBudgetSupport == -1) | ||
| { | ||
| s_MemoryBudgetSupport = 0; | ||
| uint32_t extensionCount = 0; | ||
| vkEnumerateDeviceExtensionProperties(physicalDevice, nullptr, &extensionCount, nullptr); | ||
| std::vector<VkExtensionProperties> extensions(extensionCount); | ||
| vkEnumerateDeviceExtensionProperties(physicalDevice, nullptr, &extensionCount, extensions.data()); | ||
| for (const VkExtensionProperties& extension : extensions) | ||
| { | ||
| if (strcmp(extension.extensionName, VK_EXT_MEMORY_BUDGET_EXTENSION_NAME) == 0) | ||
| { | ||
| s_MemoryBudgetSupport = 1; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (s_MemoryBudgetSupport != 1) | ||
| return false; | ||
|
|
||
| VkPhysicalDeviceMemoryBudgetPropertiesEXT budgetProperties{}; | ||
| budgetProperties.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MEMORY_BUDGET_PROPERTIES_EXT; | ||
|
|
||
| VkPhysicalDeviceMemoryProperties2 memoryProperties{}; | ||
| memoryProperties.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MEMORY_PROPERTIES_2; | ||
| memoryProperties.pNext = &budgetProperties; | ||
| vkGetPhysicalDeviceMemoryProperties2(physicalDevice, &memoryProperties); | ||
|
|
||
| outUsed = 0; | ||
| outBudget = 0; | ||
| for (uint32_t heap = 0; heap < memoryProperties.memoryProperties.memoryHeapCount; heap++) | ||
| { | ||
| if ((memoryProperties.memoryProperties.memoryHeaps[heap].flags & VK_MEMORY_HEAP_DEVICE_LOCAL_BIT) == 0) | ||
| continue; | ||
|
|
||
| outUsed += budgetProperties.heapUsage[heap]; | ||
| outBudget += budgetProperties.heapBudget[heap]; | ||
| } | ||
|
|
||
| return outBudget > 0; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Vulkan VK_EXT_memory_budget must be enabled at device creation to use VkPhysicalDeviceMemoryBudgetPropertiesEXT in vkGetPhysicalDeviceMemoryProperties2
💡 Result:
To use the VkPhysicalDeviceMemoryBudgetPropertiesEXT structure with vkGetPhysicalDeviceMemoryProperties2 to retrieve memory budget and usage information, you must enable the VK_EXT_memory_budget device extension when creating the logical device [1][2][3][4]. While vkGetPhysicalDeviceMemoryProperties2 is part of the VK_KHR_get_physical_device_properties2 extension (or Vulkan 1.1+ core), the ability to populate the VkPhysicalDeviceMemoryBudgetPropertiesEXT structure within its pNext chain is provided specifically by the VK_EXT_memory_budget extension [1][2][5]. Consequently, your application must include VK_EXT_MEMORY_BUDGET_EXTENSION_NAME in the enabled device extensions list passed to VkDeviceCreateInfo when calling vkCreateDevice [3][6][7]. If this extension is not enabled, the implementation will not fill the structure with the requested memory budget and usage data [1][2].
Citations:
- 1: https://docs.vulkan.org/refpages/latest/refpages/source/VK_EXT_memory_budget.html
- 2: https://docs.vulkan.org/refpages/latest/refpages/source/VkPhysicalDeviceMemoryBudgetPropertiesEXT.html
- 3: https://github.com/KhronosGroup/Vulkan-Samples/tree/main/samples/extensions/memory_budget
- 4: https://gpuopen-librariesandsdks.github.io/VulkanMemoryAllocator/html/staying_within_budget.html
- 5: https://registry.khronos.org/VulkanSC/specs/1.0-extensions/man/html/VkPhysicalDeviceMemoryBudgetPropertiesEXT.html
- 6: https://docs.vulkan.org/refpages/latest/refpages/source/vkCreateDevice.html
- 7: https://github.com/KhronosGroup/Vulkan-Samples/blob/ac9edc79/samples/extensions/memory_budget/memory_budget.cpp
🏁 Script executed:
#!/bin/bash
# Check whether NVRHI/device creation enables VK_EXT_memory_budget and what apiVersion is requested.
rg -nP 'VK_EXT_MEMORY_BUDGET_EXTENSION_NAME|memory_budget|enabledExtensionCount|ppEnabledExtensionNames|apiVersion|VK_API_VERSION_1_' -C2 --type-add 'cpp:*.{cpp,h,hpp}' -tcppRepository: starbounded-dev/LuxEngine
Length of output: 163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate Vulkan device creation / API-version setup in the repo.
git ls-files | rg '(^|/)(Vulkan|vulkan|NVRHI|nvrhi|Device|GraphicsDevice|Allocator|Application)\.(cpp|h|hpp|inl)$|(^|/)CMakeLists\.txt$|(^|/)premake.*\.lua$|(^|/)*.{json,cmake,txt}$' || true
echo
echo '--- matches for VK_EXT_memory_budget / Vulkan API version / enabled extensions ---'
rg -n --hidden -S 'VK_EXT_MEMORY_BUDGET_EXTENSION_NAME|VK_EXT_memory_budget|enabledExtensionCount|ppEnabledExtensionNames|VkDeviceCreateInfo|apiVersion|VK_API_VERSION_1_[12]|VK_API_VERSION_1_0|vkCreateDevice|vkGetPhysicalDeviceMemoryProperties2' .Repository: starbounded-dev/LuxEngine
Length of output: 13939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the reviewed file and nearby code paths only if present in-tree.
for f in Core/Source/Lux/Platform/Vulkan/VulkanAllocator.cpp; do
if [ -f "$f" ]; then
echo "### $f"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
fi
doneRepository: starbounded-dev/LuxEngine
Length of output: 9621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Core/Source/Lux/Platform/Vulkan/VulkanDevice.cpp:260-330 ---'
sed -n '260,330p' Core/Source/Lux/Platform/Vulkan/VulkanDevice.cpp | cat -n
echo
echo '--- Core/Source/Lux/Platform/Vulkan/VulkanDeviceManager.cpp:500-560 ---'
sed -n '500,560p' Core/Source/Lux/Platform/Vulkan/VulkanDeviceManager.cpp | cat -n
echo
echo '--- Core/Source/Lux/Renderer/DeviceManager.h:1-180 ---'
sed -n '1,180p' Core/Source/Lux/Renderer/DeviceManager.h | cat -nRepository: starbounded-dev/LuxEngine
Length of output: 15033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- search for VK_EXT_memory_budget enablement / VMA budget flag ---'
rg -n -C3 'VK_EXT_MEMORY_BUDGET_EXTENSION_NAME|VK_EXT_memory_budget|memoryBudgetExtensionEnabled|VMA_ALLOCATOR_CREATE_EXT_MEMORY_BUDGET_BIT|vulkanApiVersion|vkGetPhysicalDeviceMemoryProperties2' Core/Source Core/vendor/VulkanMemoryAllocator/vk_mem_alloc.hRepository: starbounded-dev/LuxEngine
Length of output: 28474
Enable VK_EXT_memory_budget on the device
VkPhysicalDeviceMemoryBudgetPropertiesEXT only gets populated when VK_EXT_MEMORY_BUDGET_EXTENSION_NAME is enabled at vkCreateDevice; VulkanDevice.cpp doesn’t add it to deviceExtensions, so this path won’t return driver budget data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Source/Lux/Platform/Vulkan/VulkanAllocator.cpp` around lines 44 - 102,
QueryDeviceLocalMemoryBudget in VulkanAllocator.cpp is checking for
VK_EXT_memory_budget support but that does not guarantee the budget properties
are populated. Update the device creation path in VulkanDevice.cpp to add
VK_EXT_MEMORY_BUDGET_EXTENSION_NAME to deviceExtensions when supported, so
vkGetPhysicalDeviceMemoryProperties2 can actually return valid
VkPhysicalDeviceMemoryBudgetPropertiesEXT data for this query.
Comment on lines
+15
to
+16
| RenderCommandBuffer::RenderCommandBuffer(uint32_t count, bool enableQueries, const std::string& debugName, nvrhi::CommandQueue queue) | ||
| : m_Queue(queue), m_DebugName(debugName) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== RenderCommandBuffer.h ==\n'
sed -n '1,140p' Core/Source/Lux/Renderer/RenderCommandBuffer.h
printf '\n== RenderCommandBuffer.cpp ==\n'
sed -n '1,120p' Core/Source/Lux/Renderer/RenderCommandBuffer.cpp
printf '\n== Search constructor/usages ==\n'
rg -n "RenderCommandBuffer\s*\(" Core/Source/Lux/Renderer -g '!*build*' -g '!*dist*'Repository: starbounded-dev/LuxEngine
Length of output: 8469
Match the constructor declaration to the definition
RenderCommandBuffer.h still declares the 3-argument constructor, but RenderCommandBuffer.cpp defines a 4-argument overload and Create() forwards queue to it. Add the nvrhi::CommandQueue parameter to the declaration so the factory call has a matching constructor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Source/Lux/Renderer/RenderCommandBuffer.cpp` around lines 15 - 16, The
RenderCommandBuffer constructor declaration and definition are out of sync,
since RenderCommandBuffer.cpp defines a 4-argument constructor that takes
nvrhi::CommandQueue while RenderCommandBuffer.h still exposes only the
3-argument version. Update the RenderCommandBuffer class declaration to include
the queue parameter so it matches the existing constructor implementation and
the Create() factory call that forwards queue. Make sure the signature in
RenderCommandBuffer and any related constructor references are consistent across
declaration and definition.
| { | ||
| public: | ||
| static Ref<RenderCommandBuffer> Create(uint32_t count = 0, const std::string& debugName = "", bool enableQueries = false) { return Ref<RenderCommandBuffer>::Create(count, enableQueries, debugName); } | ||
| static Ref<RenderCommandBuffer> Create(uint32_t count = 0, const std::string& debugName = "", bool enableQueries = false, nvrhi::CommandQueue queue = nvrhi::CommandQueue::Graphics) { return Ref<RenderCommandBuffer>::Create(count, enableQueries, debugName, queue); } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Create passes a 4th arg the private constructor doesn't declare.
This factory forwards queue to Ref<RenderCommandBuffer>::Create(count, enableQueries, debugName, queue), but the constructor declared at line 69 (RenderCommandBuffer(uint32_t, bool, const std::string&)) was not updated to accept the queue. The .cpp defines the 4-arg constructor, so the declaration here is the missing piece and the translation unit will not compile. See the paired fix on the constructor declaration below.
🛠 Proposed fix (line 69)
- RenderCommandBuffer(uint32_t count, bool enableQueries, const std::string& debugName);
+ RenderCommandBuffer(uint32_t count, bool enableQueries, const std::string& debugName, nvrhi::CommandQueue queue = nvrhi::CommandQueue::Graphics);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Source/Lux/Renderer/RenderCommandBuffer.h` at line 22, The
RenderCommandBuffer::Create factory is forwarding a queue argument that the
class declaration does not accept, so update the private RenderCommandBuffer
constructor declaration to match the 4-argument definition used by Create and
the .cpp implementation. Make sure the constructor signature in
RenderCommandBuffer includes the nvrhi::CommandQueue parameter in addition to
count, enableQueries, and debugName so Ref<RenderCommandBuffer>::Create can
instantiate it correctly.
| || !meshCullDrawData.empty() | ||
| || !indirectDrawData.empty() | ||
| || !gpuSceneInstanceData.empty() | ||
| || !gpuSceneRangeRows.empty() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for declarations/population/captures of the dirty-range upload vars.
rg -nP -C2 'gpuSceneRangeRows|sceneRangeRows|sceneRangeList' Core/Source/Lux/Renderer/SceneRenderer.cppRepository: starbounded-dev/LuxEngine
Length of output: 1065
🏁 Script executed:
#!/bin/bash
sed -n '5075,5205p' Core/Source/Lux/Renderer/SceneRenderer.cpp | cat -nRepository: starbounded-dev/LuxEngine
Length of output: 7517
Capture the dirty-range state in the submit lambda gpuSceneRangeRows is checked before submit, but the lambda reads sceneRangeRows and sceneRangeList without capturing either name. If these are locals, this won’t compile; make the names consistent and pass the same dirty-range data into Renderer::Submit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Core/Source/Lux/Renderer/SceneRenderer.cpp` at line 5098, The submit path is
using dirty-range state inconsistently: the pre-submit check references
gpuSceneRangeRows, but the lambda passed to Renderer::Submit reads
sceneRangeRows and sceneRangeList without capturing the same data. Update
SceneRenderer::Renderer::Submit call so the lambda captures and uses the exact
dirty-range variables being checked, or rename the locals to match consistently,
ensuring the same range data is passed through.
Comment on lines
+126
to
+131
| 4. **Aliasing coverage of big targets — investigated, REJECTED:** the exclusions in | ||
| `IsRenderGraphAliasCandidate` are correctness, not oversight. GTAO/SSR/Cloud history | ||
| buffers persist across frames (temporal accumulation reads last frame's result) and | ||
| can never be transient; PreDepth/GBuffer/SceneColor are read throughout the frame | ||
| (SSR, GTAO, debug views, TAA), leaving no dead window to alias into. Do not extend | ||
| aliasing to these. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the obsolete TAA/cloud references here.
This aliasing note still cites cloud history and TAA as active consumers, but those paths are being removed in this PR. Keeping them here makes the optimization plan look like obsolete work is still blocking aliasing.
Suggested direction
- GTAO/SSR/Cloud history buffers persist across frames ... SceneColor are read throughout the frame (..., TAA) ...
+ GTAO/SSR history buffers persist across frames ... SceneColor is read throughout the frame (...debug views...) ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 4. **Aliasing coverage of big targets — investigated, REJECTED:** the exclusions in | |
| `IsRenderGraphAliasCandidate` are correctness, not oversight. GTAO/SSR/Cloud history | |
| buffers persist across frames (temporal accumulation reads last frame's result) and | |
| can never be transient; PreDepth/GBuffer/SceneColor are read throughout the frame | |
| (SSR, GTAO, debug views, TAA), leaving no dead window to alias into. Do not extend | |
| aliasing to these. | |
| 4. **Aliasing coverage of big targets — investigated, REJECTED:** the exclusions in | |
| `IsRenderGraphAliasCandidate` are correctness, not oversight. GTAO/SSR history | |
| buffers persist across frames (temporal accumulation reads last frame's result) and | |
| can never be transient; PreDepth/GBuffer/SceneColor are read throughout the frame | |
| (SSR, GTAO, debug views), leaving no dead window to alias into. Do not extend | |
| aliasing to these. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/ENGINE_OPTIMIZATION_PLAN.md` around lines 126 - 131, The aliasing
rejection note is stale because it still mentions cloud history and TAA as
active consumers. Update the text in the optimization plan section to remove
those obsolete references and keep only the currently relevant exclusions, using
the existing IsRenderGraphAliasCandidate discussion as the anchor. Make sure the
wording reflects the PR’s current state so the note no longer implies those
removed paths still block aliasing.
Comment on lines
+208
to
+209
| - **Dist FPS uplift** from `optimize "Full"` + LTO — FPS-only comparison (Tracy is | ||
| compiled out of Dist). Regenerate projects first (`scripts/Win-GenProjects.py`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Relevant docs and scripts ==\n'
git ls-files | rg '^(docs/|scripts/|README|readme)' || true
printf '\n== Search for Win-GenProjects references ==\n'
rg -n --hidden --glob '!**/.git/**' 'Win-GenProjects|GenProjects|project-generation|regenerat' docs README* scripts || true
printf '\n== Inspect docs/RENDERER_PERF_BASELINE.md around the cited lines ==\n'
sed -n '190,220p' docs/RENDERER_PERF_BASELINE.md
printf '\n== Inspect README/setup references if present ==\n'
if [ -f README.md ]; then
sed -n '1,260p' README.md
fi
if [ -f docs/README.md ]; then
sed -n '1,260p' docs/README.md
fiRepository: starbounded-dev/LuxEngine
Length of output: 11412
Use one project-generation entrypoint consistently
docs/RENDERER_PERF_BASELINE.md points to scripts/Win-GenProjects.py here and earlier in the file, while README.md uses scripts/Win-GenProjects.bat. Keep the docs on one entrypoint so setup guidance stays consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/RENDERER_PERF_BASELINE.md` around lines 208 - 209, The docs use
inconsistent project-generation entrypoints, so update the guidance in
RENDERER_PERF_BASELINE to match the same script entrypoint used elsewhere in the
docs. Locate the references around the baseline regeneration note and make them
consistent with the shared project-generation path referenced in README so setup
instructions use one command only.
Comment on lines
+11
to
+25
| ## What works today | ||
|
|
||
| ### Rendering (Vulkan 1.4) | ||
| - **Deferred PBR pipeline** with a G-buffer, clustered (froxel) light culling for point/spot lights, and a separate forward pass for transparents. | ||
| - **Render graph** with compile caching and scratch-resource reuse; passes are skipped when their feature is off (zero-cost-when-disabled is an explicit goal). | ||
| - **Shadows** — cascaded directional shadow maps (2K default) and spot-light shadow maps. | ||
| - **Sky & atmosphere** — physically-based sky atmosphere, Preetham sky, HDR environment maps (equirect → cubemap, irradiance + prefiltered mips), skybox pass. | ||
| - **Volumetric clouds** — Nubis/RDR2-style system with baked 3D noise textures (base shape, detail, curl), temporal reprojection, and a composite pass. | ||
| - **Volumetric / atmospheric fog** — froxel fog with clustered local-light in-scattering, exponential height fog, and local fog volumes. | ||
| - **Post-processing** — GTAO (with temporal + denoise), screen-space reflections (with temporal + composite), TAA, bloom, depth of field, and HZB generation used for occlusion and SSR pre-integration. | ||
| - **Physical imaging** — exposure as manual multiplier, manual EV100, physical camera (aperture/shutter/ISO), or histogram auto-exposure; ACES and AgX tonemapping; physical light units. | ||
| - **Volume system** — blendable post-process, atmosphere, and fog volumes (box/sphere) that override settings per region. | ||
| - **GPU-driven bits** — GPU scene buffers, compute mesh culling, per-pass GPU timing. | ||
| - **2D batch renderer** — quads/sprites, circles, lines, and MSDF text rendering (msdf-atlas-gen). | ||
| - **Editor rendering** — jump-flood selection outlines, wireframe and G-buffer/AO debug views, infinite grid, debug renderer. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale renderer features from “What works today.”
This section still advertises clouds, atmosphere/fog, and TAA as live features, but this PR removes those paths. The README should reflect the post-merge renderer surface.
Suggested direction
- - **Sky & atmosphere** — physically-based sky atmosphere...
- - **Volumetric clouds** — ...
- - **Volumetric / atmospheric fog** — ...
- - **Post-processing** — ... TAA ...
+ - **Post-processing** — ... bloom, depth of field, and HZB generation ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## What works today | |
| ### Rendering (Vulkan 1.4) | |
| - **Deferred PBR pipeline** with a G-buffer, clustered (froxel) light culling for point/spot lights, and a separate forward pass for transparents. | |
| - **Render graph** with compile caching and scratch-resource reuse; passes are skipped when their feature is off (zero-cost-when-disabled is an explicit goal). | |
| - **Shadows** — cascaded directional shadow maps (2K default) and spot-light shadow maps. | |
| - **Sky & atmosphere** — physically-based sky atmosphere, Preetham sky, HDR environment maps (equirect → cubemap, irradiance + prefiltered mips), skybox pass. | |
| - **Volumetric clouds** — Nubis/RDR2-style system with baked 3D noise textures (base shape, detail, curl), temporal reprojection, and a composite pass. | |
| - **Volumetric / atmospheric fog** — froxel fog with clustered local-light in-scattering, exponential height fog, and local fog volumes. | |
| - **Post-processing** — GTAO (with temporal + denoise), screen-space reflections (with temporal + composite), TAA, bloom, depth of field, and HZB generation used for occlusion and SSR pre-integration. | |
| - **Physical imaging** — exposure as manual multiplier, manual EV100, physical camera (aperture/shutter/ISO), or histogram auto-exposure; ACES and AgX tonemapping; physical light units. | |
| - **Volume system** — blendable post-process, atmosphere, and fog volumes (box/sphere) that override settings per region. | |
| - **GPU-driven bits** — GPU scene buffers, compute mesh culling, per-pass GPU timing. | |
| - **2D batch renderer** — quads/sprites, circles, lines, and MSDF text rendering (msdf-atlas-gen). | |
| - **Editor rendering** — jump-flood selection outlines, wireframe and G-buffer/AO debug views, infinite grid, debug renderer. | |
| ## What works today | |
| ### Rendering (Vulkan 1.4) | |
| - **Deferred PBR pipeline** with a G-buffer, clustered (froxel) light culling for point/spot lights, and a separate forward pass for transparents. | |
| - **Render graph** with compile caching and scratch-resource reuse; passes are skipped when their feature is off (zero-cost-when-disabled is an explicit goal). | |
| - **Shadows** — cascaded directional shadow maps (2K default) and spot-light shadow maps. | |
| - **Post-processing** — GTAO (with temporal + denoise), screen-space reflections (with temporal + composite), bloom, depth of field, and HZB generation used for occlusion and SSR pre-integration. | |
| - **Physical imaging** — exposure as manual multiplier, manual EV100, physical camera (aperture/shutter/ISO), or histogram auto-exposure; ACES and AgX tonemapping; physical light units. | |
| - **Volume system** — blendable post-process, atmosphere, and fog volumes (box/sphere) that override settings per region. | |
| - **GPU-driven bits** — GPU scene buffers, compute mesh culling, per-pass GPU timing. | |
| - **2D batch renderer** — quads/sprites, circles, lines, and MSDF text rendering (msdf-atlas-gen). | |
| - **Editor rendering** — jump-flood selection outlines, wireframe and G-buffer/AO debug views, infinite grid, debug renderer. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 11 - 25, Update the “What works today” section so it
no longer claims removed renderer paths are available: remove the cloud,
atmosphere/fog, and TAA bullets or rewrite them to match the current renderer
surface. Keep the rest of the list aligned with the actual features still
present, and use the existing “Rendering (Vulkan 1.4)” subsection as the place
to reflect the current state.
Comment on lines
+83
to
+87
| **2. Configure dependencies:** | ||
|
|
||
| 1. Run [Setup.bat](scripts/Setup.bat) in the `scripts` folder. It validates Python packages, checks the Vulkan SDK, pulls Git LFS assets and submodules, and generates project files. | ||
| 2. The **Vulkan SDK 1.4.x** is required. If missing, the script downloads the installer and prompts you; Debug builds additionally need the SDK's shader debug libraries. | ||
| 3. After installing the SDK, run [Setup.bat](scripts/Setup.bat) again. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files README.md scripts/Setup.bat scripts/Setup.py 2>/dev/null || true
printf '\n== README snippet ==\n'
sed -n '75,95p' README.md
printf '\n== Setup.bat ==\n'
cat -n scripts/Setup.bat
printf '\n== Setup.py symbols / outline ==\n'
ast-grep outline scripts/Setup.py --view expanded || true
printf '\n== Search for Vulkan SDK / installer wording ==\n'
rg -n -i "vulkan sdk|installer|download|shader debug|setup\.bat|setup\.py" README.md scripts/Setup.bat scripts/Setup.pyRepository: starbounded-dev/LuxEngine
Length of output: 2525
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n scripts/Setup.py | sed -n '60,95p'Repository: starbounded-dev/LuxEngine
Length of output: 1710
Clarify the Vulkan SDK bootstrap flow. scripts/Setup.bat / scripts/Setup.py only checks for the Vulkan SDK and exits when it’s missing; it doesn’t download or launch an installer. Reword this step to tell users to install the SDK manually, or point to the actual helper if one exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 83 - 87, The Vulkan SDK setup instructions are
inaccurate because scripts/Setup.bat and scripts/Setup.py only detect whether
the SDK is installed and do not download or launch an installer. Update the
dependency setup section to tell users to install the Vulkan SDK manually, then
rerun Setup.bat, and if there is a real helper for bootstrap/install flow,
reference that explicitly instead of describing non-existent behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Broad performance/memory/stability pass on the renderer, plus rendering
simplification and an editor refresh. 61 commits; net +1953/−667 across 51 files.
All changes built and verified on Windows (RTX 4070 Ti); the Linux CI container
cannot build the Vulkan/NVRHI path.
Performance — CPU & frame pacing
single-copy upload captures, version-gated texture/material table copies,
upload only dirty GPUScene rows, reuse sorted draw order when the key set is
unchanged.
the swapchain and every per-frame resource were already triple-buffered.
LUX_DEBUG); itwas intercepting every call in shipping/perf builds.
in Dist.
Memory diet
VK_EXT_memory_budget.Rendering simplification
(passes and their resources no longer created; consumers null-guarded).
the AO-composite multiply into deferred lighting.
and PreIntegration when SSR is off; idle the atmosphere UBO when unused.
Stability / crash fixes
render thread); rebake only affected sets on invalidation.
permanent per-frame re-Bake and mesh disappearance on quality-preset change.
Async compute (experimental, OFF by default)
culling to the compute queue (correctness-first).
the editor and needs GPU-level debugging. With the toggle ON, async is
currently correct but does not yet overlap. Toggle lives in the Scene Renderer
panel, defaults off.
Editor & tooling
(distinct from Hazel's orange/cyan), softer rounding. Pure colour/style.
Docs
Verify before/after merge
properly multi-buffered (none expected; stopgap is a one-line revert to 1 in
Window.cpp).and use a Debug build when you need Vulkan validation output.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation