Fix disappearing images or panics when hiding and showing a window with wayland

Properly release GL resource when unmapping a window and detect stale
cache indices using a generation count. This fixes the disappearing
images (GL textures would be invalid and need to be re-created) as well
as the panics when an item's cache index ended up being re-used because
some other item was drawn first and allocated that indices. The latter
is fixed by using a generational counter on the cache that's bumped when
clearing, instead of a single "cache_ok" bool.
This commit is contained in:
Simon Hausmann 2021-05-28 09:11:03 +02:00
parent 9503ba69b3
commit d4385280eb
4 changed files with 64 additions and 10 deletions

View file

@ -93,8 +93,51 @@ impl<T> CachedGraphicsData<T> {
/// The RenderingCache, in combination with CachedGraphicsData, allows backends to store data that's either
/// intensive to compute or has bad CPU locality. Backends typically keep a RenderingCache instance and use
/// the item's cached_rendering_data() integer as index in the vec_arena::Arena.
pub type RenderingCache<T> = slab::Slab<CachedGraphicsData<T>>;
pub struct RenderingCache<T> {
slab: slab::Slab<CachedGraphicsData<T>>,
generation: usize,
}
impl<T> Default for RenderingCache<T> {
fn default() -> Self {
Self { slab: Default::default(), generation: 1 }
}
}
impl<T> RenderingCache<T> {
/// Returns the generation of the cache. The generation starts at 1 and is increased
/// whenever the cache is cleared, for example when the GL context is lost.
pub fn generation(&self) -> usize {
self.generation
}
/// Retrieves a mutable reference to the cached graphics data at index.
pub fn get_mut(&mut self, index: usize) -> Option<&mut CachedGraphicsData<T>> {
self.slab.get_mut(index)
}
/// Inserts data into the cache and returns the index for retrieval later.
pub fn insert(&mut self, data: CachedGraphicsData<T>) -> usize {
self.slab.insert(data)
}
/// Retrieves an immutable reference to the cached graphics data at index.
pub fn get(&self, index: usize) -> Option<&CachedGraphicsData<T>> {
self.slab.get(index)
}
/// Removes the cached graphics data at the given index.
pub fn remove(&mut self, index: usize) -> CachedGraphicsData<T> {
self.slab.remove(index)
}
/// Removes all entries from the cache and increases the cache's generation count, so
/// that stale index access can be avoided.
pub fn clear(&mut self) {
self.slab.clear();
self.generation += 1;
}
}
/// FontRequest collects all the developer-configurable properties for fonts, such as family, weight, etc.
/// It is submitted as a request to the platform font system (i.e. CoreText on macOS) and in exchange the
/// backend returns a Box<dyn Font>.

View file

@ -25,8 +25,10 @@ use std::cell::{Cell, RefCell};
pub struct CachedRenderingData {
/// Used and modified by the backend, should be initialized to 0 by the user code
pub(crate) cache_index: Cell<usize>,
/// Set to false initially and when changes happen that require updating the cache
pub(crate) cache_ok: Cell<bool>,
/// Used and modified by the backend, should be initilized to 0 by the user code.
/// The backend compares this generation against the one of the cache to verify
/// the validity of the cache_index field.
pub(crate) cache_generation: Cell<usize>,
}
impl CachedRenderingData {
@ -38,7 +40,7 @@ impl CachedRenderingData {
cache: &mut RenderingCache<T>,
update_fn: impl FnOnce() -> T,
) -> T {
if self.cache_ok.get() {
if self.cache_generation.get() == cache.generation() {
let index = self.cache_index.get();
if let Some(existing_entry) = cache.get_mut(index) {
if let Some(data) =
@ -48,12 +50,9 @@ impl CachedRenderingData {
}
return existing_entry.data.clone();
}
// We may see cache_ok == true but get() on the cache failed with our index. That can
// only happen when the entire cache was destroyed in the meantime (through show and hide),
// in which case we re-allocate.
}
self.cache_index.set(cache.insert(crate::graphics::CachedGraphicsData::new(update_fn)));
self.cache_ok.set(true);
self.cache_generation.set(cache.generation());
cache.get(self.cache_index.get()).unwrap().data.clone()
}
@ -61,10 +60,10 @@ impl CachedRenderingData {
/// exists, i.e. if any data was ever cached. This is typically called by the graphics backend's
/// implementation of the release_item_graphics_cache function.
pub fn release<T>(&self, cache: &mut RenderingCache<T>) {
if self.cache_ok.get() {
if self.cache_generation.get() == cache.generation() {
let index = self.cache_index.get();
cache.remove(index);
self.cache_ok.set(false);
self.cache_generation.set(0);
}
}
}

View file

@ -209,6 +209,8 @@ impl GraphicsWindow {
/// Removes the window from the screen. The window is not destroyed though, it can be show (mapped) again later
/// by calling [`PlatformWindow::map_window`].
fn unmap_window(self: Rc<Self>) {
// Release GL textures and other GPU bound resources.
self.graphics_cache.borrow_mut().clear();
self.map_state.replace(GraphicsWindowBackendState::Unmapped);
/* FIXME:
if let Some(existing_blinker) = self.cursor_blinker.borrow().upgrade() {

View file

@ -112,10 +112,20 @@ impl ItemGraphicsCacheEntry {
struct ItemGraphicsCache(RenderingCache<Option<ItemGraphicsCacheEntry>>);
impl ItemGraphicsCache {
/// Convenience method for releasing an item's cached graphics data.
fn release(&mut self, item_data: &CachedRenderingData) {
item_data.release(&mut self.0);
}
/// Clears the entire graphics cache. This is needed when for example loosing
/// the GL context (when unmapping a window) and destroying the canvas.
fn clear(&mut self) {
self.0.clear();
}
/// Convenience method that will return what's in the item's graphics cache
/// and call update_fn if the cache is outdated and needs refreshing. If
/// update_fn is called, the data is persisted in the cache.
fn ensure_up_to_date(
&mut self,
item_data: &CachedRenderingData,