Skip to content

Handle external image life time in api.delete_image() call #636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions webrender/src/internal_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ pub enum TextureUpdateOp {
Grow(u32, u32, ImageFormat, TextureFilter, RenderTargetMode),
}

pub type ExternalImageUpdateList = Vec<ExternalImageId>;

pub struct TextureUpdate {
pub id: CacheTextureId,
pub op: TextureUpdateOp,
Expand Down Expand Up @@ -395,9 +397,8 @@ impl RendererFrame {
}

pub enum ResultMsg {
UpdateTextureCache(TextureUpdateList),
RefreshShader(PathBuf),
NewFrame(RendererFrame, BackendProfileCounters),
NewFrame(RendererFrame, TextureUpdateList, ExternalImageUpdateList, BackendProfileCounters),
}

#[repr(u32)]
Expand Down
9 changes: 3 additions & 6 deletions webrender/src/render_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,18 +431,15 @@ impl RenderBackend {
&self.scene.pipeline_auxiliary_lists,
self.device_pixel_ratio);

let pending_update = self.resource_cache.pending_updates();
if !pending_update.updates.is_empty() {
self.result_tx.send(ResultMsg::UpdateTextureCache(pending_update)).unwrap();
}

frame
}

fn publish_frame(&mut self,
frame: RendererFrame,
profile_counters: &mut BackendProfileCounters) {
let msg = ResultMsg::NewFrame(frame, profile_counters.clone());
let pending_update = self.resource_cache.pending_updates();
let pending_external_image_update = self.resource_cache.pending_external_image_updates();
let msg = ResultMsg::NewFrame(frame, pending_update, pending_external_image_update, profile_counters.clone());
self.result_tx.send(msg).unwrap();
profile_counters.reset();
}
Expand Down
45 changes: 34 additions & 11 deletions webrender/src/renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use device::{TextureFilter, VAOId, VertexUsageHint, FileWatcherHandler, TextureT
use euclid::Matrix4D;
use fnv::FnvHasher;
use internal_types::{CacheTextureId, RendererFrame, ResultMsg, TextureUpdateOp};
use internal_types::{TextureUpdateList, PackedVertex, RenderTargetMode};
use internal_types::{ExternalImageUpdateList, TextureUpdateList, PackedVertex, RenderTargetMode};
use internal_types::{ORTHO_NEAR_PLANE, ORTHO_FAR_PLANE, SourceTexture};
use internal_types::{BatchTextures, TextureSampler, GLContextHandleWrapper};
use profiler::{Profiler, BackendProfileCounters};
Expand Down Expand Up @@ -710,10 +710,12 @@ impl Renderer {
// Pull any pending results and return the most recent.
while let Ok(msg) = self.result_rx.try_recv() {
match msg {
ResultMsg::UpdateTextureCache(update_list) => {
self.pending_texture_updates.push(update_list);
}
ResultMsg::NewFrame(frame, profile_counters) => {
ResultMsg::NewFrame(frame, texture_update_list, external_image_update_list, profile_counters) => {
self.pending_texture_updates.push(texture_update_list);

// When a new frame is ready, we could start to update all pending external image requests here.
self.release_external_images(external_image_update_list);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only release the external images used in previous frames when a new frame is ready.


self.backend_profile_counters = profile_counters;

// Update the list of available epochs for use during reftests.
Expand Down Expand Up @@ -1262,7 +1264,7 @@ impl Renderer {
let props = &deferred_resolve.image_properties;
let external_id = props.external_id
.expect("BUG: Deferred resolves must be external images!");
let image = handler.get(external_id);
let image = handler.lock(external_id);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turn to use lock() here.


let texture_id = match image.source {
ExternalImageSource::NativeTexture(texture_id) => TextureId::new(texture_id),
Expand All @@ -1277,13 +1279,25 @@ impl Renderer {
}
}

fn release_external_textures(&mut self) {
fn unlock_external_images(&mut self) {
if !self.external_images.is_empty() {
let handler = self.external_image_handler
.as_mut()
.expect("Found external image, but no handler set!");

for (external_id, _) in self.external_images.drain() {
handler.unlock(external_id);
}
}
}

fn release_external_images(&mut self, mut pending_external_image_updates: ExternalImageUpdateList) {
if !pending_external_image_updates.is_empty() {
let handler = self.external_image_handler
.as_mut()
.expect("found external image updates, but no handler set!");

for external_id in pending_external_image_updates.drain(..) {
handler.release(external_id);
}
}
Expand Down Expand Up @@ -1388,7 +1402,7 @@ impl Renderer {
}
}

self.release_external_textures();
self.unlock_external_images();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed the corresponding lock() call somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

pub fn debug_renderer<'a>(&'a mut self) -> &'a mut DebugRenderer {
Expand Down Expand Up @@ -1427,10 +1441,19 @@ pub struct ExternalImage {
pub source: ExternalImageSource,
}

/// Interface that an application can implement
/// to support providing external image buffers.
/// The interfaces that an application can implement to support providing
/// external image buffers.
/// When the the application passes an external image to WR, it should kepp that
/// external image life time untile the release() call.
pub trait ExternalImageHandler {
fn get(&mut self, key: ExternalImageId) -> ExternalImage;
/// Lock the external image. Then, WR could start to read the image content.
/// The WR client should not change the image content until the unlock()
/// call.
fn lock(&mut self, key: ExternalImageId) -> ExternalImage;
/// Unlock the external image. The WR should not read the image content
/// after this call.
fn unlock(&mut self, key: ExternalImageId);
/// Tell the WR client that it could start to release this external image.
fn release(&mut self, key: ExternalImageId);
}

Expand Down
33 changes: 31 additions & 2 deletions webrender/src/resource_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ use app_units::Au;
use device::TextureFilter;
use fnv::FnvHasher;
use frame::FrameId;
use internal_types::{FontTemplate, SourceTexture, TextureUpdateList};
use internal_types::{ExternalImageUpdateList, FontTemplate, SourceTexture, TextureUpdateList};
use platform::font::{FontContext, RasterizedGlyph};
use std::cell::RefCell;
use std::collections::{HashMap, HashSet};
use std::collections::hash_map::Entry::{self, Occupied, Vacant};
use std::fmt::Debug;
use std::hash::BuildHasherDefault;
use std::hash::Hash;
use std::mem;
use std::sync::{Arc, Barrier};
use std::sync::mpsc::{channel, Receiver, Sender};
use std::thread;
Expand Down Expand Up @@ -207,6 +208,7 @@ pub struct ResourceCache {
pending_image_requests: Vec<ImageRequest>,
glyph_cache_tx: Sender<GlyphCacheMsg>,
glyph_cache_result_queue: Receiver<GlyphCacheResultMsg>,
pending_external_image_update_list: ExternalImageUpdateList,
}

impl ResourceCache {
Expand All @@ -228,6 +230,7 @@ impl ResourceCache {
pending_image_requests: Vec::new(),
glyph_cache_tx: glyph_cache_tx,
glyph_cache_result_queue: glyph_cache_result_queue,
pending_external_image_update_list: ExternalImageUpdateList::new(),
}
}

Expand Down Expand Up @@ -272,6 +275,14 @@ impl ResourceCache {
bytes: Vec<u8>) {
let next_epoch = match self.image_templates.get(&image_key) {
Some(image) => {
// This image should not be an external image.
match image.data {
ImageData::External(id) => {
panic!("Update an external image with buffer, id={} image_key={:?}", id.0, image_key);
},
_ => {},
}

let Epoch(current_epoch) = image.epoch;
Epoch(current_epoch + 1)
}
Expand All @@ -294,7 +305,21 @@ impl ResourceCache {
}

pub fn delete_image_template(&mut self, image_key: ImageKey) {
self.image_templates.remove(&image_key);
let value = self.image_templates.remove(&image_key);

// If the key is associated to an external image, pass the external id to renderer for cleanup.
if let Some(image) = value {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's print a error if image is not Some(..) here as well, since that indicates a bug (such as double deletion of an image key).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

match image.data {
ImageData::External(id) => {
self.pending_external_image_update_list.push(id);
},
_ => {},
}

return;
}

println!("Delete the non-exist key:{:?}", image_key);
}

pub fn add_webgl_texture(&mut self, id: WebGLContextId, texture_id: SourceTexture, size: DeviceIntSize) {
Expand Down Expand Up @@ -343,6 +368,10 @@ impl ResourceCache {
self.texture_cache.pending_updates()
}

pub fn pending_external_image_updates(&mut self) -> ExternalImageUpdateList {
mem::replace(&mut self.pending_external_image_update_list, ExternalImageUpdateList::new())
}

pub fn get_glyphs<F>(&self,
font_key: FontKey,
size: Au,
Expand Down