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

Compiler unable to represent state property binding when the default binding is declared in a different component #1461

Open
tay64 opened this issue Aug 6, 2022 · 1 comment
Labels
a:compiler Slint compiler internal (not the codegen, not the parser) bug Something isn't working

Comments

@tay64
Copy link
Contributor

tay64 commented Aug 6, 2022

Consider

Hello := Rectangle {
    background: xxx.color;
    xxx := Text {}
 }
 
 export AppWindow := Window {
    Hello {
        states [
            highlight when width < height : {
                background: red;
            }
        ]
    }
 }

This will get lowered to something like

export AppWindow := Window {
   Hello { 
       property <int> state: ...;
       background: state == 1 ? red : xxx.color;   
   }
}

But xxx.color can't be represented properly because xxx is inaccessible from within the AppWindow context.
This may resulting in panics in the compiler in further passes.

To fix this, we need to change the representation of properties access to be able to access properties within element of components.

Original bug report

Observed with slint 0.2.5 from crates.io and with a head version from git, commit 5bc56a3.

Steps to reproduce:

  1. Start with cargo generate --git https://github.com/slint-ui/slint-rust-template.
  2. Apply this diff:
diff --git a/ui/appwindow.slint b/ui/appwindow.slint
index 5345759..03e9e94 100644
--- a/ui/appwindow.slint
+++ b/ui/appwindow.slint
@@ -12,6 +12,11 @@ export AppWindow := Window {
             clicked => {
                 request-increase-value();
             }
+            states [
+                highlight when counter > 45 : {
+                    background: red;
+                }
+            ]
         }
      }
 }

modified appwindow.slint

  1. Run cargo build

Error:

error: failed to run custom build command for `slint-bug-state-background v0.1.0 (D:\0\slint-bug-state-background)`

Caused by:
  process didn't exit successfully: `D:\0\slint-bug-state-background\target\debug\build\slint-bug-state-background-81a7ccd94231b199\build-script-build` (exit code: 101)
  --- stderr
  thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', C:\Users\user\.cargo\registry\src\github.com-1ecc6299db9ec823\i-slint-compiler-0.2.5\llr\lower_expression.rs:35:34

Error output with RUST_BACKTRACE=1

Additional observations:

  • It works just fine when I load the .slint file at runtime using slint_interpreter.
  • The problem is specifically with the background: if I comment out the line background: red; but leave the rest of the added states in place, the error disappears.
  • No error if I change some of the other properties instead of background (I tried text and opacity).
  • Assigning to background in the clicked callback instead of states works fine.
  • Changing a custom property instead of background also works and gives a workaround.

Workaround:
Create a custom property and bind background to it:

diff --git a/ui/appwindow.slint b/ui/appwindow.slint
index 5345759..2e61b0f 100644
--- a/ui/appwindow.slint
+++ b/ui/appwindow.slint
@@ -12,6 +12,13 @@ export AppWindow := Window {
             clicked => { 
                 request-increase-value();
             }
+            property<brush> my-background;
+            background: my-background;
+            states [
+                highlight when counter > 45 : {
+                    my-background: red;
+                }
+            ]
         }
      }
 }

OS: Windows
Slint style: fluent

(EDITED: added a workaround. Seems obvious but somehow didn't occur to me when I wrote the original description).

ogoffart added a commit that referenced this issue Aug 16, 2022
Some binding can't be express with the current data structures because
they reference propertis within inner elements within a component.

The fix is a bit involved and the best is to have an error istead of a
panic, for now.
ogoffart added a commit that referenced this issue Aug 16, 2022
Some binding can't be express with the current data structures because
they reference propertis within inner elements within a component.

The fix is a bit involved and the best is to have an error istead of a
panic, for now.
@ogoffart ogoffart changed the title Compiler panics on changing Button background in states Compiler unable to represent state property binding when the default binding is declared in a different component Aug 16, 2022
@ogoffart
Copy link
Member

Thanks for the well written bug report.
I was able to reproduce and found out what the problem is.
This is a difficult problem to solve so I took the liberty to edit the description and title. I'm going to keep the issue open and implemented a workaround by making this code an error for now: #1490
The actual fix will need some more refactorings in the compiler which will be done in a later release.

@ogoffart ogoffart added bug Something isn't working a:compiler Slint compiler internal (not the codegen, not the parser) labels Aug 16, 2022
ogoffart added a commit that referenced this issue Aug 17, 2022
Some binding can't be express with the current data structures because
they reference propertis within inner elements within a component.

The fix is a bit involved and the best is to have an error istead of a
panic, for now.
@ogoffart ogoffart added the a:language-slint Compiler for the .slint language (mO,bF) label Jul 13, 2023
@ogoffart ogoffart removed the a:language-slint Compiler for the .slint language (mO,bF) label Jan 22, 2024
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
Projects
None yet
Development

No branches or pull requests

2 participants