Fix resource leaks, stale bindings, dead RenderStats, naming bugs, alignment errors, window config, blend mode improvements, MRT per-attachment blending, and Debug UI pipeline leak found in full code review

[Copilot]

Full code review across 467 source files. Twenty-plus bugs and missing features fixed ranging from GPU resource leaks to always-zero stats counters to a misleading profiler API. Merged with upstream Debug UI system additions and fixed a pipeline leak in the new overlay.

Critical Fixes

GPU buffer leak — per-entity UBOs

UniformManager keyed object/material UBOs by "obj_" + entity.index() (index-only string). Buffers accumulated forever; slot reuse gave new entities stale buffers silently.

• Maps re-keyed by Handle<?> (index + generation)
• Added removeEntity(Handle<?>) to destroy buffers on entity removal
• Renderer.renderFrame() calls it for every EntityRemoved transaction before delegating to MeshRenderer

Material UBO wrong-sized on property change

UniformManager.uploadAndBindMaterial() used computeIfAbsent keyed by entity handle, allocating a buffer sized for the first set of material properties. If a material subsequently gained new scalar keys, the existing undersized buffer was reused, silently writing into unallocated GPU memory.

• Added materialUboSizes tracking map alongside materialUbos
• Buffer is destroyed and reallocated when the required size exceeds the allocated size
• Both maps are updated atomically (entries removed before destroy, inserted after create) so a failed allocation never leaves a dangling handle

Stale shader binding after entity slot reuse

entityShaders was also index-only. Added ShaderManager.removeEntityShader(), called alongside removeEntity() on EntityRemoved.

MeshRenderer map leaks on entity removal

meshDataAssignments, meshAssignments, materialAssignments were not cleared on EntityRemoved. Fixed.

Renderer.close() / Engine.shutdown() incomplete teardown

• MeshManager held VBOs, IBOs, vertex inputs that were never freed. Added MeshManager.close() (via private destroyMeshResources helper) and wired it into Renderer.close().
• Engine.shutdown() was missing assets.shutdown(), leaving FileWatcher threads alive.

DebugUiOverlay.close() pipeline leak

DebugUiOverlay.init() creates the GPU pipeline via ShaderManager.compileSlangSource(), which does not cache the pipeline in ShaderManager.shaderCache. The original close() comment ("Pipeline is owned by ShaderManager") was wrong — the pipeline is exclusively owned by DebugUiOverlay. Added device.destroyPipeline(pipeline) to close() to prevent the leak on engine shutdown.

Correctness Fixes

Vec2 alignment missing from material UBO write loop

The size-calculation loop in UniformManager.uploadAndBindMaterial() correctly aligned Vec2 fields to an 8-byte boundary. The write-offset loop did not apply that same pre-alignment, so any Vec2 property following an odd-aligned predecessor was written at the wrong GPU offset, producing corrupted material values. Fixed by adding offset = align(offset, 8) before writing Vec2 in the write loop, making both loops consistent. Also fixed missing pre-alignment for Vec4 and Mat4 in the write loop.

Profiler naming inversion

lastFrame() returned the in-progress currentFrame; previousFrame() returned the actually-completed frame. Renamed to match semantics:

• currentFrame() — in-progress frame
• lastFrame() — most-recently completed frame

Tests updated to use currentFrame().

RenderStats always zero

Nothing ever called recordDrawCall(), recordPipelineBind(), etc. Renderer.renderFrame() now records all stats and resets at frame start. Engine.renderStats() delegates to renderer.renderStats() (stats owned by Renderer).

Blend mode hardcoded / incorrect Vulkan constants

• The legacy SetBlending command hardcoded SRC_ALPHA / ONE_MINUS_SRC_ALPHA regardless of the active BlendMode. Now delegates to applyBlendMode(BlendMode.ALPHA).
• Two Vulkan blend-factor constants were wrong per the spec: VK_BLEND_FACTOR_DST_COLOR was 8 (correct: 4) and VK_BLEND_FACTOR_DST_ALPHA was 10 (correct: 8). Fixed.

WebGPU blend constants incorrect after upstream merge

Upstream introduced incorrect 1-based WgpuBindings blend factor/operation constants. Retained the correct 0-based values matching the WebGPU spec.

New Interfaces

BlendFactor and BlendEquation

New extensible interfaces (static instances, no enums — matching codebase convention) covering the full set of standard blend factors and equations. BlendMode extended with:

• enabled(), srcColorFactor(), dstColorFactor(), srcAlphaFactor(), dstAlphaFactor(), colorEquation(), alphaEquation()
• BlendMode.of(name, srcColor, dstColor, srcAlpha, dstAlpha[, colorEq, alphaEq]) factory for fully custom blending

All three backends now derive blend factors from the BlendMode object:

• GL — glBlendFuncSeparate + glBlendEquationSeparate with mapBlendFactor/mapBlendEquation helpers; all missing GL blend constants added.
• Vulkan — BlendConfig.fromBlendMode(mode) factory added to VkPipelineFactory; BlendConfig gains colorBlendOp/alphaBlendOp fields with correct VK_BLEND_OP_* mapping.
• WebGPU — mapBlendFactor/mapBlendEquation helpers added; pipeline creation passes mapped equation constants.

Per-attachment MRT blending (RenderState.BLEND_MODES)

Added RenderState.BLEND_MODES (BlendMode[]) for independent blend state per color attachment in Multiple Render Target rendering. Index i maps to attachment i; the last entry is repeated for any extra attachments.

• GL — Added glBlendFuncSeparatei / glBlendEquationSeparatei / glEnablei / glDisablei to GlBindings + LwjglGlBindings (GL 4.0+ indexed blend API). GlRenderDevice calls the indexed variants per draw buffer when BLEND_MODES is set.
• Vulkan — VkBindings.createGraphicsPipeline now accepts per-attachment arrays. LwjglVkBindings creates one VkPipelineColorBlendAttachmentState per attachment. VkPipelineFactory.create has a new BlendConfig[] overload; VkRenderDevice tracks currentBlendModes, builds the BlendConfig[] via buildBlendConfigs(), and incorporates all per-attachment mode names in the pipeline variant cache key.
• WebGPU — Current binding only surfaces a single color target; BLEND_MODES is silently ignored and BLEND_MODE applies instead (limitation documented in Javadoc).

BLEND_MODES is also registered in RenderStateManager.isRenderStateKey() so it is correctly excluded from material UBO uploads.

WindowConfig

New WindowConfig record (graphics/api) collects all window startup properties:

Property	Default
title	"Engine"
width / height	1280 / 720
resizable	true
decorated	true
vsync	false
fullscreen	false
alwaysOnTop	false

EngineConfig now holds a WindowConfig internally. Existing windowTitle() / windowSize() accessors are preserved as backward-compatible methods. Builder gains windowResizable(), windowDecorated(), windowVsync(), windowFullscreen(), windowAlwaysOnTop(). BaseApplication applies all properties to the window handle after creation. Added WindowProperty.ALWAYS_ON_TOP backed by GLFW GLFW_FLOATING.

API / Maintainability

GlobalParamNames constants

Extracted "Engine", "Camera", "Object" magic strings into dev.engine.graphics.shader.GlobalParamNames. Used in Renderer and UniformManager — including the previously missed bindGlobals() method which still had a raw "Object" literal.

DrawCommand.materialData typed correctly

Changed PropertyMap materialData (raw type) to PropertyMap<MaterialData> for compile-time type safety.

BaseApplication viewport set once

Removed unconditional per-frame setViewport call that made the WindowEvent.Resized handler redundant. Viewport is now set once from actual window dimensions before the main loop and updated only on WindowEvent.Resized.

BaseApplication Javadoc updated

Updated the launch() example to show the current .graphics(new OpenGlConfig(...)) API instead of the deprecated .graphicsBackend(...) path.

ResourceCleaner.register() made public

Was package-private, blocking use of the Cleaner safety net outside core.resource.

Additional Findings (documented in TODO.md)

The independent review passes documented open items not yet fixed. New items added from reviewing the merged Debug UI code include: DebugUiOverlay allocated in headless mode, GL/Vulkan scissor not clamped to viewport bounds, NkContext window accumulation, NkDrawList 16-bit index limit, and the debug_ui.slang [[vk::binding]] annotation propagating to all Slang targets. See TODO.md for the full list.

[zzuegg]

@copilot you should do your own code review and report if you agree with the todo or have anything to add

[Copilot]

@copilot you should do your own code review and report if you agree with the todo or have anything to add

Done an independent top-to-bottom review of all 339 source files. Here's my assessment — I agree with everything in the TODO list, and I found 9 additional issues that weren't listed. Two of them were actual bugs that I fixed in commit e5e53a9:

Two new bugs fixed:

1. UniformManager.bindGlobals() still had a hardcoded "Object" string — missed in the first pass when we introduced GlobalParamNames. Now uses GlobalParamNames.OBJECT.

2. Vec2 alignment missing from the material UBO write loop — The size-calculation loop correctly aligned Vec2 to an 8-byte offset boundary before adding its 8-byte contribution. The write loop did not apply that same alignment — the write offset fell into the else branch without any pre-alignment. Any material containing a Vec2 property after an odd-aligned predecessor had its data written at the wrong GPU offset, producing corrupted values on the GPU. Fixed by adding offset = align(offset, 8) before writing Vec2 in the write loop.

Seven new issues added to TODO.md (not yet fixed):

• Material UBO silently wrong-sized on property change — materialUbos.computeIfAbsent allocates the buffer once based on the first set of properties. If a material later gains new scalar keys, the existing buffer is too small and the excess write goes into unallocated GPU memory.
• BaseApplication Javadoc example shows deprecated API — .graphicsBackend(OpenGlBackend.factory(...)) in the example should be the new .graphics(new OpenGlConfig(...)).
• BaseApplication calls setViewport twice per frame — The WindowEvent.Resized handler and the unconditional line 129 call both set the viewport every frame. The event handler is effectively dead code.
• DrawCommand.materialData is a raw unchecked PropertyMap (missing <MaterialData> type parameter).
• DoubleBufferedPipeline + FrameSnapshot are dead code — implemented but never wired into the engine.
• EventBus is never instantiated — fully working but nothing publishes or subscribes.
• TransactionBus is never used — more appropriate for the NOTES.md architecture than the simpler TransactionBuffer currently wired in.

For the items already in the TODO list, I confirm all of them are real. The most impactful open items in my view (beyond what was already fixed) are: the Engine.run() thread-safety race, the material UBO size-mismatch, the error/fallback shader (entities silently disappear on shader failure), and the GpuResourceManager deferred-deletion frame count mismatch with Vulkan's 3-frame-in-flight case.

[zzuegg]

@copilot fix also the bugs that do not require a large architectural change, and add missing stuff. Update the todo. Make sure everything compiles, and if possible run the screenshot tests too

[Copilot]

@copilot fix also the bugs that do not require a large architectural change, and add missing stuff. Update the todo. Make sure everything compiles, and if possible run the screenshot tests too

Done in commit f6abc8f. Four additional bugs fixed, TODO.md cleaned up:

Fixed:

1. DrawCommand.materialData raw type — Changed PropertyMap to PropertyMap<MaterialData> for type safety.
2. BaseApplication viewport called every frame — Removed the unconditional per-frame setViewport call. Viewport is now set once before the loop (from actual window dimensions) and updated only on WindowEvent.Resized.
3. BaseApplication Javadoc uses deprecated API — Updated the launch() example to show .graphics(new OpenGlConfig(...)) instead of the old .graphicsBackend(...).
4. Material UBO wrong-sized on property change — Added a materialUboSizes tracking map alongside materialUbos. In uploadAndBindMaterial, if the required UBO size grows beyond the allocated size, the old buffer is cleanly removed from both maps and destroyed before a correctly-sized buffer is created. Both maps are always updated atomically (remove before destroy, insert after create) so a failed allocation never leaves a dangling handle.

TODO.md: Removed the duplicate "Hardcoded Values" section that had appeared at the bottom of the file; all four newly-fixed items marked [x].

Regarding screenshot tests: they were already failing before these changes (the Slang compiler binary isn't present in the CI environment, so every test hits SlangException). All non-GPU unit tests pass.

[zzuegg]

@copilot add also the missing window properties. They should go in the engine config, maybe in a separate window or context config?

Fix also the blend mode issues, and make sure separate blend modes are also supported, maybe create a common interface?

[Copilot]

@copilot add also the missing window properties. They should go in the engine config, maybe in a separate window or context config?

Fix also the blend mode issues, and make sure separate blend modes ...

Done in commits e5faae6 and cb018dc.

Window properties — New WindowConfig record holds all startup window properties: title, width, height, resizable (default true), decorated (default true), vsync (default false), fullscreen (default false), alwaysOnTop (default false). EngineConfig now wraps a WindowConfig internally; existing windowTitle() / windowSize() accessors are preserved as backward-compat methods, and the builder gains windowResizable(), windowDecorated(), windowVsync(), windowFullscreen(), windowAlwaysOnTop(). BaseApplication applies all properties to the window handle right after creation. Added WindowProperty.ALWAYS_ON_TOP backed by GLFW GLFW_FLOATING in GlfwWindowToolkit.

Blend modes — Created BlendFactor and BlendEquation as proper extensible interfaces (static instances, no enums, matching codebase convention). BlendMode now carries enabled(), separate color + alpha factor accessors, and equation accessors. Added a BlendMode.of(name, srcColor, dstColor, srcAlpha, dstAlpha[, colorEq, alphaEq]) factory for fully custom blending. All three backends derive blend factors from the BlendMode object:

• GL — now uses glBlendFuncSeparate + glBlendEquationSeparate with a mapBlendFactor/mapBlendEquation helper; legacy SetBlending command now delegates to applyBlendMode(BlendMode.ALPHA) instead of hardcoding SRC_ALPHA.
• Vulkan — BlendConfig.fromBlendMode(mode) factory added to VkPipelineFactory; corrected two wrong VK constants (VK_BLEND_FACTOR_DST_COLOR 8→4, VK_BLEND_FACTOR_DST_ALPHA 10→8 per spec).
• WebGPU — mapBlendFactor/mapBlendEquation helpers added; pipeline creation now passes mapped equation constants instead of hardcoded BLEND_OP_ADD.

[zzuegg]

@copilot at least opengl supports also different blendmodes on different rendertargets when using mrt. Check suppimort in the other backends, and add support for that too.

[Copilot]

@copilot at least opengl supports also different blendmodes on different rendertargets when using mrt. Check suppimort in the other backends, and add support for that too.

Done in commit bcf56fe.

Added RenderState.BLEND_MODES (BlendMode[]) for per-attachment independent blending:

// MRT: attachment 0 = alpha blend, attachment 1 = additive, attachment 2 = no blend
recorder.setRenderState(PropertyMap.<RenderState>builder()
    .set(RenderState.BLEND_MODES, new BlendMode[]{
        BlendMode.ALPHA,
        BlendMode.ADDITIVE,
        BlendMode.NONE
    })
    .build());

OpenGL: Added glBlendFuncSeparatei / glBlendEquationSeparatei / glEnablei / glDisablei to GlBindings + LwjglGlBindings (GL 4.0+ indexed blend API). GlRenderDevice iterates the array and applies per-draw-buffer state when BLEND_MODES is set. If the array is shorter than the number of active attachments, the last entry is applied to all remaining buffers.

Vulkan: VkBindings.createGraphicsPipeline now takes per-attachment arrays (boolean[], int[] × 6). LwjglVkBindings creates one VkPipelineColorBlendAttachmentState per entry. VkPipelineFactory.BlendConfig gains colorBlendOp / alphaBlendOp fields and fromBlendMode() correctly maps BlendEquation to VK_BLEND_OP_*. VkRenderDevice tracks currentBlendModes, the buildBlendConfigs() helper produces the right BlendConfig[], and the pipeline variant key covers all per-attachment mode names.

WebGPU: The current Java binding only exposes a single color target in the pipeline descriptor — BLEND_MODES is silently ignored and the single BLEND_MODE setting applies instead. This limitation is documented in the BLEND_MODES Javadoc.

[zzuegg]

@copilot then i assume i can also set the renderstate on the material? And it is working with the blendmodes too?

[Copilot]

@copilot then i assume i can also set the renderstate on the material? And it is working with the blendmodes too?

Yes — render state on materials already works through MaterialData.withRenderState():

var mat = MaterialData.unlit(new Vec3(1, 0, 0))
    .withRenderState(RenderState.BLEND_MODE, BlendMode.ALPHA)
    .withRenderState(RenderState.DEPTH_WRITE, false);
entity.add(mat);

// MRT per-attachment blend on a material:
var mrtMat = MaterialData.create()
    .withRenderState(RenderState.BLEND_MODES, new BlendMode[]{
        BlendMode.ALPHA,
        BlendMode.ADDITIVE
    });

RenderStateManager.resolve(material) merges defaults < material overrides < forced overrides and emits a SetRenderState command per draw. All three backends handle both BLEND_MODE and BLEND_MODES from that command.

Fixed a small gap in 061a58a: BLEND_MODES wasn't listed in RenderStateManager.isRenderStateKey(), so it was missing from the filter that prevents render state keys from being written into material UBOs.

[zzuegg]

@copilot rebase and redo the code review

[Copilot]

@copilot rebase and redo the code review

Done in commit cef408a. Merged origin/main (50 commits — Debug UI / NkContext system, treeNode widgets, @EngineModule/@EngineFeature annotations, new WebGPU provider) and did a fresh review of all new code.

One real bug found and fixed:

DebugUiOverlay.close() leaked the pipeline handle. The pipeline is created via ShaderManager.compileSlangSource() which does not cache it — the comment "Pipeline is owned by ShaderManager" was wrong. Added device.destroyPipeline(pipeline) to close().

Additional findings added to TODO.md (Debug UI section):

• DebugUiOverlay always initializes even in headless mode (font atlas allocated, shader compilation attempted)
• GL and Vulkan scissor paths don't clamp to viewport bounds (WebGPU does); flipScissorY can produce negative Y near the top edge
• NkContext windows accumulate forever (closed = true hides but doesn't evict)
• NkDrawList uses 16-bit indices (65535 vertex limit)
• debug_ui.slang [[vk::binding(N)]] is emitted for all Slang targets including WGSL (currently harmless since WebGPU offset is 0, but fragile)