From 2dae2a01d2cf9d1d6453dc303fb4fe0bc9c5ab55 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 10 Dec 2021 17:26:58 +0100 Subject: [PATCH] Internal cleanup in the GL renderer Separate the image cache from the texture cache. The latter needs additional data in the key. The two have less in common that it seemed. This also fixes a small issue with `CachedImage::upload_to_gpu` that for SVGs would return a decoded (rendered) SVG image, instead of a texture. That was harmless because we always call `ensure_uploaded_to_gpu` later, but now we assert that the texture cache only holds textures, because clear() (former remove_textures()) just clears everything. --- .../rendering_backends/gl/glwindow.rs | 6 +- .../rendering_backends/gl/images.rs | 88 ++++++++++++++----- sixtyfps_runtime/rendering_backends/gl/lib.rs | 2 +- 3 files changed, 71 insertions(+), 25 deletions(-) diff --git a/sixtyfps_runtime/rendering_backends/gl/glwindow.rs b/sixtyfps_runtime/rendering_backends/gl/glwindow.rs index 980ada786..bba4d33d8 100644 --- a/sixtyfps_runtime/rendering_backends/gl/glwindow.rs +++ b/sixtyfps_runtime/rendering_backends/gl/glwindow.rs @@ -15,7 +15,7 @@ use core::cell::{Cell, RefCell}; use core::pin::Pin; use std::rc::{Rc, Weak}; -use super::{ImageCache, ItemGraphicsCache}; +use super::{ItemGraphicsCache, TextureCache}; use crate::event_loop::WinitWindow; use const_field_offset::FieldOffsets; use corelib::component::ComponentRc; @@ -40,7 +40,7 @@ pub struct GLWindow { pub(crate) graphics_cache: RefCell, // This cache only contains textures. The cache for decoded CPU side images is in crate::IMAGE_CACHE. - pub(crate) texture_cache: RefCell, + pub(crate) texture_cache: RefCell, #[cfg(target_arch = "wasm32")] canvas_id: String, @@ -412,7 +412,7 @@ impl PlatformWindow for GLWindow { // Release GL textures and other GPU bound resources. self.with_current_context(|| { self.graphics_cache.borrow_mut().clear(); - self.texture_cache.borrow_mut().remove_textures(); + self.texture_cache.borrow_mut().clear(); }); self.map_state.replace(GraphicsWindowBackendState::Unmapped); diff --git a/sixtyfps_runtime/rendering_backends/gl/images.rs b/sixtyfps_runtime/rendering_backends/gl/images.rs index ad22f3bcd..e00e446ed 100644 --- a/sixtyfps_runtime/rendering_backends/gl/images.rs +++ b/sixtyfps_runtime/rendering_backends/gl/images.rs @@ -325,7 +325,11 @@ impl CachedImage { } #[cfg(feature = "svg")] ImageData::Svg(svg_tree) => match super::svg::render(svg_tree, target_size) { - Ok(rendered_svg_image) => Some(Self::new_on_cpu(rendered_svg_image)), + Ok(rendered_svg_image) => Self::new_on_cpu(rendered_svg_image).upload_to_gpu( + current_renderer, + target_size, + scaling, + ), Err(err) => { eprintln!("Error rendering SVG: {}", err); None @@ -425,34 +429,24 @@ impl CachedImage { } #[derive(PartialEq, Eq, Hash, Debug, derive_more::From)] -enum CachedImageSourceKey { +pub enum ImageCacheKey { Path(String), EmbeddedData(by_address::ByAddress<&'static [u8]>), } -#[derive(PartialEq, Eq, Hash, Debug)] -pub struct ImageCacheKey { - source_key: CachedImageSourceKey, - gpu_image_flags: ImageRendering, -} - impl ImageCacheKey { - pub fn new(resource: &ImageInner, gpu_image_flags: Option) -> Option { + pub fn new(resource: &ImageInner) -> Option { Some(match resource { ImageInner::None => return None, ImageInner::AbsoluteFilePath(path) => { if path.is_empty() { return None; } - Self { - source_key: path.to_string().into(), - gpu_image_flags: gpu_image_flags.unwrap_or_default(), - } + path.to_string().into() + } + ImageInner::EmbeddedData { data, format: _ } => { + by_address::ByAddress(data.as_slice()).into() } - ImageInner::EmbeddedData { data, format: _ } => Self { - source_key: by_address::ByAddress(data.as_slice()).into(), - gpu_image_flags: gpu_image_flags.unwrap_or_default(), - }, ImageInner::EmbeddedImage { .. } => return None, ImageInner::StaticTextures { .. } => return None, }) @@ -467,7 +461,7 @@ pub(crate) struct ImageCache(HashMap>); impl ImageCache { // Look up the given image cache key in the image cache and upgrade the weak reference to a strong one if found, // otherwise a new image is created/loaded from the given callback. - pub(crate) fn lookup_image_in_cache_or_create( + pub fn lookup_image_in_cache_or_create( &mut self, cache_key: ImageCacheKey, image_create_fn: impl Fn() -> Option>, @@ -486,7 +480,7 @@ impl ImageCache { // Try to load the image the given resource points to pub(crate) fn load_image_resource(&mut self, resource: &ImageInner) -> Option> { - ImageCacheKey::new(resource, None) + ImageCacheKey::new(resource) .and_then(|cache_key| { self.lookup_image_in_cache_or_create(cache_key, || { CachedImage::new_from_resource(resource).map(Rc::new) @@ -494,6 +488,58 @@ impl ImageCache { }) .or_else(|| CachedImage::new_from_resource(resource).map(Rc::new)) } +} + +#[derive(PartialEq, Eq, Hash, Debug)] +pub struct TextureCacheKey { + source_key: ImageCacheKey, + gpu_image_flags: ImageRendering, +} + +impl TextureCacheKey { + pub fn new(resource: &ImageInner, gpu_image_flags: ImageRendering) -> Option { + Some(match resource { + ImageInner::None => return None, + ImageInner::AbsoluteFilePath(path) => { + if path.is_empty() { + return None; + } + Self { source_key: path.to_string().into(), gpu_image_flags } + } + ImageInner::EmbeddedData { data, format: _ } => { + Self { source_key: by_address::ByAddress(data.as_slice()).into(), gpu_image_flags } + } + ImageInner::EmbeddedImage { .. } => return None, + ImageInner::StaticTextures { .. } => return None, + }) + } +} + +// Cache used to avoid repeatedly decoding images from disk. Entries with a count +// of 1 are drained after flushing the renderer commands to the screen. +#[derive(Default)] +pub struct TextureCache(HashMap>); + +impl TextureCache { + // Look up the given image cache key in the image cache and upgrade the weak reference to a strong one if found, + // otherwise a new image is created/loaded from the given callback. + pub(crate) fn lookup_image_in_cache_or_create( + &mut self, + cache_key: TextureCacheKey, + image_create_fn: impl Fn() -> Option>, + ) -> Option> { + Some(match self.0.entry(cache_key) { + std::collections::hash_map::Entry::Occupied(existing_entry) => { + existing_entry.get().clone() + } + std::collections::hash_map::Entry::Vacant(vacant_entry) => { + let new_image = image_create_fn()?; + debug_assert!(new_image.is_on_gpu()); + vacant_entry.insert(new_image.clone()); + new_image + } + }) + } pub(crate) fn drain(&mut self) { self.0.retain(|_, cached_image| { @@ -509,8 +555,8 @@ impl ImageCache { }); } - pub(crate) fn remove_textures(&mut self) { - self.0.retain(|_, cached_image| !cached_image.is_on_gpu()); + pub(crate) fn clear(&mut self) { + self.0.clear(); } } diff --git a/sixtyfps_runtime/rendering_backends/gl/lib.rs b/sixtyfps_runtime/rendering_backends/gl/lib.rs index 3c0f29572..271844e88 100644 --- a/sixtyfps_runtime/rendering_backends/gl/lib.rs +++ b/sixtyfps_runtime/rendering_backends/gl/lib.rs @@ -902,7 +902,7 @@ impl GLItemRenderer { let image = source_property.get(); let image_inner = (&image).into(); - ImageCacheKey::new(image_inner, Some(image_rendering)) + TextureCacheKey::new(image_inner, image_rendering) .and_then(|cache_key| { self.graphics_window .texture_cache