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

Techdraw: finish Qt6 port #9104

Merged
merged 3 commits into from Mar 31, 2023
Merged

Techdraw: finish Qt6 port #9104

merged 3 commits into from Mar 31, 2023

Conversation

wwmayer
Copy link
Contributor

@wwmayer wwmayer commented Mar 28, 2023

Thank you for creating a pull request to contribute to FreeCAD! Place an "X" in between the brackets below to "check off" to confirm that you have satisfied the requirement, or ask for help in the FreeCAD forum if there is something you don't understand.

  • Your Pull Request meets the requirements outlined in section 5 of CONTRIBUTING.md for a Valid PR

Please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 1.0 Changelog Forum Thread.


@github-actions github-actions bot added the WB TechDraw Related to the TechDraw Workbench label Mar 28, 2023
@chennes
Copy link
Member

chennes commented Mar 28, 2023

Was this the last thing that we were waiting on for full Qt6 support?

@luzpaz luzpaz added the 3rd party: Qt 6 Issue related to Qt 6 label Mar 28, 2023
@freecadci
Copy link

pipeline status for feature branch PR_9104. Pipeline 820522117 was triggered at 372f578. All CI branches and pipelines.

@wwmayer
Copy link
Contributor Author

wwmayer commented Mar 28, 2023

Yes, I think so.

However, we still have tons of slot functions of this form on_objectname_signalname that are automatically connected with QMetaObject::connectSlotsByName. Since several signals have been renamed in Qt6 some automatic connections may fail.

@wwmayer
Copy link
Contributor Author

wwmayer commented Mar 28, 2023

BTW, the algorithm for Qt6 can be easily tested with Qt5. Therefore replace #if QT_VERSION < QT_VERSION_CHECK(6,0,0) with #if 0 in XMLQuery.cpp. When creating a TD page you will see the green bullets where you can change the editable fields.

Now if you comment out the line processElements inside processItems you will realize that the green bullets are missing.

@chennes
Copy link
Member

chennes commented Mar 28, 2023

However, we still have tons of slot functions of this form on_objectname_signalname that are automatically connected with QMetaObject::connectSlotsByName. Since several signals have been renamed in Qt6 some automatic connections may fail.

Should we be eliminating the automatic connections and doing it manually?

@wwmayer
Copy link
Contributor Author

wwmayer commented Mar 29, 2023

Should we be eliminating the automatic connections and doing it manually?

This is what clazy recommends. I don't have a commit right now that shows the warning but you have already seen it for sure that clazy says that the automatic connections are error-prone. IMO, it's similar to the old-style connect with SIGNAL and SLOT which has raised many issues with the Qt6 port.

So, as a long-term goal we should migrate to the manual connection but this is not of high priority.

@wwmayer
Copy link
Contributor Author

wwmayer commented Mar 31, 2023

@WandererFan please have a look and merge if all is OK.

@WandererFan
Copy link
Contributor

This works fine with Qt5. I don't have Qt6 installed yet.

Thanks for handling this.

@WandererFan WandererFan merged commit f2aa219 into FreeCAD:master Mar 31, 2023
7 checks passed
@wwmayer wwmayer deleted the techdraw_qt6 branch March 31, 2023 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party: Qt 6 Issue related to Qt 6 WB TechDraw Related to the TechDraw Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants