From 042adeeed54c1f582eb642331e751e6720515a26 Mon Sep 17 00:00:00 2001 From: Jay Dolan Date: Mon, 17 Aug 2026 10:29:38 -0400 Subject: [PATCH 1/2] Acquire the swapchain in endFrame, not beginFrame beginFrame acquired the swapchain texture at the top of the frame, so the drawable was held across all of the caller's per-frame CPU work before the presenting blit finally submitted it. On Metal that starves CAMetalLayer's drawable pool: [layer nextDrawable] returns nil under contention, SDL does not check it, and the frame is silently never presented. Nothing needs the swapchain until the blit, so acquire it there. Size the present framebuffer from SDL_GetWindowSizeInPixels rather than the swapchain dimensions, so a transient acquire hiccup can no longer tear down and rebuild every attachment (which also reset double-buffered attachment parity). The two agree by construction; the blit now scales if they ever disagree mid-resize. When no drawable is available the command buffer is still submitted, so the frame's GPU work completes and only presentation is skipped. Also treat a NULL texture from either acquire wrapper as unavailable. SDL documents returning success with a NULL handle when the window is minimised or too many frames are in flight, and that handle must not be passed back into SDL; both backends do exactly this on the non-blocking path. Refs jdolan/quetoo#945 --- Sources/ObjectivelyGPU/CommandBuffer.c | 31 ++++++++++------ Sources/ObjectivelyGPU/CommandBuffer.h | 22 ++++++++---- Sources/ObjectivelyGPU/RenderDevice.c | 50 +++++++++++++++----------- Sources/ObjectivelyGPU/RenderDevice.h | 43 +++++++++++++--------- 4 files changed, 91 insertions(+), 55 deletions(-) diff --git a/Sources/ObjectivelyGPU/CommandBuffer.c b/Sources/ObjectivelyGPU/CommandBuffer.c index 857b55d..b20b05f 100644 --- a/Sources/ObjectivelyGPU/CommandBuffer.c +++ b/Sources/ObjectivelyGPU/CommandBuffer.c @@ -64,12 +64,17 @@ static bool acquireSwapchainTexture(const CommandBuffer *self, SwapchainTexture &swapchain->texture, &w, &h); - if (ok) { - swapchain->size = (SDL_Size) { (int) w, (int) h }; - swapchain->format = SDL_GetGPUSwapchainTextureFormat(self->device->device, self->device->window); + // SDL reports success with a NULL texture when too many frames are in flight; + // that NULL must never be handed back to SDL, so report it as unavailable. + if (!ok || swapchain->texture == NULL) { + *swapchain = (SwapchainTexture) { 0 }; + return false; } - - return ok; + + swapchain->size = (SDL_Size) { (int) w, (int) h }; + swapchain->format = SDL_GetGPUSwapchainTextureFormat(self->device->device, self->device->window); + + return true; } /** @@ -305,12 +310,18 @@ static bool waitAndAcquireSwapchainTexture(const CommandBuffer *self, SwapchainT self->device->window, &swapchain->texture, &w, &h); - if (ok) { - swapchain->size = (SDL_Size) { (int) w, (int) h }; - swapchain->format = SDL_GetGPUSwapchainTextureFormat(self->device->device, self->device->window); + + // SDL reports success with a NULL texture when the window is minimised or the + // swapchain is otherwise unavailable; that NULL must never be handed back to SDL. + if (!ok || swapchain->texture == NULL) { + *swapchain = (SwapchainTexture) { 0 }; + return false; } - - return ok; + + swapchain->size = (SDL_Size) { (int) w, (int) h }; + swapchain->format = SDL_GetGPUSwapchainTextureFormat(self->device->device, self->device->window); + + return true; } #pragma mark - Class lifecycle diff --git a/Sources/ObjectivelyGPU/CommandBuffer.h b/Sources/ObjectivelyGPU/CommandBuffer.h index 8be1933..eb4d272 100644 --- a/Sources/ObjectivelyGPU/CommandBuffer.h +++ b/Sources/ObjectivelyGPU/CommandBuffer.h @@ -134,11 +134,13 @@ struct CommandBufferInterface { /** * @fn bool CommandBuffer::acquireSwapchainTexture(const CommandBuffer *self, SwapchainTexture *swapchain) * @brief Acquires the next swapchain texture for rendering. - * @details Returns `false` (without asserting) when the window is minimised - * or the swapchain is temporarily unavailable. The caller should skip - * rendering for that frame. + * @details Returns `false` (without asserting) when the window is minimised, too many + * frames are in flight, or the swapchain is temporarily unavailable; SDL reports those + * cases as success with a NULL texture, which MUST NOT be passed back into SDL, so + * they are reported here as failure and @p swapchain is zeroed. The caller should skip + * presenting for that frame. * @param self The CommandBuffer. - * @param swapchain Output structure populated with the texture and dimensions. + * @param swapchain Output structure populated with the texture and dimensions, or zeroed. * @return True on success, false when the swapchain is unavailable this frame. * @memberof CommandBuffer */ @@ -322,10 +324,16 @@ struct CommandBufferInterface { * @fn bool CommandBuffer::waitAndAcquireSwapchainTexture(const CommandBuffer *self, SwapchainTexture *swapchain) * @brief Blocks until a swapchain texture is available, then acquires it. * @details Prefer `acquireSwapchainTexture` unless you must guarantee a - * texture this frame (e.g. during resize). + * texture this frame (e.g. during resize). Blocking bounds how many frames may be in + * flight, but a drawable is still not guaranteed: as with `acquireSwapchainTexture`, + * an unavailable swapchain is reported as `false` with @p swapchain zeroed. + * + * Call this as late in the frame as possible. The acquired drawable is held until this + * CommandBuffer is submitted, and holding it across a frame's CPU work starves the + * drawable pool. * @param self The CommandBuffer. - * @param swapchain Output structure populated with the texture and dimensions. - * @return True on success, false on error. + * @param swapchain Output structure populated with the texture and dimensions, or zeroed. + * @return True on success, false when the swapchain is unavailable this frame. * @memberof CommandBuffer */ bool (*waitAndAcquireSwapchainTexture)(const CommandBuffer *self, SwapchainTexture *swapchain); diff --git a/Sources/ObjectivelyGPU/RenderDevice.c b/Sources/ObjectivelyGPU/RenderDevice.c index 1990f85..df74e4d 100644 --- a/Sources/ObjectivelyGPU/RenderDevice.c +++ b/Sources/ObjectivelyGPU/RenderDevice.c @@ -155,15 +155,17 @@ static CommandBuffer *beginFrame(RenderDevice *self) { GPU_Assert(self->framebuffer, "no framebuffer set; call setFramebuffer first"); GPU_Assert(self->commands == NULL, "beginFrame called with a frame already in flight"); + // The swapchain is deliberately *not* acquired here; see endFrameAndFence. Size the + // framebuffer from the window instead, so a transient swapchain hiccup can never + // churn every attachment. + int w = 0, h = 0; + if (!SDL_GetWindowSizeInPixels(self->window, &w, &h) || w <= 0 || h <= 0) { + return NULL; + } + self->commands = $(self, acquireCommandBuffer); - const bool ok = $(self->commands, waitAndAcquireSwapchainTexture, &self->swapchain); - if (ok) { - $(self->framebuffer, resize, &self->swapchain.size); - } else { - $(self->commands, cancel); - self->commands = release(self->commands); - } + $(self->framebuffer, resize, &(SDL_Size) { w, h }); return self->commands; } @@ -179,20 +181,26 @@ static Fence *endFrameAndFence(RenderDevice *self) { Texture *color = $(self->framebuffer, resolveColorTexture, 0); GPU_Assert(color, "framebuffer has no color attachment to present"); - $(self->commands, blitTexture, &(SDL_GPUBlitInfo) { - .source = { - .texture = color->texture, - .w = (Uint32) self->swapchain.size.w, - .h = (Uint32) self->swapchain.size.h, - }, - .destination = { - .texture = self->swapchain.texture, - .w = (Uint32) self->swapchain.size.w, - .h = (Uint32) self->swapchain.size.h, - }, - .load_op = SDL_GPU_LOADOP_DONT_CARE, - .filter = SDL_GPU_FILTER_NEAREST, - }); + // Acquire the swapchain as late as possible: the drawable is held only for this blit + // rather than for the whole frame, which is what Metal's CAMetalLayer wants and what + // keeps `nextDrawable` from starving under contention. When no drawable is available + // the frame's GPU work is still submitted -- it simply isn't presented. + if ($(self->commands, waitAndAcquireSwapchainTexture, &self->swapchain)) { + $(self->commands, blitTexture, &(SDL_GPUBlitInfo) { + .source = { + .texture = color->texture, + .w = (Uint32) self->framebuffer->size.w, + .h = (Uint32) self->framebuffer->size.h, + }, + .destination = { + .texture = self->swapchain.texture, + .w = (Uint32) self->swapchain.size.w, + .h = (Uint32) self->swapchain.size.h, + }, + .load_op = SDL_GPU_LOADOP_DONT_CARE, + .filter = SDL_GPU_FILTER_NEAREST, + }); + } Fence *fence = $(self->commands, submitAndFence); diff --git a/Sources/ObjectivelyGPU/RenderDevice.h b/Sources/ObjectivelyGPU/RenderDevice.h index 5e39c9d..59ec499 100644 --- a/Sources/ObjectivelyGPU/RenderDevice.h +++ b/Sources/ObjectivelyGPU/RenderDevice.h @@ -98,8 +98,8 @@ struct RenderDevice { /** * @brief The present-target Framebuffer driven by `beginFrame`/`endFrame`, or `NULL`. - * @details Set via `setFramebuffer` (retained). `beginFrame` resizes it to the - * swapchain each frame and `endFrame` blits its resolved color to the swapchain. + * @details Set via `setFramebuffer` (retained). `beginFrame` resizes it to the window's + * pixel size each frame and `endFrame` blits its resolved color to the swapchain. */ Framebuffer *framebuffer; @@ -112,8 +112,9 @@ struct RenderDevice { CommandBuffer *commands; /** - * @brief The swapchain texture acquired for the current frame. - * @details Valid only between `beginFrame` and `endFrame`. + * @brief The swapchain texture acquired to present the current frame. + * @details Acquired inside `endFrame`, immediately before the presenting blit, and + * cleared again before it returns. Zeroed when no drawable was available. * @private */ SwapchainTexture swapchain; @@ -155,16 +156,20 @@ struct RenderDeviceInterface { /** * @fn CommandBuffer *RenderDevice::beginFrame(RenderDevice *self) - * @brief Begins a frame: acquires a command buffer and the swapchain, and prepares the framebuffer. - * @details Convenience over the manual acquire→wait-swapchain→resize boilerplate. - * Acquires a CommandBuffer, blocks for the swapchain texture, resizes the framebuffer - * set via `setFramebuffer` to the swapchain dimensions, and returns the command buffer - * so the caller can record passes into `framebuffer`. Returns `NULL` when the swapchain - * is unavailable (e.g. the window is minimised); the command buffer is cancelled and the - * frame should be skipped. Pair every non-NULL return with `endFrame`. The returned - * CommandBuffer is owned by the device; do not release it. Applications that render - * directly to the swapchain can ignore `beginFrame`/`endFrame` and drive the command - * buffer themselves. + * @brief Begins a frame: acquires a command buffer and prepares the framebuffer. + * @details Acquires a CommandBuffer, resizes the framebuffer set via `setFramebuffer` + * to the window's pixel size, and returns the command buffer so the caller can record + * passes into `framebuffer`. Returns `NULL` when the window has no drawable area (e.g. + * it is minimised) and the frame should be skipped entirely. + * + * The swapchain is deliberately **not** acquired here. `endFrame` acquires it + * immediately before the presenting blit, so the drawable is held for that blit alone + * rather than for the whole frame. Holding a Metal drawable across a frame's CPU work + * starves `CAMetalLayer`'s drawable pool and costs presented frames. + * + * Pair every non-NULL return with `endFrame`. The returned CommandBuffer is owned by + * the device; do not release it. Applications that render directly to the swapchain can + * ignore `beginFrame`/`endFrame` and drive the command buffer themselves. * @param self The RenderDevice. * @return The frame's CommandBuffer (borrowed), or `NULL` to skip the frame. * @memberof RenderDevice @@ -394,10 +399,14 @@ struct RenderDeviceInterface { /** * @fn void RenderDevice::endFrame(RenderDevice *self) * @brief Ends the frame begun by `beginFrame`: presents the framebuffer and submits. - * @details Blits the framebuffer's resolved color (`Framebuffer::resolveColorTexture`, - * the resolve target when multisampled) into the acquired swapchain texture, submits - * the frame's command buffer, and releases it. Must be paired with a non-NULL + * @details Acquires the swapchain texture, blits the framebuffer's resolved color + * (`Framebuffer::resolveColorTexture`, the resolve target when multisampled) into it, + * submits the frame's command buffer, and releases it. Must be paired with a non-NULL * `beginFrame` return. + * + * When no drawable is available the blit is skipped but the command buffer is still + * submitted, so the frame's GPU work completes and resource cycling stays correct; the + * frame simply is not presented and the previous one remains on screen. * @param self The RenderDevice. * @memberof RenderDevice */ From 31aef12f0665845e47755f901f9c32d6a0f0115b Mon Sep 17 00:00:00 2001 From: Jay Dolan Date: Mon, 17 Aug 2026 13:24:17 -0400 Subject: [PATCH 2/2] Remove superfluous comments. --- Sources/ObjectivelyGPU/CommandBuffer.c | 5 ----- Sources/ObjectivelyGPU/RenderDevice.c | 17 +++++------------ 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/Sources/ObjectivelyGPU/CommandBuffer.c b/Sources/ObjectivelyGPU/CommandBuffer.c index b20b05f..617d792 100644 --- a/Sources/ObjectivelyGPU/CommandBuffer.c +++ b/Sources/ObjectivelyGPU/CommandBuffer.c @@ -63,9 +63,6 @@ static bool acquireSwapchainTexture(const CommandBuffer *self, SwapchainTexture self->device->window, &swapchain->texture, &w, &h); - - // SDL reports success with a NULL texture when too many frames are in flight; - // that NULL must never be handed back to SDL, so report it as unavailable. if (!ok || swapchain->texture == NULL) { *swapchain = (SwapchainTexture) { 0 }; return false; @@ -311,8 +308,6 @@ static bool waitAndAcquireSwapchainTexture(const CommandBuffer *self, SwapchainT &swapchain->texture, &w, &h); - // SDL reports success with a NULL texture when the window is minimised or the - // swapchain is otherwise unavailable; that NULL must never be handed back to SDL. if (!ok || swapchain->texture == NULL) { *swapchain = (SwapchainTexture) { 0 }; return false; diff --git a/Sources/ObjectivelyGPU/RenderDevice.c b/Sources/ObjectivelyGPU/RenderDevice.c index df74e4d..b0ba3eb 100644 --- a/Sources/ObjectivelyGPU/RenderDevice.c +++ b/Sources/ObjectivelyGPU/RenderDevice.c @@ -152,21 +152,19 @@ static CommandBuffer *acquireCommandBuffer(const RenderDevice *self) { */ static CommandBuffer *beginFrame(RenderDevice *self) { + GPU_Assert(self->window, "no SDL_Window for RenderDevice"); GPU_Assert(self->framebuffer, "no framebuffer set; call setFramebuffer first"); - GPU_Assert(self->commands == NULL, "beginFrame called with a frame already in flight"); - // The swapchain is deliberately *not* acquired here; see endFrameAndFence. Size the - // framebuffer from the window instead, so a transient swapchain hiccup can never - // churn every attachment. int w = 0, h = 0; if (!SDL_GetWindowSizeInPixels(self->window, &w, &h) || w <= 0 || h <= 0) { return NULL; } - - self->commands = $(self, acquireCommandBuffer); - + $(self->framebuffer, resize, &(SDL_Size) { w, h }); + GPU_Assert(self->commands == NULL, "beginFrame called with a frame already in flight"); + + self->commands = $(self, acquireCommandBuffer); return self->commands; } @@ -181,10 +179,6 @@ static Fence *endFrameAndFence(RenderDevice *self) { Texture *color = $(self->framebuffer, resolveColorTexture, 0); GPU_Assert(color, "framebuffer has no color attachment to present"); - // Acquire the swapchain as late as possible: the drawable is held only for this blit - // rather than for the whole frame, which is what Metal's CAMetalLayer wants and what - // keeps `nextDrawable` from starving under contention. When no drawable is available - // the frame's GPU work is still submitted -- it simply isn't presented. if ($(self->commands, waitAndAcquireSwapchainTexture, &self->swapchain)) { $(self->commands, blitTexture, &(SDL_GPUBlitInfo) { .source = { @@ -205,7 +199,6 @@ static Fence *endFrameAndFence(RenderDevice *self) { Fence *fence = $(self->commands, submitAndFence); self->commands = release(self->commands); - self->swapchain = (SwapchainTexture) { 0 }; return fence;