8000 Improvements to Qt/QML interface by m7913d · Pull Request #945 · zxing-cpp/zxing-cpp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improvements to Qt/QML interface #945

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improvements to Qt/QML interface #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9862112
Improvement to Qt/QML interface
m7913d May 4, 2025
39200a0
Changes based on review.
m7913d May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/Flags.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class Flags
int count() const noexcept { return BitHacks::CountBitsSet(i); }

constexpr inline bool operator==(Flags other) const noexcept { return i == other.i; }
constexpr inline bool operator!=(Flags other) const noexcept { return i != other.i; }

inline Flags& operator&=(Flags mask) noexcept { return i &= mask.i, *this; }
inline Flags& operator&=(Enum mask) noexcept { return i &= Int(mask), *this; }
Expand Down
14 changes: 13 additions & 1 deletion example/ZXingQt6CamReader.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import QtQuick.Layouts
import QtQuick.Shapes
import QtMultimedia
import ZXing
import QtCore // required for CameraPermission (Qt 6.6 and above)

Window {
visible: true
Expand Down Expand Up @@ -58,12 +59,23 @@ Window {
id: devices
}

// Starting from Qt 6.6, camera permission should be requested manually. For older Qt versions, CameraPermission object can be simply removed as Qt automatically requests the permission.
CameraPermission {
id: cameraPermission
Component.onCompleted: {
if (status !== Qt.PermissionStatus.Granted)
request();
}
}
// End of Qt 6.6+ section

Camera {
id: camera
cameraDevice: devices.videoInputs[camerasComboBox.currentIndex] ? devices.videoInputs[camerasComboBox.currentIndex] : devices.defaultVideoInput
focusMode: Camera.FocusModeAutoNear
onErrorOccurred: console.log("camera error:" + errorString)
active: true
active: cameraPermission.status === Qt.PermissionStatus.Granted // for Qt 6.6 and above
// active: true // pre Qt 6.6
}

CaptureSession {
Expand Down
2 changes: 0 additions & 2 deletions example/ZXingQtCamReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ int main(int argc, char *argv[])
QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
#endif

ZXingQt::registerQmlAndMetaTypes();

QGuiApplication app(argc, argv);
app.setApplicationName("ZXingQtCamReader");
QQmlApplicationEngine engine;
Expand Down
87 changes: 56 additions & 31 deletions example/ZXingQtReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <QDebug>
#include <QMetaType>
#include <QScopeGuard>
#include <QtQmlIntegration/qqmlintegration.h>

#ifdef QT_MULTIMEDIA_LIB
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
Expand All @@ -27,11 +28,12 @@

namespace ZXingQt {

Q_NAMESPACE

//TODO: find a better way to export these enums to QML than to duplicate their definition
// #ifdef Q_MOC_RUN produces meta information in the moc output but it does end up working in qml
#ifdef QT_QML_LIB
namespace QML {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a sub-namespace QML here`? See also below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advantage of moving the redefinition of the enums in order to expose it to QML, to a separate namespace is that the enums in ZXingQt can be a typedef to their ZXing versions. This allows your ZXingQtReader.cpp example to compile even if linked with the QML (Qt::Quick) library, i.e. ZXingQt::ReaderOptions().setFormats(ZXingQt::BarcodeFormat::MatrixCodes) is valid in all cases (which was previously not the case if linked with Qt::Quick).

Nobody should uses this QML namespace directly, as from C++, one should use ZXingQt::BarcodeFormat and in QML it is exposed as ZXingQt.BarcodeFormat. So it could be considered as an implementation detail.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the explanation. I'm wondering whether there would be any benefit to be had on the QWidgets side if we simply used those Q_... augmented redefinitions in ZXingQt instead of the typedefs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, not much. Almost the same as using Q_PROPERTY outside a QML environment. If I'm not mistaken, one advantage may be that qDebug() << ZXingQt::BarcodeFormat::MatrixCodes prints the name instead of the integer value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be one advantage. Another would be that offering a QFlag based API is obviously more "native" to Qt developers and offering the most natural API in the different wrappers is definitively a design goal. And if we need to duplicate the enums for QML anyway, then why not use them consistently throughout the Qt wrapper, remove the need for the QML namespace as well as the 'duplication' of the format property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that imply that you also want to create a Qt wrapper for ZXing::ReaderOptions instead of using a simple typedef as ZXing::ReaderOptions expects ZXing::BarcodeFormat instead of ZXingQt::BarcodeFormat?
6D40

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an excellent point, which I've overlooked. While this obviously worsens the cost-benefit ratio, I'd argue that it would still be worth it. First I thought: simply subclass ReaderOptions inside ZXingQt and add an appropriate set of QFlag<> overloads. But then I realized I also need a TextMode overload set. Further, it would make sens to have a signal/slot-ified object for convenient parameter configuration. Now I'm thinking: maybe better go directly the way I outlined in #724 (especially here). To make the Qt wrapper not unnecessarily complex, I'd probably only implement the BarcodeReader class and not the ReadBarcodes + ReaderOptions alternative. This is not obviously not completely thought out but I'm running out of time for now.

If you have general comments about the fluid API idea (which is still in 'flow'...), please add them below the above linked comment for better discoverability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably only implement the BarcodeReader class and not the ReadBarcodes + ReaderOptions alternative.

So, you would like to implement all use cases inside BarcodeReader, i.e. inline QList<Barcode> ReadBarcodes(const QImage& img, const ReaderOptions& opts = {}) should become a member of BarcodeReader?

Concerning the fluent approach, I'm not sure how well it works in case of a QObject base class as it doesn't provide a copy/move constructor.

Q_NAMESPACE
enum class BarcodeFormat
{
None = 0, ///< Used as a return value if no valid barcode has been detected
Expand Down Expand Up @@ -64,20 +66,24 @@ enum class ContentType { Text, Binary, Mixed, GS1, ISO15434, UnknownECI };

enum class TextMode { Plain, ECI, HRI, Hex, Escaped };

#else
Q_DECLARE_FLAGS(BarcodeFormats, BarcodeFormat)
Q_DECLARE_OPERATORS_FOR_FLAGS(BarcodeFormats)
Q_FLAG_NS(BarcodeFormats)
Q_ENUM_NS(BarcodeFormat)

Q_ENUM_NS(ContentType)
Q_ENUM_NS(TextMode)
} // namespace QML
#endif

using ZXing::BarcodeFormat;
using ZXing::ContentType;
using ZXing::TextMode;
#endif

using ZXing::ReaderOptions;
using ZXing::Binarizer;
using ZXing::BarcodeFormats;

Q_ENUM_NS(BarcodeFormat)
Q_ENUM_NS(ContentType)
Q_ENUM_NS(TextMode)

template<typename T, typename = decltype(ZXing::ToString(T()))>
QDebug operator<<(QDebug dbg, const T& v)
{
Expand Down Expand Up @@ -129,15 +135,16 @@ class Barcode : private ZXing::Barcode

using ZXing::Barcode::isValid;

BarcodeFormat format() const { return static_cast<BarcodeFormat>(ZXing::Barcode::format()); }
ContentType contentType() const { return static_cast<ContentType>(ZXing::Barcode::contentType()); }
BarcodeFormat format() const { return ZXing::Barcode::format(); }
ContentType contentType() const { return ZXing::Barcode::contentType(); }
QString formatName() const { return QString::fromStdString(ZXing::ToString(ZXing::Barcode::format())); }
QString contentTypeName() const { return QString::fromStdString(ZXing::ToString(ZXing::Barcode::contentType())); }
const QString& text() const { return _text; }
const QByteArray& bytes() const { return _bytes; }
const Position& position() const { return _position; }
};

namespace Detail {
inline QList<Barcode> ZXBarcodesToQBarcodes(ZXing::Barcodes&& zxres)
{
QList<Barcode> res;
Expand All @@ -149,6 +156,7 @@ inline QList<Barcode> ZXBarcodesToQBarcodes(ZXing::Barcodes&& zxres)
#endif
return res;
}
} // namespace Detail

inline QList<Barcode> ReadBarcodes(const QImage& img, const ReaderOptions& opts = {})
{
Expand All @@ -172,7 +180,7 @@ inline QList<Barcode> ReadBarcodes(const QImage& img, const ReaderOptions& opts
};

auto exec = [&](const QImage& img) {
return ZXBarcodesToQBarcodes(ZXing::ReadBarcodes(
return Detail::ZXBarcodesToQBarcodes(ZXing::ReadBarcodes(
{img.bits(), img.width(), img.height(), ImgFmtFromQImg(img), static_cast<int>(img.bytesPerLine())}, opts));
};

Expand Down Expand Up @@ -282,7 +290,7 @@ inline QList<Barcode> ReadBarcodes(const QVideoFrame& frame, const ReaderOptions
}
QScopeGuard unmap([&] { img.unmap(); });

return ZXBarcodesToQBarcodes(ZXing::ReadBarcodes(
return Detail::ZXBarcodesToQBarcodes(ZXing::ReadBarcodes(
{img.bits(FIRST_PLANE) + pixOffset, img.width(), img.height(), fmt, img.bytesPerLine(FIRST_PLANE), pixStride}, opts));
}
else {
Expand Down Expand Up @@ -330,6 +338,9 @@ class BarcodeReader : public QObject, private ReaderOptions
{
Q_OBJECT

Q_PROPERTY(QML::BarcodeFormats formats READ formatsQML WRITE setFormatsQML NOTIFY formatsChanged)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm overlooking something but does this mean the formats property of the (non-QML) class ZXingQt::BarcodeReader is of type QML::BarcodeFormats?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, ZXingQt::BarcodeReader does not store the formats property itself. The configured formats are only stored by the base class ZXingQt::ReaderOptions (=ZXing::ReaderOptions). So, formats is stored as ZXing::BarcodeFormats.

The Q_PROPERTY macro does not add a member to the class itself, it only tells to QML (i.e. Qt's meta-system) that a property formats is available for this class, which can be accessed using formatsQML and changed using the setFormatsQML, which returns/accepts a QML::BarcodeFormats value.

Technically, you can access the properties exposed by the Q_PROPERTY macro from C++ using Qt's meta system. If one would do so, he would indeed retrieve a QML::BarcodeFormats value, but I guess nobody will do this as just using BarcodeReader::formats and BarcodeReader::setFormats members is much easier from C++.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, you can access the properties exposed by the Q_PROPERTY macro from C++ using Qt's meta system.

That is what I was talking about. My point is, that the meta-object-system from Qt which is used for signal dispatching and also cross-thread-boundary message passing and I forgot for what else more, has an application outside (and predates) QML. Advertising the formats property as being of type QML::BarcodeFormats even without that type even being defined, looks like a source of trouble to me. This would obviously also just go away, if we did away with this extra namespace (see above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q_PROPERTY is not needed/used for signal dispatching or cross-thread-boundary message passing. As far as I know, Q_PROPERTY indeed existed before QML but was mainly for QtDesigner integration. One could also query the value using QObject::property, but I'm not sure why one would do so (maybe for integration with a scripting environment?).

To be honest, I only use Q_PROPERTY to expose properties to QML.

Q_PROPERTY(QML::TextMode textMode READ textModeQML WRITE setTextModeQML NOTIFY textModeChanged)

public:
#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
BarcodeReader(QObject* parent = nullptr) : QAbstractVideoFilter(parent) {}
Expand All @@ -346,35 +357,23 @@ class BarcodeReader : public QObject, private ReaderOptions

#endif

// TODO: find out how to properly expose QFlags to QML
// simply using ZQ_PROPERTY(BarcodeFormats, formats, setFormats)
// results in the runtime error "can't assign int to formats"
Q_PROPERTY(int formats READ formats WRITE setFormats NOTIFY formatsChanged)
int formats() const noexcept
{
auto fmts = ReaderOptions::formats();
return *reinterpret_cast<int*>(&fmts);
}
Q_SLOT void setFormats(int newVal)
BarcodeFormats formats() const noexcept { return ReaderOptions::formats(); }
Q_SLOT void setFormats(BarcodeFormats newVal)
{
if (formats() != newVal) {
ReaderOptions::setFormats(static_cast<ZXing::BarcodeFormat>(newVal));
ReaderOptions::setFormats(newVal);
emit formatsChanged();
qDebug() << ReaderOptions::formats();
}
}
Q_SIGNAL void formatsChanged();

Q_PROPERTY(TextMode textMode READ textMode WRITE setTextMode NOTIFY textModeChanged)
TextMode textMode() const noexcept { return static_cast<TextMode>(ReaderOptions::textMode()); }
TextMode textMode() const noexcept { return ReaderOptions::textMode(); }
Q_SLOT void setTextMode(TextMode newVal)
{
if (textMode() != newVal) {
ReaderOptions::setTextMode(static_cast<ZXing::TextMode>(newVal));
ReaderOptions::setTextMode(newVal);
emit textModeChanged();
}
}
Q_SIGNAL void textModeChanged();

ZQ_PROPERTY(bool, tryRotate, setTryRotate)
ZQ_PROPERTY(bool, tryHarder, setTryHarder)
Expand Down Expand Up @@ -408,6 +407,20 @@ public slots:
void failedRead();
void foundBarcode(ZXingQt::Barcode barcode);

void formatsChanged();
void textModeChanged();

private:
QML::BarcodeFormats formatsQML() const noexcept
{
auto fmts = formats();
return QML::BarcodeFormats(*reinterpret_cast<int*>(&fmts));
}
void setFormatsQML(QML::BarcodeFormats newVal) { setFormats(static_cast<ZXing::BarcodeFormat>(newVal.operator int())); }

QML::TextMode textModeQML() const noexcept { return static_cast<QML::TextMode>(textMode()); }
void setTextModeQML(QML::TextMode newVal) { setTextMode(static_cast<ZXing::TextMode>(newVal)); }

#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
public:
QVideoFilterRunnable *createFilterRunnable() override;
Expand Down Expand Up @@ -489,6 +502,8 @@ Q_DECLARE_METATYPE(ZXingQt::Barcode)

namespace ZXingQt {

namespace Detail {

inline void registerQmlAndMetaTypes()
{
qRegisterMetaType<ZXingQt::BarcodeFormat>("BarcodeFormat");
Expand All @@ -497,14 +512,24 @@ inline void registerQmlAndMetaTypes()

// supposedly the Q_DECLARE_METATYPE should be used with the overload without a custom name
// but then the qml side complains about "unregistered type"
qRegisterMetaType<ZXingQt::Position>("Position");
qRegisterMetaType<ZXingQt::Barcode>("Barcode");
qmlRegisterType<ZXingQt::Position>("ZXing", 1, 0, "position");
qmlRegisterType<ZXingQt::Barcode>("ZXing", 1, 0, "barcode");

qmlRegisterUncreatableMetaObject(
ZXingQt::staticMetaObject, "ZXing", 1, 0, "ZXing", "Access to enums & flags only");
ZXingQt::QML::staticMetaObject, "ZXing", 1, 0, "ZXing", "Access to enums & flags only");
#ifdef QT_MULTIMEDIA_LIB
qmlRegisterType<ZXingQt::BarcodeReader>("ZXing", 1, 0, "BarcodeReader");
#endif // QT_MULTIMEDIA_LIB
}

struct ZXingQtInitializer
{
ZXingQtInitializer() {registerQmlAndMetaTypes();}
} inline zxingQtInitializer;

} // namespace Detail


} // namespace ZXingQt

#endif // QT_QML_LIB
Loading
0