Qr code reader #1185

sklencar · 2021-02-12T15:40:05Z

QR code reader implementation using ZXing library.

Supported multiple platforms
- full support for Android and iOS,
- supported also MacOS desktop build, but seems to have bug that it is capturing invalid image for ZXing library,
- WIN build not supported;
available for TextEdit widget if an attribute's name contains qrcode case insensitive. Note that name is taken from field.name or overwriten by field.alias if exists.
added invalid data changed notification
basic style with simple header and camera overlay

Related manual-test: lutraconsulting/input-manual-tests#28
closes #408

Known issues

camera issues
- blank page when camera is being loaded (affects some devices)
- QImage qt_imageFromVideoFrame(const QVideoFrame &) : unsupported pixel format Format_ABGR32 while converting videoOutput to QImage (seems to be Android only).
- sometimes not QR decoder not responses well

RPReplay_Final1613558974.MP4

tomasMizera

Very good job!

I did not go through the whole code yet, though what I have seen so far looks great. I tested it on Android and it works well. I could not get Linux build running but I guess we will be able to sort it out.

Some notes:

Permission for camera on Android is now again requested at startup (this was changed in Camera permission crash #1081). I think what we want is to opt it when needed - when user clicks on take a photo or now qr code icon. It is possible that it comes from ZXing library, but the problem is that it will be forbidden in upcoming version of Android to immediately ask for permissions without any information.
If permission is not granted and user opens QR code scanner, it says something like "Camera service is not operating?" which is not true, user only did not provide permission.
Maybe icon for QR Code can be smaller?
When user scans invalid (not convertible) value for field, maybe we could display some more user friendly message, now:

.github/workflows/android.yml

sklencar · 2021-02-24T10:42:19Z

All remarks from notes should be resolved. However I kept the QR code icon unchanged. I would suggest to revise all icons in quick widgets to use its size variable or derive size from a ratio of rowHeight.

tomasMizera

Based on a phone discussion, please add some more comment to have an idea how this is working. It is working very nice on Android! Thanks for resolving the permission problems, it is now working as expected.

app/android.pri

qgsquick/from_qgis/plugin/editor/qgsquicktextedit.qml

tomasMizera · 2021-02-24T20:18:07Z

qgsquick/from_qgis/plugin/qgsquickfeatureform.qml

@@ -474,6 +497,10 @@ Item {
               return form.loadWidgetFn(widget.toLowerCase())
            else return ''
          }
+
+          Component.onCompleted: {
+            supportDataImport = importDataHandler.supportImportData(Name)


supportImportData function is not defined in default importDataHandler in qgsquickfeatureform

Sorry, I can't see how this one was resolved

tomasMizera · 2021-02-24T20:31:35Z

qgsquick/from_qgis/plugin/qgsquickfeatureform.qml

+
+          Component.onCompleted: {
+            supportDataImport = importDataHandler.supportImportData(Name)


I understand the need for this function to be called there, however, it is not very common that library would ask for change. I guess it should be the other way around - Input letting qgsquick know we want to supportDataImport.

I know it is hard to get the Name variable but I think this is too tightly coupled.
I was thinking you could instead of invoking this method when component completes, invoke it by setting the object

It is simplified to use the function from handler directly to assign supportDataImport attribute. Added also default function to default handler definition.

app/qml/FeaturePanel.qml

tomasMizera · 2021-02-25T08:41:25Z

Also regarding the manual test, I vote to add it here lutraconsulting/input-manual-tests#5 rather than creating new test case. Thanks!

sklencar · 2021-02-25T16:53:08Z

Remarks resolved + manual test relocated.

I have checked the issue with scanning the same code for multiple fields. The issue seems to be rather linked with the library (or, of course, something else). This use case is possible, but the scanner acts like it has problem to scan it sometimes. Usually when I was moving camera a bit and wait longer time, it eventually scan the code. This could be possibly improved to add some flags to the DecodeHints in decoder (e.g Binarizer which I keep as default since I was having some issues with reading barcodes when i set it to Binarizer::FixedThreshold). In any case, this is something that has to be investigated further.

tomasMizera

Thanks for resolving the notes, however, I unresolved one, I could not find how it was resolved (the one with Component.onCompleted)

Other than that there are some notes about memory. After clearing these I am ok to merge so passing to you @wonder-sk 🟢

app/codefilter.h

tomasMizera · 2021-02-26T08:28:05Z

qgsquick/from_qgis/plugin/qgsquickfeatureform.qml

@@ -474,6 +497,10 @@ Item {
               return form.loadWidgetFn(widget.toLowerCase())
            else return ''
          }
+
+          Component.onCompleted: {
+            supportDataImport = importDataHandler.supportImportData(Name)


Sorry, I can't see how this one was resolved

app/codefilter.cpp

tomasMizera · 2021-02-26T12:14:19Z

app/codefilter.cpp

+
+QVideoFilterRunnable *CodeFilter::createFilterRunnable()
+{
+  return new QRRunnable( this );


We should probably save this address to a new attribute and free it in CodeFilter's destructor or somehow destroy QRRunnable instance when this (CodeFilter) is deleted

From documentation, it seems that destructions is handled. Also added docs to the function.
However, I am easy to change it if needed.
http://doc.qt.io/qt-5/qabstractvideofilter.html#createFilterRunnable

app/codefilter.cpp

app/codefilter.h

app/qml/CodeReader.qml

mostly related to memory

sklencar changed the title ~~WIP: Qr code reader~~ Qr code reader Feb 17, 2021

sklencar requested a review from tomasMizera February 17, 2021 15:08

sklencar force-pushed the qr_code branch from af867ce to 55ecc53 Compare February 18, 2021 09:47

tomasMizera reviewed Feb 18, 2021

View reviewed changes

.github/workflows/android.yml Show resolved Hide resolved

sklencar force-pushed the qr_code branch from 55ecc53 to 19a2405 Compare February 24, 2021 10:32

sklencar requested a review from tomasMizera February 24, 2021 10:42

tomasMizera reviewed Feb 25, 2021

View reviewed changes

tomasMizera approved these changes Feb 26, 2021

View reviewed changes

vsklencar added 12 commits March 1, 2021 11:48

QR code reader

b0e8cd1

QR code reader

21461f4

QR code reader - upgraded input-sdk

511c2bb

QR code reader

2d7ffca

QR code reader enabled for desktop

116c132

bump qgis deps version for autotest

ebc401f

qr reader - mostly added some docs

e6945a3

qr reader - barcode fixes

7603d5f

Code reader - android permissions

5477fc1

Code reader - user friendly message

f2f74b6

Qr code reader improvements according review

bebb858

Changes after review

03798e3

mostly related to memory

sklencar force-pushed the qr_code branch from 1dcc620 to 03798e3 Compare March 1, 2021 10:49

sklencar requested a review from tomasMizera March 1, 2021 10:49

qr code - fix TextEdit widget padding after rebasing

ecfd0b6

sklencar requested a review from wonder-sk March 1, 2021 12:42

tomasMizera approved these changes Mar 1, 2021

View reviewed changes

wonder-sk merged commit 3ae5476 into master Mar 1, 2021

PeterPetrik deleted the qr_code branch April 15, 2021 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qr code reader #1185

Qr code reader #1185

sklencar commented Feb 12, 2021 •

edited

Loading

tomasMizera left a comment

sklencar commented Feb 24, 2021

tomasMizera left a comment

tomasMizera Feb 24, 2021

tomasMizera Feb 26, 2021

tomasMizera Feb 24, 2021

sklencar Mar 1, 2021

tomasMizera commented Feb 25, 2021

sklencar commented Feb 25, 2021

tomasMizera left a comment

tomasMizera Feb 26, 2021

tomasMizera Feb 26, 2021

sklencar Mar 1, 2021 •

edited

Loading


		Component.onCompleted: {
		supportDataImport = importDataHandler.supportImportData(Name)

Qr code reader #1185

Qr code reader #1185

Conversation

sklencar commented Feb 12, 2021 • edited Loading

tomasMizera left a comment

Choose a reason for hiding this comment

sklencar commented Feb 24, 2021

tomasMizera left a comment

Choose a reason for hiding this comment

tomasMizera Feb 24, 2021

Choose a reason for hiding this comment

tomasMizera Feb 26, 2021

Choose a reason for hiding this comment

tomasMizera Feb 24, 2021

Choose a reason for hiding this comment

sklencar Mar 1, 2021

Choose a reason for hiding this comment

tomasMizera commented Feb 25, 2021

sklencar commented Feb 25, 2021

tomasMizera left a comment

Choose a reason for hiding this comment

tomasMizera Feb 26, 2021

Choose a reason for hiding this comment

tomasMizera Feb 26, 2021

Choose a reason for hiding this comment

sklencar Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

sklencar commented Feb 12, 2021 •

edited

Loading

sklencar Mar 1, 2021 •

edited

Loading