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

cxx-qt: remove handle_property_change instead rely on QML/C++ #151

Merged
merged 1 commit into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions book/src/concepts/threading.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ This means that Rust code, such as invokables and handlers, which are directly c

We provide a solution to prevent entering deadlocks from signal connections, eg if a property change signal was connected to an invokable on the C++/QML side this wouldn't be able to acquire a lock if the property change was triggered from a Rust invokable. The solution is to post events to a queue which could cause deadlocks, eg signal emisson, these are then executed once the next event loop occurs, and crucially, after the lock from the Rust invokable is released.

If Rust code needs to listen to property changes, handlers can be implemented (eg PropertyChangeHandler) in the [RustObj Handlers](../qobject/handlers.md). These are called directly in the event loop from the Qt thread.

<div style="background-color: white; padding: 1rem; text-align: center;">

![Threading Abstract](../images/threading_abstract.svg)
Expand Down
Binary file modified book/src/images/threading_abstract.dia
Binary file not shown.
5 changes: 2 additions & 3 deletions book/src/images/threading_abstract.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 0 additions & 4 deletions book/src/qobject/data_struct.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ Note that you can also use serde on the Data struct and derive `Deserialize` and

If you want to provide default values for your QObject, then instead of deriving implement the `Default` trait for the struct `Data`.

## Property enum

An enum called `Property` is automatically generated from the names of the fields in the `Data` struct, this can then be used in the [`PropertyChangeHandler`](./handlers.md).

## Deserialisation or Serialisation

Using [Serde](https://serde.rs/) the Data struct can be (de)seralised, by adding the dervive attributes as normal.
Expand Down
13 changes: 0 additions & 13 deletions book/src/qobject/handlers.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,8 @@ Handlers are used to react to events on the Qt event loop thread. This allows Ru

The following handlers are available

* PropertyChangeHandler to handle when a property value has changed
* UpdateRequestHandler to process update requests on the Qt event loop thread, see [threading](../concepts/threading.md) for more info.

## PropertyChangeHandler

When a property defined in the [data struct](./data_struct.md) is changed, either via Rust calling a setter or via QML / C++ calling a setter, we can be notified of this change by using the `PropertyChangeHandler`.

The example below listens to the number property and `handle_property_change` is triggered when the property `number` changes. It uses a `Property` enum which is automatically generated from the names of the properties defined in the [data struct](./data_struct.md).

Note that this is called from the Qt event loop thread.

```rust,ignore,noplayground
{{#include ../../../examples/qml_features/src/handler_property_change.rs:book_macro_code}}
```

## UpdateRequestHandler

When a background Rust thread uses an `UpdateRequester` to request the Qt thread to synchronise via calling `request_update` this triggers the `handle_update_request` method of the `UpdateRequestHandler`.
Expand Down
9 changes: 0 additions & 9 deletions cxx-qt-gen/src/extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,6 @@ pub struct QObject {
pub(crate) original_passthrough_decls: Vec<Item>,
/// The Rust impl that has optionally been provided to handle updates
pub(crate) handle_updates_impl: Option<ItemImpl>,
/// The Rust impl that has optionally been provided to handle property changes
pub(crate) handle_property_change_impl: Option<ItemImpl>,
}

/// Describe the error type from extract_qt_type and extract_type_ident
Expand Down Expand Up @@ -806,9 +804,6 @@ pub fn extract_qobject(
// Determines if (and how) this object can respond to update requests
let mut handle_updates_impl = None;

// Determines if (and how) this object can respond to property changes
let mut handle_property_change_impl = None;

// Process each of the items in the mod
for item in items.drain(..) {
match item {
Expand Down Expand Up @@ -935,9 +930,6 @@ pub fn extract_qobject(
"UpdateRequestHandler" => {
handle_updates_impl = Some(original_impl.to_owned())
}
"PropertyChangeHandler" => {
handle_property_change_impl = Some(original_impl.to_owned())
}
_others => original_trait_impls.push(original_impl.to_owned()),
}
} else {
Expand Down Expand Up @@ -1024,7 +1016,6 @@ pub fn extract_qobject(
original_trait_impls,
original_passthrough_decls,
handle_updates_impl,
handle_property_change_impl,
})
}

Expand Down
78 changes: 23 additions & 55 deletions cxx-qt-gen/src/gen_cpp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,6 @@ fn generate_invokables_cpp(
fn generate_properties_cpp(
struct_ident: &Ident,
properties: &[Property],
has_property_change_handler: bool,
) -> Result<Vec<CppProperty>, TokenStream> {
let mut items: Vec<CppProperty> = vec![];

Expand Down Expand Up @@ -513,24 +512,6 @@ fn generate_properties_cpp(
// this is used for the owned member and extra pointer specific methods
let parameter_ident_pascal = parameter.ident.to_case(Case::Pascal);

let call_property_change_handler = if has_property_change_handler {
formatdoc! {
r#"
const auto handlePropertySuccess = QMetaObject::invokeMethod(this, [&]() {{
const std::lock_guard<std::mutex> guard(m_rustObjMutex);
m_rustObj->handlePropertyChange(*this, Property::{ident});
}}, Qt::QueuedConnection);
Q_ASSERT(handlePropertySuccess);
"#,
ident = parameter_ident_pascal
}
} else {
"".to_owned()
};

// TODO: we eventually want to also support handling property changes inside subobjects
// by wiring up the relevant changed signals to m_rustObj->handlePropertyChange.

// If we are a pointer type then add specific methods
if parameter.type_ident.is_ptr() {
// Pointers are stored in the C++ object, so build a member and owned ident
Expand Down Expand Up @@ -560,8 +541,6 @@ fn generate_properties_cpp(

const auto signalSuccess = QMetaObject::invokeMethod(this, "{ident_changed}", Qt::QueuedConnection);
Q_ASSERT(signalSuccess);

{call_property_change_handler}
}}
}}
"#,
Expand All @@ -575,7 +554,6 @@ fn generate_properties_cpp(
member_owned_ident = member_owned_ident,
struct_ident = struct_ident.to_string(),
type_ident = type_ident,
call_property_change_handler = call_property_change_handler,
});

// Add members to the reference and own it
Expand Down Expand Up @@ -626,8 +604,6 @@ fn generate_properties_cpp(

const auto signalSuccess = QMetaObject::invokeMethod(this, "{ident_changed}", Qt::QueuedConnection);
Q_ASSERT(signalSuccess);

{call_change_handler}
}}
"#,
ident_changed = ident_changed,
Expand All @@ -637,7 +613,6 @@ fn generate_properties_cpp(
member_owned_ident = member_owned_ident,
struct_ident = struct_ident.to_string(),
type_ident = type_ident,
call_change_handler = call_property_change_handler,
));
} else {
cpp_property.source.push(formatdoc! {
Expand All @@ -661,8 +636,6 @@ fn generate_properties_cpp(

const auto signalSuccess = QMetaObject::invokeMethod(this, "{ident_changed}", Qt::QueuedConnection);
Q_ASSERT(signalSuccess);

{call_change_handler}
}}
}}
"#,
Expand All @@ -675,7 +648,6 @@ fn generate_properties_cpp(
struct_ident = struct_ident.to_string(),
type_ident = type_ident,
member_ident = format!("m_{}", parameter.ident),
call_change_handler = call_property_change_handler,
});

// Own the member on the C++ side
Expand Down Expand Up @@ -824,33 +796,29 @@ pub fn generate_qobject_cpp(obj: &QObject) -> Result<CppObject, TokenStream> {
}

// Build CppProperty's for the object, then drain them into our CppPropertyHelper
let properties = generate_properties_cpp(
&obj.ident,
&obj.properties,
obj.handle_property_change_impl.is_some(),
)?
.drain(..)
.fold(
CppPropertyHelper {
headers_includes: vec![],
headers_members: vec![],
headers_meta: vec![],
headers_public: vec![],
headers_signals: vec![],
headers_slots: vec![],
sources: vec![],
},
|mut acc, mut property| {
acc.headers_includes.append(&mut property.header_includes);
acc.headers_meta.append(&mut property.header_meta);
acc.headers_members.append(&mut property.header_members);
acc.headers_public.append(&mut property.header_public);
acc.headers_signals.append(&mut property.header_signals);
acc.headers_slots.append(&mut property.header_slots);
acc.sources.append(&mut property.source);
acc
},
);
let properties = generate_properties_cpp(&obj.ident, &obj.properties)?
.drain(..)
.fold(
CppPropertyHelper {
headers_includes: vec![],
headers_members: vec![],
headers_meta: vec![],
headers_public: vec![],
headers_signals: vec![],
headers_slots: vec![],
sources: vec![],
},
|mut acc, mut property| {
acc.headers_includes.append(&mut property.header_includes);
acc.headers_meta.append(&mut property.header_meta);
acc.headers_members.append(&mut property.header_members);
acc.headers_public.append(&mut property.header_public);
acc.headers_signals.append(&mut property.header_signals);
acc.headers_slots.append(&mut property.header_slots);
acc.sources.append(&mut property.source);
acc
},
);

// A helper which allows us to flatten data from vec of invokables
struct CppInvokableHelper {
Expand Down
64 changes: 0 additions & 64 deletions cxx-qt-gen/src/gen_rs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,43 +510,19 @@ pub fn generate_qobject_cxx(
quote! {}
};

// Define a function to handle property changes if we have one
let handle_property_change = if obj.handle_property_change_impl.is_some() {
quote! {
#[cxx_name = "handlePropertyChange"]
fn call_handle_property_change(self: &mut #rust_class_name, cpp: Pin<&mut #class_name>, property: Property);
}
} else {
quote! {}
};

// Build the import path for the C++ header
let import_path = format!("cxx-qt-gen/include/{}.cxxqt.h", ident_snake);

// Generate an enum representing all the properties that the object has
let property_enum = generate_property_enum(obj);

// TODO: ideally we only want to add the "type QString = cxx_qt_lib::QStringCpp;"
// if we actually generate some code that uses QString.

// Build the namespace string, rust::module
let namespace = obj.namespace.join("::");

// Build the properties FFI
let property_ffi = if !obj.properties.is_empty() {
quote! {
pub type Property = ffi::Property;
}
} else {
quote! {}
};

// Build the CXX bridge
let output = quote! {
#[cxx::bridge(namespace = #namespace)]
mod ffi {
#property_enum

unsafe extern "C++" {
include!(#import_path);

Expand Down Expand Up @@ -601,12 +577,10 @@ pub fn generate_qobject_cxx(
fn initialise_cpp(cpp: Pin<&mut #class_name>);

#handle_update_request
#handle_property_change
}
}

pub type FFICppObj = ffi::#class_name;
#property_ffi
};

Ok(output.into_token_stream())
Expand All @@ -627,25 +601,6 @@ fn generate_cpp_object_initialiser(obj: &QObject) -> TokenStream {
output.into_token_stream()
}

/// Generate an enum representing all the properties that a QObject has
fn generate_property_enum(obj: &QObject) -> TokenStream {
let properties = obj.properties.iter().map(|property| {
let ident_str = &property.ident.rust_ident.to_string();
let ident = format_ident!("{}", ident_str.to_case(Case::Pascal));
quote! { #ident }
});

if properties.len() > 0 {
quote! {
enum Property {
#(#properties),*
}
}
} else {
quote! {}
}
}

fn generate_property_methods_rs(obj: &QObject) -> Result<Vec<TokenStream>, TokenStream> {
// Build a list of property methods impls
let mut property_methods = Vec::new();
Expand Down Expand Up @@ -1090,26 +1045,13 @@ pub fn generate_qobject_rs(
quote! {}
};

// Define a function to handle property changes if we have one
let handle_property_change = if obj.handle_property_change_impl.is_some() {
quote! {
fn call_handle_property_change(&mut self, cpp: std::pin::Pin<&mut FFICppObj>, property: Property) {
let mut cpp = CppObj::new(cpp);
self.handle_property_change(&mut cpp, property);
}
}
} else {
quote! {}
};

let rust_struct_impl = quote! {
impl #rust_class_name {
#(#invokable_method_wrappers)*
#(#invokable_methods)*
#(#normal_methods)*

#handle_update_request
#handle_property_change
}
};

Expand Down Expand Up @@ -1176,14 +1118,10 @@ pub fn generate_qobject_rs(
if obj.handle_updates_impl.is_some() {
use_traits.push(quote! { use cxx_qt_lib::UpdateRequestHandler; });
}
if obj.handle_property_change_impl.is_some() {
use_traits.push(quote! { use cxx_qt_lib::PropertyChangeHandler; });
}
// TODO: only push if we have an opaque param or return type?
use_traits.push(quote! { use cxx_qt_lib::ToUniquePtr; });

let handle_updates_impl = &obj.handle_updates_impl;
let handle_property_change_impl = &obj.handle_property_change_impl;

// Build our rewritten module that replaces the input from the macro
let output = quote! {
Expand Down Expand Up @@ -1213,8 +1151,6 @@ pub fn generate_qobject_rs(

#handle_updates_impl

#handle_property_change_impl

fn create_rs() -> std::boxed::Box<RustObj> {
std::default::Default::default()
}
Expand Down
6 changes: 0 additions & 6 deletions cxx-qt-gen/test_inputs/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ mod my_object {
#[derive(Default)]
struct RustObj;

impl PropertyChangeHandler<CppObj, Property> for RustObj {
fn handle_property_change(&mut self, _cpp: &mut CppObj, _property: Property) {
println!("change")
}
}

impl UpdateRequestHandler<CppObj> for RustObj {
fn handle_update_request(&mut self, _cpp: &mut CppObj) {
println!("update")
Expand Down
Loading