kas-gui / kas

Another GUI toolkit
Apache License 2.0
892 stars 27 forks source link

Widget pods / method shims #407

Closed dhardy closed 9 months ago

dhardy commented 10 months ago

In Druid, any widget must be stored in a WidgetPod. This is achieved by requiring containers store this type, not some W: Widget.

WidgetPods or an alternative mechanism would enable the library to insert shim code around some widget methods, e.g. Layout::draw.

Motvation (in brief): attaching WidgetId to draw context (see DrawCx::recurse), avoiding the repetitive if at the start of all user-defined fn find_id methods, widget status tracking.

Want: wrapper fns around Layout::draw etc.

Problem: user implements Layout::draw etc. but this impl does not behave quite the way we'd like it to; we'd like to automatically wrap this with another method which is the one that gets called.

Solution 1

Use two different method names, e.g. impl_draw and do_draw. They may or may not live in the same trait.

This is workable and easy to understand, if a little messy. Users may be confused that both self.child.do_draw(cx) and self.child.impl_draw(cx) appear to work.

Solution 2

Use the same method name on a different trait, e.g. Layout::draw and Widget::draw (where Layout is not dyn-safe).

Problem is error[E0034]: multiple applicable items in scope.

Solution 3

The user implements Layout::draw, the #[widget] macro implements Widget::draw and an inherent method draw on the defined type. The compiler should resolve the correct method given a Layout, a Widget or a concrete type.

Problem: the user only sees the impl for Layout and may be surprised by differing behaviour and multiple methods (i.e. it is confusing). This may function as expected but could easily cause other problems.

Solution 4

Store all widgets in a "pod", like Druid.

Have constructors and chain methods return the inner "widget" type. Pod<W> may support From<W>.

To support introspection we require a dyn-safe API on the pod. We do not require this on the inner "widget", hence trait Widget would be implemented by the pod, not the inner widget.

Very likely the pod would support Deref and DerefMut to the inner "widget" so that users can easily call custom methods on typed child widgets.

Solution 4a

#[widget] fields are automatically type-wrapped with Pod<..>. This partially hides the Pod wrapper but is confusing; e.g. constructed widgets must be pod-wrapped via .into() and error messages will use the Pod<..> type.

The problem of 3 applies, but slightly differently: users see child #[widget] foo: Foo where Foo: Widget, and they even see the definition of <Foo as Layout>::draw, yet it acts slightly differently.

Solution 4b

#[widget] fields must have type Pod<W> where W: Widget making the pod explicit. This is much clearer than (4a).

Example: adaptation of ListEntry from examples/data-list.rs:

impl_scope! {
    // The list entry
    #[widget{
        layout = column! [
            row! [self.label, self.radio],
            self.edit,
        ];
    }]
    struct ListEntry {
        core: widget_core!(),
        #[widget(&())]
        // NOTE: use of Pod<...>
        label: Pod<StringLabel>,
        #[widget(&data.active)]
        radio: Pod<RadioButton<usize>>,
        #[widget]
        edit: Pod<EditBox<ListEntryGuard>>,
    }

    impl Events for Self {
        type Data = Data;

        fn handle_messages(&mut self, cx: &mut EventCx, data: &Data) {
            if let Some(SelectEntry(n)) = cx.try_pop() {
                if data.active != n {
                    cx.push(Control::Select(n, self.edit.get_string()));
                }
            }
        }
    }

    impl Self {
        fn new(n: usize) -> Self {
            ListEntry {
                core: Default::default(),
                // NOTE: use of .into()
                label: Label::new(format!("Entry number {}", n + 1)).into(),
                radio: RadioButton::new_msg(
                    "display this entry",
                    move |_, active| *active == n,
                    move || SelectEntry(n),
                ).into(),
                edit: EditBox::new(ListEntryGuard(n)).with_text(format!("Entry #{}", n + 1)).into(),
            }
        }
    }
}

Solution 5

Wrap all widgets with a Pod such that the widget has an inner type implementing Layout, Events and and an outer type implementing the dyn-safe Widget API. This is like (4), but give the wrapped type a public name (e.g. pub type Button = Pod<ButtonInner>).

Solution 5a

Implicitly rename the type declared by #[widget] and declare a wrapping type with the original name.

Problem: user-defined constructors like Foo::new just won't work as expected (well, a macro could modify/wrap them, but only around those declared within the impl_scope! and only where the output type is Self or a known derivative like Option<Self> or Result<Self, _> — this would definitely cause some confusing issues).

Solution 5b

Widget definitions explicitly declare an inner type and an outer type, implementing Events, Layout on the inner type and public inherent methods (including constructors) on the outer type.

Problem: custom widget definitions are somewhat verbose. impl_scope! and singleton! are designed for the definition of a single type.

Example: adaptation of ListEntry from examples/data-list.rs:


impl_scope! {
    // The list entry
    #[widget{
        layout = column! [
            row! [self.label, self.radio],
            self.edit,
        ];
    }]
    struct ListEntryImpl {
        core: widget_core!(),
        #[widget(&())]
        label: StringLabel,
        #[widget(&data.active)]
        radio: RadioButton<usize>,
        #[widget]
        edit: EditBox<ListEntryGuard>,
    }

    impl Events for Self {
        type Data = Data;

        fn handle_messages(&mut self, cx: &mut EventCx, data: &Data) {
            if let Some(SelectEntry(n)) = cx.try_pop() {
                if data.active != n {
                    cx.push(Control::Select(n, self.edit.get_string()));
                }
            }
        }
    }
}
// NOTE: type alias; impl outside of impl_scope!
type ListEntry = Pod<ListEntryImpl>;
impl ListEntry {
    fn new(n: usize) -> Self {
        Pod::new(ListEntryImpl {
            core: Default::default(),
            label: Label::new(format!("Entry number {}", n + 1)),
            radio: RadioButton::new_msg(
                "display this entry",
                move |_, active| *active == n,
                move || SelectEntry(n),
            ),
            edit: EditBox::new(ListEntryGuard(n)).with_text(format!("Entry #{}", n + 1)),
        })
    }
}

Solution 6

The dyn-safe widget API has no public methods; only hidden ones. This forces users to interact with the widget API via methods provided by contexts (e.g. draw.recurse(&mut self.child)) or extension traits (e.g. fn WidgetExt::id) when using a dyn-widget child.

Problem: Layout is used as a dyn-safe API (e.g. for fn get_child). We would need separate traits for the implementation target (which could be merged with Events) and the dataless dyn-safe API.

Problem: when using a typed child widget users may be tempted to call self.child.draw(cx) and the like; we can document against this, but still, it's the obvious way to draw the child and it mostly works.

Minor problem: Layout::find_id needs to recurse but has no context. We can pass cx: &mut ConfigCx.

Solution 6b

As above, but pass all contexts by value without being publically clonable. This forces users not to do e.g. self.child.draw(cx) except for the ultimate use of cx.

It also forces users to write mut cx: DrawCx where any mut method is used.

Solution 7

Give up: keep draw.recurse(&mut self.child) for that particular issue, and continue to allow users to call self.child.draw(cx) directly.

Solution 8

Custom widget definitions do not implement a trait; instead a custom (macro-parsed) DSL is used.

This already is the case to some extent: e.g. the layout property.

Problem: like Slint, this alienates the toolkit from the Rust language. Some things become easier, but users have more to learn, especially when it (inevitably) comes to interaction between the DSL and Rust.

Example of soln 8

This version still has "Rustic" syntax but not true Rust (traits).

Advantages:

Disadvantages:

Solution 9

Let the #[widget] macro modify methods. We already require that, if a user provides a definition of Layout::draw , this be within the impl_scope!; technically this is sufficient to let the macro modify the method definition (for example, by prepending a line like draw.set_id(self.id());).

This solution is relatively simple. There could be issues parsing method definitions in macros, but likely injecting complete statements at the start of method is sufficient to solve all motivating issues, in which case the only parsing needed is to check for leading inner attributes.

Problem: methods don't behave as written. Should documentation mention that fn Layout::find_id behaves slightly differently when called to how it appears? If not, should we avoid modifying this method and limit modifications to those with less obvious side-effects? Even then, this approach could probably cause confusing bugs.

Conclusion

Most of the above "solutions" cause confusion of the form "user implemented fn draw but it behaves differently when called". To some extent, this is the intention of the "solutions", yet there is too much potential for bugs and confusion.

Essentially, we have to choose between:

dhardy commented 10 months ago

Side note: Druid uses WidgetPod to store toolkit data. Kas uses widget_core!() to store, among other things, child widgets defined via macro layout syntax; moving these to the WidgetPod (outside the user-defined type) would imply that Layout methods could only be implemented on the pod (Pod<W> not W for user-defined W). It would also mean the WidgetId and Rect would have to be either explicitly stored by all user-defined widgets or passed through the "context" parameters. While none of these issues are blockers, they do appear to out-weigh the advantage of removing widget_core!(), hence this is not proposed for the above options using a Pod type.

dhardy commented 9 months ago

Solution 9 is easily the simplest in terms of impact on user code. We can avoid confusion over behavioural differences by limiting modifications to those with no observable effect beyond assertion-failure in erroneous scenarios.

Status: this is not solved, but #410 removes the strongest motivation for a resolution here. I will close for now since none of the above solutions have are especially appealing.