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

ability to embed slint component from different source #2390

Open
ogoffart opened this issue Mar 21, 2023 · 9 comments
Open

ability to embed slint component from different source #2390

ogoffart opened this issue Mar 21, 2023 · 9 comments
Labels
enhancement New feature or request rfc Request for comments: proposals for changes roadmap Tickets that we can use for roadmapping

Comments

@ogoffart
Copy link
Member

Use case:

  • Slint Editor
  • Editor that allow to place Slint Component
  • Dynamic widget from a model
  • Dynamic load .slint files at runtime

We would introduce a new component property type, and a Embed builtin element:

component Editor {
   in property <component> foo;
   // ...
        Embed { component: foo; }
}

The generated code for the setter would be something like

fn set_foo(_ : Option<impl slint::ComponentHandle>);

We should try to create the window adapter lazily because the Embed will set it when the sub-tree is visited (by its surrounding WindowAdapter)

The geometry constraints are forwarded from the embedded component to the Embedd element.
There will be a two way binding for the width and height of the Embedd element, and its component.

If a component is used several times, we need to do either:

  1. exit the event loop with a specific error saying this hapenned
  2. just print a error in stderr
    The Embed should know its source location at runtime to give the developer some idea on where the error came from.
@ogoffart ogoffart added enhancement New feature or request roadmap Tickets that we can use for roadmapping rfc Request for comments: proposals for changes labels Mar 21, 2023
@ogoffart
Copy link
Member Author

We were also discussing that the component type shouldn't be copyable/clonable.
This would introduce no-clone type in the Slint language, they couldn't be used in property, but could be used as argument/return value to functions.
In rust / C++, we should either have no strong reference counting in the component handle (then Weak cannot be converted to strong, but we can imagine a weak.with(|_: &Foo| {...}) Or there could be a owner and more strong reference but only the owner can be passed to the slint code / made into a window.
This is probably a bit more complicated and would be too much change for now.

ogoffart referenced this issue May 4, 2023
Add that type as well;-)

This is rust-only for now!
@ogoffart
Copy link
Member Author

ogoffart commented May 5, 2023

The question is whether a component is the right property type name. Would it be confusing?

I'm sure people will try things like this

// This is a component, can i put it in a property?
component Foo { ... }

component Bar {
   foo1 := Foo { }
   // not allowed  (foo1 is an element)
   property <component> p1: foo1;  
   // not allowed. We actually need an instance of a component
   property <component> p2: Foo;
   // Grammar don't currently support that. Should it? what is the parent then?  How can i even refer to other id in the current document, that won't work without a garbage collector
   property <component> p3: Foo { width: parent.width; height: foo1.height; };
   // If we allow something like the previous line that means that Foo {} becomes an expression,
   // then the gramar should alow things like that
   property <component> p4: true ? Foo { ... } : SomethingElse { ... };
}

So i'm just afraid that the component type might be confusing and people will try to do things with it that is not what it is meant for. Another name would be component-instence

@tronical
Copy link
Member

tronical commented May 5, 2023

You're right, component is not a good name for the type. component-instance is clearer, I agree. If component-instance is a weak pointer, should there be also a strong equivalent?

@ogoffart
Copy link
Member Author

ogoffart commented May 5, 2023

Back in the meeting, we also discussed the concept of component factory. And in fact, i think we should revisit this option.
I don't have a name for it, but it is just like a model, but instead of creating a T, it creates a component.
So given this Slint code

component FooBar {
    in property <component-factory> my_component_model;
    // ...
         for xxx in 42: Rectangle {
              // ...
                  Embed {
                       component: my_component_model;
                  }
         }
}

in rust

let foobar = FooBar::new().unwrap();
// `component-factory` takes a `Rc<dyn slint::ComponentFactory>`
foobar.set_my_component_factory(Rc::new(MyComponentFactory::new()));


impl slint::ComponentFactory for MyComponentFactory {
    fn build_component(&self) -> ComponentHandle {
           let result = self.the_component_definition.create();
           // store so we can update it with property changes.
           self.current_component.borrow_mut() = result.as_weak()
           result
    }
}

@hunger
Copy link
Member

hunger commented May 7, 2023

I think the ComponentHandle will need to have it's Inner type to be defined. So we end up with a generic function. Or we just return a ComponentWeak from the build_component function (or the corresponding VRc).

The problem I am struggling with right now is that I have a ComponentVTable in my hand. That is fine, except for the parent_node function which returns None, claiming to be at the root of the tree of components.

Setting the parent node on a component is not supported via the ComponentVTable at this time. Instead of just adding a set_parent_node function, I want to wrap my component with one that basically just overrides the parent_node function (to return the right component/index) and forwards everything else. If we only look at the functionality exposed by the ComponentVTable such a wrapper should be possible and should even enable embedding the same component in different places. I am sure some detail or another (like passing size information, etc.) will break that, but I still want to give that a try on Monday.

@ogoffart
Copy link
Member Author

ogoffart commented May 8, 2023

I think the ComponentHandle will need to have it's Inner type to be defined. So we end up with a generic function. Or we just return a ComponentWeak from the build_component function (or the corresponding VRc).

True, we may need to change ComponentHandle to no longer have an Inner, or create a new trait that can be used as a trait object.

Instead of just adding a set_parent_node function, I want to wrap my component with one that basically just overrides the parent_node function (to return the right component/index) and forwards everything else.

I'm thinking having a set_parent_node may be the simpler. The wrapper option might be complicated, because the inner node of a repeated component will have a weak pointer to the non-wrapper as a parent.

@ogoffart
Copy link
Member Author

ogoffart commented May 9, 2023

think the ComponentHandle will need to have it's Inner type to be defined

Only if we use dyn but anyway, we can't return a trait from a function in an object safe trait. So we'll have to do something like

/// Wrapper around something that implements ComponentHandle.
/// Can be constructed with `ComponentInstance::from(my_component)` where my_component immplements
/// the ComponentHandle trait. (eg: slint_interpreter::ComponentInstance or the type generated from 
/// the slint code.
///
/// FIXME: naming?
pub struct ComponentInstance(crate::component::ComponentRc);
impl<T: ComponentHandle> From<T> for ComponentInstance
where
    T::Inner: vtable::HasStaticVTable<ComponentVTable> + 'static,
{
    fn from(value: T) -> Self {
        Self(vtable::VRc::into_dyn(value.as_weak().inner().upgrade().unwrap()))
    }
}


/// The type `component-factory`  in slint maps to a Rc<dyn ComponentFactory>
pub trait ComponentFactory {
    fn build(&self) -> ComponentInstance;
}

This is going to make C++ a bit tricky because Rc<dyn ComponentFactory> is not ffi safe, so we'll have to use the same trick as with WindowAdapterRcOpaque. That adds one memory allocation indirection, but the alternative is to use a vtable::VRc<ComponentFactoryVTable> or a wrapper around it, and that sucks.

@ales-tsurko
Copy link

I see ComponentFactory in the rust docs. Can I somehow use it to add components at the runtime?

@hunger
Copy link
Member

hunger commented Sep 18, 2023

Only if you set SLINT_ENABLE_EXPERIMENTAL_FEATURES=1 in your environment while building slint.

We do have an experimental feature adding a component-factory type as well as a ComponentContainer element to Slint. This is currently rust only and we have not committed to the API (or even to the overall feature!) yet. So use with a lot of care :-)

But yes, the idea is to allow arbitrary components (e.g. generated by the slint interpreter or by some dynamically loaded rust code) by having a ComponentContainer element and setting/changing its component factory.

Again: Use with care, this may or may not land in Slint and it will definitely change API/ABI before it becomes stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rfc Request for comments: proposals for changes roadmap Tickets that we can use for roadmapping
Projects
None yet
Development

No branches or pull requests

4 participants