Fix panic about multiple mutable borrows with the software renderer

When using repeaters - like in the slide puzzle - and during renderer a component
gets deleted, we call free_graphics_resources and try to free
the dirty rectangle list in the partial renderer cache. At that point the cache is
already mutably borrowed, which causes a panic.

As remedy, apply the mutable borrow more fine grained and not right when calling
render().
This commit is contained in:
Simon Hausmann 2022-06-09 17:55:37 +02:00 committed by Simon Hausmann
parent 100eb305a3
commit 2d2eb70b51
4 changed files with 26 additions and 20 deletions

View file

@ -131,7 +131,7 @@ mod the_backend {
items: &mut dyn Iterator<Item = Pin<i_slint_core::items::ItemRef<'a>>>,
) {
super::LINE_RENDERER.with(|renderer| {
renderer.borrow_mut().free_graphics_resources(items);
renderer.borrow().free_graphics_resources(items);
});
}
@ -346,7 +346,7 @@ mod the_backend {
};
LINE_RENDERER.with(|renderer| {
renderer.borrow_mut().render(
renderer.borrow().render(
runtime_window,
window.initial_dirty_region_for_next_frame.take(),
buffer_provider,

View file

@ -143,7 +143,7 @@ impl PlatformWindow for SimulatorWindow {
items: &mut dyn Iterator<Item = std::pin::Pin<i_slint_core::items::ItemRef<'a>>>,
) {
super::LINE_RENDERER.with(|cache| {
cache.borrow_mut().free_graphics_resources(items);
cache.borrow().free_graphics_resources(items);
});
}
@ -325,7 +325,7 @@ impl WinitWindow for SimulatorWindow {
}
}
super::LINE_RENDERER.with(|renderer| {
renderer.borrow_mut().render(
renderer.borrow().render(
runtime_window,
self.initial_dirty_region_for_next_frame.take(),
BufferProvider {

View file

@ -382,7 +382,7 @@ pub type DirtyRegion = euclid::default::Box2D<Coord>;
/// Put this structure in the renderer to help with partial rendering
pub struct PartialRenderer<'a, T> {
cache: &'a mut PartialRenderingCache,
cache: &'a RefCell<PartialRenderingCache>,
/// The region of the screen which is considered dirty and that should be repainted
pub dirty_region: DirtyRegion,
actual_renderer: T,
@ -391,7 +391,7 @@ pub struct PartialRenderer<'a, T> {
impl<'a, T> PartialRenderer<'a, T> {
/// Create a new PartialRenderer
pub fn new(
cache: &'a mut PartialRenderingCache,
cache: &'a RefCell<PartialRenderingCache>,
initial_dirty_region: DirtyRegion,
actual_renderer: T,
) -> Self {
@ -404,7 +404,9 @@ impl<'a, T> PartialRenderer<'a, T> {
crate::item_tree::visit_items(
component,
crate::item_tree::TraversalOrder::BackToFront,
|_, item, _, offset| match item.cached_rendering_data_offset().get_entry(self.cache)
|_, item, _, offset| match item
.cached_rendering_data_offset()
.get_entry(&mut self.cache.borrow_mut())
{
Some(CachedGraphicsData { data, dependency_tracker: Some(tr) }) => {
if tr.is_dirty() {
@ -435,11 +437,11 @@ impl<'a, T> PartialRenderer<'a, T> {
}
fn do_rendering(
cache: &mut PartialRenderingCache,
cache: &RefCell<PartialRenderingCache>,
rendering_data: &CachedRenderingData,
render_fn: impl FnOnce() -> Rect,
) {
if let Some(entry) = rendering_data.get_entry(cache) {
if let Some(entry) = rendering_data.get_entry(&mut cache.borrow_mut()) {
entry
.dependency_tracker
.get_or_insert_with(|| Box::pin(crate::properties::PropertyTracker::default()))
@ -447,6 +449,7 @@ impl<'a, T> PartialRenderer<'a, T> {
.evaluate(render_fn);
} else {
let cache_entry = crate::graphics::CachedGraphicsData::new(render_fn);
let mut cache = cache.borrow_mut();
rendering_data.cache_index.set(cache.insert(cache_entry));
rendering_data.cache_generation.set(cache.generation());
}
@ -461,7 +464,7 @@ impl<'a, T> PartialRenderer<'a, T> {
macro_rules! forward_rendering_call {
(fn $fn:ident($Ty:ty)) => {
fn $fn(&mut self, obj: Pin<&$Ty>, item_rc: &ItemRc) {
Self::do_rendering(&mut self.cache, &obj.cached_rendering_data, || {
Self::do_rendering(&self.cache, &obj.cached_rendering_data, || {
self.actual_renderer.$fn(obj, item_rc);
type Ty = $Ty;
let width = Ty::FIELD_OFFSETS.width.apply_pin(obj).get_untracked();
@ -477,7 +480,8 @@ macro_rules! forward_rendering_call {
impl<'a, T: ItemRenderer> ItemRenderer for PartialRenderer<'a, T> {
fn filter_item(&mut self, item: Pin<ItemRef>) -> (bool, Rect) {
let rendering_data = item.cached_rendering_data_offset();
let item_geometry = match rendering_data.get_entry(self.cache) {
let mut cache = self.cache.borrow_mut();
let item_geometry = match rendering_data.get_entry(&mut cache) {
Some(CachedGraphicsData { data, dependency_tracker }) => {
dependency_tracker
.get_or_insert_with(|| Box::pin(crate::properties::PropertyTracker::default()))
@ -489,8 +493,8 @@ impl<'a, T: ItemRenderer> ItemRenderer for PartialRenderer<'a, T> {
let cache_entry =
crate::graphics::CachedGraphicsData::new(|| item.as_ref().geometry());
let geom = cache_entry.data;
rendering_data.cache_index.set(self.cache.insert(cache_entry));
rendering_data.cache_generation.set(self.cache.generation());
rendering_data.cache_index.set(cache.insert(cache_entry));
rendering_data.cache_generation.set(cache.generation());
geom
}
};

View file

@ -15,6 +15,7 @@ use crate::textlayout::{FontMetrics as _, TextParagraphLayout};
use crate::{Color, Coord, ImageInner, StaticTextures};
use alloc::rc::Rc;
use alloc::{vec, vec::Vec};
use core::cell::RefCell;
use core::pin::Pin;
pub use draw_functions::TargetPixel;
@ -41,7 +42,7 @@ pub trait LineBufferProvider {
#[derive(Default)]
pub struct LineRenderer {
partial_cache: crate::item_rendering::PartialRenderingCache,
partial_cache: RefCell<crate::item_rendering::PartialRenderingCache>,
}
impl LineRenderer {
@ -56,7 +57,7 @@ impl LineRenderer {
/// (can we call the line_buffer function from different thread?)
/// TODO: should `initial_dirty_region` be set from a different call?
pub fn render(
&mut self,
&self,
window: Rc<crate::window::Window>,
initial_dirty_region: crate::item_rendering::DirtyRegion,
line_buffer: impl LineBufferProvider,
@ -73,18 +74,19 @@ impl LineRenderer {
window_item.background(),
size.cast(),
initial_dirty_region,
&mut self.partial_cache,
&self.partial_cache,
line_buffer,
);
}
}
pub fn free_graphics_resources(
&mut self,
&self,
items: &mut dyn Iterator<Item = Pin<crate::items::ItemRef<'_>>>,
) {
for item in items {
let cache_entry = item.cached_rendering_data_offset().release(&mut self.partial_cache);
let cache_entry =
item.cached_rendering_data_offset().release(&mut self.partial_cache.borrow_mut());
drop(cache_entry);
}
}
@ -95,7 +97,7 @@ fn render_window_frame(
background: Color,
size: PhysicalSize,
initial_dirty_region: crate::item_rendering::DirtyRegion,
cache: &mut PartialRenderingCache,
cache: &RefCell<PartialRenderingCache>,
mut line_buffer: impl LineBufferProvider,
) {
let mut scene =
@ -367,7 +369,7 @@ fn prepare_scene(
size: PhysicalSize,
initial_dirty_region: crate::item_rendering::DirtyRegion,
line_buffer: &mut impl LineBufferProvider,
cache: &mut PartialRenderingCache,
cache: &RefCell<PartialRenderingCache>,
) -> Scene {
let factor = ScaleFactor::new(runtime_window.scale_factor());
let prepare_scene = PrepareScene::new(size, factor, runtime_window.default_font_properties());