Connecting to the 'draw' signal

[cameronwhite]

For https://gtk-rs.org/docs/gtk/trait.WidgetExt.html#tymethod.connect_draw the callback is expected to return Inhibit instead of nothing, which causes issues since the provided callback is expected to return a Message type and is wrapped in another function that always returns ()

To reproduce, add <DrawingArea on draw=|w, c| Message::Exit /> to the UI in examples/inc/src/main.rs

[LiHRaM]

I'm blocked on this problem as well. The code is generated in https://github.com/bodil/vgtk/blob/master/macros/src/gtk.rs#L273-L288. After calling scope.send_message(msg);, we want to return vgtk::lib::glib::signal::Inhibit(true|false) if the return type of object.#connect is Inhibit.

I'm not sure how to deduce the type, but I'm going to see if I can come up with something today.

[LiHRaM]

So I feel like this should work, but it doesn't:

let inhibit = if body_string.contains("Inhibit(true)") {
    quote!(vgtk::lib::glib::signal::Inhibit(true))
} else if body_string.contains("Inhibit(false)") {
    quote!(vgtk::lib::glib::signal::Inhibit(false))
} else {
    quote!()
};
let inner_block = if async_keyword.is_some() {
    quote!({
        let scope = scope.clone();
        vgtk::lib::glib::MainContext::ref_thread_default().spawn_local(
            async move {
                let msg = async move { #body_s }.await;
                scope.send_message(msg);
                #inhibit
            }
        )
    })
} else {
    quote!({
        let msg = { #body_s };
        scope.send_message(msg);
        #inhibit
    })
};

It still complains about the same thing, although cargo expand shows me a valid expansion, with the correct returning of Inhibit.

[cameronwhite]

Maybe another idea would be to allow returning a tuple with the message type and the callback's return type? I'm not sure offhand though if there are any other GTK callbacks with return types other than Inhibit

[bodil]

I'm honestly not sure this is a problem that can be solved at all in a straightforward way. The tuple return value sounds compelling, but I'm not sure it's going to be possible to implement - I imagine it would require some creative use of the From trait if it's even feasible. I'll give it a go, but not sure I can promise anything...

[LiHRaM]

The Tuple return value was a great idea!

I was able to get it working by deconstructing the result of { #body_s } and returning the second tuple element.

In gtk.rs:

let inner_block = if async_keyword.is_some() {
    quote!({
        let scope = scope.clone();
        vgtk::lib::glib::MainContext::ref_thread_default().spawn_local(
            async move {
                let (msg, ret) = async move { #body_s }.await;
                scope.send_message(msg);
                ret
            }
        )
    })
} else {
    quote!({
        let (msg, ret) = { #body_s };
        scope.send_message(msg);
        ret
    })
};

Used like so:

<Application::new_unwrap(Some("example.signal.app"), ApplicationFlags::empty())>
    <Window on destroy=|_| (Message::Exit, ())>
        <DrawingArea on draw=|_,_| (Message::NoOp, Inhibit(false)) />
    </Window>
</Application>

This needs some more work to work with custom components though. The PropTransform trait impls need to handle it right, and I have no idea how to work with traits atm. :man_shrugging:

[piegamesde]

To complicate things even further: Inhibit is not the only type that a callback may be required to return. I've seen bool as well (essentially doing about the same as an Inhibit), and we cannot be sure about any other types in the future.

[emirror-de]

Hey there, will there be a decision in the near future about how to fix this?

I just tried the fork of marhkb, by adding a <Switch on state_set=|_, _| (Message::Inc, Inhibit(false)) /> to the inc example and it works fine. Would it be a possible solution to merge it?

I would love to use vgtk in my next project, but this issue keeps me hesitating.

[pepijndevos]

I have found a potential workaround for this problem.

<Layout on realize=|l| { l.connect_draw(draw_layout); Message::None } />

I just added a dummy message because the update function isn't supposed to do anything with it I think.

I think the downside of this is that the draw callback can't send a message to the update function. So the procedural Cairo drawing code will just live in its own little bubble. Coming to think of it, it also can't access the state, which makes it kind of useless beyond drawing hello world?

[pepijndevos]

If I understand the solution by @LiHRaM correctly, that would require all signals to return a tuple, which is a breaking change and kinda inconvenient when most return ().

This needs some more work to work with custom components though. The PropTransform trait impls need to handle it right, and I have no idea how to work with traits atm. :man_shrugging:

Could you clarify what is the problem here? Trying to look into it now.

I came up with a conversion trait and a blanket implementation that lets you return your Message type, and implement this trait to tell how your message type maps to Inhibit for example. This is backwards compatible, but I'm not sure if it needs special handling for subcomponents.

I think this does not need special handling int subcomponents, since a subcomponent is never going to require returning an Inhibit. Or is there some black magic going on that would break on draw in subcomponents?

[duvholt]

I hacked together a syntax addition to solve this issue for myself a while ago (and to learn more about lalrpop): https://github.com/duvholt/vgtk/commit/620ca4ff8e2a323e903b0b47838fa7d2b3096af7

Return values from handlers can be specified like this: <DrawingArea on draw=|w, c| Message::Exit and_return=Inhibit(false) />

I don't really remember why I went with and_return as I don't think it's a good name, but the idea is to allow specifying the return value as a separate property. This is a backwards compatible change, but the syntax is a bit weird.

[pepijndevos]

Oooh! I was completely unaware of lalrpop. I guess that explains some of the macro magic. I'll look into that, thanks.

In terms of actually solving this issue, what do you think of the different solutions so far? Any downsides to my trait compared to your macro syntax? I think I mostly agree with Bodil that it'd be good to minimize special syntax. It makes things less complex and less surprising.

[pepijndevos]

Okay, I merged my solution, but will keep this issue open for using the draw signal to do anything useful. If I try to use my model in the handler, it will complain that the closure captures it and it can't be moved, even if you try to clone it. I feel like this should be possible somehow, but can't figure out a way that Rust allows the state to be moved into the closure. Any ideas?

error[E0507]: cannot move out of `state`, a captured variable in an `Fn` closure
   --> src/main.rs:175:59
    |
170 |         let state = self.clone();
    |             ----- captured outer variable
...
175 |                     <Layout on draw=|l, cr| { draw_layout(state, l, cr); Message::None }
    |                                                           ^^^^^ move occurs because `state` has type `Model`, which does not implement the `Copy` trait
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0507]: cannot move out of `state`, a captured variable in an `Fn` closure
   --> src/main.rs:175:43
    |
170 |            let state = self.clone();
    |                ----- captured outer variable
171 |  /         gtk! {
172 |  |             <Application::new_unwrap(Some("nl.pepijndevos.mosaic"), ApplicationFlags::empty())>
173 |  |                 <Window default_width=500 default_height=500
174 |  |                         border_width=20 on destroy=|_| Message::Exit>
175 |  |                     <Layout on draw=|l, cr| { draw_layout(state, l, cr); Message::None }
    |  |                                           ^               -----
    |  |                                           |               |
    |  |                                           |               move occurs because `state` has type `Model`, which does not implement the `Copy` trait
    |  |___________________________________________|               move occurs due to use in closure
    | ||
176 | ||                             /*on motion_notify_event=|l, e| Message::Coord(e.get_coords().unwrap())*/>
177 | ||                         <Image Layout::x=220 Layout::y=200 pixbuf=Some(self.nmos.clone())/>
178 | ||                         <Image Layout::x=240 Layout::y=220
...   ||
220 | ||             </Application>
221 | ||         }
    | ||_________- in this macro invocation
...   |
    |
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

I'm kinda thinking of adding a new API to vgtk::ext that handles it better.

I guess the dream would be a JSX SVG vgtk kinda thing, but that might be out of my league for now. Anything that allows me to draw things based on my model would be nice.

[emirror-de]

Hey @pepijndevos, have you tried <Layout on draw=move |l, cr| { draw_layout(state, l, cr); Message::None } already?

[pepijndevos]

The !gtk macro does not support it it seems.

error: expected "async" or "|"

The macro would have to be expanded to allow move, I think?

[emirror-de]

Seems like. Another approach would be to define the callback method outside of the gtk! macro and passing the input parameters to it. Something like:

let callback = move |l, cr| { draw_layout(state, l, cr); Message::None };
...
gtk! {
<Layout on draw=|l, cr| callback(l, cr) ...

I tried it on a small example on a slider that I had locally and it worked. The compiler only wanted to know the parameter type for the callback, so you will be required to add it.

[pepijndevos]

I started adding a move keyword to the parser, but then stopped to look at the expanded output, and it seems all the closures are already wrapped in move:

                            handlers.push(VHandler {
                                name: "clicked",
                                id: "#0 bytes(1331..1332)",
                                set: std::boxed::Box::new(
                                    move |object: &vgtk::lib::glib::Object, scope: &Scope<_>| {
                                        use vgtk::lib::glib::object::Cast;
                                        let object: &Button =
                                            object.downcast_ref().unwrap_or_else(|| {
                                                ::std::rt::begin_panic_fmt(
                                                    &::core::fmt::Arguments::new_v1(
                                                        &[
                                                            "downcast to ",
                                                            " failed in signal setter",
                                                        ],
                                                        &match (&Button::static_type(),) {
                                                            (arg0,) => {
                                                                [::core::fmt::ArgumentV1::new(
                                                                    arg0,
                                                                    ::core::fmt::Debug::fmt,
                                                                )]
                                                            }
                                                        },
                                                    ),
                                                )
                                            });
                                        let scope: Scope<_> = scope.clone();
                                        object.connect_clicked(move |_| {
                                            let msg = { Message::Inc };
                                            scope.send_message(msg);
                                        })
                                    },
                                ),
                            });

So why does it work to define your own move callback, but complains if you use the existing closure that is already moved. Is there some non-move wrapper that I'm missing?

While looking into that I ran into this proc-macro-hack stuff that I don't understand, and from there found that it is superseded by native support for whatever it was doing. But so far I've been unsuccessful in untangling this mess to get rid of it.