gtk-rs / gtk-rs-core

Rust bindings for GNOME libraries
https://gtk-rs.org/gtk-rs-core
MIT License
279 stars 112 forks source link

[FEATURE REQUEST] SignalProxy #1219

Open ranfdev opened 10 months ago

ranfdev commented 10 months ago

This is an exploratory idea. I welcome everyone who want's to provide feedback.

In Vala, you can work with signals by doing

btn.clicked(); // emit
btn.clicked.connect(fun);
btn.clicked.connect_after(fun); // after default handler

While in Rust we currently do

btn.emit_clicked(); // emit
btn.connect_clicked(fun);
btn.connect("clicked", true, fun); // after default handler

Clearly we are losing the type checker with the last statement;

But I don't think that's required. We can introduce a type

struct SignalProxy<'a, T: 'a, const PROPERTY_NUMBER: usize> {
  object: &'a T
}

// let's say 1 is the index of the signal `clicked` of `GtkButton`
impl<'a> SignalProxy<'a, gtk::Button, 1> {
  const NAME: &str = "clicked";
  fn connect(&self, f: impl FnMut(&'a T) -> ()) {
    self.object.connect(Self::NAME, false, f);
  }
  fn connect_after(&self, f: impl FnMut(&'a T) -> ()) {
    self.object.connect(Self::NAME, true, f);
  }
  fn emit(&self) -> ()) {
    self.object.emit(...);
  }
}

We can then return a SignalProxy from a clicked() call, in a similar way to what Vala is doing.

btn.clicked().emit();
btn.clicked().connect(fun);
btn.clicked().connect_after(fun);
// We also unlock the ability to do a lot of other things, while still being type safe
btn.clicked().stop_emission();
btn.clicked().metadata(); // returns `glib::subclass::signal::Signal`

The proposed implementation doesn't bring a runtime performance impact, because SignalProxy is really just a reference to the object, so btn.clicked().emit() is exactly equivalent to the current btn.emit_clicked().

Conclusions

The proposed API is a zero-cost abstraction to provide additional type-safe methods to work with signals. It also provides a useful association between a signal and it's name in the Rust type system, with SignalProxy::NAME, which is currently missing.

sdroege commented 10 months ago

const PROPERTY_NUMBER: usize

What is this good for? Doesn't seem to be used.

fn connect(&self, f: impl FnMut(&'a T) -> ()) {

FnMut is not correct for signal handlers. They can be called a) from multiple threads at once depending on the object, b) even in a single threaded application the same signal handler can be called from inside itself again.

fn connect(&self, f: impl FnMut(&'a T) -> ()) {

How do you want to handle the arbitrary parameters and return type of the signals here and in emit()?

The proposed API is a zero-cost abstraction to provide additional type-safe methods to work with signals. It also provides a useful association between a signal and it's name in the Rust type system, with SignalProxy::NAME, which is currently missing.

I like the approach and had a similar idea in the past, just never got to follow up on it. IMHO all the connect_specific_signal_name() should be replaced by something like this.

bilelmoussaoui commented 10 months ago

How do you want to handle the arbitrary parameters and return type of the signals here and in emit()?

Right, that is the main issue I had when I tried to rewrite the generated signals code to go through ObjectExt methods instead of ffi.

ranfdev commented 10 months ago

The generic constant SIGNAL_NUMBER enables us to impl SignalProxy multiple times.

That means we can have two different functions, SignalProxy<Button, 1>::emit(&self, data) and SignalProxy<Button, 2>::emit(&self, data, additional_parameter_specific_for_this_signal).

Without the generic constant we cannot have different emit functions for different signals. Also, the index of the signal becomes useful when retrieving the metadata of the signal from the list returned by ObjectImpl::signals

sdroege commented 10 months ago

Wouldn't it make sense to make this a const SIGNAL_NAME: &str instead of the counter and an associated constant then?

ranfdev commented 10 months ago

Wouldn't it make sense to make this a const SIGNAL_NAME: &str instead of the counter and an associated constant then?

I'd like to do that, but generic strings constants are not stable. usize and bool generics are stable

sdroege commented 3 months ago

As part of this it would also be good to handle g_signal_connect_object() style behaviour (disconnect when the object goes out of scope automatically)