Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge absolute-x and absolute-y "virtual" properties into absolute-position #2942

Merged
merged 8 commits into from Jun 21, 2023
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -28,7 +28,7 @@ All notable changes to this project are documented in this file.
- Added functions on color: `transparentize`, `mix`, and `with-alpha`.
- Added a `close()` function and a `close-on-click` boolean property to `PopupWindow`.
- Added basic translation infrastructure with `@tr("...")`
- Added `absolute-x` and `absolute-y` properties to every element, for computing window-absolute positions.
- Added `absolute-coordinates` property to every element, for computing window-absolute positions.
- Added `primary` boolean property to `Button` to configure its visual appearance.
- Added `current-row` to `StandardTableView`.

Expand Down
2 changes: 1 addition & 1 deletion api/cpp/cbindgen.rs
Expand Up @@ -63,7 +63,7 @@ fn default_config() -> cbindgen::Config {
("VoidArg".into(), "void".into()),
("KeyEventArg".into(), "KeyEvent".into()),
("PointerEventArg".into(), "PointerEvent".into()),
("PointArg".into(), "Point".into()),
("PointArg".into(), "slint::LogicalPosition".into()),
("FloatArg".into(), "float".into()),
("Coord".into(), "float".into()),
]
Expand Down
2 changes: 1 addition & 1 deletion docs/language/src/builtins/elements.md
Expand Up @@ -9,7 +9,7 @@ These properties are valid on all visible items:
- **`width`** and **`height`** (_in_ _length_): The size of the element. When set, this overrides the default size.
- **`x`** and **`y`** (_in_ _length_): The position of the element relative to its parent.
- **`z`** (_in_ _float_): Allows to specify a different order to stack the items with its siblings. (default value: 0)
- **`absolute-x`** and **`absolute-y`** (_in_ _length_): The position of the element within the contained window.
- **`absolute-coordinates`** (_in_ _Point_): The position of the element within the contained window.
tronical marked this conversation as resolved.
Show resolved Hide resolved

### Layout

Expand Down
8 changes: 0 additions & 8 deletions internal/compiler/builtins.slint
Expand Up @@ -172,13 +172,6 @@ export component BoxShadow inherits Empty {
//-is_internal
}

// Keep an eye on typeregister::logical_point_type() that creates the same type.
export struct Point {
//-name:slint::private_api::Point
x: length,
y: length,
}

export component TextInput {
in-out property <string> text;
in property <string> font-family;
Expand Down Expand Up @@ -408,7 +401,6 @@ export struct StandardListViewItem {
}

export struct TableColumn {
//-name:slint::private_api::TableColumn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used, but we have a public slint::TableColumn after all :-)

title: string,
min-width: length,
horizontal-stretch: float,
Expand Down
2 changes: 1 addition & 1 deletion internal/compiler/generator/cpp.rs
Expand Up @@ -2841,7 +2841,7 @@ fn compile_builtin_function_call(
BuiltinFunction::ItemAbsolutePosition => {
if let [llr::Expression::PropertyReference(pr)] = arguments {
let item_rc = access_item_rc(pr, ctx);
format!("slint_item_absolute_position(&{item_rc})")
format!("slint::LogicalPosition(slint_item_absolute_position(&{item_rc}))")
} else {
panic!("internal error: invalid args to ItemAbsolutePosition {:?}", arguments)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/compiler/generator/rust.rs
Expand Up @@ -2536,7 +2536,7 @@ fn compile_builtin_function_call(
if let [Expression::PropertyReference(pr)] = arguments {
let item_rc = access_item_rc(pr, ctx);
quote!(
(*#item_rc).map_to_window(Default::default()).to_untyped()
slint::LogicalPosition::from_euclid((*#item_rc).map_to_window(Default::default()))
)
} else {
panic!("internal error: invalid args to MapPointToWindow {:?}", arguments)
Expand Down
52 changes: 32 additions & 20 deletions internal/compiler/passes/lower_absolute_coordinates.rs
Expand Up @@ -19,13 +19,14 @@ pub fn lower_absolute_coordinates(component: &Rc<Component>) {

recurse_elem_including_sub_components_no_borrow(component, &(), &mut |elem, _| {
visit_all_named_references_in_element(elem, |nr| {
if nr.name() == "absolute-x" || nr.name() == "absolute-y" {
if nr.name() == "absolute-coordinates" {
to_materialize.insert(nr.clone());
}
});
});

let absolute_point_prop_name = "cached-absolute-xy".to_string();
// Absolute item coordinates without the item local x/y.
let cached_absolute_item_prop_name = "cached-absolute-xy".to_string();
tronical marked this conversation as resolved.
Show resolved Hide resolved
let point_type = match BuiltinFunction::ItemAbsolutePosition.ty() {
crate::langtype::Type::Function { return_type, .. } => return_type.as_ref().clone(),
_ => unreachable!(),
Expand All @@ -36,13 +37,13 @@ pub fn lower_absolute_coordinates(component: &Rc<Component>) {

elem.borrow_mut()
.property_declarations
.entry(absolute_point_prop_name.clone())
.entry(cached_absolute_item_prop_name.clone())
.or_insert_with(|| PropertyDeclaration {
property_type: point_type.clone(),
..PropertyDeclaration::default()
});

if !elem.borrow().bindings.contains_key(&absolute_point_prop_name) {
if !elem.borrow().bindings.contains_key(&cached_absolute_item_prop_name) {
let point_binding = Expression::FunctionCall {
function: Box::new(Expression::BuiltinFunctionReference(
BuiltinFunction::ItemAbsolutePosition,
Expand All @@ -53,24 +54,35 @@ pub fn lower_absolute_coordinates(component: &Rc<Component>) {
};
elem.borrow_mut()
.bindings
.insert(absolute_point_prop_name.clone(), RefCell::new(point_binding.into()));
.insert(cached_absolute_item_prop_name.clone(), RefCell::new(point_binding.into()));
}

// Create a binding to the hidden point property. The materialize properties pass is going to create the actual property
// for absolute-x/y.
let x_or_y = nr.name().strip_prefix("absolute-").unwrap().to_string();
let binding = Expression::BinaryExpression {
lhs: Expression::StructFieldAccess {
base: Expression::PropertyReference(NamedReference::new(
&elem,
&absolute_point_prop_name,
))
.into(),
name: x_or_y.clone(),
}
.into(),
rhs: Expression::PropertyReference(NamedReference::new(&elem, &x_or_y)).into(),
op: '+',
// Create a binding to the hidden point property and add item local x/y. The
// materialize properties pass is going to create the actual property for
// absolute-coordinates.
let binding = Expression::Struct {
ty: point_type.clone(),
values: IntoIterator::into_iter(["x", "y"])
.map(|coord| {
(
coord.to_string(),
Expression::BinaryExpression {
lhs: Expression::StructFieldAccess {
base: Expression::PropertyReference(NamedReference::new(
&elem,
&cached_absolute_item_prop_name,
))
.into(),
name: coord.to_string(),
}
.into(),
rhs: Expression::PropertyReference(NamedReference::new(&elem, &coord))
.into(),
op: '+',
},
)
})
.collect(),
};
elem.borrow_mut().bindings.insert(nr.name().to_string(), RefCell::new(binding.into()));
}
Expand Down
9 changes: 3 additions & 6 deletions internal/compiler/typeregister.rs
Expand Up @@ -21,9 +21,6 @@ pub const RESERVED_GEOMETRY_PROPERTIES: &[(&str, Type)] = &[
("z", Type::Float32),
];

pub(crate) const RESERVED_ABSOLUTE_GEOMETRY_PROPERTIES: &[(&str, Type)] =
&[("absolute-x", Type::LogicalLength), ("absolute-y", Type::LogicalLength)];

pub const RESERVED_LAYOUT_PROPERTIES: &[(&str, Type)] = &[
("min-width", Type::LogicalLength),
("min-height", Type::LogicalLength),
Expand Down Expand Up @@ -115,13 +112,13 @@ pub const RESERVED_ACCESSIBILITY_PROPERTIES: &[(&str, Type)] = &[
pub fn reserved_properties() -> impl Iterator<Item = (&'static str, Type)> {
RESERVED_GEOMETRY_PROPERTIES
.iter()
.chain(RESERVED_ABSOLUTE_GEOMETRY_PROPERTIES.iter())
.chain(RESERVED_LAYOUT_PROPERTIES.iter())
.chain(RESERVED_OTHER_PROPERTIES.iter())
.chain(RESERVED_DROP_SHADOW_PROPERTIES.iter())
.chain(RESERVED_ROTATION_PROPERTIES.iter())
.chain(RESERVED_ACCESSIBILITY_PROPERTIES.iter())
.map(|(k, v)| (*k, v.clone()))
.chain(IntoIterator::into_iter([("absolute-coordinates", logical_point_type())]))
.chain(IntoIterator::into_iter([
("forward-focus", Type::ElementReference),
("focus", BuiltinFunction::SetFocusItem.ty()),
Expand Down Expand Up @@ -234,6 +231,7 @@ impl TypeRegister {
register.insert_type(Type::Angle);
register.insert_type(Type::Brush);
register.insert_type(Type::Rem);
register.types.insert("Point".into(), logical_point_type());

BUILTIN_ENUMS.with(|e| e.fill_register(&mut register));

Expand Down Expand Up @@ -402,15 +400,14 @@ impl TypeRegister {
}
}

/// This is identical with builtins.slint's Point type (TODO: use that in the future)
pub fn logical_point_type() -> Type {
Type::Struct {
fields: IntoIterator::into_iter([
("x".to_owned(), Type::LogicalLength),
("y".to_owned(), Type::LogicalLength),
])
.collect(),
name: Some("Point".into()),
name: Some("slint::LogicalPosition".into()),
node: None,
rust_attributes: None,
}
Expand Down
3 changes: 2 additions & 1 deletion internal/core/api.rs
Expand Up @@ -47,7 +47,8 @@ impl LogicalPosition {
pub(crate) fn to_euclid(&self) -> crate::lengths::LogicalPoint {
[self.x as _, self.y as _].into()
}
pub(crate) fn from_euclid(p: crate::lengths::LogicalPoint) -> Self {
#[doc(hidden)]
pub fn from_euclid(p: crate::lengths::LogicalPoint) -> Self {
tronical marked this conversation as resolved.
Show resolved Hide resolved
Self::new(p.x as _, p.y as _)
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/core/items.rs
Expand Up @@ -1448,7 +1448,7 @@ pub struct PointerEvent {
pub unsafe extern "C" fn slint_item_absolute_position(
self_component: &vtable::VRc<crate::component::ComponentVTable>,
self_index: usize,
) -> Point {
) -> LogicalPoint {
let self_rc = ItemRc::new(self_component.clone(), self_index);
self_rc.map_to_window(Default::default()).to_untyped()
self_rc.map_to_window(Default::default())
}
10 changes: 9 additions & 1 deletion tests/cases/absolute_coords.slint
Expand Up @@ -5,7 +5,7 @@ export component TestCase {
width: 500phx;
height: 500phx;

property <bool> simple-inner-ok: simple-inner.absolute-x == 40phx && simple-inner.absolute-y == 60phx;
property <bool> simple-inner-ok: simple-inner.absolute-coordinates.x == 40phx && simple-inner.absolute-coordinates.y == 60phx;
Rectangle {
x: 10phx;
y: 20phx;
Expand All @@ -16,23 +16,31 @@ export component TestCase {
}
}
out property <bool> test: simple-inner-ok;
out property <Point> coords <=> simple-inner.absolute-coordinates;
}

/*
```rust
let instance = TestCase::new().unwrap();
assert!(instance.get_test());
let pos: slint::LogicalPosition = instance.get_coords();
assert_eq!(pos.x, 40.);
assert_eq!(pos.y, 60.);
```

```cpp
auto handle = TestCase::create();
const TestCase &instance = *handle;
assert(instance.get_test());
slint::LogicalPosition pos = instance.get_coords();
assert_eq(pos.x, 40);
assert_eq(pos.y, 60);
```

```js
let instance = new slint.TestCase({});
assert(instance.test, 1);
assert.deepEqual(instance.coords, { x: 40, y: 60});
```

*/