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

register QML types at compile time #241

Closed
Be-ing opened this issue Sep 8, 2022 · 17 comments
Closed

register QML types at compile time #241

Be-ing opened this issue Sep 8, 2022 · 17 comments
Assignees
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 🙋 good first issue Good for newcomers
Milestone

Comments

@Be-ing
Copy link
Contributor

Be-ing commented Sep 8, 2022

This feature was introduced in Qt 5.15 and allows for getting rid of the qmlRegisterType boilerplate in C++:

https://www.qt.io/blog/qml-type-registration-in-qt-5.15
https://doc-snapshots.qt.io/qt5-5.15/qtqml-cppintegration-definetypes.html

I suggest an API something like:

#[cxx_qt::qobject(qml_element)]
pub struct MyObject {
    #[qproperty]
    number: i32,
    #[qproperty]
    string: UniquePtr<QString>,
}

If an application wants to support Qt < 5.15, I think they could simply omit the qml_element from the attribute and call qmlRegisterType in their C++.

Removing the requirement of calling qmlRegisterType would make it feasible to launch a QML application from Rust with bindings to QGuiApplication and QQmlApplication without needing to write any C++.

@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 8, 2022

This is a prerequisite for ahead-of-time compilation of QML: #242

@LeonMatthesKDAB
Copy link
Collaborator

I think we should even register the type as a qml type by default?
i.e. instead of opt-in, opt-out of QML.

E.g.:
Instead of cxx_qt::qobject(qml_element), use cxx_qt::qobject(cpp_only) to register as a cpp only type.

@LeonMatthesKDAB
Copy link
Collaborator

Ah, also we should probably have options for QML_IMPORT_NAME/QML_IMPORT_MAJOR_VERSION in our cmake 🤔
Such that we make sure they are set to some sane default.

@Be-ing
Copy link
Contributor Author

Be-ing commented Sep 12, 2022

Qt doesn't automatically register all QObjects as QML types by default, so I don't think we should.

@ahayzen-kdab
Copy link
Collaborator

I think we should follow Qt as this then links to doing singletons which would then look like this (from #25 )

#[cxx_qt::qobject(qml_element, qml_singleton)]
pub struct MyObject {

@ahayzen-kdab
Copy link
Collaborator

ahayzen-kdab commented Nov 1, 2022

Ah, also we should probably have options for QML_IMPORT_NAME/QML_IMPORT_MAJOR_VERSION in our cmake thinking Such that we make sure they are set to some sane default.

Looks like this would be set by

qt_add_qml_module(my_target
  URI com.mycompany.qmlcomponents
  VERSION 1.0
)

However i think this was only added 6.x series, so how would you set this in 5.15 ? https://doc.qt.io/qt-6/qt-add-qml-module.html#qt6-add-qml-module

@ahayzen-kdab ahayzen-kdab added ⬆️ feature New feature or request 🙋 good first issue Good for newcomers 🤔 discussion Feedback welcome labels Nov 2, 2022
@LeonMatthesKDAB
Copy link
Collaborator

LeonMatthesKDAB commented Nov 3, 2022

Hm, maybe that's not possible in Qt 5 with CMake at all?
https://stackoverflow.com/a/64168976

We could only enable the feature if Qt6 is used...
And require Qt5 users to manually register their QML types :/

@vimpostor
Copy link
Member

@ahayzen-kdab I am working on this issue, can you assign it to me? :)

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 1, 2022

Hm, maybe that's not possible in Qt 5 with CMake at all?

It's not possible to register QML types at build time with Qt5 and CMake, however, it is possible with qmake, meaning all the tooling exists in both Qt5 and Qt6, just not the CMake wrapper in Qt5. Qt6QmlMacros.cmake (/usr/lib64/cmake/Qt6Qml/Qt6QmlMacros.cmake from Fedora's qt6-qtdeclarative-devel package) has a function _qt_internal_qml_type_registration which runs an undocumented CLI tool called qmltyperegistrar which generates some C++ code. We could run that from qt-build-utils & cxx-qt-build, which AFAIK should work with both Qt5.15 and Qt6 and also work for Cargo-only builds without CMake. It seems qmltyperegistar needs a JSON file that moc outputs, but I'll let @vimpostor figure out those details.

@vimpostor
Copy link
Member

Hi I have the proof of concept working now (Qt6 only for now), but there are 2 major issues I encountered that probably require some API redesign:

  1. Generated sources don't play well together with qt_add_qml_module():
    If we use QML_ELEMENT on the C++ side and want cmake to actually register the type, we need to add the sources to the SOURCES argument of qt_add_qml_module(). There are several problems with this:
    Firstly, the sources are generated so it would be wild to require the user to add some hardcoded path like ${CMAKE_CURRENT_BINARY_DIR}/cxxqt/qml-minimal/cxx-qt-gen/my_object.cxxqt.h to their own CMakeLists.txt, ideally cxx-qt should take care of this itself.
    Second this won't even be possible to be added in the CMakeLists.txt because the sources do not exist at configure time yet, so cmake will complain that the files don't exist.
    Third, these sources may include other files that the QML compiler won't be able to find, unless we add a generated directory to include_directories(). Again we hardly can expect the user to include a hardcoded generated directory location like include_directories(${CMAKE_CURRENT_BINARY_DIR}/cxxqt/qml-minimal/cxx-qt-gen) (which I am manually doing at the moment), instead cxx-qt should take care of this itself.
    Do you have any ideas on how we can solve this problem from the cxx-qt side, without requiring the user to manually include weird generated files in their CMakeLists.txt? We must somehow find a way to add the generated sources to the qt_add_qml_module() call in the CMakeLists.txt, but without cmake complaining that the file doesn't exist at configure time.

  2. QML_ELEMENT requires us to include and link against QtQml in the cxxqt-build stage:
    Right now I solve this by letting the user manually add Qml to the CxxQtBuilder::qt_modules() function, e.g. in build.rs: CxxQtBuilder::new().file("src/cxxqt_object.rs").qt_modules(&["Qml"]).build();
    However it might be better to let cxx-qt deal with this automatically, so that the user doesn't have to modify this.
    I was thinking of automatically adding Qml to qt_modules whenever a #[cxx_qt::qobject(qml_element)] is used.
    But there might be an easier solution by just adding Qml to qt_modules by default. If I understood correctly, cxx-qt is only used with QML anyway. What's your opinion on this?

@Be-ing I think adding Qt5 cmake support for this would be out of scope for a PR for the base implementation as it is already getting complicated enough, but it surely can be added later.

@ahayzen-kdab
Copy link
Collaborator

@vimpostor Thanks for the detailed information!

So I think that for 1), @Be-ing was talking about looking at what CMake does, then adding support into our qt-build-utils crate, so that then in the build.rs step of the project that currently generates the C++ files it would then be able to effectively run the qt_add_qml_module step. (assuming this can then work when it's linked to the final lib/exec).

For 2) we currently by default always link to Core and Gui, so guess we could add Qml too. We were also thinking in #290 to add features into cxx-qt-lib for the different modules so you could have something that just uses Core. But this would be in the -build and -gen phases. I wonder if we can also have a feature and/or ifdef so that if Qml support is enabled in cxx-qt/cxx-qt-build/cxx-qt-gen it causes the QML_ELEMENT to occur. This feature would be enabled by default but eg if you were working on a widgets or QtCore only projefct you could disable this feature ? tl;dr; yes enabling Qml module linking by default makes sense, but would be good if we can disable with a feature or similar somewhere.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2022

Generated sources don't play well together with qt_add_qml_module():

Oof, yeah, it seems the only reasonable way to implement this robustly is to not use qt_add_aml_module in CMake and do it all from build.rs with qt-build-utils and cxx-qt-build.

@Be-ing
Copy link
Contributor Author

Be-ing commented Dec 5, 2022

Initially I was thinking a new method would need to be added to CxxQtBuilder to specify the information that is passed to qt_add_qml_module (URI and version at least), but I think it would be better to have the user specify this information in the Rust macro like #[cxx_qt::qobject(qml_element, qml_uri = "foo.bar", qml_version = "1.4.6")]. Come to think of it, do we even need the qml_element part in the Rust code? We could automatically infer that when qml_uri is specified.

@ahayzen-kdab
Copy link
Collaborator

#[cxx_qt::qobject(qml_element, qml_uri = "foo.bar", qml_version = "1.4.6")].

So this could be quite repetitive for multiple objects with the same uri and version. Maybe we need a fallback global uri / version that can be set with CxxQtBuilder, then a local override in the macro ?

Come to think of it, do we even need the qml_element part in the Rust code? We could automatically infer that when qml_uri is specified.

So there is also QML_SINGLETON, QML_ANONYMOUS, QML_INTERFACE that you could use instead of QML_ELEMENT etc. And in C++ you need to specify the QML_ELEMENT when you want that one so it was matching that and going with the "do less magic" as we have been recently.

@ahayzen-kdab
Copy link
Collaborator

@Be-ing is looking into the CMake / qt-build-utils side further :-)

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 2, 2023

I've been investigating how the qt_add_qml_module CMake function works and it's quite involved (and the internals aren't documented much):

  1. Run moc with --output-json CLI argument and pass generated JSON file to the qmltyperegistrar CLI tool, which generates a C++ file. I have a rough proof-of-concept of this working on top of a rebase of @vimpostor's branch. However, by itself this doesn't do anything.
  2. Generate QQmlEngineExtensionPlugin which has a constructor that calls the QML type registration function from the C++ file generated by qmltyperegistrar
  3. Use Q_IMPORT_PLUGIN C++ macro to load the plugin built as a static library
  4. Generate a qmldir file and add it to the Qt resource system

This is quite a lot to avoid 2 lines of boilerplate C++ code. 🙃

@vimpostor
Copy link
Member

vimpostor commented Feb 2, 2023

This is quite a lot to avoid 2 lines of boilerplate C++ code. 🙃

Yes, I noticed that too :D But very impressive, if you are able to get it working.

Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 7, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing pushed a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 8, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 9, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 9, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Be-ing added a commit to Be-ing/cxx-qt that referenced this issue Feb 9, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
przempore pushed a commit to przempore/cxx-qt that referenced this issue Mar 15, 2023
by specifying
[cxxqt::qobject(qml_uri = "foo.bar", qml_verison = "1.0")]

Fixes KDAB#241
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤔 discussion Feedback welcome ⬆️ feature New feature or request 🙋 good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants