From 3002c45184bfe9b5af907b44478aeb13d9ad1933 Mon Sep 17 00:00:00 2001 From: Alan Jeffrey Date: Wed, 5 Jul 2017 10:45:49 -0500 Subject: [PATCH] Don't delete webrender image keys immediately. --- components/canvas/canvas_paint_thread.rs | 20 ++++++- components/canvas/webgl_paint_thread.rs | 70 +++++++++++++++++++----- 2 files changed, 73 insertions(+), 17 deletions(-) diff --git a/components/canvas/canvas_paint_thread.rs b/components/canvas/canvas_paint_thread.rs index e2937b097965..c285f3aa63d1 100644 --- a/components/canvas/canvas_paint_thread.rs +++ b/components/canvas/canvas_paint_thread.rs @@ -59,6 +59,10 @@ pub struct CanvasPaintThread<'a> { saved_states: Vec>, webrender_api: webrender_api::RenderApi, image_key: Option, + /// An old webrender image key that can be deleted when the next epoch ends. + old_image_key: Option, + /// An old webrender image key that can be deleted when the current epoch ends. + very_old_image_key: Option, } #[derive(Clone)] @@ -111,6 +115,8 @@ impl<'a> CanvasPaintThread<'a> { saved_states: vec![], webrender_api: webrender_api, image_key: None, + old_image_key: None, + very_old_image_key: None, } } @@ -548,7 +554,10 @@ impl<'a> CanvasPaintThread<'a> { self.saved_states.clear(); // Webrender doesn't let images change size, so we clear the webrender image key. if let Some(image_key) = self.image_key.take() { - self.webrender_api.delete_image(image_key); + // If this executes, then we are in a new epoch since we last recreated the canvas, + // so `old_image_key` must be `None`. + debug_assert!(self.old_image_key.is_none()); + self.old_image_key = Some(image_key); } } @@ -588,6 +597,10 @@ impl<'a> CanvasPaintThread<'a> { } } + if let Some(image_key) = mem::replace(&mut self.very_old_image_key, self.old_image_key.take()) { + self.webrender_api.delete_image(image_key); + } + let data = CanvasImageData { image_key: self.image_key.unwrap(), }; @@ -745,7 +758,10 @@ impl<'a> CanvasPaintThread<'a> { impl<'a> Drop for CanvasPaintThread<'a> { fn drop(&mut self) { - if let Some(image_key) = self.image_key { + if let Some(image_key) = self.old_image_key.take() { + self.webrender_api.delete_image(image_key); + } + if let Some(image_key) = self.very_old_image_key.take() { self.webrender_api.delete_image(image_key); } } diff --git a/components/canvas/webgl_paint_thread.rs b/components/canvas/webgl_paint_thread.rs index e0fd8b7b8486..abd6e4d00a00 100644 --- a/components/canvas/webgl_paint_thread.rs +++ b/components/canvas/webgl_paint_thread.rs @@ -11,6 +11,7 @@ use offscreen_gl_context::{ColorAttachmentType, GLContext, GLLimits}; use offscreen_gl_context::{GLContextAttributes, NativeGLContext, OSMesaContext}; use servo_config::opts; use std::borrow::ToOwned; +use std::mem; use std::sync::Arc; use std::sync::mpsc::channel; use std::thread; @@ -102,7 +103,15 @@ impl GLContextWrapper { enum WebGLPaintTaskData { WebRender(webrender_api::RenderApi, webrender_api::WebGLContextId), - Readback(GLContextWrapper, webrender_api::RenderApi, Option), + Readback { + context: GLContextWrapper, + webrender_api: webrender_api::RenderApi, + image_key: Option, + /// An old webrender image key that can be deleted when the next epoch ends. + old_image_key: Option, + /// An old webrender image key that can be deleted when the current epoch ends. + very_old_image_key: Option, + }, } pub struct WebGLPaintThread { @@ -119,7 +128,13 @@ fn create_readback_painter(size: Size2D, let limits = context.get_limits(); let painter = WebGLPaintThread { size: size, - data: WebGLPaintTaskData::Readback(context, webrender_api, None) + data: WebGLPaintTaskData::Readback { + context: context, + webrender_api: webrender_api, + image_key: None, + old_image_key: None, + very_old_image_key: None, + }, }; Ok((painter, limits)) @@ -154,8 +169,8 @@ impl WebGLPaintThread { WebGLPaintTaskData::WebRender(ref api, id) => { api.send_webgl_command(id, message); } - WebGLPaintTaskData::Readback(ref ctx, _, _) => { - ctx.apply_command(message); + WebGLPaintTaskData::Readback { ref context, .. } => { + context.apply_command(message); } } } @@ -165,7 +180,7 @@ impl WebGLPaintThread { WebGLPaintTaskData::WebRender(ref api, id) => { api.send_vr_compositor_command(id, message); } - WebGLPaintTaskData::Readback(..) => { + WebGLPaintTaskData::Readback { .. } => { error!("Webrender is required for WebVR implementation"); } } @@ -229,14 +244,20 @@ impl WebGLPaintThread { fn send_data(&mut self, chan: IpcSender) { match self.data { - WebGLPaintTaskData::Readback(ref ctx, ref webrender_api, ref mut image_key) => { + WebGLPaintTaskData::Readback { + ref context, + ref webrender_api, + ref mut image_key, + ref mut old_image_key, + ref mut very_old_image_key, + } => { let width = self.size.width as usize; let height = self.size.height as usize; - let mut pixels = ctx.gl().read_pixels(0, 0, - self.size.width as gl::GLsizei, - self.size.height as gl::GLsizei, - gl::RGBA, gl::UNSIGNED_BYTE); + let mut pixels = context.gl().read_pixels(0, 0, + self.size.width as gl::GLsizei, + self.size.height as gl::GLsizei, + gl::RGBA, gl::UNSIGNED_BYTE); // flip image vertically (texture is upside down) let orig_pixels = pixels.clone(); let stride = width * 4; @@ -276,6 +297,10 @@ impl WebGLPaintThread { } } + if let Some(image_key) = mem::replace(very_old_image_key, old_image_key.take()) { + webrender_api.delete_image(image_key); + } + let image_data = CanvasImageData { image_key: image_key.unwrap(), }; @@ -291,7 +316,7 @@ impl WebGLPaintThread { #[allow(unsafe_code)] fn recreate(&mut self, size: Size2D) -> Result<(), &'static str> { match self.data { - WebGLPaintTaskData::Readback(ref mut context, ref webrender_api, ref mut image_key) => { + WebGLPaintTaskData::Readback { ref mut context, ref mut image_key, ref mut old_image_key, .. } => { if size.width > self.size.width || size.height > self.size.height { self.size = context.resize(size)?; @@ -301,7 +326,10 @@ impl WebGLPaintThread { } // Webrender doesn't let images change size, so we clear the webrender image key. if let Some(image_key) = image_key.take() { - webrender_api.delete_image(image_key); + // If this executes, then we are in a new epoch since we last recreated the canvas, + // so `old_image_key` must be `None`. + debug_assert!(old_image_key.is_none()); + *old_image_key = Some(image_key); } } WebGLPaintTaskData::WebRender(ref api, id) => { @@ -314,7 +342,7 @@ impl WebGLPaintThread { } fn init(&mut self) { - if let WebGLPaintTaskData::Readback(ref context, _, _) = self.data { + if let WebGLPaintTaskData::Readback { ref context, .. } = self.data { context.make_current(); } } @@ -322,9 +350,21 @@ impl WebGLPaintThread { impl Drop for WebGLPaintThread { fn drop(&mut self) { - if let WebGLPaintTaskData::Readback(_, ref mut wr, image_key) = self.data { + if let WebGLPaintTaskData::Readback { + ref mut webrender_api, + image_key, + old_image_key, + very_old_image_key, + .. + } = self.data { if let Some(image_key) = image_key { - wr.delete_image(image_key); + webrender_api.delete_image(image_key); + } + if let Some(image_key) = old_image_key { + webrender_api.delete_image(image_key); + } + if let Some(image_key) = very_old_image_key { + webrender_api.delete_image(image_key); } } }