From 1c436d0df5092f9236346756cb827acda8a68e7f Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Tue, 10 Sep 2024 16:12:31 +0100 Subject: [PATCH] More resource handling --- vello_pacing/src/lib.rs | 87 +++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/vello_pacing/src/lib.rs b/vello_pacing/src/lib.rs index 326c6be44..fd9ece91a 100644 --- a/vello_pacing/src/lib.rs +++ b/vello_pacing/src/lib.rs @@ -162,8 +162,9 @@ pub struct VelloPacing { /// The refresh rate reported "by the system". refresh_rate: u64, mapped_unused_download_buffers: Vec<(Buffer, Arc)>, + mapped_unused_download_buffers_scratch: Vec<(Buffer, Arc)>, // TODO: Smallvec? - unmapped_download_buffers: Vec<(Buffer, Arc)>, + free_download_buffers: Vec<(Buffer, Arc)>, /// Details about the previous frame, which has already been presented. presenting_frame: Option, /// Details about the frame whose work has been submitted to the. @@ -276,28 +277,17 @@ impl VelloPacing { // This is accurate since we never make a `Weak` for the `work_complete` buffers // If that became untrue, the only risk is that the buffer and value would be dropped instead of reused. if Arc::strong_count(&frame.work_complete) == 1 { - self.handle_completed_unmapped(frame.download_map_buffer, frame.work_complete); + Self::handle_completed_mapping_finished( + &mut self.free_download_buffers, + frame.download_map_buffer, + frame.work_complete, + ); } else { self.mapped_unused_download_buffers .push((frame.download_map_buffer, frame.work_complete)); } } - fn handle_completed_unmapped(&mut self, buffer: Buffer, mut work_complete: Arc) { - if let Some(value) = Arc::get_mut(&mut work_complete) { - let value = value.get_mut(); - if !*value { - tracing::error!("Tried to unmap buffer which was never assigned for mapping?"); - } else { - buffer.unmap(); - } - *value = false; - if self.unmapped_download_buffers.len() < 4 { - self.unmapped_download_buffers.push((buffer, work_complete)); - } - } - } - fn paint_frame(&mut self, scene: Scene, to: SurfaceTexture) { // TODO: It could happen that frames get sent not in response to our handling code (e.g. as an immediate frame) // Do we just always want to abandon the current working frame? @@ -329,16 +319,18 @@ impl VelloPacing { .expect("We know this value is present"); { - let value = presenting.download_map_buffer.slice(..).get_mapped_range(); - eprintln!("Downloaded: {value:?}"); - presenting.download_map_buffer.unmap(); - // Will be calculated from `value`, as part of https://github.com/linebender/vello/pull/606 + let map_result = + presenting.download_map_buffer.slice(..).get_mapped_range(); + eprintln!("Downloaded: {map_result:?}"); + // Will be calculated from `map_result`, as part of https://github.com/linebender/vello/pull/606 failed = false; if failed { // Perform reallocation. Will be part of https://github.com/linebender/vello/pull/606. } // The time which we should not present before. - // If `value` indicates we really badly overflowed, then this will be later than otherwise expected. + // If `map_result` indicates we really badly overflowed the available time, + // then this will be later than otherwise expected, to avoid a stutter. + // See https://developer.android.com/games/sdk/frame-pacing desired_present_time = None; self.stats.push_back(( presenting.id, @@ -353,15 +345,18 @@ impl VelloPacing { }, )); } - self.unmapped_download_buffers - .push((presenting.download_map_buffer, presenting.work_complete)); + Self::handle_completed_mapping_finished( + &mut self.free_download_buffers, + presenting.download_map_buffer, + presenting.work_complete, + ); } else { unreachable!( "Buffer mapping/work complete callback dropped without being called." ) } } else { - // Probably nothing to do? + // The presenting frame's work isn't complete. Probably nothing to do? } } if self.presenting_frame.is_none() { @@ -414,12 +409,52 @@ impl VelloPacing { self.presenting_frame = Some(working_frame); } } + assert!(self.mapped_unused_download_buffers_scratch.is_empty()); + for (buffer, work_complete) in self.mapped_unused_download_buffers.drain(..) { + // We know that the `work_complete` will not change again. + if Arc::strong_count(&work_complete) == 1 { + Self::handle_completed_mapping_finished( + &mut self.free_download_buffers, + buffer, + work_complete, + ); + } else { + self.mapped_unused_download_buffers_scratch + .push((buffer, work_complete)); + } + } + std::mem::swap( + &mut self.mapped_unused_download_buffers_scratch, + &mut self.mapped_unused_download_buffers, + ); } - // fn presented_frame_status(&self) -> FrameStatus {} fn frame_in_flight(&self) -> bool { self.gpu_working_frame.is_some() } + + /// Handle a completed buffer mapping for reuse. + /// `buffer` should be finished with, but mapped. + /// + /// Associated function for borrow checker purposes + fn handle_completed_mapping_finished( + unmapped_download_buffers: &mut Vec<(Buffer, Arc)>, + buffer: Buffer, + mut work_complete: Arc, + ) { + if let Some(value) = Arc::get_mut(&mut work_complete) { + let value = value.get_mut(); + if !*value { + tracing::error!("Tried to unmap buffer which was never assigned for mapping?"); + } else { + buffer.unmap(); + } + *value = false; + if unmapped_download_buffers.len() < 4 { + unmapped_download_buffers.push((buffer, work_complete)); + } + } + } } #[derive(Copy, Clone, Debug, PartialEq, Eq)]