Qt4 -> Qt5/6: scrollbar signal handling

[Jörg W. (dl8dtl) (Moderator)]

Ich versuche gerade,

https://github.com/aitjcize/QCamber

auf Qt5/Qt6 zu bringen.

https://github.com/aitjcize/QCamber/issues/23

"This app was build onto of Qt 4.7. I've never tried compiling for Qt 5. 
You are welcome to help port it to Qt 5.0."

Nun, das Projekt ist ein Opensource-ODB++-Viewer – einen anderen konnte 
ich zumindest auf die Schnelle nicht finden. Insofern könnte sich ein 
wenig Aufwand ja mal lohnen.

Am Ende stolpere ich über die Art und Weise, wie dort Scrollbar-Events 
gehandhabt werden:

  connect(horizontalScrollBar(), SIGNAL(valueChanged(int)),
      this, SLOT(updateLayerViewport(void)));

Das wirft eine Reihe von Fehlern, die wohl in erster Linie darauf hin 
deuten, dass sich das erste Argument nicht in einen QObject* wandeln 
ließe.

../../src/graphicsview/odbppgraphicsminimapview.cpp:55:3: error: no matching member function for call to 'connect'
   55 |   connect(horizontalScrollBar(), SIGNAL(valueChanged(int)),
      |   ^~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:222:36: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  222 |     static QMetaObject::Connection connect(const QObject *sender, const char *signal,
      |                                    ^       ~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:225:36: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  225 |     static QMetaObject::Connection connect(const QObject *sender, const QMetaMethod &signal,
      |                                    ^       ~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:481:41: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  481 | inline QMetaObject::Connection QObject::connect(const QObject *asender, const char *asignal,
      |                                         ^       ~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:242:43: note: candidate template ignored: substitution failure [with Func1 = const char *, Func2 = const char *]: no type named 'Object' in 'QtPrivate::FunctionPointer<const char *>'
  242 |     static inline QMetaObject::Connection connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal,
      |                                           ^                                                         ~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:283:13: note: candidate template ignored: requirement 'int(QtPrivate::FunctionPointer<const char *>::ArgumentCount) >= 0' was not satisfied [with Func1 = const char *, Func2 = const char *]
  283 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, const QObject *context, Func2 slot,
      |             ^
/usr/local/include/qt5/QtCore/qobject.h:322:13: note: candidate template ignored: substitution failure [with Func1 = const char *, Func2 = const char *]: no type named 'Object' in 'QtPrivate::FunctionPointer<const char *>'
  322 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, const QObject *context, Func2 slot,
      |             ^                                                         ~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:274:13: note: candidate function template not viable: requires 3 arguments, but 4 were provided
  274 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, Func2 slot)
      |             ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:314:13: note: candidate function template not viable: requires 3 arguments, but 4 were provided
  314 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, Func2 slot)
      |             ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/graphicsview/odbppgraphicsminimapview.cpp:57:3: error: no matching member function for call to 'connect'
   57 |   connect(verticalScrollBar(), SIGNAL(valueChanged(int)),
      |   ^~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:222:36: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  222 |     static QMetaObject::Connection connect(const QObject *sender, const char *signal,
      |                                    ^       ~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:225:36: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  225 |     static QMetaObject::Connection connect(const QObject *sender, const QMetaMethod &signal,
      |                                    ^       ~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:481:41: note: candidate function not viable: cannot convert argument of incomplete type 'QScrollBar *' to 'const QObject *' for 1st argument
  481 | inline QMetaObject::Connection QObject::connect(const QObject *asender, const char *asignal,
      |                                         ^       ~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:242:43: note: candidate template ignored: substitution failure [with Func1 = const char *, Func2 = const char *]: no type named 'Object' in 'QtPrivate::FunctionPointer<const char *>'
  242 |     static inline QMetaObject::Connection connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal,
      |                                           ^                                                         ~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:283:13: note: candidate template ignored: requirement 'int(QtPrivate::FunctionPointer<const char *>::ArgumentCount) >= 0' was not satisfied [with Func1 = const char *, Func2 = const char *]
  283 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, const QObject *context, Func2 slot,
      |             ^
/usr/local/include/qt5/QtCore/qobject.h:322:13: note: candidate template ignored: substitution failure [with Func1 = const char *, Func2 = const char *]: no type named 'Object' in 'QtPrivate::FunctionPointer<const char *>'
  322 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, const QObject *context, Func2 slot,
      |             ^                                                         ~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:274:13: note: candidate function template not viable: requires 3 arguments, but 4 were provided
  274 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, Func2 slot)
      |             ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/qt5/QtCore/qobject.h:314:13: note: candidate function template not viable: requires 3 arguments, but 4 were provided
  314 |             connect(const typename QtPrivate::FunctionPointer<Func1>::Object *sender, Func1 signal, Func2 slot)
      |             ^       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Kann mir jemand mit tieferen Qt-Kenntnissen hier auf die Sprüngen 
helfen, wie man das heutzutage ausdrücken würde?

[Urban (ubn)]

Jörg W. schrieb:
> /usr/local/include/qt5/QtCore/qobject.h:222:36: note: candidate function
> not viable: cannot convert argument of incomplete type 'QScrollBar *' to
> 'const QObject *' for 1st argument

Er kennt QScrollBar nicht. Evtl. einfach QWidgets includen?

#include <QtWidgets>

Im Übrigen würde ich empfehlen die SIGNAL() und SLOT() durch 
Funktionspointer-Schreibweise zu ersetzen, weil es nicht mehr zu 
Runtime-Errors führt (sondern compile-time) falls die signal/slot 
connection nicht erstellt werden kann. Ungefähr so (ungetestet):

  connect(horizontalScrollBar(), &QScrollBar::valueChanged,
          this, &ODBPPGraphicsMiniMapView::updateLayerViewport);

Falls es wegen inkompatiblen Argumenten nicht klappt, Lambdas verwenden:

  connect(horizontalScrollBar(), &QScrollBar::valueChanged,
          this, [this](int){ updateLayerViewport(); });

[Jörg W. (dl8dtl) (Moderator)]

Danke für die schnelle Antwort, aber jetzt ist es schon etwas spät. ;-) 
Ich schau mir das an.

[Oliver S. (oliverso)]

Urban schrieb:
> Er kennt QScrollBar nicht. Evtl. einfach QWidgets includen?

Besser QScrollbar ;)

Oliver

[Jörg W. (dl8dtl) (Moderator)]

Urban schrieb:
> Er kennt QScrollBar nicht. Evtl. einfach QWidgets includen?

Danke nochmal. Dass man QtWidgets explizit angeben muss (schon im 
qmake-Projekt und dann in den includes) ist offenbar eine der 
wesentlichen API-Änderungen zwischen Qt4 und Qt5 gewesen. Das include 
fehlte in diesem Projekt an vielen Stellen.

Jetzt habe ich es soweit mit Qt5 am Compilieren. Es gibt aber immer noch 
ein paar deprecation warnings, die würde ich auch noch aufräumen, dann 
sollte es vielleicht auch gleich Qt6-kompatibel sein.

[Rolf M. (rmagnus)]

Bei Qt hat man mit der Zeit immer weiter die sich gegenseitig 
inkludierenden Header reduziert, so das etwas, das in Qt4 noch implizit 
durch einen anderen Header mit reingezogen wurde, in Qt5 ggf. explizit 
eingebunden werden muss.
Ich würde aber auch nicht QtWidgets inkludieren, weil das ein 
Rundumschlag mit allem ist. Besser wäre es, nur die Widgets, die auch 
tatsächlich benötigt werden, einzeln einzubunden.

[Jörg W. (dl8dtl) (Moderator)]

Rolf M. schrieb:
> Ich würde aber auch nicht QtWidgets inkludieren, weil das ein
> Rundumschlag mit allem ist. Besser wäre es, nur die Widgets, die auch
> tatsächlich benötigt werden, einzeln einzubunden.

Das wäre jetzt schon sehr viel Feinschliff. Im Moment habe ich es mit 
QtWidgets (und ein paar anderen Änderungen) wenigstens erstmal mit Qt5 
zum Compilieren bekommen. Qt6 braucht noch paar Handgriffe mehr (u.a. 
weil QRegExp dann wegfällt oder nur noch compat ist).

Erstmal möchte ich es aber auch sinnvoll laufen sehen. Soweit ist es 
leider im Moment noch nicht.

[Bernd B. (bbrand)]

Hi,

angehängt ist ein Patch gegen den aktuellen Stand in GitHub. Damit lässt 
sich QCamber zumindest unter Linux sowohl mit Qt5 als auch Qt6 (aber 
nicht mehr mit Qt4) bauen. Die regulären Ausdrücke sind auf 
QRegularExpression umgestellt, die Compat-Library wird daher unter Qt6 
nicht benötigt.
Es scheint soweit auch zu laufen, allerdings habe ich es bisher nur mit 
der Beispieldatei von odbplusplus.com getestet.
(https://odbplusplus.com/wp-content/resource-docs/designodb_rigidflex.tgz)

Gruß,
Bernd

[Jörg W. (dl8dtl) (Moderator)]

Danke! Magst du den selbst noch als pull request eintüten oder soll ich 
das für dich machen?

[Jörg W. (dl8dtl) (Moderator)]

Qt4 hatte ich auch in meinem PR schon zu den Akten gelegt. Ist schon so 
lange obsolet, dass man das wohl nicht mehr unterstützen muss.

[Bernd B. (bbrand)]

Jörg W. schrieb:
> Magst du den selbst noch als pull request eintüten oder soll ich
> das für dich machen?

Mach Du ruhig, ich habe keinen GitHub Account :)

[Bernd B. (bbrand)]

Hmm, ich habe es jetzt mal mit einer aus KiCad exportierten ODB++-Datei 
getestet. Dabei stürzt QCamber direkt ab. Das Fehlerhandling der 
aktuellen Implementierung ist eher bescheiden, die Software verlässt 
sich sehr darauf, dass der Input genauso aussieht wie erwartet. Im 
konkreten Fall wird versucht, eine Datei "steps/pcb/attrlist" zu laden, 
die im KiCad-Export aber gar nicht existiert. Das wird aber nicht 
abgefangen, sondern führt in der Folge zu einem NULL-Pointer Zugriff.

Es ist auch nicht schön, dass QCamber nur ODB++-Dateien im Format 
.tar.gz lesen kann. KiCad exportiert aber nur .zip, das muss dann erst 
mal umgepackt werden.

Korrektur: Stimmt gar nicht, KiCad kann auch .tgz exportieren.

[Jörg W. (dl8dtl) (Moderator)]

Ja, mindestens einen der Nullpointer-Abstürze habe ich hier im Debugger 
schon analysiert und korrigiert. Ich vermute, dass das noch nicht der 
letzte ist.

Man müsste das Teil auch noch umstellen vom externen Aufruf eines 
tar-Kommandos auf zlib, dann wäre sowohl .tar.gz als auch .zip 
gleichermaßen handhabbar, und man muss nicht noch auf Windows extra ein 
tar.exe beilegen, wie es die Demo-Binaries machen (die bei mir unter 
Wine aber auch nur crashen).

[Jörg W. (dl8dtl) (Moderator)]

Jörg W. schrieb:
> Ja, mindestens einen der Nullpointer-Abstürze habe ich hier im Debugger
> schon analysiert und korrigiert. Ich vermute, dass das noch nicht der
> letzte ist.

Scheint wirklich nur der eine zu sein:

diff --git a/src/parser/odbpp/featuresparser.cpp b/src/parser/odbpp/featuresparser.cpp
index 13dfba8..111b7e1 100644
--- a/src/parser/odbpp/featuresparser.cpp
+++ b/src/parser/odbpp/featuresparser.cpp
@@ -122,6 +122,8 @@ FeaturesDataStore* FeaturesParser::parse(void)
 
 void FeaturesParser::putAttrlist(const StructuredTextDataStore* ds)
 {
+  if (ds == NULL)
+    return;
   const StructuredTextDataStore::ValueType d = ds->getValueData();
   for (StructuredTextDataStore::ValueType::const_iterator it = d.begin();
       it != d.end(); ++it) {

Nachdem ich rausgefunden habe, wie man die Farben umstellt (wenngleich 
mir nicht klar ist, warum es nur 6 Farben sind und wie diese den Lagen 
zugeordnet werden), kann ich jetzt auch einen Kicad-Export damit 
anzeigen.

[Bernd B. (bbrand)]

In einem anderen Projekt habe ich vor kurzem die KArchive-Library 
(https://github.com/KDE/karchive) benutzt.
Damit müsste man die Dateien noch nicht mal auf die Platte einpacken, 
sondern könnte die Inhalte direkt aus dem Archiv lesen. Könnte 
allerdings einen größeren Umbau erfordern. KArchive stammt zwar vom 
KDE-Projekt; anders als der Name vermuten lässt werden aber nur Qt- und 
keine KDE-Libraries benötigt. Ausserdem erfordert zumindest der aktuelle 
Stand von KArchive Qt6.

[Bernd B. (bbrand)]

Jörg W. schrieb:
> kann ich jetzt auch einen Kicad-Export damit
> anzeigen.

Stimmt, funktioniert mit dem Patch prima :)

[Jörg W. (dl8dtl) (Moderator)]

Bernd B. schrieb:
> Jörg W. schrieb:
>> kann ich jetzt auch einen Kicad-Export damit
>> anzeigen.
>
> Stimmt, funktioniert mit dem Patch prima :)

Bin jetzt (mit diesen Demo-ODB++-Daten von oben) in noch einen Crash 
mehr getapst.

diff --git a/src/symbol/usersymbol.cpp b/src/symbol/usersymbol.cpp
index 8a96e90..4b00393 100644
--- a/src/symbol/usersymbol.cpp
+++ b/src/symbol/usersymbol.cpp
@@ -34,13 +34,14 @@ UserSymbol::UserSymbol(const QString& def, const Polarity& polarity,
 {
   QString path = ctx.loader->featuresPath("symbols/" + def);
   FeaturesDataStore* ds = CachedFeaturesParser::parse(path);
-
-  for (QList<Record*>::const_iterator it = ds->records().begin();
-      it != ds->records().end(); ++it) {
-    Symbol* symbol = (*it)->createSymbol();
-    addChild(symbol);
-    m_symbols.append(symbol);
-  }
+  
+  if (ds != NULL)
+    for (QList<Record*>::const_iterator it = ds->records().begin();
+        it != ds->records().end(); ++it) {
+      Symbol* symbol = (*it)->createSymbol();
+      addChild(symbol);
+      m_symbols.append(symbol);
+    }
 
   setHandlesChildEvents(true);
 }

Ich denke, das ganze Projekt könnte mal einen Audit gebrauchen auf alle 
"ds->".

Bernd B. schrieb:
> In einem anderen Projekt habe ich vor kurzem die KArchive-Library
> (https://github.com/KDE/karchive) benutzt.
> Damit müsste man die Dateien noch nicht mal auf die Platte einpacken,
> sondern könnte die Inhalte direkt aus dem Archiv lesen.

Naja gut, für ein größeres Projekt ist es sicher gar nicht so schlecht, 
die Platte gewissermaßen als Cache zu haben. Zip lässt sich ja random 
access zugreifen, aber .tar.gz nicht, das geht immer nur im ganzen 
Stream.

Außerdem ist zlib halt (wie wir seit dem letzten security issue wissen 
;-) praktisch mittlerweile omnipräsent.

[Jörg W. (dl8dtl) (Moderator)]

Außerdem bräuchte das Ganze wohl zumindest eine leicht zu findende 
Hilfe-Funktion und vielleicht ja auch noch ein paar Standard-Dinge wie 
File -> Exit. Das Herausfinden der Bedienung war jedenfalls teilweise 
schon mal eine Herausforderung.

[Jörg W. (dl8dtl) (Moderator)]

Ich sehe auch gerade, dass im Projekt sonst an vielen Stellen "if (!ds)" 
als Test benutzt wird. Das würde ich daher aus stilistischen Gründen 
dann auch so machen.

[Jörg W. (dl8dtl) (Moderator)]

Jörg W. schrieb:
> Bin jetzt (mit diesen Demo-ODB++-Daten von oben) in noch einen Crash
> mehr getapst.

Ich sehe gerade, dass du den ja auch schon korrigiert hattest.

[Jörg W. (dl8dtl) (Moderator)]

Bernd B. schrieb:

> angehängt ist ein Patch gegen den aktuellen Stand in GitHub.

Ich habe das als PR eingereicht.

Ich hatte gerade noch festgestellt, dass bei mir der Zoom via Mausrad 
nicht funktionierte. Ich habe in

ODBPPGraphicsView::wheelEvent()

mal ->pixelDelta durch ->angleDelta ersetzt. So, wie ich die Doku 
verstehe, gibt es angleDelta immer (ggf. halt mit großen Sprüngen), 
während pixelDelta nicht immer ein Ergebnis bringen muss. Extra noch 
eine Fallunterscheidung fand ich für das bissel Zoom jetzt auch nicht 
sinnvoll.

[Jörg W. (dl8dtl) (Moderator)]

Jörg W. schrieb:
> Bernd B. schrieb:
>
>> angehängt ist ein Patch gegen den aktuellen Stand in GitHub.
>
> Ich habe das als PR eingereicht.

Der ist übrigens jetzt bearbeitet worden. Ich glaube, er ist noch nicht 
gemergt, aber zumindest begutachtet worden.

[Jörg W. (dl8dtl) (Moderator)]

Merge ist jetzt ebenfalls durch. Danke nochmal an dich, Bernd!

[Bernd B. (bbrand)]

Gern geschehen.

Dann werde ich mal meinen Stand mit der Repo synchronisieren. Ich habe 
noch ein paar weitere Änderungen gemacht, z.B. den StructuredTextParser 
von Hand implementiert, so dass Flex & Bison nicht mehr gebraucht 
werden. Ich habe zwar prinzipiell nichts gegen Flex/Bison, aber es ist 
immer etwas mühsam, Versionen zu finden, die auch mit Windows 
funktionieren. Ein alternativer und einfacherer Ansatz wäre, die 
generierten Files mit in die Versionsverwaltung aufzunehmen; da wird ja 
normalerweise nichts mehr geändert werden müssen.
Ich spiele auch immer noch mit dem Gedanken, KArchive einzubinden. Naja, 
vielleicht irgendwann einmal, in den letzten drei Wochen habe ich nichts 
mehr daran gemacht.

[Jörg W. (dl8dtl) (Moderator)]

Bernd B. schrieb:
> Ein alternativer und einfacherer Ansatz wäre, die generierten Files mit
> in die Versionsverwaltung aufzunehmen; da wird ja normalerweise nichts
> mehr geändert werden müssen.

Selbst wenn, könnte man ja die neu generierten Dateien ebenfalls mit 
aufnehmen.

Erscheint mir zumindest ein sinnvollerer Ansatz, als Zeit in einen 
handgestrickten Scanner/Parser zu investieren.

[Bernd B. (bbrand)]

Jörg W. schrieb:
> ... als Zeit in einen handgestrickten Scanner/Parser zu investieren.

Das stimmt zwar, aber nur weil der Parser bereits fertig implementiert 
war.
Der von Hand geschriebene Parser ist mit 73 Zeilen Code sogar kleiner 
als die Flex/Bison Definitionen, ganz zu schweigen vom daraus 
generierten Code.