Use Mutex instead of a RefCell to acquire/release instance buffers (#7291)

This fixes a panic happening when releasing an instance buffer.
Releasing the buffer happens on a different thread but the borrow
checker was not catching it because the metal buffer completion handler
API doesn't have a `Send` marker on it.

Release Notes:

- N/A
This commit is contained in:
Antonio Scandurra 2024-02-02 09:53:07 -07:00 committed by GitHub
parent 074acacdf7
commit 659423a4a1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -15,14 +15,9 @@ use foreign_types::ForeignType;
use media::core_video::CVMetalTextureCache; use media::core_video::CVMetalTextureCache;
use metal::{CommandQueue, MTLPixelFormat, MTLResourceOptions, NSRange}; use metal::{CommandQueue, MTLPixelFormat, MTLResourceOptions, NSRange};
use objc::{self, msg_send, sel, sel_impl}; use objc::{self, msg_send, sel, sel_impl};
use parking_lot::Mutex;
use smallvec::SmallVec; use smallvec::SmallVec;
use std::{ use std::{cell::Cell, ffi::c_void, mem, ptr, sync::Arc};
cell::{Cell, RefCell},
ffi::c_void,
mem, ptr,
rc::Rc,
sync::Arc,
};
#[cfg(not(feature = "runtime_shaders"))] #[cfg(not(feature = "runtime_shaders"))]
const SHADERS_METALLIB: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/shaders.metallib")); const SHADERS_METALLIB: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/shaders.metallib"));
@ -44,7 +39,8 @@ pub(crate) struct MetalRenderer {
polychrome_sprites_pipeline_state: metal::RenderPipelineState, polychrome_sprites_pipeline_state: metal::RenderPipelineState,
surfaces_pipeline_state: metal::RenderPipelineState, surfaces_pipeline_state: metal::RenderPipelineState,
unit_vertices: metal::Buffer, unit_vertices: metal::Buffer,
instance_buffers: Rc<RefCell<Vec<metal::Buffer>>>, #[allow(clippy::arc_with_non_send_sync)]
instance_buffers: Arc<Mutex<Vec<metal::Buffer>>>,
sprite_atlas: Arc<MetalAtlas>, sprite_atlas: Arc<MetalAtlas>,
core_video_texture_cache: CVMetalTextureCache, core_video_texture_cache: CVMetalTextureCache,
} }
@ -185,7 +181,7 @@ impl MetalRenderer {
polychrome_sprites_pipeline_state, polychrome_sprites_pipeline_state,
surfaces_pipeline_state, surfaces_pipeline_state,
unit_vertices, unit_vertices,
instance_buffers: Rc::default(), instance_buffers: Arc::default(),
sprite_atlas, sprite_atlas,
core_video_texture_cache, core_video_texture_cache,
} }
@ -215,7 +211,7 @@ impl MetalRenderer {
); );
return; return;
}; };
let mut instance_buffer = self.instance_buffers.borrow_mut().pop().unwrap_or_else(|| { let mut instance_buffer = self.instance_buffers.lock().pop().unwrap_or_else(|| {
self.device.new_buffer( self.device.new_buffer(
INSTANCE_BUFFER_SIZE as u64, INSTANCE_BUFFER_SIZE as u64,
MTLResourceOptions::StorageModeManaged, MTLResourceOptions::StorageModeManaged,
@ -341,7 +337,7 @@ impl MetalRenderer {
let instance_buffer = Cell::new(Some(instance_buffer)); let instance_buffer = Cell::new(Some(instance_buffer));
let block = ConcreteBlock::new(move |_| { let block = ConcreteBlock::new(move |_| {
if let Some(instance_buffer) = instance_buffer.take() { if let Some(instance_buffer) = instance_buffer.take() {
instance_buffers.borrow_mut().push(instance_buffer); instance_buffers.lock().push(instance_buffer);
} }
}); });
let block = block.copy(); let block = block.copy();