Build with Qt6 #532

doug1234 · 2024-02-24T16:26:06Z

Builds and the example runs with qt 6.6.0 on rhel9 with no new warnings. Currently using the qt5 compatibility library as was suggested in other pull requests.

Issue: The cursor is drawn farther to the right then it should be. This was not an issue for me with qt5. Looing into.
Issue: Text is sized differently when selected. This makes an odd effect. This was not an issue for me with qt5.

TODO: Update documentation.

doug1234 · 2024-02-24T17:04:52Z

Not sure how to update the CI to use qt6, the qt6 build of lxqt-build-tools and the appropriate python tools for qt6.

yan12125 · 2024-02-25T11:17:56Z

Not sure how to update the CI to use qt6, the qt6 build of lxqt-build-tools and the appropriate python tools for qt6.

You can ignore it for now. It needs lxqt/lxqt-build-tools#77 to be merged first.

stefonarch · 2024-02-25T20:36:04Z

Builds fine, just those warnings:


[ 82%] Building CXX object CMakeFiles/qtermwidget6.dir/lib/tools.cpp.o
/home/stef/git/doug1234/qtermwidget/lib/TerminalCharacterDecoder.cpp: In member function ‘virtual void Konsole::PlainTextDecoder::decodeLine(const Konsole::Character*, int, Konsole::LineProperty)’:
/home/stef/git/doug1234/qtermwidget/lib/TerminalCharacterDecoder.cpp:85:28: warning: comparing the result of pointer addition ‘(((const Konsole::Character*)characters) + ((sizetype)(((long unsigned int)i) * 16)))’ and NULL [-Waddress]
   85 |         if (characters + i == nullptr)
      |             ~~~~~~~~~~~~~~~^~~~~~~~~~
[ 84%] Building CXX object CMakeFiles/qtermwidget6.dir/lib/Vt102Emulation.cpp.o
[ 85%] Building CXX object CMakeFiles/qtermwidget6.dir/lib/moc_Emulation.cpp.o
[ 86%] Building CXX object CMakeFiles/qtermwidget6.dir/lib/moc_Filter.cpp.o
/home/stef/git/doug1234/qtermwidget/lib/Vt102Emulation.cpp: In member function ‘virtual void Konsole::Vt102Emulation::sendMouseEvent(int, int, int, int)’:
/home/stef/git/doug1234/qtermwidget/lib/Vt102Emulation.cpp:949:56: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size between 7 and 26 [-Wformat-truncation=]
  949 |         snprintf(command, sizeof(command), "\033[%d;%d;%dM", cb + 0x20, cx, cy);
      |                                                        ^~
/home/stef/git/doug1234/qtermwidget/lib/Vt102Emulation.cpp:949:44: note: directive argument in the range [1, 2147483647]
  949 |         snprintf(command, sizeof(command), "\033[%d;%d;%dM", cb + 0x20, cx, cy);
      |                                            ^~~~~~~~~~~~~~~~
/home/stef/git/doug1234/qtermwidget/lib/Vt102Emulation.cpp:949:17: note: ‘snprintf’ output between 9 and 37 bytes into a destination of size 32
  949 |         snprintf(command, sizeof(command), "\033[%d;%d;%dM", cb + 0x20, cx, cy);
      |         ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

marcusbritanicus · 2024-02-26T02:40:19Z

Issue: The cursor is drawn farther to the right then it should be. This was not an issue for me with qt5. Looing into.

@doug1234 I took the strain to build konsole Qt6, and they seem to do this properly, I know not how. Since you're working on this, you might have a better idea. On first glance, I noticed that their TerminalDisplay class is much more sophisticated than the one we have here.

Would it help to port that code to QTermWidget?

yan12125 · 2024-02-26T05:03:50Z

Builds fine, just those warnings:

Those warnings have been there for a long tine. I have a fix at https://github.com/lxqt/qtermwidget/commits/eliminate-warnings/, but I forgot why I didn't push it.

Would it help to port that code to QTermWidget?

That may help but will likely take much more efforts than porting to Qt 6, and thus it's out of scope for this PR in my opinion. A better place for such discussions can be lxqt/qterminal#320

marcusbritanicus · 2024-02-26T06:02:10Z

@yan12125 I did have a look at it before I posted here. That thread seems be sleeping since 2018, and was rather unwilling to wake it up - I seriously disliked the direction it was taking.

I do have a vested interest in QTermWidget - we have two projects based on it (CoreTerminal and DesQ Term) and would like to keep our projects free from KDE parts. So I am willing to spend some time to port TerminalDisplay if that is of any help at all.

Edit: I will see if I can port it without much efforts.

marcusbritanicus · 2024-02-26T09:24:39Z

@doug1234 Fantastic work...!

Issue: The cursor is drawn farther to the right then it should be. This was not an issue for me with qt5. Looing into.

I think I fixed this issue. Following are the changes I made:

diff --git a/lib/TerminalDisplay.cpp b/lib/TerminalDisplay.cpp
index 3018f65..0ccfb75 100644
--- a/lib/TerminalDisplay.cpp
+++ b/lib/TerminalDisplay.cpp
@@ -291,6 +291,15 @@ void TerminalDisplay::setVTFont(const QFont& f)
   // Disabling kerning saves some computation when rendering text.
   font.setKerning(false);
 
+  // QFont::ForceIntegerMetrics has been removed.
+  // Set full hinting instead to ensure the letters are aligned properly.
+  font.setHintingPreference(QFont::PreferFullHinting);
+
+  // "Draw intense colors in bold font" feature needs to use different font weights. StyleName
+  // property, when set, doesn't allow weight changes. Since all properties (weight, stretch,
+  // italic, etc) are stored in QFont independently, in almost all cases styleName is not needed.
+  font.setStyleName(QString());
+
   QWidget::setFont(font);
   fontChange(font);
 }

@yan12125 Looks like porting of TerminalDisplay is not required.

PS: @doug1234 I have opened a PR in your repo with these changes.

Fix issue cursor positioning issues

doug1234 · 2024-02-26T12:23:21Z

It looks great now thanks to help from @marcusbritanicus ! The only reason I can see not to merge is documentation since you don't want to update the ci here. I can tweak the documentation if you want but I am going to remove the draft tag now.

yan12125 · 2024-03-01T04:07:52Z

Thank you both for awesome efforts! I should be able to have a look in coming days.

The only reason I can see not to merge is documentation since you don't want to update the ci here.

Yes almost. Just need lxqt/lxqt-build-tools#77 and cleanup lxqt/qterminal#1067

I seriously disliked the direction it was taking.

That's just a discussion. If there is a need to port Konsole codes in the future, feel free to propose alternative approaches.

yan12125

(Only first few files checked)

examples/cpp/main.cpp

lib/ColorScheme.cpp

yan12125

Second batch. Codes related to KPty and TerminalDisplay are remaining.

lib/TerminalDisplay.cpp

lib/qtermwidget.cpp

lib/qtermwidget.h

lib/qtermwidget_interface.h

yan12125 · 2024-03-10T13:46:55Z

Not sure how to update the CI to use qt6, the qt6 build of lxqt-build-tools and the appropriate python tools for qt6.

I started testing Qt 6 CI in https://github.com/lxqt/qtermwidget/commits/qt6-ci/. The C++ part looks good, while the PyQt part may need some tricks.

yan12125

The final batch of reviews! Sorry it takes long with many rounds.

lib/kptyprocess.h

lib/kptyprocess.cpp

lib/TerminalDisplay.cpp

yan12125 · 2024-03-16T16:16:49Z

TODO: Update documentation.

What documentation needs updates?

Another question: is this an independent work or based on earlier pull requests in this repo? If the latter, it's better to give credits to authors of other pull requests.

doug1234 · 2024-03-18T13:30:36Z

I definitely looked at the original branch and used it as a starting point for some things. Let me know how you would like me to give credit to the original author. I am willing to do it however you want.

doug1234 · 2024-03-18T14:00:48Z

As far as documentation goes, I think we may need to change references to qt5 to qt6. Not sure if there is anything else or not. I can look into it if you want.

lib/kptyprocess.cpp

lib/TerminalDisplay.cpp

doug1234 · 2024-04-06T23:15:50Z

I bumped the version to 2.0.0. Please give a final review @yan12125 as I think this is looking pretty good now.

yan12125 · 2024-04-10T12:04:16Z

Thanks for the update. I tried non-BMP emoji, emoji flag sequence, and emoji variation selector, and there is no case working in Qt 5 but not in Qt 6. I think the implementation is good enough - will take a look this week.

tsujan · 2024-04-10T12:22:53Z

.... there is no case working in Qt 5 but not in Qt 6.

That's a Qt6 problem/regression I encountered years ago. It's visible in all Qt6 apps.

tsujan · 2024-04-10T12:50:02Z

Oh, sorry, I misread your comment! (Read "no case" as "one case".)

I meant that there are emojis which are shown correctly in Qt5 apps but not in Qt6 apps. Otherwise, I haven't tried the Qt6-based QTerminal yet.

yan12125 · 2024-04-11T06:59:10Z

Thank you very much for the efforts! I will merge this along with lxqt/qterminal#1067, after cleaning up remaining Qt 5 stuff in the latter.

That's a Qt6 problem/regression I encountered years ago. It's visible in all Qt6 apps.

Well, a case that breaks many Qt apos may be out of scope of this PR

tsujan · 2024-04-11T08:43:28Z

Well, a case that breaks many Qt apos may be out of scope of this PR

And that was my point: If there are some emojis that aren't shown correctly in the Qt6 version, the problem might be in Qt6 itself, not in QTerminal.

yan12125 · 2024-04-13T05:56:42Z

Found an issue: after 0f2cbfe, URLs are not underlined when hovered. I suspect changes in Filter.cpp break it, as that class is for filtering specific patterns (ex: URLs) from terminal contents.

Will the Qt6 port of QTerminal be ready before the next release, which will happen around Apr 15, 2024?

@tsujan It seems remaining work may not be trivial. I prefer to postpone Qt 6 port of qtermwidget to the next LXQt release.

tsujan · 2024-04-13T10:44:05Z

It seems remaining work may not be trivial.

I understand. It could be released whenever it's ready and tested.

doug1234 · 2024-04-13T12:07:35Z

Found an issue: after 0f2cbfe, URLs are not underlined when hovered. I suspect changes in Filter.cpp break it, as that class is for filtering specific patterns (ex: URLs) from terminal contents.

Will the Qt6 port of QTerminal be ready before the next release, which will happen around Apr 15, 2024?

@tsujan It seems remaining work may not be trivial. I prefer to postpone Qt 6 port of qtermwidget to the next LXQt release.

I should be able to take a look early next week. I wouldn't expect the fix to be too difficult.

yan12125 · 2024-04-14T12:09:27Z

Thanks for the kind help. If the URL issue is resolved before LXQt 2.0, it's still good to postpone the Qt 6 version, as overall changes in this PR are non-trivial and require intensive testing.

doug1234 · 2024-04-14T12:16:16Z

Thanks for the kind help. If the URL issue is resolved before LXQt 2.0, it's still good to postpone the Qt 6 version, as overall changes in this PR are non-trivial and require intensive testing.

OK. I am pretty sure I already see the mistake that caused this. It should be a super quick fix. But schedule the release for whenever you think is best.

tsujan · 2024-04-14T12:24:57Z

No need to worry about the release date. We could release the port whenever it's thoroughly tested. I added a note to the draft of the release announcement of LXQt 2.0.0:. Check if it's good enough:

"QTerminal is the only app whose Qt6 port will be released separately -- complications were encountered due to the removal of legacy encodings from Qt6. Until then, its Qt5 version 1.4.0 could be used."

luis-pereira

GTM.
Should land on master ASAP.
Any issues can be address there,
@doug1234 Nice work.

…rking like qt5.

doug1234 · 2024-04-15T14:56:32Z

Anchored pattern was not necessary for urls. Removing that got URLs working as they do in qt5.

yan12125 · 2024-04-16T13:48:35Z

Thanks! I will test again and finish cleaning up the qterminal PR these two days, and then both can be merged. If you wish, you can do a rebase and cleanup some commits (ex: previous attempts for Qt6 compatibility). It's also fine to skip further cleanups - I will merge all commits as-is.

doug1234 · 2024-04-16T23:19:37Z

Great! I think I am going to leave it as is unless you find something else.

yan12125 · 2024-04-19T11:31:55Z

Here we go 🎉

isf63 · 2024-04-20T14:08:31Z

I'm unsure on how to get the QTermWidget bindings from PyQt6 to build. With both python-pyqt5 and python-pyqt6 installed it selects PyQt5. Didn't see anything in the Arch PKGBUILD that changes that.

marcusbritanicus · 2024-04-20T16:47:12Z

@isf63 have a look at this. We're using this command to generate the PyQt6 bindings.

isf63 · 2024-04-24T11:24:42Z

@marcusbritanicus: Thanks, that got me further, but still no complete success:

Generating the QTermWidget bindings...
/usr/lib/python3.11/site-packages/PyQt6/bindings/QtCore/QtCoremod.sip: line 64: '%Plugin' is deprecated and will be removed in SIP v7.0.0
Generating the .pro file for the QTermWidget module...
Generating the top-level .pro file...
Generating the Makefiles...
/usr/bin/qmake6 -recursive QTermWidget.pro
Info: creating stash file /tmp/tmpx8qs774o/.qmake.stash
Reading /tmp/tmpx8qs774o/QTermWidget/QTermWidget.pro
Compiling the project...
make
cd QTermWidget/ && ( test -e Makefile || /usr/bin/qmake6 -o Makefile /tmp/tmpx8qs774o/QTermWidget/QTermWidget.pro ) && make -f Makefile 
make[1]: Entering directory '/tmp/tmpx8qs774o/QTermWidget'
g++ -c -pipe -I/tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../lib -I/tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../build/lib -fno-exceptions -Wall -Wextra -mno-direct-extern-access -D_REENTRANT -fPIC -DPy_LIMITED_API=0x03080000 -DSIP_PROTECTED_IS_PUBLIC -Dprotected=public -DQT_NO_EXCEPTIONS -DQT_NO_DEBUG -DQT_PLUGIN -DQT_WIDGETS_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I. -I. -I.. -I/usr/include/python3.11 -I/usr/include/qt6 -I/usr/include/qt6/QtWidgets -I/usr/include/qt6/QtGui -I/usr/include/qt6/QtCore -I. -I/usr/lib/qt6/mkspecs/linux-g++ -o sipQTermWidgetcmodule.o sipQTermWidgetcmodule.cpp
g++ -c -pipe -I/tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../lib -I/tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../build/lib -fno-exceptions -Wall -Wextra -mno-direct-extern-access -D_REENTRANT -fPIC -DPy_LIMITED_API=0x03080000 -DSIP_PROTECTED_IS_PUBLIC -Dprotected=public -DQT_NO_EXCEPTIONS -DQT_NO_DEBUG -DQT_PLUGIN -DQT_WIDGETS_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I. -I. -I.. -I/usr/include/python3.11 -I/usr/include/qt6 -I/usr/include/qt6/QtWidgets -I/usr/include/qt6/QtGui -I/usr/include/qt6/QtCore -I. -I/usr/lib/qt6/mkspecs/linux-g++ -o sipQTermWidgetQTermWidget.o sipQTermWidgetQTermWidget.cpp
In file included from /tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../lib/qtermwidget.h:25,
                 from /tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/sip/qtermwidget.sip:14:
/tmp/makepkg/qtermwidget-git/src/qtermwidget/pyqt/../lib/Emulation.h:36:10: fatal error: qtermwidget_export.h: No such file or directory
   36 | #include "qtermwidget_export.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

marcusbritanicus · 2024-04-24T18:26:11Z

@isf63 Right. I tried to compile on a fresh setup. I too hit this error. The simplest solution is to install the C++ library first. Then you can compile and install the pyqt bindings. How this works on the ci I am not sure.

lib/TerminalDisplay.cpp

      while( ((right.x()<_usedColumns-1) || (right.y()<_usedLines-1 && (_lineProperties[right.y()] & LINE_WRAPPED) ))
-                      && charClass(_image[i+1].character) == selClass )
+                      && charClass(QChar(static_cast<ushort>(_image[i-1].character))) == selClass )


tsujan · 2024-06-25T13:57:53Z

lib/Emulation.cpp

-     setCodec(LocaleCodec);
-
-  delete _decoder;
-  _decoder = _codec->makeDecoder();


https://doc.qt.io/qt-6/qstringencoder.html → "The encoder remembers any state that is required between calls,..."

Doug added 4 commits February 24, 2024 11:39

Builds with qt6.

b1fa777

Fixed some warnings.

b40b77b

Fixed another warning.

f288547

Fix the final warnings.

184c9fe

doug1234 force-pushed the Qt6Update branch from 9e9d476 to 184c9fe Compare February 24, 2024 16:40

yan12125 mentioned this pull request Feb 25, 2024

Continue the work from https://github.com/lxqt/qtermwidget/pull/468 #515

Closed

marcusbritanicus and others added 2 commits February 26, 2024 15:39

Fix issue cursor positioning issues

c880ab2

Merge pull request #1 from marcusbritanicus/Qt6Update

7052dc2

Fix issue cursor positioning issues

doug1234 changed the title ~~Draft: Build with Qt6~~ Build with Qt6 Feb 26, 2024

yan12125 reviewed Mar 5, 2024

View reviewed changes

examples/cpp/main.cpp Show resolved Hide resolved

lib/ColorScheme.cpp Outdated Show resolved Hide resolved

Doug and others added 2 commits March 5, 2024 09:41

Removed tab

f085cd8

Fix the example.

08fc85c

yan12125 requested changes Mar 8, 2024

View reviewed changes

lib/TerminalDisplay.cpp Outdated Show resolved Hide resolved

lib/qtermwidget.cpp Outdated Show resolved Hide resolved

lib/qtermwidget.h Outdated Show resolved Hide resolved

lib/qtermwidget_interface.h Outdated Show resolved Hide resolved

doug1234 added 2 commits March 11, 2024 07:04

Tab to space in terminal display.

c5e5f1f

Remove chagnes that were accidently reverted.

94ee191

yan12125 reviewed Mar 16, 2024

View reviewed changes

lib/kptyprocess.h Outdated Show resolved Hide resolved

lib/kptyprocess.cpp Show resolved Hide resolved

lib/TerminalDisplay.cpp Show resolved Hide resolved

yan12125 reviewed Mar 25, 2024

View reviewed changes

lib/kptyprocess.cpp Outdated Show resolved Hide resolved

lib/kptyprocess.cpp Show resolved Hide resolved

lib/TerminalDisplay.cpp Show resolved Hide resolved

yan12125 approved these changes Apr 11, 2024

View reviewed changes

luis-pereira mentioned this pull request Apr 12, 2024

Qt6 Only #538

Closed

luis-pereira approved these changes Apr 15, 2024

View reviewed changes

Removed anchored pattern for email and url. URL underlining is now wo…

a760241

…rking like qt5.

yan12125 merged commit 48ca3dd into lxqt:master Apr 19, 2024
1 check passed

yan12125 mentioned this pull request May 3, 2024

Fix a typo #544

Merged

yan12125 mentioned this pull request May 12, 2024

Update build.sh lxqt/qterminal#1122

Closed

correabuscar reviewed Jun 24, 2024

View reviewed changes

correabuscar reviewed Jun 25, 2024

View reviewed changes

correabuscar mentioned this pull request Jun 25, 2024

replacement character aka � appears out of nowhere sometimes lxqt/qterminal#1143

Closed

Build with Qt6 #532

Build with Qt6 #532

Conversation

doug1234 commented Feb 24, 2024

doug1234 commented Feb 24, 2024

yan12125 commented Feb 25, 2024

stefonarch commented Feb 25, 2024

marcusbritanicus commented Feb 26, 2024

yan12125 commented Feb 26, 2024

marcusbritanicus commented Feb 26, 2024 • edited Loading

marcusbritanicus commented Feb 26, 2024 • edited Loading

doug1234 commented Feb 26, 2024

yan12125 commented Mar 1, 2024

yan12125 left a comment

Choose a reason for hiding this comment

yan12125 left a comment

Choose a reason for hiding this comment

yan12125 commented Mar 10, 2024

yan12125 left a comment

Choose a reason for hiding this comment

yan12125 commented Mar 16, 2024

doug1234 commented Mar 18, 2024

doug1234 commented Mar 18, 2024

doug1234 commented Apr 6, 2024

yan12125 commented Apr 10, 2024

tsujan commented Apr 10, 2024 • edited Loading

tsujan commented Apr 10, 2024 • edited Loading

yan12125 commented Apr 11, 2024

tsujan commented Apr 11, 2024

yan12125 commented Apr 13, 2024

tsujan commented Apr 13, 2024

doug1234 commented Apr 13, 2024

yan12125 commented Apr 14, 2024

doug1234 commented Apr 14, 2024

tsujan commented Apr 14, 2024

luis-pereira left a comment

Choose a reason for hiding this comment

doug1234 commented Apr 15, 2024

yan12125 commented Apr 16, 2024

doug1234 commented Apr 16, 2024

yan12125 commented Apr 19, 2024

isf63 commented Apr 20, 2024

marcusbritanicus commented Apr 20, 2024

isf63 commented Apr 24, 2024

marcusbritanicus commented Apr 24, 2024

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

tsujan Jun 25, 2024

Choose a reason for hiding this comment

marcusbritanicus commented Feb 26, 2024 •

edited

Loading

marcusbritanicus commented Feb 26, 2024 •

edited

Loading

tsujan commented Apr 10, 2024 •

edited

Loading

tsujan commented Apr 10, 2024 •

edited

Loading