diff --git a/CHANGELOG.md b/CHANGELOG.md index 5540078b4a..b39ce9ef61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Fixed +- Fixed bug that would sometimes cause taking a screenshot to panic because a buffer was mapped a the wrong time. ## [0.12.0] - 2022-02-19 diff --git a/voxygen/src/render/renderer/drawer.rs b/voxygen/src/render/renderer/drawer.rs index 95581b33cf..26549224d6 100644 --- a/voxygen/src/render/renderer/drawer.rs +++ b/voxygen/src/render/renderer/drawer.rs @@ -553,42 +553,48 @@ impl<'frame> Drop for Drawer<'frame> { // If taking a screenshot and the blit pipeline is available // NOTE: blit pipeline should always be available for now so we don't report an // error if it isn't - if let Some((screenshot, blit)) = self + let download_and_handle_screenshot = self .taking_screenshot .take() .zip(self.borrow.pipelines.blit()) - { - // Image needs to be copied from the screenshot texture to the swapchain texture - let mut render_pass = encoder.scoped_render_pass( - "screenshot blit", - self.borrow.device, - &wgpu::RenderPassDescriptor { - label: Some("Blit screenshot pass"), - color_attachments: &[wgpu::RenderPassColorAttachment { - view: &self.swap_tex.view, - resolve_target: None, - ops: wgpu::Operations { - load: wgpu::LoadOp::Clear(wgpu::Color::TRANSPARENT), - store: true, - }, - }], - depth_stencil_attachment: None, - }, - ); - render_pass.set_pipeline(&blit.pipeline); - render_pass.set_bind_group(0, screenshot.bind_group(), &[]); - render_pass.draw(0..3, 0..1); - drop(render_pass); - // Issues a command to copy from the texture to a buffer and then sends the - // buffer off to another thread to be mapped and processed - screenshot.download_and_handle(&mut encoder); - } + .map(|(screenshot, blit)| { + // Image needs to be copied from the screenshot texture to the swapchain texture + let mut render_pass = encoder.scoped_render_pass( + "screenshot blit", + self.borrow.device, + &wgpu::RenderPassDescriptor { + label: Some("Blit screenshot pass"), + color_attachments: &[wgpu::RenderPassColorAttachment { + view: &self.swap_tex.view, + resolve_target: None, + ops: wgpu::Operations { + load: wgpu::LoadOp::Clear(wgpu::Color::TRANSPARENT), + store: true, + }, + }], + depth_stencil_attachment: None, + }, + ); + render_pass.set_pipeline(&blit.pipeline); + render_pass.set_bind_group(0, screenshot.bind_group(), &[]); + render_pass.draw(0..3, 0..1); + drop(render_pass); + // Issues a command to copy from the texture to a buffer and returns a closure + // that will send the buffer off to another thread to be mapped + // and processed. + screenshot.copy_to_buffer(&mut encoder) + }); let (mut encoder, profiler) = encoder.end_scope(); profiler.resolve_queries(&mut encoder); // It is recommended to only do one submit per frame self.borrow.queue.submit(std::iter::once(encoder.finish())); + // Need to call this after submit so the async mapping doesn't occur before + // copying the screenshot to the buffer which will be mapped. + if let Some(f) = download_and_handle_screenshot { + f(); + } profiler .end_frame() diff --git a/voxygen/src/render/renderer/screenshot.rs b/voxygen/src/render/renderer/screenshot.rs index f8460cf9f9..6a4220e07e 100644 --- a/voxygen/src/render/renderer/screenshot.rs +++ b/voxygen/src/render/renderer/screenshot.rs @@ -88,9 +88,13 @@ impl TakeScreenshot { /// swapchain image pub fn bind_group(&self) -> &wgpu::BindGroup { &self.bind_group.bind_group } - /// NOTE: spawns thread /// Call this after rendering to the screenshot texture - pub fn download_and_handle(self, encoder: &mut wgpu::CommandEncoder) { + /// + /// Issues a command to copy from the texture to a buffer and returns a + /// closure that needs to be called after submitting the encoder + /// to the queue. When called, the closure will spawn a new thread for + /// async mapping of the buffer and downloading of the screenshot. + pub fn copy_to_buffer(self, encoder: &mut wgpu::CommandEncoder) -> impl FnOnce() { // Calculate padded bytes per row let padded_bytes_per_row = padded_bytes_per_row(self.width, self.bytes_per_pixel); // Copy image to a buffer @@ -114,14 +118,17 @@ impl TakeScreenshot { depth_or_array_layers: 1, }, ); - // Send buffer to another thread for async mapping, downloading, and passing to - // the given handler function (which probably saves it to the disk) - std::thread::Builder::new() - .name("screenshot".into()) - .spawn(move || { - self.download_and_handle_internal(); - }) - .expect("Failed to spawn screenshot thread"); + + move || { + // Send buffer to another thread for async mapping, downloading, and passing to + // the given handler function (which probably saves it to the disk) + std::thread::Builder::new() + .name("screenshot".into()) + .spawn(move || { + self.download_and_handle_internal(); + }) + .expect("Failed to spawn screenshot thread"); + } } fn download_and_handle_internal(self) {