-
Notifications
You must be signed in to change notification settings - Fork 293
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}; | ||
|
@@ -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); | ||
|
||
self.backend_profile_counters = profile_counters; | ||
|
||
// Update the list of available epochs for use during reftests. | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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), | ||
|
@@ -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); | ||
} | ||
} | ||
|
@@ -1388,7 +1402,7 @@ impl Renderer { | |
} | ||
} | ||
|
||
self.release_external_textures(); | ||
self.unlock_external_images(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you missed the corresponding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
pub fn debug_renderer<'a>(&'a mut self) -> &'a mut DebugRenderer { | ||
|
@@ -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); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 { | ||
|
@@ -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(), | ||
} | ||
} | ||
|
||
|
@@ -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) | ||
} | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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, | ||
|
There was a problem hiding this comment.
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.