Outputs: Defensive checks, and don't wait for a Metal drawable twice in a frame

- Metal: when a layer has no drawable to give (it can take a second to
  say so), don't ask again in the same frame for every pass to it.
- Swapchains other than the primary are created, resized and destroyed
  between frames (Metal asserts it), and destroyed with the last
  reference to them (ReleaseAssertLast, Metal and Vulkan).
- BindFramebufferSamplesAsTexture asserts it isn't given a swapchain,
  like BindFramebufferAsTexture.
- An offscreen Metal swapchain can't be resized, asserted.
- VulkanRenderManager asserts that no additional swapchain outlives it.
- Native_RemoveOutputAndWait asserts it isn't called on the thread that
  handles the requests, where it would always time out.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Henrik RydgårdandClaude Opus 5.5 committed 2026-10-22 23:08:52 +02:00
1 parent ba55ee7e5f
commit da33eab776
6 files changed
+26 -3

No files matched your search

+4
View File
@@ -235,6 +235,8 @@ struct MetalSwapchain {
bool drawn = false; // Something has rendered to it.
// Replay side. Acquired by the first pass that renders to it, presented at the end of the frame.
id<CAMetalDrawable> drawable;
// The replayed frame nextDrawable gave none in. Not asked again that frame, as each try can wait a second.
uint64_t noDrawableFrame = UINT64_MAX;
};
struct MetalRecordedPass {
@@ -383,6 +385,7 @@ public:
void DestroySwapchain(MetalSwapchain *swapchain);
void Resize(MetalSwapchain *swapchain, int width, int height);
void Resize(int width, int height) { Resize(backbuffer_, width, height); }
bool InFrame() const { return inFrame_; }
int BackbufferWidth() const { return backbuffer_->width; }
int BackbufferHeight() const { return backbuffer_->height; }
MTLPixelFormat BackbufferFormat() const { return MTLPixelFormatBGRA8Unorm; }
@@ -556,6 +559,7 @@ private:
id<MTLBlitCommandEncoder> rtUploadBlit_;
id<MTLComputeCommandEncoder> rtUploadCompute_;
std::vector<MetalSwapchain *> rtAcquired_; // Swapchains with a drawable to present.
uint64_t replayFrame_ = 0; // Counts the presents replayed.
// Replay scratch, reused between chunks.
std::vector<MetalStep> rtSteps_;
size_t rtStepCount_ = 0;
+4 -1
View File
@@ -926,12 +926,14 @@ id<MTLTexture> MetalRenderManager::ReplaySwapchainTexture(MetalSwapchain *swapch
if (!swapchain->layer) {
return swapchain->offscreen;
}
if (!swapchain->drawable) {
if (!swapchain->drawable && swapchain->noDrawableFrame != replayFrame_) {
@autoreleasepool {
swapchain->drawable = [swapchain->layer nextDrawable];
}
if (swapchain->drawable) {
rtAcquired_.push_back(swapchain);
} else {
swapchain->noDrawableFrame = replayFrame_;
}
}
return swapchain->drawable ? swapchain->drawable.texture : nil;
@@ -1039,6 +1041,7 @@ void MetalRenderManager::Replay(MetalCommandChunk *chunk) {
swapchain->drawable = nil;
}
rtAcquired_.clear();
replayFrame_++;
}
dispatch_semaphore_t frameSemaphore = chunk->end == MetalChunkEnd::PRESENT ? frameSemaphore_ : nullptr;
+9 -1
View File
@@ -1247,6 +1247,7 @@ void MetalDrawContext::BindFramebufferAsTexture(Framebuffer *fbo, int binding, A
void MetalDrawContext::BindFramebufferSamplesAsTexture(Framebuffer *fbo, int binding, int layer) {
_dbg_assert_(binding < (int)MAX_TEXTURE_SLOTS);
_dbg_assert_(fbo && !fbo->IsSwapchain());
MetalFramebuffer *fb = (MetalFramebuffer *)fbo;
boundTextures_[binding] = fb->msaaColor ? fb->msaaColor : fb->color;
boundThin3DTextures_[binding] = nullptr;
@@ -1263,6 +1264,8 @@ void MetalDrawContext::GetFramebufferDimensions(Framebuffer *fbo, int *w, int *h
}
Swapchain *MetalDrawContext::CreateSwapchain(const SwapchainDesc &desc, std::string *error) {
// Swapchains other than the primary come and go between frames.
_dbg_assert_(!rm_->InFrame());
if (desc.winsys != WINDOWSYSTEM_METAL_EXT) {
*error = "Metal swapchains need a CAMetalLayer";
return nullptr;
@@ -1278,15 +1281,20 @@ Swapchain *MetalDrawContext::CreateSwapchain(const SwapchainDesc &desc, std::str
void MetalDrawContext::DestroySwapchain(Swapchain *swapchain) {
_dbg_assert_(swapchain && swapchain != primarySwapchain_);
_dbg_assert_(!rm_->InFrame());
MetalSwapchainFB *fb = (MetalSwapchainFB *)swapchain;
rm_->DestroySwapchain(fb->swapchain);
fb->swapchain = nullptr;
fb->Release();
// Nothing else may hold it, it no longer has a swapchain to be.
fb->ReleaseAssertLast();
}
void MetalDrawContext::ResizeSwapchain(Swapchain *swapchain, int width, int height) {
MetalSwapchain *metalSwapchain = ToSwapchain(swapchain);
_dbg_assert_(metalSwapchain);
// An offscreen one keeps its size, as there's no layer to resize.
_dbg_assert_(metalSwapchain->layer || swapchain == primarySwapchain_);
_dbg_assert_(swapchain == primarySwapchain_ || !rm_->InFrame());
rm_->Resize(metalSwapchain, width, height);
if (swapchain == primarySwapchain_) {
primarySwapchain_->SetSize(metalSwapchain->width, metalSwapchain->height);
@@ -759,6 +759,8 @@ void VulkanRenderManager::CheckNothingPending() {
VulkanRenderManager::~VulkanRenderManager() {
INFO_LOG(Log::G3D, "VulkanRenderManager destructor");
// Their owners (the outputs) destroy them first, on device loss. Left over, they'd outlive the device.
_dbg_assert_(extraSwapchains_.empty());
{
std::unique_lock<std::mutex> lock(compileQueueMutex_);
+3 -1
View File
@@ -1999,6 +1999,7 @@ void VKContext::BindFramebufferAsTexture(Framebuffer *fbo, int binding, Aspect c
}
void VKContext::BindFramebufferSamplesAsTexture(Framebuffer *fbo, int binding, int layer) {
_dbg_assert_(fbo && !fbo->IsSwapchain());
VKFramebuffer *fb = (VKFramebuffer *)fbo;
_assert_(binding >= 0 && binding < MAX_BOUND_TEXTURES);
_assert_(fb != curFramebuffer_);
@@ -2034,7 +2035,8 @@ void VKContext::DestroySwapchain(Swapchain *swapchain) {
VKSwapchain *vkSwapchain = (VKSwapchain *)swapchain;
renderManager_.DestroyExtraSwapchain(vkSwapchain->swapchain);
vkSwapchain->swapchain = nullptr;
vkSwapchain->Release();
// Nothing else may hold it, it no longer has a swapchain to be.
vkSwapchain->ReleaseAssertLast();
}
void VKContext::ResizeSwapchain(Swapchain *swapchain, int width, int height) {
+4
View File
@@ -217,6 +217,8 @@ static uint64_t g_outputRequestsQueued = 0;
static uint64_t g_outputRequestsDone = 0;
// Whether the outputs have swapchains, between NativeInitGraphics and NativeShutdownGraphics.
static bool g_outputsLive = false;
// The thread that handles the requests, which mustn't wait for them.
static std::thread::id g_outputRequestThread;
static std::atomic<int> g_nextOutputId{1};
static void ProcessOutputRequests() {
@@ -236,6 +238,7 @@ static void ProcessOutputRequests() {
}
std::lock_guard<std::mutex> guard(g_outputRequestLock);
g_outputRequestsDone = queued;
g_outputRequestThread = std::this_thread::get_id();
g_outputRequestsDoneCond.notify_all();
}
@@ -2098,6 +2101,7 @@ void Native_RemoveOutput(int id) {
bool Native_RemoveOutputAndWait(int id) {
Native_RemoveOutput(id);
std::unique_lock<std::mutex> lock(g_outputRequestLock);
_dbg_assert_msg_(std::this_thread::get_id() != g_outputRequestThread, "Waiting for output requests on the thread that handles them");
const uint64_t queued = g_outputRequestsQueued;
// Without a device, the outputs have no swapchains to let go of.
const bool done = g_outputRequestsDoneCond.wait_for(lock, std::chrono::seconds(3), [=] {