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

Failure to compile layout: binding loop reported #7693

Closed
tronical opened this issue Feb 21, 2025 · 4 comments · Fixed by #7697
Closed

Failure to compile layout: binding loop reported #7693

tronical opened this issue Feb 21, 2025 · 4 comments · Fixed by #7697
Labels
a:compiler Slint compiler internal (not the codegen, not the parser) bug Something isn't working need triaging Issue that the owner of the area still need to triage priority:blocker Highest priority for issues needed to be done as fast as possibe

Comments

@tronical
Copy link
Member

tronical commented Feb 21, 2025

Bug Description

After #7680 , in particular commit bf716ff, the following provided test-case fails to compile.

It compiles when reverting that change.

Error message:

error: The binding for the property 'layout-cache' is part of a binding loop
  --> /Users/simon/test.slint:8:5
   |
8  |     HorizontalLayout {
   |     ^
error: The binding for the property 'width' is part of a binding loop
  --> /Users/simon/test.slint:8:5
   |
8  |     HorizontalLayout {
   |     ^
error: The binding for the property 'width' is part of a binding loop
  --> /Users/simon/test.slint:10:13
   |
10 |             Foo { }
   |             ^
error: The binding for the property 'layoutinfo-h' is part of a binding loop
  --> /Users/simon/test.slint:2:5
   |
2  |     VerticalLayout {
   |     ^
error: The binding for the property 'layoutinfo-h' is part of a binding loop
  --> /Users/simon/test.slint:10:13
   |
10 |             Foo { }
   |             ^
error: The binding for the property 'layoutinfo-h' is part of a binding loop
  --> /Users/simon/test.slint:9:9
   |
9  |         Rectangle {
   |         ^

Reproducible Code (if applicable)

component Foo inherits Rectangle {
    VerticalLayout {
        if root.width >= 10px: HorizontalLayout { }
    }
}

export component AppShell {
    HorizontalLayout {
        Rectangle {
            Foo { }
        }
    }
}

Environment Details

  • Slint Version:
  • Platform/OS:
  • Programming Language:
  • Backend/Renderer:

Product Impact

No response

@tronical tronical added bug Something isn't working need triaging Issue that the owner of the area still need to triage a:compiler Slint compiler internal (not the codegen, not the parser) priority:blocker Highest priority for issues needed to be done as fast as possibe labels Feb 21, 2025
@tronical
Copy link
Member Author

At first glance... I wonder if this is perhaps a valid error and it was just somehow masked before?

@ogoffart
Copy link
Member

Looks like a layout bug in which layout behave differently depending on inlining:

Example

In that example, i'd have expected t1 and t2 and t3 to have the same minimal size.
But in 1.9.2 they don't have the same size because we somehow don't propagate the constraints from components

@ogoffart
Copy link
Member

I tried to "fix" the problem by removing this lines

if child_infos.is_empty() {
return;
}

(indeed, when we see something like abc := Abc {}, then the element abc has no children info directly, but it still has some "implicit" constraint that we should take care of for the layout info from Abc's component children.)

But then we panic later in other testcase in

panicked at internal/compiler/passes/inlining.rs:253:13:
not yet implemented: Merge layout infos

Which would need to be fixed but i don't have the time to do that now.

@ogoffart
Copy link
Member

ogoffart commented Feb 21, 2025

Another similar bug in https://github.com/slint-ui/slint/actions/runs/13460934634/job/37615873276
https://github.com/somantics/retrodungeon/blob/9f8b7b5aa0d5d68b331c6a710b9c6dff42011447/ui/old.slint#L122

  error: The property 'y' cannot be set for elements placed in this layout, because the layout is already setting it
     --> /home/runner/the_repo/ui/old.slint:122:6
      |
  122 |   y: 0;
      |      ^

This is again caused because of inlining and the y property on the root cause that. which is yet another bug.

I think the best is to revert commit bf716ff for this release and try to fix these bug before we do the inlining.

Edit: another bug caused by that commit in https://github.com/slint-ui/slint/actions/runs/13460934634/job/37615849402 when compiling https://github.com/DASPRiD/vrc-osc-manager

  thread 'main' panicked at /home/olivier/slint/internal/compiler/llr/lower_expression.rs:43:34:
  called `Option::unwrap()` on a `None` value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:compiler Slint compiler internal (not the codegen, not the parser) bug Something isn't working need triaging Issue that the owner of the area still need to triage priority:blocker Highest priority for issues needed to be done as fast as possibe
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants