metal: Fix SDL_RenderReadPixels to wait for the GPU to finish rendering to the active texture before reading its pixels.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44
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;
}}