Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-03 12:40:06 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #17606 - asajeffrey:canvas-delay-deleting-image-keys, r=jdm

Browse source 
Don't delete webrender image keys immediately.

<!-- Please describe your changes on the following line: -->

We currently delete webrender image id's immediately on resizing a canvas, which can cause panics. This PR delays deleting the image id until an epoch has passed.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #17534
- [X] These changes do not require tests because the intermittent failure is already triggered by the css-paint-api tests.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17606)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-07-17 13:25:37 -07:00 • committed by GitHub
commit 7f1278a329
2 changed files with 73 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

20
components/canvas/canvas_paint_thread.rs
View file

@ -59,6 +59,10 @@ pub struct CanvasPaintThread<'a> {
saved_states: Vec<CanvasPaintState<'a>>,
webrender_api: webrender_api::RenderApi,
image_key: Option<webrender_api::ImageKey>,
/// An old webrender image key that can be deleted when the next epoch ends.
old_image_key: Option<webrender_api::ImageKey>,
/// An old webrender image key that can be deleted when the current epoch ends.
very_old_image_key: Option<webrender_api::ImageKey>,
}
#[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);
}
}

70
components/canvas/webgl_paint_thread.rs
View file

@ -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<webrender_api::ImageKey>),
Readback {
context: GLContextWrapper,
webrender_api: webrender_api::RenderApi,
image_key: Option<webrender_api::ImageKey>,
/// An old webrender image key that can be deleted when the next epoch ends.
old_image_key: Option<webrender_api::ImageKey>,
/// An old webrender image key that can be deleted when the current epoch ends.
very_old_image_key: Option<webrender_api::ImageKey>,
},
}
pub struct WebGLPaintThread {
@ -119,7 +128,13 @@ fn create_readback_painter(size: Size2D<i32>,
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<CanvasData>) {
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<i32>) -> 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);
}
}
}