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

Stabilizing 1.0 API #555

Open
LeonMatthesKDAB opened this issue May 29, 2023 · 7 comments
Open

Stabilizing 1.0 API #555

LeonMatthesKDAB opened this issue May 29, 2023 · 7 comments
Labels
🤔 discussion Feedback welcome 🥳🎉 1.0 This issue is part of stabilization for 1.0 release

Comments

@LeonMatthesKDAB
Copy link
Collaborator

LeonMatthesKDAB commented May 29, 2023

After discussions following #531, we believe we've found an improved API that we want to target for Cxx-Qt 1.0.

Anyone using Cxx-Qt, feel free to leave feedback on this issue.

Solved API problems

Drawbacks

  • Around 6 lines of boilerplate, compared to the current API for the example below

1.0 API Overview

The point of this API is to be even closer to CXX than before, by only putting declarations within the bridge itself.
We've found this preferable for #[inherit], as well as for #[qsignal] for a while now, but haven't found a good API for QProperty.
We believe by simply using a #[qproperty()] attribute this can be solved nicely!

// the module name now defines the module that your qobjects show up in.
#[cxxqt::bridge]
mod qobject {
    // Like an `extern "C++"` block, but every type is assumed to be a QObject/QGadget
    extern "C++Qt" {
        // Just passed through to the `extern "C++"` block without additional generation
        type QAbstractItemModel = cxx_qt_lib::QAbstractItemModel;

        type QButton;

        // Passed through straight to CXX + generates on_clicked/connect_clicked methods
        #[qsignal]
        fn clicked(self: &QButton);
    }

    #[namespace="MyNamespace"]
    enum MyEnum {
        First,
        Second
    }

    // Any type in here is a QObject that comes from a Rust struct, like with `extern "Rust"`
    extern "RustQt" {
        // This mimicks C++ pretty closely, as everything is now split into its own attribute
        #[base="QAbstractItemModel"]
        #[qml_element]
        #[qenum(MyEnum)]
        #[qproperty(i32, propOne)] // needs to access field .prop - shortcut for #[qproperty(i32, prop, READ, WRITE, NOTIFY)]
        #[qproperty(i32, propTwo, READ=get_my_prop, WRITE)]
        #[qproperty(QString, propThree, READ=get_my_prop_three, WRITE=set_my_prop_three, NOTIFY)]
        type MyObject = super::MyObject;
        // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This solves the duality of types, you define a new type
        // that's within a `mod qobject` yourself, you can choose any name you like, etc.

        // invokables are now declared in the bridge, but defined outside.
        // That solves the issues we had with `Result<..>`
        #[qinvokable]
        fn my_invokable(self: Pin<&mut MyObject>, ..., *mut OtherObject) -> Result<()>;
    
       // Signals are now just functions!
       #[qsignal]
       #[inherit]
        fn data(self: Pin<&mut MyObject>, ...) -> ...;

        // Signal and function inheritance now works the same way
        #[cxx_name = "hasChildren"]
        #[inherit]
        fn has_children_super(self: &MyObject, parent: &QModelIndex) -> bool;
        //                          ^^^^^^^^
        // We no longer have to write qobject:: inside the bridge.
        
        // setters/getters must be exposed to C++
        fn get_my_prop(self: &MyObject) -> i32;
        fn get_my_prop_three(self: Pin<&mut MyObject>) -> QString;
        fn set_my_prop_three(self: Pin<&mut MyObject>, value: &QString);
    }

    // use traits to shape code generation, similar to CXX
    impl cxx_qt::Constructor<(i32)> for MyObject {
        todo!{}
    }

    impl cxx_qt::Threading for MyObject {}
    // to disable CXXQt locking!
    unsafe impl !cxx_qt::Locking for MyObject {}
}

struct MyObject {
    pub prop: i32,

    private_stuff: MySpecialType,
}

use cxx_qt_lib::prelude::*;

// Prefix here defined by module name, no more magic type!
impl qobject::MyObject {
    fn get_my_prop(&self) -> i32 {
        todo!{}
    }
    
    fn get_my_prop_three(&self) -> QString {
        todo!{}
    }

    fn set_my_prop_three(self: Pin<&mut Self>, value: i32) {
        todo!{}
        // this should call self.notify_prop_three() in the end to signal the change.
    }

    pub fn my_invokable(self: core::pin::Pin<&mut Self>) ->  anyhow::Result<()> {
        todo! { }
    }
}
@ahayzen-kdab
Copy link
Collaborator

From #584, for auto generated getter/setter should of properties should they always be pub methods ? Then if the developer wants something different they can use the custom getter/setter?

I think this makes sense but just wanted to check if we need any more changes in the API, but seems likely not.

Original comments

what do we do about the visibility of the getter/setter/notify when they are auto generated?

At the moment we always generate them as pub, maybe this is fine ?

Maybe if you want the getter/setter/notify to be pub(crate) or not pub you should use a custom getter/setter/notify ?

As in Qt/C++ if you have a Q_PROPERTY even if the methods aren't public you can always ( ? ) use the QMetaProperty / meta object system to read / write the properties ? So they are always "public" ?
As #[qproperty(T, NAME)] is supposed to be a shorthand for #[qproperty(T, NAME, read = Self::my_getter, write = Self::my_setter)], to me it seems that the shorthand could just always generate as pub visibility then custom stuff can do anything else.

Otherwise we'd need the API to be #[qproperty(pub, T, NAME)] or #[qproperty(pub T, NAME)] #[qproperty(T, pub NAME)] or something thinking

@ahayzen-kdab
Copy link
Collaborator

Does #[qinvokable(cxx_override)] become

#[qvinokable]
#[cxx_override]

As we need to be able to support making an override on just a C++ method and not a qinvokable as well ? eg #298

@LeonMatthesKDAB
Copy link
Collaborator Author

Does #[qinvokable(cxx_override)] become

#[qvinokable]
#[cxx_override]

As we need to be able to support making an override on just a C++ method and not a qinvokable as well ? eg #298

I think that makes sense, yes.

@axelkar
Copy link

axelkar commented Oct 5, 2023

Is the following code legacy/deprecated? Should I use extern "RustQt" like in the examples folder?
Edit: Is the book up to date?
Edit2: How do I include cxx-qt-gen headers in a CMake project directly in C++, not QML?

#[cxx_qt::qobject]
pub struct FooBar {
    #[qproperty]
    number: i32,
    #[qproperty]
    string: QString,
}

// ...

impl qobject::FooBar {
    #[qinvokable]
    pub fn increment_number(self: Pin<&mut Self>) {
        let previous = *self.as_ref().number();
        self.set_number(previous + 1);
    }

    #[qinvokable]
    pub fn say_hi(&self, string: &QString, number: i32) {
        println!("Hi from Rust! String is '{string}' and number is {number}");
    }
}

@LeonMatthesKDAB
Copy link
Collaborator Author

LeonMatthesKDAB commented Oct 6, 2023

Hi @axelkar ,

happy to hear you're interested in CXX-Qt.
Yes, the code you're showing is unfortunately going to be deprecated with the next release (we're aiming to release in the next few weeks).
For most of our public documentation (e.g. the book and docs.rs) it always shows the currently stable version.
The book is therefore only up-to-date to the current release, not the current development version.
We will of course update the book with the next release as well.
If you want to preview the new book, take a look at PR #687 .

The core concepts of CXX-Qt are not going to change. However the API will be different and a lot closer to CXX.
Porting to 0.6 should be a relatively straight-forward, if a bit boring task.
If you're feeling adventurous, feel free to depend directly on CXX-Qt main to try out the new API.
Most existing 0.5 features are working there with the new API already, though some details can change without notice on master, as CXX-Qt is not stable yet.

In our opinion, the new API is ever so slightly more verbose, but it should make the aim of CXX-Qt much clearer.
The goal is to provide a bridge similar to CXX that just declares, but doesn't define the concepts that should be available on both sides of the bridge. CXX-Qt just adds qobject, qenum, qsignals, qinvokables and other Qt concepts to this bridge.
This should make it easy to learn CXX-Qt for existing CXX, as well as Qt developers, as the API is very similar to existing CXX, as well as to a normal QObject declaration in C++. That's why we think that's the API we can stabilize on.

We're always looking for feedback, so please feel free to leave any thoughts, concerns and suggestions under this issue 😊

@LeonMatthesKDAB
Copy link
Collaborator Author

Regarding your second question:
Take a look at the CMakeLists.txt in the qml_minimal example.
CXX-Qt's output directory is controlled using the CXXQT_EXPORT_DIR environment variable.
Inside that each crate gets its own directory.
Inside each crates directory, you'll find header files in these subdirectories:

  • cxx-qt-gen: The files generated by this crate
  • cxx-qt-common: Header files necessary for CXX-Qt core functions
  • cxx-qt-lib (optional): Header files necessary for cxx-qt-lib wrapped types
  • rust: Header files needed by CXX

So to include the header file for the qobject generated by rust/src/cxxqt_object.rs inside qml_minimal you'd need to write:

#include <cxx-qt-gen/qobject.cxxqt.h>

As the ${CXXQT_EXPORT_DIR}/${CRATE} is added to the include path.

@ahayzen-kdab
Copy link
Collaborator

We need to figure out a plan for QFlags maybe something with https://crates.io/crates/bitflags and https://crates.io/crates/flagset etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤔 discussion Feedback welcome 🥳🎉 1.0 This issue is part of stabilization for 1.0 release
Projects
None yet
Development

No branches or pull requests

3 participants