Lay out grids at runtime rather than at compile time

[dfaure-kdab]

This opens the door for using computed values in "row"/"col" properties, and later on for dynamic grid layouts (for/if support)

PROBLEM: it's missing the "row" and "col" properties from my previous commit... This is WIP, so we can discuss how to bring those properties back.

[ogoffart]

I think this is going in the right direction.

We'll have to have good documentation for the exact behavior and tests.

[ogoffart]

slightly concerned about the extra allocation here at runtime. But I hope this is fine and we can optimize later if needed.

[dfaure-kdab]

Ah, maybe I could change the generated code to generate a Vec (that I can mutate) instead of
&sp::GridLayoutData { cells: sp::Slice::from_slice(&[ ... ]) ) ?

[dfaure-kdab]

I think this is going in the right direction.

Phew ;-)

We'll have to have good documentation for the exact behavior and tests.

Documentation, I agree (will do).
Tests, I want to reassure you, we do already. While working on this, I have had a LOT of feedback from the existing layout tests.

[dfaure-kdab]

Now including row and col properties, implemented using the layout cache.
It works, but it has an issue: if e.g. a widget's text property is set to "row" or "col", it creates an infinite recursion on the layout-cache binding, incorrectly. It makes sense to x,y,width,height to depend on the widget text, but not for row and col. So I should create a different kind of cache for that?

The other issue is the one mentioned in the commit log: how to react to changes to row/col and trigger layouting again?
I'll dig into it, but hints welcome.

[ogoffart]

It works, but it has an issue: if e.g. a widget's text property is set to "row" or "col", it creates an infinite recursion on the layout-cache binding, incorrectly.

I see, because the row and col are both input and output of the layout computation?

It makes sense to x,y,width,height to depend on the widget text, but not for row and col. So I should create a different kind of cache for that?

Perhaps it would help to have another property to cache these values instead of doing it all in solve_layout, yes.
(Then we would also not need this cache if the row and col are entirely known at compile time)

But this can be a known issue and doesn't need to be solve, as long as it is detected at compile time and the logic in the binding_analysis that detect the loop is catching that properly.

[dfaure-kdab]

Perhaps it would help to have another property to cache these values instead of doing it all in solve_layout, yes

Hmm, solve_grid_layout returns the x,y,width,height cache (for use in the property binding for layoutcache) and takes the spec from the user as input, so row/col determination has to happen in there too (it's obviously needed to determine x,y,width,height)... but the result of that row/col determination needs to be stored into another property... I wonder how.

The current solution is already to duplicate the input data between layoutcache (the call to solve_grid_layout) and layoutinfo (the call to grid_layout_info). Somehow we should be able to extract that into a function that returns a Vec, that I could also use from the third property that is computed from that same data?
I means doing some processing 3 times but I guess this is unavoidable, e.g. to compute the binding dependencies correctly... or we could use the output of the first step as input to the other two (layoutcache and layoutinfo), to optimize this.

(Then we would also not need this cache if the row and col are entirely known at compile time)

That is an excellent idea, it would address your concern about memory usage.

But this can be a known issue and doesn't need to be solve, as long as it is detected at compile time and the logic in the binding_analysis that detect the loop is catching that properly.

Yes, it's caught properly, but it's still a bug, and one that requires a big refactoring of the code in this MR. But indeed if we want the feature already (or if you simply want to avoid re-reviewing the same thing many times), you can merge this and I can build on top.

[dfaure-kdab]

The other issue is the one mentioned in the commit log: how to react to changes to row/col and trigger layouting again? I'll dig into it, but hints welcome.

I was wrong, this works well in an interactive testcase. It just doesn't work in a unittest like
grid_variable_row_col.slint.txt
(extension hacked so github allows it). get_final_grid_ok() fails. But in an interactive testcase with a button to call change_grid(), it works fine. I guess one needs to wait for the relayout to happens before testing the new properties, any idea how to do that? EDIT - PEBCAK, sorry for the noise.

[ogoffart]

   out property <bool> test: {
       initial_grid_ok
       // how to call change_grid??
   }

Maybe something like

init => {
   if !initial_grid_ok { root.error = "!initial_grid_ok"; }
   change_grid()
}
out property <string> error;
out property <bool> test: error == "" && final_grid_ok;

[dfaure-kdab]

Maybe something like [...]

Thanks, this worked, now the new test passes (both when compiled and when interpreted)

[dfaure-kdab]

we could use the output of the first step as input to the other two (layoutcache and layoutinfo), to optimize this.

I quite like this idea, actually. I am currently implementing this solution, on top of this MR, to make this cleaner and solve the binding loop.

pub struct GridLayoutInputData {
    /// whether this cell is the first one in a Row element
    pub new_row: bool,
    /// col and row number (u16::MAX means auto).
    pub col: u16,
    pub row: u16,
    /// colspan and rowspan
    pub colspan: u16,
    pub rowspan: u16,
}

/// The organized layout data for a GridLayout, after row/col determination:
/// For each cell, stores col, colspan, row, rowspan
pub type GridLayoutOrganizedData = SharedVector<Coord>;
}

A new property "layout-organized-data" contains the result of the call that uses the input data to determine row/col values and store that into the organized data (which is a SharedVector in order to reuse the LayoutCacheAccess mechanism, just on a different cache property), and this (plus layout constraints...) will be the source of information for both solve_grid_layout and grid_layout_info. Currently only implemented for the interpreter (works 100% - grid_layout_data() reads from the organized data and adds the constraints, it all works including a text property that uses row and col), I will now work on the compiler side of things.