From 1d87d643bb501e3309e99cb89e7081b2ca966b03 Mon Sep 17 00:00:00 2001 From: Corbin Crutchley Date: Thu, 17 Sep 2026 20:39:01 -0700 Subject: [PATCH] fix: release completed RetroArch command buffers while idle --- ScreenSlanger/librashader.swift | 28 ++++++++++- ScreenSlangerTests/ShaderTests.swift | 73 ++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/ScreenSlanger/librashader.swift b/ScreenSlanger/librashader.swift index 07bde88..3e34c64 100644 --- a/ScreenSlanger/librashader.swift +++ b/ScreenSlanger/librashader.swift @@ -64,6 +64,19 @@ final class RetroArchFilterChain: Sendable { frameCount: frameCount, framesPerSecond: framesPerSecond, frameTimeMilliseconds: frameTimeMilliseconds, parameterValues: parameterValues) } + let commandID = ObjectIdentifier(commandBuffer) + commandBuffer.addCompletedHandler { [weak self] _ in + self?.releaseCompletedFrame(id: commandID) + } + } + + /// Release completed command storage even when a static desktop produces no + /// next frame. Never block a Metal callback on the worker: teardown on that + /// thread may itself be waiting for command completion. + func releaseCompletedFrame(id: ObjectIdentifier) { + // Metal can invoke completion while destroying an abandoned command. + // Carry its identity, never retain the callback's potentially dying buffer. + worker.schedule { state in state?.releaseCompletedFrame(id: id) } } /// If later encoding fails and the caller drops the unsubmitted command buffer, @@ -75,7 +88,7 @@ final class RetroArchFilterChain: Sendable { } private struct LibraCommand: @unchecked Sendable { - // As with LibraFrame, the caller lends exclusive encoding ownership for this call. + // The worker only inspects identity/status here; it never encodes commands. let buffer: any MTLCommandBuffer } @@ -158,6 +171,10 @@ private final class LibraWorker: Sendable { deinit { queue.finish() } + func schedule(_ body: @escaping @Sendable (inout LibraState?) -> Void) { + queue.append(body) + } + func perform(_ body: @escaping @Sendable (inout LibraState?) throws -> Value) throws -> Value { let reply = Reply() queue.append { state in reply.complete(Result { try body(&state) }) } @@ -259,6 +276,15 @@ private final class LibraState { lastCommandBuffer = commandBuffer } + func releaseCompletedFrame(id: ObjectIdentifier) { + // A new frame can be encoded before an older completion reaches this + // queue. Its busy gate and retained resources belong to that new frame. + if let commandBuffer = lastCommandBuffer, ObjectIdentifier(commandBuffer) == id, + commandBuffer.status == .completed || commandBuffer.status == .error { + lastCommandBuffer = nil + } + } + func discardUnsubmittedFrame(_ commandBuffer: any MTLCommandBuffer) { if lastCommandBuffer === commandBuffer, commandBuffer.status == .notEnqueued { lastCommandBuffer = nil diff --git a/ScreenSlangerTests/ShaderTests.swift b/ScreenSlangerTests/ShaderTests.swift index 8c71668..5b1e451 100644 --- a/ScreenSlangerTests/ShaderTests.swift +++ b/ScreenSlangerTests/ShaderTests.swift @@ -606,6 +606,79 @@ struct ShaderTests { try expectPixels(finishRendering(next, to: nextDestination), expected: inputPixels) } + @Test("An idle RetroArch chain releases its completed command buffer") + func completedFrameStorageIsReleased() async throws { + let shared = SharedMetalResources.shared + defer { shared.clear() } + try await shared.loadEffect(ShaderLoadRequest(url: fixtureURL("retroarch-helper"))) + let core = ShaderRenderCore(resources: shared) + weak var completed: (any MTLCommandBuffer)? + try autoreleasepool { + let commands = try #require(shared.commandQueue.makeCommandBuffer()) + let destination = try makeTexture() + completed = commands + try core.encode(commandBuffer: commands, source: makeTexture(pixels: inputPixels), + destination: destination, context: ShaderFrameContext(outputSize: SIMD2(2, 2))) + _ = try finishRendering(commands, to: destination) + } + // Completion schedules cleanup on the chain's own thread. No additional + // render or clear may be needed to release the last command's storage. + for _ in 0..<200 { + if completed == nil { break } + try await Task.sleep(for: .milliseconds(10)) + } + #expect(completed == nil, "The idle chain retained its completed command buffer") + #expect(shared.effect != nil) + } + + @Test("Late and premature completion notifications cannot release a newer frame") + func completionCannotReleaseAnotherFrame() async throws { + let shared = SharedMetalResources.shared + defer { shared.clear() } + try await shared.loadEffect(ShaderLoadRequest(url: fixtureURL("retroarch-helper"))) + let effect = try #require(shared.effect) + guard case .retroArch(let chains) = effect.backend else { + Issue.record("Expected a RetroArch filter chain") + return + } + let chain = try #require(chains[0]) + let core = ShaderRenderCore(resources: shared) + let source = try makeTexture(pixels: inputPixels) + let destination = try makeTexture() + let context = ShaderFrameContext(outputSize: SIMD2(2, 2)) + let first = try #require(shared.commandQueue.makeCommandBuffer()) + try core.encode(commandBuffer: first, source: source, destination: destination, context: context) + _ = try finishRendering(first, to: destination) + + let second = try #require(shared.commandQueue.makeCommandBuffer()) + let third = try #require(shared.commandQueue.makeCommandBuffer()) + defer { + if second.status == .notEnqueued { + _ = try? finishRendering(second, to: destination) + } + chain.discardUnsubmittedFrame(commandBuffer: third) + } + try core.encode(commandBuffer: second, source: source, destination: destination, context: context) + // A completion may arrive after the next frame has claimed the slot. + // Queue these before the next encode so the worker processes them first. + chain.releaseCompletedFrame(id: ObjectIdentifier(first)) + chain.releaseCompletedFrame(id: ObjectIdentifier(second)) + shared.parameterState.setValue(1, for: "GAIN") + do { + try core.encode(commandBuffer: third, source: source, destination: destination, context: context) + Issue.record("A completion notification released an unsubmitted frame") + return + } catch RetroArchRuntimeError.frameInFlight { + // The rejected attempt must not alter the second frame's uniforms. + } + try expectPixels(finishRendering(second, to: destination), expected: [ + 0, 0, 128, 255, 0, 128, 0, 255, + 128, 0, 0, 255, 128, 128, 128, 255, + ]) + try core.encode(commandBuffer: third, source: source, destination: destination, context: context) + try expectPixels(finishRendering(third, to: destination), expected: inputPixels) + } + @Test("Discarding an unsubmitted frame releases its chain for the next render") func discardedFrameReleasesBusyGate() async throws { let shared = SharedMetalResources.shared -- 2.51.2