Commit 8c41e2624e49b994dcd1b1fe39be077c4d0bab5b

Alex Szpakowski 2018-10-13T03:36:42

metal: Fix SDL_RenderReadPixels to wait for the GPU to finish rendering to the active texture before reading its pixels.

diff --git a/src/render/metal/SDL_render_metal.m b/src/render/metal/SDL_render_metal.m
index 1fc0a4f..5b4d8ea 100644
--- a/src/render/metal/SDL_render_metal.m
+++ b/src/render/metal/SDL_render_metal.m
@@ -1351,10 +1351,23 @@ static int
 METAL_RenderReadPixels(SDL_Renderer * renderer, const SDL_Rect * rect,
                     Uint32 pixel_format, void * pixels, int pitch)
 { @autoreleasepool {
+    METAL_RenderData *data = (__bridge METAL_RenderData *) renderer->driverdata;
+
+    /* Make sure we have a valid MTLTexture to read from, and an active command
+     * buffer we can wait for. */
     METAL_ActivateRenderCommandEncoder(renderer, MTLLoadActionLoad);
 
-    // !!! FIXME: this probably needs to commit the current command buffer, and probably waitUntilCompleted
-    METAL_RenderData *data = (__bridge METAL_RenderData *) renderer->driverdata;
+    /* Wait for the current command buffer to finish, so we don't read from the
+     * texture before the GPU finishes rendering to it. */
+    if (data.mtlcmdencoder) {
+        [data.mtlcmdencoder endEncoding];
+        [data.mtlcmdbuffer commit];
+        [data.mtlcmdbuffer waitUntilCompleted];
+
+        data.mtlcmdencoder = nil;
+        data.mtlcmdbuffer = nil;
+    }
+
     id<MTLTexture> mtltexture = data.mtlpassdesc.colorAttachments[0].texture;
     MTLRegion mtlregion = MTLRegionMake2D(rect->x, rect->y, rect->w, rect->h);
 
@@ -1370,6 +1383,13 @@ METAL_RenderReadPixels(SDL_Renderer * renderer, const SDL_Rect * rect,
     const Uint32 temp_format = (mtltexture.pixelFormat == MTLPixelFormatBGRA8Unorm) ? SDL_PIXELFORMAT_ARGB8888 : SDL_PIXELFORMAT_ABGR8888;
     const int status = SDL_ConvertPixels(rect->w, rect->h, temp_format, temp_pixels, temp_pitch, pixel_format, pixels, pitch);
     SDL_free(temp_pixels);
+
+    /* Set up an active command buffer and encoder once we're done. It will use
+     * the same texture that was active before (even if it's part of the swap
+     * chain), since we didn't clear that when waiting for the command buffer to
+     * complete. */
+    METAL_ActivateRenderCommandEncoder(renderer, MTLLoadActionLoad);
+
     return status;
 }}