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

QML modules with cmake #12380

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

s-lisovenko
Copy link
Contributor

Create separated qml plugins used qt6 cmake functionality

Create next separated qml plugins:

  • FactControls
  • ScreenTools
  • PlanView
  • Toolbar
  • Main UI plugin

First step in reworking old QML integration with .qrc files

Checklist:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@DonLakeFlyer
Copy link
Contributor

This looks like a lot of rearranging the deck chairs. But I don't quite understand the underlying benefit of this change? Can you explain what's gained here?

@DonLakeFlyer
Copy link
Contributor

Is it basically all about this:

qt_add_qml_module(${PROJECT_NAME}
    URI QGroundControl.FactControls
    VERSION 1.0
    STATIC
    NO_PLUGIN_OPTIONAL
    RESOURCE_PREFIX /qml
    QML_FILES ${MODULE_QML_FILES}

Which does all the qml module magic whereas before we need to set the resources correctly and qmldir manually and so forth?

@s-lisovenko
Copy link
Contributor Author

This looks like a lot of rearranging the deck chairs. But I don't quite understand the underlying benefit of this change? Can you explain what's gained here?

The main benefit is the migration to the mechanisms for working with QML modules in qt6 as old approach with qrc files is deprecated and don't recommended for modern qt applications.
New approach allows optimization like qt quick compiler as others benefits

@s-lisovenko
Copy link
Contributor Author

s-lisovenko commented Feb 1, 2025

Which does all the qml module magic whereas before we need to set the resources correctly and qmldir manually and so forth?

This code creates an independent qml module that is compiled as a static library and embedded into the application. Then you can use the import QGroundControl.FactControls and use the module elements.
https://doc.qt.io/qt-6/qt-add-qml-module.html

@HTRamsey
Copy link
Collaborator

HTRamsey commented Feb 1, 2025

Yeah the improved static analysis and performance from compiling is really nice. I wanted to get to this but it was a decent amount of work to get working

@s-lisovenko
Copy link
Contributor Author

Yes, it's a lot of work. I don't have much free time, but I already migrated FlightDisplay, FlightMap and json files, and if this approach works for the original qgc, I can create another PR's with these changes.

@DonLakeFlyer
Copy link
Contributor

Looks like all good stuff then. I"m concerned about leaving it half done. We need to work towards wrapping up 5.0. If this could get fully done in the next couple months then I'm ok with it.

@DonLakeFlyer
Copy link
Contributor

Can someone create an Issue for completing the work on this? And then if anyone picks up a setup to convert make a note in there so we don't end up working on the same things.

@s-lisovenko
Copy link
Contributor Author

Looks like all good stuff then. I"m concerned about leaving it half done. We need to work towards wrapping up 5.0. If this could get fully done in the next couple months then I'm ok with it.

I think I can do it next month if I have enough time. But I don't see any conflict even in partial migration, these 2 approaches can be combined without problems

@DonLakeFlyer
Copy link
Contributor

But I don't see any conflict even in partial migration

There is no conflict. It just makes it harder to navigate the source since it's inconsistent.

@s-lisovenko
Copy link
Contributor Author

#12382

@s-lisovenko
Copy link
Contributor Author

But I don't see any conflict even in partial migration

There is no conflict. It just makes it harder to navigate the source since it's inconsistent.

What do you mean? To me, this is a much simpler approach to navigating and organizing code. Am I missing something?

@DonLakeFlyer
Copy link
Contributor

Am I missing something?

Not at all. I'm just saying for someone looking through the source for the first time it can be confusing to see something done two different ways.

@DonLakeFlyer
Copy link
Contributor

@s-lisovenko So I wondering about the movement of qml files into a parallel universe of src/UI directories. For example: Before the PlanView directory included all the stuff associated with the PlanView. Both C++ and qml controls. But now things are in two places. I'm not sure that's better from a discoverability standpoint. What is the reasoning behind those moves?

@HTRamsey
Copy link
Collaborator

HTRamsey commented Feb 2, 2025

I know Qt used to be designed in a way where it was recommended to put QML files in their own directory tree, although they now provide options in CMake making that less important. One of the perks of autogenerated qmldir files

@s-lisovenko
Copy link
Contributor Author

s-lisovenko commented Feb 2, 2025

@s-lisovenko So I wondering about the movement of qml files into a parallel universe of src/UI directories. For example: Before the PlanView directory included all the stuff associated with the PlanView. Both C++ and qml controls. But now things are in two places. I'm not sure that's better from a discoverability standpoint. What is the reasoning behind those moves?

This is a good question, there are different strategies. What logic did I follow and what do I use in my production applications:

  1. Only UI module(without share backend to other app modules):
  • I prefer to put a pure UI module with qml only files in the UI folder (PlanView is no exception, since it is a pure QML module, at least what I moved to UI). This allows, in my opinion, to better organize the project since in any case now we have a common UI folder
  • If a module contains qml and some C++ files, but is only used on the QML side, I prefer to put that module in the UI folder as well.
  • if module is a big and have a lot c++ stuff I put it to separate folder into src.
  • Exeption is only if module is optional(not core part of app) in this case better is to put it to separate folder inside src(what I do in my app's)
  1. Mixed UI module with sharing backend
  • if module has share backend I think better is to put it into separate folder. This is structure what I use in my projects:
    /ModuleName
    |_ ./Backend files
    |_UI/ frontend related files
  • If Module is a optional(as I mentioned before) I also prefer to put it to separate folder in src For example Viewer3D

As for PlanView, PlanView should be part of the MissionManager module(my opinion) with backend and frontend. But since at the moment it was a separate folder of qml files, I placed it in the UI folder, since unfortunately I do not have much time to radically rework the application structure.
There are many approaches, which one would be better for QGC is difficult for me to answer, but I work mainly with what I described above and do not experience difficulties

@s-lisovenko
Copy link
Contributor Author

s-lisovenko commented Feb 2, 2025

I know Qt used to be designed in a way where it was recommended to put QML files in their own directory tree, although they now provide options in CMake making that less important. One of the perks of autogenerated qmldir files

Exactly, especially starting with qt 6.8 you have no any restrictions with project organization

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Feb 2, 2025

So last night I realized this new mechanism caused a significant problem with custom builds. The concept of resource overrides in custom builds is a fundamental aspect of omodifying the QGC ui to fit the needs of a custom build. The custom build example has a number of example usages of this. By moving to this new model as far as I can tell this is no longer possible. That's a really big problem. Do you see any way to move to this new model but keep the resource override concept?

@s-lisovenko
Copy link
Contributor Author

So last night I realized this new mechanism caused a significant problem with custom builds. The concept of resource overrides in custom builds is a fundamental aspect of omodifying the QGC ui to fit the needs of a custom build. The custom build example has a number of example usages of this. By moving to this new model as far as I can tell this is no longer possible. That's a really big problem. Do you see any way to move to this new model but keep the resource override concept?

I'm not very familiar with the concept of custom build in QGC, as I've never used it. When I quickly looked through it, I didn't see anything overridden except the icons. The concept of QML plugins is not new, it is a separate module that can be reused in different parts of the application. Could you describe what exactly the problem is? About icons for now we can leave it as part of qrc system as it more flexible.

@DonLakeFlyer
Copy link
Contributor

https://docs.qgroundcontrol.com/master/en/qgc-dev-guide/custom_build/resource_override.html#resource-overrides

You can see examples of this being used in custom-example/qgroundcontrol.exclusion and friends. It allows the custom example build to add custom actions to the toolstrip and overlay things onto the FlyView

@s-lisovenko
Copy link
Contributor Author

https://docs.qgroundcontrol.com/master/en/qgc-dev-guide/custom_build/resource_override.html#resource-overrides

You can see examples of this being used in custom-example/qgroundcontrol.exclusion and friends. It allows the custom example build to add custom actions to the toolstrip and overlay things onto the FlyView

looks like we can control this with a cmake array? We can add any resources we want to the module and control it all at the configure stage.

@HTRamsey
Copy link
Collaborator

HTRamsey commented Feb 2, 2025

There's that, but there's also the concept of import path priority. I'm pretty sure if you have a path like Qt/Qml/QGroundControl, and Qt/Qml/Custom, whichever you import first will take precedence when loading modules with the same name. Theres also cmake options like QT_QML_SKIP_QMLDIR_ENTRY and QT_DISCARD_FILE_CONTENTS and QT_RESOURCE_ALIAS that may help otherwise.

@HTRamsey
Copy link
Collaborator

HTRamsey commented Feb 2, 2025

For reference here are some new gstreamer cmake scripts from 1.25.1
gst_cmake.tar.gz

Edit: Wrong comment section but whatever

@DonLakeFlyer
Copy link
Contributor

Theres also cmake options like QT_QML_SKIP_QMLDIR_ENTRY

That seems promising. Sure would be nice to move into the 21st century with this qml cmake stuff.

@s-lisovenko s-lisovenko marked this pull request as draft February 10, 2025 09:26
@DonLakeFlyer
Copy link
Contributor

There's that, but there's also the concept of import path priority. I'm pretty sure if you have a path like Qt/Qml/QGroundControl, and Qt/Qml/Custom, whichever you import first will take precedence when loading modules with the same name.

I don't quite understand what you mean here. Take this for example:

file(GLOB_RECURSE MODULE_QML_FILES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/*.qml)

qt_add_qml_module(${PROJECT_NAME}
    URI QGroundControl.FactControls
    VERSION 1.0
    STATIC
    RESOURCE_PREFIX /qml
    QML_FILES ${MODULE_QML_FILES}
)

How would import path help you with that?

@HTRamsey
Copy link
Collaborator

Basically I just mean the order of the following should matter:

qmlEngine->addImportPath("qrc:/qt/qml/Custom");
qmlEngine->addImportPath("qrc:/qt/qml/QGroundControl");

In that case, if you have some module called MainRootWindow.qml in both paths, then it should take the one imported first. But I think it could be affected by things done in CMake too, just not 100% as I've never tried and it's been a while since I remember reading a post about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants