lvgl / lv_binding_rust

LVGL bindings for Rust. A powerful and easy-to-use embedded GUI with many widgets, advanced visual effects (opacity, antialiasing, animations) and low memory requirements (16K RAM, 64K Flash).
MIT License
687 stars 71 forks source link

Automatically delete `Obj` and related on drop #144

Open nia-e opened 1 year ago

nia-e commented 1 year ago

Yet more work towards #140, currently very broken and will require removing some quality-of-life functions i.e. new().

nia-e commented 1 year ago

Considering changing removal of functions to be behind a feature gate i.e. static-obj-memory so that platforms where the overhead is "tolerated" don't need to worry about all of this and can still make everything 'static. Alternatively, we can just have this done anyways with e.g. Box<T>::leak(mut self) -> &'static mut T which is still in the code but commented out and expose .leak() on T: NativeObject types. Would like to hear some opinions

cydergoth commented 1 year ago

One other nice feature would be a solution (macro?) to support styles in Flash not ram ....

On Wed, Jun 21, 2023, 11:13 AM Nia @.***> wrote:

Considering removal of functions to be behind a feature gate i.e. static-obj-memory so that platforms where the overhead is "tolerated" don't need to worry about all of this and can still make everything 'static. Alternatively, we can just have this done anyways with e.g. Box::leak(mut self) -> &'static mut T which is still in the code but commented out and expose .leak() on T: NativeObject types. Would like to hear some opinions

— Reply to this email directly, view it on GitHub https://github.com/lvgl/lv_binding_rust/pull/144#issuecomment-1601137244, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPWAWV66JIC6QLJA5BXUYLXMMMTDANCNFSM6AAAAAAZOU6LTU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

nia-e commented 1 year ago

How is this handled in C?

cydergoth commented 1 year ago

With a variable declaration attribute

On Wed, Jun 21, 2023, 11:17 AM Nia @.***> wrote:

How is this handled in C?

— Reply to this email directly, view it on GitHub https://github.com/lvgl/lv_binding_rust/pull/144#issuecomment-1601142085, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPWAWVNN6DTV33LOF2FHSTXMMNAVANCNFSM6AAAAAAZOU6LTU . You are receiving this because you commented.Message ID: @.***>

nia-e commented 1 year ago

Alright. I'll open an issue for it when I get back to working on this

cydergoth commented 1 year ago

See an example here: static LV_STYLE_CONST_INIT(focus_style, focus_style_props);

cydergoth commented 1 year ago

Working from this branch, I get

 (cd ~/workspace/rust/lvgl-mm/ && RUST_LOG=debug RUST_BACKTRACE=1 DEP_LV_CONFIG_PATH=$(pwd) cargo build)
  5    Compiling lvgl-mm v0.1.0 (/home/jcrisp/workspace/rust/lvgl-mm)
  6 error[E0597]: `screen_style` does not live long enough
  7    --> src/main.rs:144:34
  8     |
  9 144 |     screen.add_style(Part::Main, &mut screen_style);
 10     |     -----------------------------^^^^^^^^^^^^^^^^^-
 11     |     |                            |
 12     |     |                            borrowed value does not live long enough
 13     |     argument requires that `screen_style` is borrowed for `'static`
 14 ...
 15 187 | }
 16     | - `screen_style` dropped here while still borrowed
 17
 18 error[E0597]: `screen` does not live long enough
 19    --> src/main.rs:151:45
 20     |
 21 151 |     let root_id = build_tree(None, Box::new(&mut screen), cfg, &mut widget_tree);
 22     |                   --------------------------^^^^^^^^^^^-------------------------                                                                                                        23     |                   |                         |                                                                                                                                           24     |                   |                         borrowed value does not live long enough
 25     |                   argument requires that `screen` is borrowed for `'static`                                                                                                             26 ...
 27 187 | }
 28     | - `screen` dropped here while still borrowed
 115 async fn display() -> Result<(), LvError> {
 116     const HOR_RES: u32 = 480;
 117     const VER_RES: u32 = 320;
 118
 119     let sim_display: SimulatorDisplay<Rgb565> =
 120         SimulatorDisplay::new(Size::new(HOR_RES, VER_RES));
 121     let output_settings = OutputSettingsBuilder::new().scale(1).build();
 122     let mut window = Window::new("LVGL-MM", &output_settings);
 123
 124     // LVGL will render the graphics here first, and seed the rendered image to the
 125     // display. The buffer size can be set freely.
 126     let buffer = DrawBuffer::<{ (HOR_RES * VER_RES) as usize }>::default();
 127
 128     // Register your display update callback with LVGL. The closure you pass here will be called
 129     // whenever LVGL has updates to be painted to the display.
 130 //    let display = Display::register(buffer, HOR_RES, VER_RES, |refresh| {
 131 //        sim_display.draw_iter(refresh.as_pixels()).unwrap();
 132 //    })?;
 133
 134     println!("Before all widgets: {:?}", mem_info());
 135
 136     let mut screen_style = Style::default();
 137     screen_style.set_bg_color(Color::from_rgb((0, 0, 0)));
 138     screen_style.set_text_color(Color::from_rgb((255, 255, 255)));
 139     screen_style.set_bg_opa(Opacity::OPA_COVER);
 140     screen_style.set_radius(0);
 141
 142     // Create screen and widgets
 143     let (_display, mut screen) = lv_drv_disp_sdl!(buffer, HOR_RES, VER_RES)?;
 144     screen.add_style(Part::Main, &mut screen_style);
.....

Not sure how this is supposed to work with code which isn't in the main() function or 'static scope

nia-e commented 1 year ago

This is "intended" and fixable but needs documentation. The issue is that theoretically Box::new(x: T) requires that T lives arbitrarily long, which is fine to say for structs etc. but not for most references. I think what you could do is make the Boxing happen sooner and access the screen through it, so

let boxed_screen = Box::new(screen); // not &mut screen
boxed_screen.add_style(&mut style);

Not sure how this is supposed to work with code which isn't in the main() function or 'static scope

Assigning the style in a function that doesn't move that style into the same scope or greater than the object/screen it's assigned to is technically a use-after-free. You can get around this by leaking the style object (with mem::leak(), or by wrapping it in a ManuallyDrop) or by returning the style(s) from the called function.

nia-e commented 1 year ago

Also note that this branch is WIP and will segfault if you do anything with it - tests pass, examples don't work though. I'm currently in the middle of a move so can't really work more until wednesday, sorry.

cydergoth commented 1 year ago

No hurry, this is hobby space for me