Don't steal the x and y properties for dummy parents

Parents surch as Opacity, Clip, and co, used to steal the x and y
property of their children, making the property not what they ought to
be.

Now that we refactored recently the code so that geometry need not to be
always linked to a property of the same name, we can dissociate the x
and y property of these generated elements and their content so that the
actual "x" property of the former elementstay some value, despite its
relative item property is now 0.

Had to change a bit of code that was still assuming a literal "height"
or "width" or "y" or "x" property that no longer worked when the
geometry is dissociated from its property

Fix #1072

internal/compiler/llr/lower_to_item_tree.rs

@@ -521,12 +521,9 @@ fn lower_repeated_component(elem: &ElementRc, ctx: &ExpressionContext) -> Repeat
     let component = e.base_type.as_component().clone();
     let repeated = e.repeated.as_ref().unwrap();
 
-    let sc = lower_sub_component(&component, ctx.state, Some(ctx));
+    let sc: LoweredSubComponent = lower_sub_component(&component, ctx.state, Some(ctx));
 
-    let map_inner_prop = |p| {
-        sc.mapping
-            .map_property_reference(&NamedReference::new(&component.root_element, p), ctx.state)
-    };
+    let geom = component.root_element.borrow().geometry_props.clone().unwrap();
 
     let listview = repeated.is_listview.as_ref().map(|lv| ListViewInfo {
         viewport_y: ctx.map_property_reference(&lv.viewport_y),
@@ -534,10 +531,9 @@ fn lower_repeated_component(elem: &ElementRc, ctx: &ExpressionContext) -> Repeat
         viewport_width: ctx.map_property_reference(&lv.viewport_width),
         listview_height: ctx.map_property_reference(&lv.listview_height),
         listview_width: ctx.map_property_reference(&lv.listview_width),
-
-        prop_y: map_inner_prop("y"),
-        prop_width: map_inner_prop("width"),
-        prop_height: map_inner_prop("height"),
+        prop_y: sc.mapping.map_property_reference(&geom.y, ctx.state),
+        prop_width: sc.mapping.map_property_reference(&geom.width, ctx.state),
+        prop_height: sc.mapping.map_property_reference(&geom.height, ctx.state),
     });
 
     RepeatedElement {

internal/compiler/object_tree.rs

@@ -2544,32 +2544,20 @@ pub fn inject_element_as_repeated_element(repeated_element: &ElementRc, new_root
 /// Make the geometry of the `injected_parent` that of the old_elem. And the old_elem
 /// will cover the `injected_parent`
 pub fn adjust_geometry_for_injected_parent(injected_parent: &ElementRc, old_elem: &ElementRc) {
-    // The values for properties that affect the geometry may be supplied in two different ways:
-    //
-    //   * When coming from the outside, for example by the repeater being inside a layout, we need
-    //     the values to apply to the new root element and the old root just needs to follow.
-    //   * When coming from the inside, for example when the repeater just creates rectangles that
-    //     calculate their own position, we need to move those bindings as well to the new root.
-    injected_parent.borrow_mut().bindings.extend(Iterator::chain(
-        ["x", "y", "z"].iter().filter_map(|x| old_elem.borrow_mut().bindings.remove_entry(*x)),
-        ["width", "height"].iter().map(|x| {
-            (
-                x.to_string(),
-                BindingExpression::from(Expression::PropertyReference(NamedReference::new(
-                    old_elem, x,
-                )))
-                .into(),
-            )
-        }),
-    ));
-    injected_parent.borrow().property_analysis.borrow_mut().extend(
-        ["x", "y", "z"].into_iter().filter_map(|x| {
-            old_elem
-                .borrow()
-                .property_analysis
-                .borrow()
-                .get_key_value(x)
-                .map(|(k, v)| (k.clone(), v.clone()))
-        }),
+    let mut injected_parent_mut = injected_parent.borrow_mut();
+    injected_parent_mut.bindings.insert(
+        "z".into(),
+        RefCell::new(BindingExpression::new_two_way(NamedReference::new(old_elem, "z".into()))),
     );
+    // (should be removed by const propagation in the llr)
+    injected_parent_mut.property_declarations.insert(
+        "dummy".into(),
+        PropertyDeclaration { property_type: Type::LogicalLength, ..Default::default() },
+    );
+    let mut old_elem_mut = old_elem.borrow_mut();
+    injected_parent_mut.default_fill_parent = std::mem::take(&mut old_elem_mut.default_fill_parent);
+    injected_parent_mut.geometry_props = old_elem_mut.geometry_props.clone();
+    drop(injected_parent_mut);
+    old_elem_mut.geometry_props.as_mut().unwrap().x = NamedReference::new(injected_parent, "dummy");
+    old_elem_mut.geometry_props.as_mut().unwrap().y = NamedReference::new(injected_parent, "dummy");
 }

internal/compiler/passes/default_geometry.rs

@@ -52,20 +52,40 @@ pub fn default_geometry(root_component: &Rc<Component>, diag: &mut BuildDiagnost
                 match builtin_type.default_size_binding {
                     DefaultSizeBinding::None => {
                         if elem.borrow().default_fill_parent.0 {
-                            w100 |= make_default_100(elem, parent, "width");
+                            let e_width =
+                                elem.borrow().geometry_props.as_ref().unwrap().width.clone();
+                            let p_width =
+                                parent.borrow().geometry_props.as_ref().unwrap().width.clone();
+                            w100 |= make_default_100(&e_width, &p_width);
                         } else {
                             make_default_implicit(elem, "width");
                         }
                         if elem.borrow().default_fill_parent.1 {
-                            h100 |= make_default_100(elem, parent, "height");
+                            let e_height =
+                                elem.borrow().geometry_props.as_ref().unwrap().height.clone();
+                            let p_height =
+                                parent.borrow().geometry_props.as_ref().unwrap().height.clone();
+                            h100 |= make_default_100(&e_height, &p_height);
                         } else {
                             make_default_implicit(elem, "height");
                         }
                     }
                     DefaultSizeBinding::ExpandsToParentGeometry => {
                         if !elem.borrow().child_of_layout {
-                            w100 |= make_default_100(elem, parent, "width");
-                            h100 |= make_default_100(elem, parent, "height");
+                            let (e_width, e_height) = elem
+                                .borrow()
+                                .geometry_props
+                                .as_ref()
+                                .map(|g| (g.width.clone(), g.height.clone()))
+                                .unwrap();
+                            let (p_width, p_height) = parent
+                                .borrow()
+                                .geometry_props
+                                .as_ref()
+                                .map(|g| (g.width.clone(), g.height.clone()))
+                                .unwrap();
+                            w100 |= make_default_100(&e_width, &p_width);
+                            h100 |= make_default_100(&e_height, &p_height);
                         }
                     }
                     DefaultSizeBinding::ImplicitSize => {
@@ -282,9 +302,9 @@ fn fix_percent_size(
 
 /// Generate a size property that covers the parent.
 /// Return true if it was changed
-fn make_default_100(elem: &ElementRc, parent_element: &ElementRc, property: &str) -> bool {
-    elem.borrow_mut().set_binding_if_not_set(property.to_string(), || {
-        Expression::PropertyReference(NamedReference::new(parent_element, property))
+fn make_default_100(prop: &NamedReference, parent_prop: &NamedReference) -> bool {
+    prop.element().borrow_mut().set_binding_if_not_set(prop.name().to_string(), || {
+        Expression::PropertyReference(parent_prop.clone())
     })
 }
 

internal/compiler/passes/lower_component_container.rs

@@ -5,7 +5,6 @@ use crate::diagnostics::BuildDiagnostics;
 use crate::langtype::ElementType;
 use crate::object_tree::*;
 use crate::typeregister::TypeRegister;
-use std::cell::RefCell;
 use std::rc::Rc;
 
 pub fn lower_component_container(
@@ -34,7 +33,7 @@ fn diagnose_component_container(element: &ElementRc, diag: &mut BuildDiagnostics
 fn process_component_container(element: &ElementRc, empty_type: &ElementType) {
     let mut elem = element.borrow_mut();
 
-    let embedded_element = Rc::new(RefCell::new(Element {
+    let embedded_element = Element::make_rc(Element {
         base_type: empty_type.clone(),
         id: elem.id.clone(),
         node: elem.node.clone(),
@@ -44,7 +43,7 @@ fn process_component_container(element: &ElementRc, empty_type: &ElementType) {
         inline_depth: elem.inline_depth,
         is_component_placeholder: true,
         ..Default::default()
-    }));
+    });
 
     elem.children.push(embedded_element);
 }

internal/compiler/passes/lower_shadows.rs

@@ -163,16 +163,7 @@ pub fn lower_shadow_properties(
                     }
                 };
 
-                // Install bindings from the remaining properties of the shadow element to the
-                // original, such as x/y/width/height.
-                for (prop, _) in crate::typeregister::RESERVED_GEOMETRY_PROPERTIES.iter() {
-                    let prop = prop.to_string();
-                    shadow_elem.bindings.entry(prop.clone()).or_insert_with(|| {
-                        let binding_ref =
-                            Expression::PropertyReference(NamedReference::new(&child, &prop));
-                        RefCell::new(binding_ref.into())
-                    });
-                }
+                shadow_elem.geometry_props = elem.borrow().geometry_props.clone();
 
                 elem.borrow_mut().children.push(Element::make_rc(shadow_elem));
             }

internal/compiler/passes/visible.rs

@@ -64,8 +64,9 @@ pub fn handle_visible(
                         let clip_elem = create_visibility_element(&root_elem, &native_clip);
                         object_tree::inject_element_as_repeated_element(&child, clip_elem.clone());
                         // The width and the height must be null
-                        clip_elem.borrow_mut().bindings.remove("width");
-                        clip_elem.borrow_mut().bindings.remove("height");
+                        let d = NamedReference::new(&clip_elem, "dummy");
+                        clip_elem.borrow_mut().geometry_props.as_mut().unwrap().width = d.clone();
+                        clip_elem.borrow_mut().geometry_props.as_mut().unwrap().height = d;
                     }
                 } else if has_visible_binding(&child) {
                     let new_child = create_visibility_element(&child, &native_clip);

tests/cases/layout/issue_1072_opacity_geometry.slint

@@ -0,0 +1,57 @@
+// Copyright © SixtyFPS GmbH <info@slint.dev>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-1.1 OR LicenseRef-Slint-commercial
+
+// Test that the x and y property of a Rectangle with opacity are consistant
+
+export component TestCase  {
+
+    width: 100px;
+    height: 100px;
+
+
+    r1 := Rectangle {
+        x: 20px;
+        y: 15px;
+        width: 12px;
+        height: 5px;
+        opacity: 0.5;
+        background: red;
+        TouchArea {
+            clicked => {
+                root.clicked = (self.absolute-position.y + self.mouse-y) / 1px;
+                r1.y += 50px;
+            }
+        }
+    }
+
+    in-out property xx <=> r1.x;
+    out property<int> clicked;
+
+    out property <bool> test:
+        r1.x == 20px && r1.y == 15px;
+}
+
+/*
+```cpp
+auto handle = TestCase::create();
+const TestCase &instance = *handle;
+assert(instance.get_test());
+```
+
+
+```rust
+let instance = TestCase::new().unwrap();
+assert!(instance.get_test());
+
+slint_testing::send_mouse_click(&instance, 21., 16.);
+assert_eq!(instance.get_clicked(), 16);
+instance.set_xx(80.);
+slint_testing::send_mouse_click(&instance, 81., 66.);
+assert_eq!(instance.get_clicked(), 66);
+```
+
+```js
+var instance = new slint.TestCase({});
+assert(instance.test);
+```
+*/