[BUILD] 066a10e6 causes QT6 builds to fail

[MatthewCroughan]

Commit Hash

066a10e

Platform

Linux

Summary

Whilst creating a reproducible Nix derivation for olive-editor. I was building for QT6 and found via a bisect that 066a10e causes builds with QT6 to fail like this

olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp: In member function 'virtual olive::ProjectSerializer::LoadData olive::ProjectSerializer230220::Load(olive::Project*, QXmlStreamReader*, olive::ProjectSerializer::LoadType, void*) const':
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:192:23: error: 'QStringRef' was not declared in this scope; did you mean 'QStringView'?
olive-editor-unstable>   192 |               QVector<QStringRef> l = attr.value().split(',');
olive-editor-unstable>       |                       ^~~~~~~~~~
olive-editor-unstable>       |                       QStringView
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:192:33: error: template argument 1 is invalid
olive-editor-unstable>   192 |               QVector<QStringRef> l = attr.value().split(',');
olive-editor-unstable>       |                                 ^
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:192:57: error: cannot convert 'QList<QStringView>' to 'int' in initialization
olive-editor-unstable>   192 |               QVector<QStringRef> l = attr.value().split(',');
olive-editor-unstable>       |                                       ~~~~~~~~~~~~~~~~~~^~~~~
olive-editor-unstable>       |                                                         |
olive-editor-unstable>       |                                                         QList<QStringView>
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:193:31: error: request for member 'size' in 'l', which is of non-class type 'int'
olive-editor-unstable>   193 |               items.reserve(l.size());
olive-editor-unstable>       |                               ^~~~
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:194:26: error: 'QStringRef' does not name a type; did you mean 'QStringView'?
olive-editor-unstable>   194 |               for (const QStringRef &s : l) {
olive-editor-unstable>       |                          ^~~~~~~~~~
olive-editor-unstable>       |                          QStringView
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:196:16: error: expected ';' before '}' token
olive-editor-unstable>   196 |               }
olive-editor-unstable>       |                ^
olive-editor-unstable>       |                ;
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ~
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:197:13: error: expected primary-expression before '}' token
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ^
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:196:16: error: expected ';' before '}' token
olive-editor-unstable>   196 |               }
olive-editor-unstable>       |                ^
olive-editor-unstable>       |                ;
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ~
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:197:13: error: expected primary-expression before '}' token
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ^
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:196:16: error: expected ')' before '}' token
olive-editor-unstable>   196 |               }
olive-editor-unstable>       |                ^
olive-editor-unstable>       |                )
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ~
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:194:19: note: to match this '('
olive-editor-unstable>   194 |               for (const QStringRef &s : l) {
olive-editor-unstable>       |                   ^
olive-editor-unstable> /build/source/app/node/project/serializer/serializer230220.cpp:197:13: error: expected primary-expression before '}' token
olive-editor-unstable>   197 |             }
olive-editor-unstable>       |             ^

Additional Information / Output

Every commit after 06a10e6 builds successfully with QT5.

The reason I am creating this derivation is to update the very out of date Nix package

[MatthewCroughan]

The reason is because QStringRef is removed in Qt6, so you'll need to port this to use QStringView instead. You could use Qt5Compat, but Is there a valid reason to cling to Qt5 support since Qt5 is now deprecated? Let's get rid of Qt5 altogether in the Makefile and make Qt6 default.

[MatthewCroughan]

I can confirm that simply renaming QStringRef to QStringView in the following files works:

./app/node/project/serializer/serializer.h
./app/node/project/serializer/serializer.cpp
./app/node/project/serializer/serializer230220.cpp

I believe this will break QT5 builds, not entirely sure. I won't make a patch for this since the choice is yours as to whether to get rid of QT5 and replace with QT6.

[MatthewCroughan]

NixOS/nixpkgs#222710

[itsmattkc]

Is there a valid reason to cling to Qt5 support since Qt5 is now deprecated?

There are still some unresolved UI issues that get introduced when compiling with Qt 6. From memory, it's mostly mouse related issues pertaining to the sliders, though I can't remember fully and haven't really tested since first adding Qt 6 support. There's also an argument that there are still OSes in use that Olive officially supports and that Qt 6 doesn't, however this is obviously becoming less the case over time.

[MatthewCroughan]

I've been using the latest commit on Wayland QT6 for more than 24 hours now, it's rock solid and I'm blown away by olive in general. It is state of the art.

The mouse/cursor does not change its appearance on various parts of the UI such as when enlarging clips, but I really do not mind that at all, if that's the bug you're referring to.

[Simran-B]

The current target is CY2022, but CY2023 is also Qt 5. It will be Qt 6 for CY2024:
http://vfxplatform.com/

[Quackdoc]

I can confirm that simply renaming QStringRef to QStringView in the following files works:

if the changes are as simple as this, it probably warrants just using #ifdef to keep support for both. it should be clean enough to remove if/when the decision to deprecate QT5 is made

[itsmattkc]

I agree that this should be solved with ifdefs.

[ThomasWilshaw]

There have been several commits upgrading QT6 support, is the issue still relevant?

[rathann]

Yes. It's throwing a different error when compiling app/node/project/serializer/serializer230220.cpp as of 617ff87:

/builddir/build/BUILD/olive-0.2.0_20240825git617ff87-build/olive-617ff8761250aa4c9f1dd8fde6dec45e0b96639a/app/node/project/serializer/serializer230220.cpp: In member function 'virtual olive::ProjectSerializer::LoadData olive::ProjectSerializer230220::Load(olive::Project*, QXmlStreamReader*, olive::ProjectSerializer::LoadType, void*) const':
/builddir/build/BUILD/olive-0.2.0_20240825git617ff87-build/olive-617ff8761250aa4c9f1dd8fde6dec45e0b96639a/app/node/project/serializer/serializer230220.cpp:193:21: error: 'QStringRef' was not declared in this scope; did you mean 'QStringView'?
  193 |               QList<QStringRef> l = attr.value().split(',');
      |                     ^~~~~~~~~~
      |                     QStringView

After adding #include <QStringRef>, I get:

/builddir/build/BUILD/olive-0.2.0_20240825git617ff87-build/olive-617ff8761250aa4c9f1dd8fde6dec45e0b96639a/app/node/project/serializer/serializer230220.cpp: In member function 'virtual olive::ProjectSerializer::LoadData olive::ProjectSerializer230220::Load(olive::Project*, QXmlStreamReader*, olive::ProjectSerializer::LoadType, void*) const':
/builddir/build/BUILD/olive-0.2.0_20240825git617ff87-build/olive-617ff8761250aa4c9f1dd8fde6dec45e0b96639a/app/node/project/serializer/serializer230220.cpp:194:55: error: conversion from 'QList<QStringView>' to non-scalar type 'QList<QStringRef>' requested
  194 |               QList<QStringRef> l = attr.value().split(',');
      |                                     ~~~~~~~~~~~~~~~~~~^~~~~

@MatthewCroughan , it looks like switching to QStringView is the way here, especially since it is available in Qt 5.10 and above, not just in Qt 6.x:
https://www.kdab.com/qstringview-diaries-eagle-landed/
https://doc.qt.io/qt-5/qstringview.html

Would you take a PR that does s/QStringRef/QStringView/g here?