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

LSP crash when specifying padding based on parent width #3989

Closed
bjorn opened this issue Nov 22, 2023 · 1 comment · Fixed by #4229 · May be fixed by #3997
Closed

LSP crash when specifying padding based on parent width #3989

bjorn opened this issue Nov 22, 2023 · 1 comment · Fixed by #4229 · May be fixed by #3997

Comments

@bjorn
Copy link
Contributor

bjorn commented Nov 22, 2023

Platform: Linux, Language: Rust, Editor: VS Code

The following component is crashing the LSP when I press "Show Preview":

component LspCrash inherits Rectangle {
    HorizontalLayout {
        padding-left: parent.width * 0.015;
    }
}

It appears to be triggered by the padding-left: parent.width * 0.015; line, since the crash disappears when the line is removed.

The backtrace:

thread 'main' panicked at internal/core/properties.rs:486:9:
Recursion detected
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: i_slint_core::properties::Property<T>::get
   3: i_slint_core::properties::alloc_binding_holder::evaluate
   4: i_slint_core::properties::Property<T>::get
   5: <i_slint_core::rtti::MaybeAnimatedPropertyInfoWrapper<Item,i_slint_core::properties::Property<T>> as i_slint_core::rtti::PropertyInfo<Item,Value>>::get
   6: <&dyn i_slint_core::rtti::PropertyInfo<Item,slint_interpreter::api::Value> as slint_interpreter::eval::ErasedPropertyInfo>::get
   7: slint_interpreter::eval::load_property_helper
   8: slint_interpreter::eval::eval_expression
   9: slint_interpreter::eval::eval_expression
  10: slint_interpreter::dynamic_item_tree::make_binding_eval_closure::{{closure}}
  11: i_slint_core::properties::alloc_binding_holder::evaluate
  12: i_slint_core::properties::Property<T>::get
  13: <i_slint_core::rtti::MaybeAnimatedPropertyInfoWrapper<Item,i_slint_core::properties::Property<T>> as i_slint_core::rtti::PropertyInfo<Item,Value>>::get
  14: slint_interpreter::eval::load_property_helper
  15: slint_interpreter::eval_layout::compute_layout_info::{{closure}}
  16: slint_interpreter::eval_layout::compute_layout_info
  17: slint_interpreter::dynamic_item_tree::make_binding_eval_closure::{{closure}}
  18: i_slint_core::properties::alloc_binding_holder::evaluate
  19: i_slint_core::properties::Property<T>::get
  20: <field_offset::FieldOffset<Item,i_slint_core::properties::Property<T>,field_offset::AllowPin> as i_slint_core::rtti::PropertyInfo<Item,Value>>::get
  21: slint_interpreter::eval::load_property_helper
  22: slint_interpreter::eval::eval_expression
  23: slint_interpreter::eval::eval_expression
  24: slint_interpreter::dynamic_item_tree::make_binding_eval_closure::{{closure}}
  25: i_slint_core::properties::alloc_binding_holder::evaluate
  26: i_slint_core::properties::Property<T>::get
  27: <field_offset::FieldOffset<Item,i_slint_core::properties::Property<T>,field_offset::AllowPin> as i_slint_core::rtti::PropertyInfo<Item,Value>>::get
  28: slint_interpreter::eval::load_property_helper
  29: slint_interpreter::eval::eval_expression
  30: slint_interpreter::eval::eval_expression
  31: slint_interpreter::dynamic_item_tree::make_binding_eval_closure::{{closure}}
  32: i_slint_core::properties::alloc_binding_holder::evaluate
  33: i_slint_core::properties::Property<T>::get
  34: <field_offset::FieldOffset<Item,i_slint_core::properties::Property<T>,field_offset::AllowPin> as i_slint_core::rtti::PropertyInfo<Item,Value>>::get
  35: slint_interpreter::eval::load_property_helper
  36: slint_interpreter::eval_layout::get_layout_info
  37: slint_interpreter::dynamic_item_tree::layout_info
  38: slint_interpreter::dynamic_item_tree::COMPONENT_BOX_VT::layout_info
  39: <i_slint_core::items::component_container::ComponentContainer as i_slint_core::items::Item_vtable_mod::Item>::layout_info
  40: core::ops::function::FnOnce::call_once
  41: i_slint_core::properties::alloc_binding_holder::evaluate
  42: i_slint_core::properties::Property<T>::get
  43: core::ops::function::FnOnce::call_once
  44: i_slint_core::properties::alloc_binding_holder::evaluate
  45: i_slint_core::properties::Property<T>::get
  46: <slint_lsp::preview::ui::slint_generatedPreviewUi::InnerPreviewUi as i_slint_core::item_tree::ItemTree_vtable_mod::ItemTree>::item_geometry
  47: <slint_lsp::preview::ui::slint_generatedPreviewUi::InnerPreviewUi as const_field_offset::PinnedDrop>::drop::VT::item_geometry
  48: i_slint_core::item_rendering::ItemRenderer::filter_item
  49: i_slint_core::item_tree::ItemVisitor_vtable_mod::ItemVisitorVTable::new::visit_item
  50: i_slint_core::item_tree::visit_item_tree::{{closure}}
  51: <slint_lsp::preview::ui::slint_generatedPreviewUi::InnerPreviewUi as i_slint_core::item_tree::ItemTree_vtable_mod::ItemTree>::visit_children_item
  52: i_slint_core::item_tree::ItemVisitor_vtable_mod::ItemVisitorVTable::new::visit_item
  53: i_slint_core::item_tree::visit_item_tree::{{closure}}
  54: <slint_lsp::preview::ui::slint_generatedPreviewUi::InnerPreviewUi as i_slint_core::item_tree::ItemTree_vtable_mod::ItemTree>::visit_children_item
  55: i_slint_core::item_tree::ItemVisitor_vtable_mod::ItemVisitorVTable::new::visit_item
  56: i_slint_core::item_tree::visit_item_tree::{{closure}}
  57: <slint_lsp::preview::ui::slint_generatedPreviewUi::InnerPreviewUi as i_slint_core::item_tree::ItemTree_vtable_mod::ItemTree>::visit_children_item
  58: i_slint_renderer_femtovg::FemtoVGRenderer::internal_render_with_post_callback::{{closure}}
  59: <i_slint_backend_winit::renderer::femtovg::GlutinFemtoVGRenderer as i_slint_backend_winit::renderer::WinitCompatibleRenderer>::render
  60: i_slint_backend_winit::event_loop::EventLoopState::process_event
  61: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
  62: i_slint_backend_winit::event_loop::EventLoopState::run
  63: <i_slint_backend_winit::Backend as i_slint_core::platform::Platform>::run_event_loop
  64: slint_lsp::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@ogoffart
Copy link
Member

ogoffart commented Nov 23, 2023

In order to understand this, we must understand that the preview UI uses itself Slint and uses this UI:

            i-preview-area-container := ComponentContainer {
                width: max(self.min-width, min(self.max-width, drawing-rect.width));
                height: max(self.min-height, min(self.max-height, drawing-rect.height));
            }

The ComponentContainer will get the inner's window property and create this binding:

width: i-preview-area-container.width;
height: i-preview-area-container.height;

It comes naturally that we then have this binding loop:

ComponentContainer::layout_info -> root_window-1.layoutinfo-h -> root_window-1.root-2-layoutinfo-h -> root_window-1.empty-3-layoutinfo-h -> root_window-1.empty-3-padding-left -> root_window-1.width -> i-preview-area-container.width -> i-preview-area-container.max-width -> ComponentContainer::layout_info -> load_property_helper root_window-1.layoutinfo-h !!!!

We'll have to break that loop somehow.

Note that in principle, this kind of loop already exist and should probably be forbidden by the compiler as they tend to cause issues like #2902 This works because when not using a ComponentContainer, the normal window break the layout by letting one event loop iteration in between the min-max size query and width/height adjustement. But if the loop is actually real and doesn't converge, it creates infinite changing size.

bjorn added a commit to bjorn/raccoin that referenced this issue Nov 23, 2023
It did not like this binding since it caused a binding loop in the
preview UI (see slint-ui/slint#3989).

Fortunately we can just replace it with a stretching rectangle.
ogoffart added a commit that referenced this issue Nov 23, 2023
The Window geometry depends on its constraints, so its constraints
cannot depends on its geometry

This fixes Infinitely growing layout, and other panics

Fixes #3989
Fixes #2902
ogoffart added a commit that referenced this issue Dec 28, 2023
 - make sure that the initial size is proper by calling show() on the preview ui
   after the factory has been set
 - ensure that there is no recursion if the inner layout info depends on the size
   (Fixes #3989)
 - Ensure that the geometry constraints are respected when previewing a component
   that was already resized
ogoffart added a commit that referenced this issue Jan 2, 2024
 - make sure that the initial size is proper by calling show() on the preview ui
   after the factory has been set
 - ensure that there is no recursion if the inner layout info depends on the size
   (Fixes #3989)
 - Ensure that the geometry constraints are respected when previewing a component
   that was already resized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants