Fix panic when trying to access layout cache of destroyed items

This can be reproduced by deleting the last item of the printer queue in the
printer demo.
It is a regression showing up because we now emit the MouseExit event after
the mouse grab as released.
The problem is that we upgrade the weak item, and call geometry() on it.
Calling geometry will re-evaluate the layout cache which will re-evaluate
the model which will result in the component being removed and the cache
entry having less item than expected.

It is ok to simply return 0. for these layout location since the item will
disapear anyway.
This commit is contained in:
Olivier Goffart 2021-09-08 09:36:38 +02:00 committed by Olivier Goffart
parent e8c1fccbbc
commit fafcbfde2c
5 changed files with 99 additions and 4 deletions

View file

@ -403,6 +403,12 @@ inline bool operator!=(const LayoutInfo &a, const LayoutInfo &b)
} }
namespace private_api { namespace private_api {
/// Access the layout cache of an item within a repeater
inline float layout_cache_access(const SharedVector<float> &cache, int offset, int repeater_index) {
size_t idx = size_t(cache[offset]) + repeater_index * 2;
return idx < cache.size() ? cache[idx] : 0;
}
// models // models
struct AbstractRepeaterView struct AbstractRepeaterView
{ {

View file

@ -1854,7 +1854,7 @@ fn compile_expression(
Expression::LayoutCacheAccess { layout_cache_prop, index, repeater_index } => { Expression::LayoutCacheAccess { layout_cache_prop, index, repeater_index } => {
let cache = access_named_reference(layout_cache_prop, component, "self"); let cache = access_named_reference(layout_cache_prop, component, "self");
if let Some(ri) = repeater_index { if let Some(ri) = repeater_index {
format!("[&](auto cache) {{ return cache[int(cache[{}]) + {} * 2]; }} ({}.get())", index, compile_expression(ri, component), cache) format!("sixtyfps::private_api::layout_cache_access({}.get(), {}, {})", cache, index, compile_expression(ri, component))
} else { } else {
format!("{}.get()[{}]", cache, index) format!("{}.get()[{}]", cache, index)
} }

View file

@ -1573,7 +1573,7 @@ fn compile_expression(expr: &Expression, component: &Rc<Component>) -> TokenStre
let offset = compile_expression(ri, component); let offset = compile_expression(ri, component);
quote!({ quote!({
let cache = #cache.get(); let cache = #cache.get();
cache[(cache[#index] as usize) + #offset as usize * 2] *cache.get((cache[#index] as usize) + #offset as usize * 2).unwrap_or(&0.)
}) })
} else { } else {
quote!(#cache.get()[#index]) quote!(#cache.get()[#index])

View file

@ -450,6 +450,13 @@ pub struct BoxLayoutCellData {
/// Solve a BoxLayout /// Solve a BoxLayout
pub fn solve_box_layout(data: &BoxLayoutData, repeater_indexes: Slice<u32>) -> SharedVector<Coord> { pub fn solve_box_layout(data: &BoxLayoutData, repeater_indexes: Slice<u32>) -> SharedVector<Coord> {
let mut result = SharedVector::<f32>::default();
result.resize(data.cells.len() * 2 + repeater_indexes.len(), 0.);
if data.cells.is_empty() {
return result;
}
let mut layout_data: Vec<_> = data let mut layout_data: Vec<_> = data
.cells .cells
.iter() .iter()
@ -514,8 +521,6 @@ pub fn solve_box_layout(data: &BoxLayoutData, repeater_indexes: Slice<u32>) -> S
} }
} }
let mut result = SharedVector::<f32>::default();
result.resize(data.cells.len() * 2 + repeater_indexes.len(), 0.);
let res = result.make_mut_slice(); let res = result.make_mut_slice();
// The index/2 in result in which we should add the next repeated item // The index/2 in result in which we should add the next repeated item

View file

@ -0,0 +1,84 @@
/* LICENSE BEGIN
This file is part of the SixtyFPS Project -- https://sixtyfps.io
Copyright (c) 2021 Olivier Goffart <olivier.goffart@sixtyfps.io>
Copyright (c) 2021 Simon Hausmann <simon.hausmann@sixtyfps.io>
SPDX-License-Identifier: GPL-3.0-only
This file is also available under commercial licensing terms.
Please contact info@sixtyfps.io for more information.
LICENSE END */
// issue #177
export TestCase := Window {
width: 100px;
height: 100px;
callback clicked;
clicked => { debug("Hello"); model= []; }
property <bool> hover <=> under.has-hover;
property<[int]> model: [1];
VerticalLayout {
under := TouchArea {
HorizontalLayout {
for value in model: TouchArea {
horizontal-stretch: 5;
vertical-stretch: 5;
clicked => { root.clicked(); }
Rectangle { background: blue; }
}
}
}
Rectangle {
horizontal-stretch: 0;
vertical-stretch: 0;
background: yellow;
}
}
}
/*
```cpp
auto handle = TestCase::create();
const TestCase &instance = *handle;
auto vec_model = std::make_shared<sixtyfps::VectorModel<int>>(std::vector<int>{1, 2});
instance.set_model(vec_model);
instance.on_clicked([vec_model] { vec_model->erase(vec_model->row_count()-1); });
sixtyfps::testing::send_mouse_click(&instance, 95., 5.);
assert_eq(instance.get_model()->row_count(), 1);
assert(instance.get_hover());
sixtyfps::testing::send_mouse_click(&instance, 95., 5.);
assert_eq(instance.get_model()->row_count(), 0);
assert(!instance.get_hover());
```
```rust
use sixtyfps::Model;
let instance = TestCase::new();
let vec_model = std::rc::Rc::new(sixtyfps::VecModel::from(vec![1i32, 2i32]));
instance.set_model(sixtyfps::ModelHandle::from(vec_model.clone() as std::rc::Rc<dyn Model<Data = i32>>));
instance.on_clicked(move || dbg!(vec_model.remove(vec_model.row_count() - 1)));
sixtyfps::testing::send_mouse_click(&instance, 95., 5.);
assert_eq!(instance.get_model().row_count(), 1);
assert!(instance.get_hover());
sixtyfps::testing::send_mouse_click(&instance, 95., 5.);
assert_eq!(instance.get_model().row_count(), 0);
assert!(!instance.get_hover());
```
```js
var instance = new sixtyfps.TestCase({
clicked: function() { var x = instance.model; x.pop(); instance.model = x; }
});
instance.model = [1, 2];
instance.send_mouse_click(5., 5.);
assert.equal(instance.model.length, 1);
assert(instance.hover);
instance.send_mouse_click(5., 5.);
assert.equal(instance.model.length, 0);
assert(!instance.hover);
```
*/