Skip to content

Commit cc9e947

Browse files
committed
METAL: Route texture load through video thread when threaded video active
The Metal driver had a narrow threading issue when used with the threaded video wrapper. Unlike the other graphics APIs where both load and unload have races, Metal's case is specifically about the shared blit command buffer used during mipmapped texture creation. The race ======== When metal_load_texture is called with a TEXTURE_FILTER_MIPMAP_* filter, the execution path reaches: - [self newTexture:image mipmapped:YES] in metal_renderer.m - which creates an MTLTexture (thread-safe on MTLDevice) - calls replaceRegion: (thread-safe) - accesses self.blitCommandBuffer (a lazily-created MTLCommandBuffer stored on the shared Context instance) - encodes mipmap generation via [cb blitCommandEncoder] MTLCommandBuffer and MTLCommandEncoder are documented as single-thread-only. The blit command buffer is committed and nulled at frame end, on the video thread: - (void)end { ... [_blitCommandBuffer commit]; _blitCommandBuffer = nil; ... } If the main thread calls metal_load_texture mid-frame and encodes mipmap commands on the same blit command buffer that the video thread is about to commit, this is undefined behaviour per Apple's Metal threading rules. Other Metal paths are already safe ================================== Most of the Metal driver operations are fine: - [_device newTextureWithDescriptor:] is thread-safe - [texture replaceRegion:] is thread-safe and synchronous - metal_unload_texture releases an ARC-retained handle; Metal command buffers retain their referenced resources, so the underlying MTLTexture stays alive until the GPU is done. No cross-thread race on unload (unlike D3D12/Vulkan where command buffers don't refcount resources). The only remaining race is the blit command buffer sharing during mipmapped load. The fix ======= Route metal_load_texture through video_thread_texture_handle when threaded video is active, matching the pattern established in the D3D10/11/12 and Vulkan patches. This serialises all Context.blitCommandBuffer access on the video thread. Implementation: - Split metal_load_texture into an internal function that does the actual texture creation, and an outer function that dispatches. - metal_texture_cmd_t carries video_data (as void* to avoid ARC issues with Obj-C refs in C structs), image, filter_type, and handle (output). - Handle is passed back via cmd->handle rather than the int return channel of custom_command_method_t to avoid truncation on 64-bit Apple platforms. - The dispatch happens unconditionally for any threaded load, not just mipmapped ones -- consistent with other drivers and avoids branching on filter type. - metal_unload_texture is left unchanged (ARC refcounting is sufficient). Depends on: 8d55232 (VIDEO/THREAD: barrier + wrapper hardening), which hardens video_thread_texture_handle to be safe when the wrapper is inactive. Cost: texture load now blocks briefly while the video thread processes the dispatched command. Texture loads happen during menu navigation, not frame-critical paths, so latency is imperceptible. No behavioural change on non-threaded video.
1 parent 0306e17 commit cc9e947

1 file changed

Lines changed: 73 additions & 2 deletions

File tree

gfx/drivers/metal.m

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@
4949

5050
#include "../font_driver.h"
5151
#include "../video_driver.h"
52+
#ifdef HAVE_THREADS
53+
#include "../video_thread_wrapper.h"
54+
#endif
5255

5356
#include "../common/metal_common.h"
5457

@@ -2568,8 +2571,31 @@ static bool metal_read_viewport(void *data, uint8_t *buffer, bool is_idle)
25682571
return [md.frameView readViewport:buffer isIdle:is_idle];
25692572
}
25702573

2571-
static uintptr_t metal_load_texture(void *video_data, void *data,
2572-
bool threaded, enum texture_filter_type filter_type)
2574+
#ifdef HAVE_THREADS
2575+
typedef struct
2576+
{
2577+
void *video_data; /* unretained MetalDriver * */
2578+
struct texture_image *image;
2579+
enum texture_filter_type filter_type;
2580+
uintptr_t handle;
2581+
} metal_texture_cmd_t;
2582+
#endif
2583+
2584+
/* Inner load function -- performs the actual texture creation.
2585+
* Must run on the same thread that owns the Metal context's
2586+
* blit command buffer (the video thread when threaded video is
2587+
* active, otherwise the main thread).
2588+
*
2589+
* Metal's blit command buffer is lazily created on first use
2590+
* and committed during frame end. It is stored on the shared
2591+
* Context instance, not thread-local. Mipmapped texture loads
2592+
* encode mipmap-generation commands into this buffer via a
2593+
* BlitCommandEncoder -- and MTLCommandBuffer / MTLCommandEncoder
2594+
* are explicitly documented as single-thread-only. Concurrent
2595+
* access from the main thread (mid-load) and the video thread
2596+
* (mid-frame-end commit) is undefined behaviour. */
2597+
static uintptr_t metal_load_texture_internal(void *video_data, void *data,
2598+
enum texture_filter_type filter_type)
25732599
{
25742600
MetalDriver *md = (__bridge MetalDriver *)video_data;
25752601
struct texture_image *img = (struct texture_image *)data;
@@ -2581,11 +2607,56 @@ static uintptr_t metal_load_texture(void *video_data, void *data,
25812607
return (uintptr_t)(__bridge_retained void *)(t);
25822608
}
25832609

2610+
#ifdef HAVE_THREADS
2611+
/* Wrap function invoked on the video thread via
2612+
* CMD_CUSTOM_COMMAND. Writes the handle to cmd->handle rather
2613+
* than the int return channel to avoid truncation on 64-bit
2614+
* Apple platforms where uintptr_t is 64 bits. */
2615+
static int metal_texture_load_wrap(void *data)
2616+
{
2617+
metal_texture_cmd_t *cmd = (metal_texture_cmd_t*)data;
2618+
cmd->handle = metal_load_texture_internal(
2619+
cmd->video_data, cmd->image, cmd->filter_type);
2620+
return 0;
2621+
}
2622+
#endif
2623+
2624+
static uintptr_t metal_load_texture(void *video_data, void *data,
2625+
bool threaded, enum texture_filter_type filter_type)
2626+
{
2627+
#ifdef HAVE_THREADS
2628+
/* When threaded video is active, dispatch to the video
2629+
* thread so Context.blitCommandBuffer access is serialised
2630+
* with the video thread's frame-end commit. This is only
2631+
* required for mipmapped filters (which encode into the
2632+
* blit buffer), but we dispatch unconditionally for
2633+
* consistency and simplicity. */
2634+
if (threaded)
2635+
{
2636+
metal_texture_cmd_t cmd;
2637+
cmd.video_data = video_data;
2638+
cmd.image = (struct texture_image *)data;
2639+
cmd.filter_type = filter_type;
2640+
cmd.handle = 0;
2641+
video_thread_texture_handle(&cmd, metal_texture_load_wrap);
2642+
return cmd.handle;
2643+
}
2644+
#endif
2645+
2646+
return metal_load_texture_internal(video_data, data, filter_type);
2647+
}
2648+
25842649
static void metal_unload_texture(void *data,
25852650
bool threaded, uintptr_t handle)
25862651
{
25872652
if (!handle)
25882653
return;
2654+
/* Metal command buffers retain their referenced resources,
2655+
* so ARC-releasing the handle here does not free the
2656+
* underlying MTLTexture if the video thread's command
2657+
* buffer is still using it -- the Metal runtime refcounts
2658+
* resources across CPU and GPU. No cross-thread
2659+
* serialisation needed for unload. */
25892660
Texture *t = (__bridge_transfer Texture *)(void *)handle;
25902661
t = nil;
25912662
}

0 commit comments

Comments
 (0)