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

[BUG] Rendering with Smartstate in basic-example-incremental-redraw #3

Open
Ddystopia opened this issue Dec 3, 2024 · 8 comments
Open

Comments

@Ddystopia
Copy link

image

Steps to reproduce: click 10 times on "+" button. Then layout will shift (as "10" is longer then "9") and that bug will happen.

image

That is the state when you click on "-" button after previous screenshot.

@Yandrik
Copy link
Owner

Yandrik commented Dec 17, 2024

You're right - this is a limitation of the way in which the widgets are placed. The best way to fix this is to do one of two things:

  1. Set a consistent amount of padding for the number, e.g. to have a number between 0 and 999, and have 3 spaces of padding (e.g. format!("{:<3}", num);, this preserves the length of the label)
  2. When the number switches from 9 to 10 or from 10 to 9, redraw to the end of the row using ui.clear_row_to_end(). This would require clearing the smartstate for the button in the case of a change of the number.

Honestly, both solutions are somewhat suboptimal. I believe that the best solution for now is to keep the label length as consistent as it can be, and once smartstates are integrated into the UI, clearing smartstates until a new row would probably be an optimal solution.

@Ddystopia
Copy link
Author

Are you going to continue working on that crate? Or is it dead?

@Yandrik
Copy link
Owner

Yandrik commented Dec 20, 2024

I do plan to continue working on the crate. in fact, there's quite a few changes on the optimization's branch that will be merged into the main branch soon, and then a 0.0.1 release to be done. I don't have a lot of free time at the moment sadly, so I'm not sure what the frequency of updates will be, but even in its current state, Kolibri is pretty usable for now. I'm using it in production on our homebuilt injection molding machine, and it hasn't shown any problems so far.

However, workarounds like the one discussed in this issue will remain necessary for a while. This is mostly the case because of the way the immediate mode paradigm works.
Even so, overall I think there's still a lot of ergonomic improvements that can / should be done, but they aren't essential to the crate functioning.

@Yandrik
Copy link
Owner

Yandrik commented Dec 27, 2024

The example is fixed in #9 using one of the methods proposed above, namely keeing the string length constant

@Ddystopia
Copy link
Author

Ddystopia commented Dec 27, 2024

I think it is not really an acceptable as a general solution... What if user wants length to change? It more of a hack I would say.

@Yandrik
Copy link
Owner

Yandrik commented Dec 27, 2024

That's a fair argument. Kolibri has an alternative mechanism for this (at the moment wrongly called clear_to_end_of_col(), to be renamed to clear_to_end_of_row() I just noticed), which would allow this.

This would look something like this:

// ...
let mut len = 0;
loop {
    let new_len = ...;
    ui.add(Label::new(...).smartstate(...));
    if new_len =! len {
        len = new_len;
        ui.clear_to_end_of_col(); // clear to the end of the current UI
        smartstates.peek().force_redraw(); // ensure button is redrawn
    }
    // add the button
}

However, I think an even better solution would be to give the labels a fixed size property, with an align parameter of how the labels should be aligned in this fixed size (e.g. Center or Left). This would give a more intuitive way of doing this than the available methods.

I'll see if I can implement this fixed size property - I think it should be pretty easy. There's some text centering in the optimizationsbranch already, so it should be pretty similar to that

@Ddystopia
Copy link
Author

Ddystopia commented Dec 27, 2024

In my opinion, optimizations are meant to improve performance without reducing use cases. If usage of smartstate requires user to change the logic of painting etc, then it is not an optimization anymore.

I think that at some point in future we would contribute and use this library. From what I see, we would not use smartstate and only trigger rerenders when state is changed. But I think it is great to give labels a fixed size.

By the way, what do you think about this? High performance no_std implementation of css flexbox and css grid algorithms https://github.com/DioxusLabs/taffy ? It would've been neet to have something like a grid or flexbox. Or is it incompatible with immediate mode ui? EDIT: egui has some prototype of flex box and egui_taffy. Found here

@Yandrik
Copy link
Owner

Yandrik commented Dec 28, 2024

The reason why Smartstates are so important to the library is because they allow performance improvements of up to 100x compared to full redraws, which, especially on slower displays like an ILI9341 over serial change the amount of FPS achieved from something like 3 to well over 60, which reduces interaction times with buttons from like a second to unnoticeably fast. This comes with drawbacks, but this optimization is essential to the entire library. You can, however, use the entire library without Smartstates, as long as you don't mind the hit in performance, or have a high bandwidth connection to your display. (I have some direct comparisons to lvgl and Slint actually, which I do plan to publish at some point soon. Real life speedups are approximately 7-to-25ish in most cases compared to those frameworks using smartstates)

Because of the immediate mode nature of the framework, Smartstates will always come with some downsides like this, and will require more mental effort by the programmer for thinking about what to redraw when and how unfortunately. However, fairly complex user interfaces are still very much possible, but do require a little bit more thinking than without the smartstate system of course.

The flexbox layout looks really cool, I definitely have to take a look at how egui solved this! Thanks for bringing this up. The major problem with this is is that layout is notoriously difficult in immediate mode GUIs because the size of all widgets in a row is not known at the time the first widget is added. My idea on how to work around this was to pass an entire array of widgets, and then implement a size function for widgets, which would then allow computing the positions in advance. To keep widget creation simple, I think a trait method that returns the bounds of the widget and in its default implementation simply panics would be a good trade-off between complexity and capability here. Widgets that allow layouting (as in can compute their size before being rendered) would then implement this method. This would need a rework of the current widget trait, but that is in order anyways, as the current implementation does not allow passing colors to widgets as the color type of the UI is not known at creation time, which makes styling tedious and requires creating sub-UIs and changing the native style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants