Fix priority when merging two_way_binding

We should not increase the priority when merging two way binding, only when
inlining.

This fixes the iot-dashboard's devices widget which were sometimes not
transparent as they should have been.
This was not deterministic because the order in which the two way binding are
merged is not deterministic because of hash table, and sometimes one binding
ended up having a higher priority as it should have had.
This commit is contained in:
Olivier Goffart 2021-08-17 11:57:35 +02:00
parent a2966bdafa
commit 2c1e39f00e
3 changed files with 38 additions and 13 deletions

View file

@ -1120,37 +1120,42 @@ impl BindingExpression {
/// Merge the other into this one. Normally, &self is kept intact (has priority), unless two ways binding are /// Merge the other into this one. Normally, &self is kept intact (has priority), unless two ways binding are
/// involved then they need to be merged. /// involved then they need to be merged.
/// Also the animation is taken if the other don't have one /// Also the animation is taken if the other don't have one
pub fn merge_with(&mut self, other: &Self) { ///
/// Returns true if the other expression was taken
pub fn merge_with(&mut self, other: &Self) -> bool {
fn maybe_merge_two_ways( fn maybe_merge_two_ways(
binding: &mut Expression, binding: &mut Expression,
priority: &mut i32, priority: &mut i32,
original: &BindingExpression, original: &BindingExpression,
) { ) -> bool {
match (binding, &original.expression) { match (binding, &original.expression) {
(Expression::TwoWayBinding(_, Some(ref mut x)), _) => { (Expression::TwoWayBinding(_, Some(ref mut x)), _) => {
maybe_merge_two_ways(&mut *x, priority, original); maybe_merge_two_ways(&mut *x, priority, original)
} }
(Expression::TwoWayBinding(_, x), o) => { (Expression::TwoWayBinding(_, x), o) => {
*priority = original.priority + 1; *priority = original.priority;
*x = Some(Box::new(o.clone())); *x = Some(Box::new(o.clone()));
true
} }
(ref mut x, Expression::TwoWayBinding(nr, _)) => { (ref mut x, Expression::TwoWayBinding(nr, _)) => {
let n = let n =
Expression::TwoWayBinding(nr.clone(), Some(Box::new(std::mem::take(*x)))); Expression::TwoWayBinding(nr.clone(), Some(Box::new(std::mem::take(*x))));
**x = n **x = n;
false
} }
_ => *priority += 1, _ => false,
} }
} }
if matches!(self.expression, Expression::Invalid) {
self.priority = other.priority + 1;
self.expression = other.expression.clone();
} else {
maybe_merge_two_ways(&mut self.expression, &mut self.priority, other);
}
if self.animation.is_none() { if self.animation.is_none() {
self.animation = other.animation.clone(); self.animation = other.animation.clone();
} }
if matches!(self.expression, Expression::Invalid) {
self.priority = other.priority;
self.expression = other.expression.clone();
true
} else {
maybe_merge_two_ways(&mut self.expression, &mut self.priority, other)
}
} }
} }

View file

@ -114,7 +114,9 @@ fn inline_element(
entry.insert(val.clone()).priority += 1; entry.insert(val.clone()).priority += 1;
} }
std::collections::btree_map::Entry::Occupied(mut entry) => { std::collections::btree_map::Entry::Occupied(mut entry) => {
entry.get_mut().merge_with(val); if entry.get_mut().merge_with(val) {
entry.get_mut().priority += 1;
}
} }
} }
} }

View file

@ -8,6 +8,13 @@
Please contact info@sixtyfps.io for more information. Please contact info@sixtyfps.io for more information.
LICENSE END */ LICENSE END */
export OpacityTwoWay := Rectangle {
property <float> sub_opacity <=> rect.opacity;
rect := Rectangle {
opacity: 0.75;
}
}
export Compo := Rectangle { export Compo := Rectangle {
property<int> foo <=> self.bar; property<int> foo <=> self.bar;
property<int> bar: 120; property<int> bar: 120;
@ -18,8 +25,16 @@ TestCase := Window {
compo1 := Compo {} compo1 := Compo {}
compo2 := Compo { foo: compo1.foo + 10; } compo2 := Compo { foo: compo1.foo + 10; }
Rectangle {
otw := OpacityTwoWay { sub_opacity: 0.5; }
otw2 := OpacityTwoWay { sub_opacity: 0.5; }
}
property <int> compo0_foo: compo0.foo; property <int> compo0_foo: compo0.foo;
property <int> compo2_foo: compo2.foo; property <int> compo2_foo: compo2.foo;
property <float> otw_opacity <=> otw.sub_opacity;
property <bool> test: compo0_foo == 140 && compo2_foo == 130 && otw_opacity == 0.5 && otw2.sub_opacity == 0.5;
} }
/* /*
@ -28,6 +43,7 @@ TestCase := Window {
let instance = TestCase::new(); let instance = TestCase::new();
assert_eq!(instance.get_compo0_foo(), 140); assert_eq!(instance.get_compo0_foo(), 140);
assert_eq!(instance.get_compo2_foo(), 130); assert_eq!(instance.get_compo2_foo(), 130);
assert_eq!(instance.get_otw_opacity(), 0.5);
``` ```
@ -37,6 +53,7 @@ auto handle = TestCase::create();
const TestCase &instance = *handle; const TestCase &instance = *handle;
assert_eq(instance.get_compo0_foo(), 140); assert_eq(instance.get_compo0_foo(), 140);
assert_eq(instance.get_compo2_foo(), 130); assert_eq(instance.get_compo2_foo(), 130);
assert_eq(instance.get_otw_opacity(), 0.5);
``` ```
@ -44,6 +61,7 @@ assert_eq(instance.get_compo2_foo(), 130);
let instance = new sixtyfps.TestCase({}); let instance = new sixtyfps.TestCase({});
assert.equal(instance.compo0_foo, 140); assert.equal(instance.compo0_foo, 140);
assert.equal(instance.compo2_foo, 130); assert.equal(instance.compo2_foo, 130);
assert.equal(instance.otw_opacity, 0.5);
``` ```
*/ */