Refactor QR code reader

[adzialocha]

This PR tackles a couple of issues around scanning QR codes:

1. Make QrReader an independent React component which can be re-used in other places in the future
2. Use jsqr library with inlined WebWorker (before it was wrapped in https://github.com/deltachat/react-qr-reader)
3. Stop camera properly when component gets unmounted (closes camera stays sometimes on after qr code scan #1730 and QR Code scanner does not unwind properly when prematurely closed #3599)
4. Add UI to allow selecting a camera and facing (closes Cant select proper camera. #2634)
5. Bring "Scan from Clipboard" and "Upload image" functionality into UI
6. Accept images from clipboard (closes qr code scanner: also accept images from clipboard and scan from them #3658)
7. Added a spinner to show something while camera is loading
8. Added an visual error mode when something goes wrong
9. Confirmation dialogs did not fire the callback when pressing ESC-keys Scanning a DCACCOUNT QR code doesn't happen if aborted first #3081
10. Remove progress bar dialog parsing QR code, it was almost never shown since parsing is too short and makes it look buggy due to the flickering

Preview

[farooqkz]

Hello there. I see you are trying to add this with WASM. Isn't it better to use native? It has better performance for sure. Not that it really matters since QR code scanning is not compute intensive nor the user would scan 100 codes a day. Just asking.

[adzialocha]

Hello there. I see you are trying to add this with WASM. Isn't it better to use native? It has better performance for sure. Not that it really matters since QR code scanning is not compute intensive nor the user would scan 100 codes a day. Just asking.

Good question! It would totally make sense to do that at one point. I think for now the low hanging fruit is to fix these bugs which would already be a large improvement and just being able to pull this library in is less work than looking into integration into the core. The call to quircs (via Wasm) is roughly 2 lines of code, replacing this with a future RPC etc. call would not be a biggie, so I definitely think there's a path of using a fully native version in the future.

[Simon-Laux]

Could make sense to make a hybrid solution of also still using jsqr in parallel because there are some cases where quircs does not detect the qr codes.
Also for performance processing the images in a web worker would make sense.

• jsqr is what the library, that we currently use, is using for qr code detection/decoding
• quircs is a rust port of quirc made by dignifiedquire.
  • quircs can detect multiple qr codes in one scan and can also give us the position of the qr code in the image (not that we would use it here but still nice feature, I tried adding a qr scan feature to the gallery with it (highlighted clickable qr codes in fullscreen media view), but it was not good enough in detecting and parsing qr codes (first prototype of highlight qr codes in fullscreen image view #3346)
  • I made the wasm wrapper you pulled in, I also experimented with apis to sharpen the image and manipulate it otherwise before passing it with quircs, which seemed to improve detection rate on a low res camera, but did not make any noticeable difference when I tested it with a high-res webcam. (currently it tries again after sharpening if it couldn't find a qr code: https://github.com/Simon-Laux/quircs_wasm/blob/0bc826f35b1249211908a8a352145b6ef8f9bfe1/src/lib.rs#L35)
  • Anyways feel free to also adapt that wasm layer if you need to modify the apis when it is not good enough yet. (https://github.com/Simon-Laux/quircs_wasm/tree/master, I can also move the repo to the deltachat organisation, if we now start using it for real)

Whatever we do we should not use the library that we use on android, as that has detection problems sometimes (see https://support.delta.chat/t/error-qr-code-could-not-be-decoded/2923 and deltachat/deltachat-core-rust#5256).

Also we thought about integrating quircs into core a few times already, but I doubt that it would help us here in desktop much because the jsonrpc does not support binary data, so you would need to encode it as base64 before sending it over, not sure if its still faster with the ipc, but because we wanted to but quircs in core anyway we could also try it at some point, at least android would also benefit from having access to a "better" qr code scanning library

[adzialocha]

Thank you for these insights @Simon-Laux!

What was the reasoning to not use zxing? It looks like unlinke other libraries its further maintained, maybe it got better and we can give it a try again?

[Simon-Laux]

What was the reasoning to not use zxing?

jsqr is just what react-qr-reader is using.

dc android uses zxing, It is a very versatile library detecting all kinds of codes, there is this report of not being able to detect an image of an qr code that quircs can (I linked that report above). Could maybe be that the library got better and we haven't updated it in android?

Better ask some android people if that is still a problem and about their experience with zxing.

[adzialocha]

jsqr is just what react-qr-reader is using.

Ah, interesting. This must have been at the point where you've forked it because in upstream it also uses zxing now.

The QR library doesn't seem to be the main issue of this refactoring, I'll keep that in the background and maybe just use what we have already in case that's just fine.

[adzialocha]

I've integrated zbar-wasm now and it seems to work very well! Based on this paper it should work better https://www.ncbi.nlm.nih.gov/pmc/articles/PMC8321072/

[adzialocha]

I've introduced an Camera and Facing selector to some sort of "Settings" menu which floats over the video image.

How do you feel about adding the functionality for "Paste from Clipboard" and "Upload image" somewhere there as well, either in the same menu or in separate buttons?

This is currently how it looks like (not happy with it):

How about we have two buttons, one "Camera" icon for selecting Camera and Facing, one "Upload" icon for pasting from clipboard and uploading an image? Or even three separate buttons, one for settings, and then one for clipboard and one for uploading an image?

Pinging @Simon-Laux and @r10s

[Simon-Laux]

I imagined it as a rounded square button in the top left corner with white background (in darkmode maybe different bg).
basically a "Floating action button"
I think a single button would be fine if we find the right icon, otherwise we could also do two next or beneath each other. one with a camera icon and the other one with another fitting icon (maybe a generic scanner / view finder icon).
Thinking about it we could also do three buttons, as finding a fitting/intuitive icon for both scan from image file and scan from clipboard feels hard, but then again a single button with all 3 options might be better.

why 3 menu items instead of 4? because I believe we can combine facing mode and cameras to make the interface simpler to understand.

[r10s]

i just tried out the pr on macos with a linked iPhone (so with macos having the option to use the iphone camera, this is possible since ) - works like a charm! this is how it looks like:

for the "Facing Mode" - this did not make a difference, not even when using the iphone camera which has front and back (macos has only one camera). so, maybe we can just strike that option for now until someone complains sth. is missing.

for the menu:

• i think, a single menu will do
• i would move the camera selection as the very first options, if possible technically, without a submenu or an extra title
• after a separator the (since invite links) advanced paste/load options can be added. it also makes sense logically: all of them, "Camera 1", "Camera 2", "Paste From Clipboard", "Load QR code as image" are input methods
• for the menu icon: maybe go for the "flip" image used often for switching camera. but the cogwheel is okay as well. more important than the icon seems that it is clearly visible and not too small

[Simon-Laux]

(since invite links) advanced

I don't completely agree with the "advanced" term here yet, the invite links work most of the time on mobile directly, but on desktop the situation is not as good yet, see #3657.

maybe go for the "flip" image

not fitting if someone only has one camera and/or is looking for the other options.

[r10s]

but on desktop the situation is not as good yet, see #3657

but even then: the browser opens and you tap "Open Chat" with the OPENPGP4FRP: scheme - no need to transfer data more complicated via file or clipboard. or did i missed sth and the issue is larger?

to be clear: maybe the term i've chosen is confusing. i meant, that for most users, switching camera is probably needed more often than the other options. and therefore, make them first, and not hidden in a submenu. i do not suggest to hide or remove paste/load further

for the icon: maybe just stay with the cogwheel then, just make it a bit larger

[Simon-Laux]

or did i missed sth and the issue is larger?

I wouldn't trust it too much there are many platforms (I recently fixed sth about file/url association in the flatpak package for example) and there are also different browsers like people using the tor browser which on purpose does not integrate with the system.

i meant, that for most users, switching camera is probably needed more often than the other options

I think it is the opposite, I do think it's not that often that you are a Mac user with connected iPhone, a live-streamed with multiple connected cameras or a linux phone user that managed to install dc desktop on their phone.

Though I still agree with your conclusion that it would be nice to not have an extra menu for the camera

[r10s]

I wouldn't trust it too much there are many platforms [...] and there are also different browsers like people using the tor browser which on purpose does not integrate with the system.

I think it is the opposite, I do think it's not that often that you are a Mac user with connected iPhone [...]

thanks for sharing thoughts, very interesting!

tbh, when we discussed the menu ordering, i did not expect that this raises a discussion :) so:

note, that the now recommended workflow is scanning QR codes and sharing invite links.

the camera options belong to the recommended workflow. despite tor or broken systems, it is unquestionable also on desktop that load/paste, as supported fallback, plays less a role with the new invite links.

ordering recommending workflow options first makes sense even if we assume for a moment, that there are more ppl using tor or have a broken system than ppl having two webcams ¹

honestly, these are all weak arguments, but they sum up. and at the end, one has to make a decision to move forward :)

if the decision is wrong, we can correct that easily. again: we're discussing ordering of 3 options, not functionality as such

maybe go for the "flip" image

not fitting if someone only has one camera and/or is looking for the other options.

good point, i agree

Footnotes

1. additional webcams are often used for old/broken laptops. or just look at a random office. but also on macos, there is a good share of macos users that have an iphone as well - and get two cameras by that - just look eg. at some random us coworking place or café ↩

[adzialocha]

I think this is ready for review now, some testing with different devices / cameras etc. would be appreciated! Thank you 💖

Happy to also work further on the design if there's any ideas.

[r10s]

very nice! also the new, direct camera selection

i leave code review to the experts, functionality-wise, i can say it works nicely on my mac.

for the UI, some minors, not blocking or so:

• if possible, add a separator line after the camera selection. in general, it is best-practise to group radio items in menus that way
• maybe sort "Load QR code as image" very last - these images are not very well supported on the platforms - where "Paste" has a direct pendent already on desktop
• also, the menu flows out of screen when close to the edge - this would not happen easily when using a a left-hand corner, which is maybe is nicer in general

and when on that: "Hold your camera over the QR code." is often not practical on desktop - when on that, we could add a more fitting string à la "Move the QR code to the camera" - if you agree, i can do a PR on android to get the string in

[Simon-Laux]

if possible, add a separator line after the camera selection. in general, it is best-practise to group radio items in menus that way

our context menu implementation does not support seperators nor radio buttons yet, so we would have to add the functionality there first:

• seperator would just be a visual element, with logic changes to keyboard navigation to skip separator items when selecting the next context menu item.
• I think we can do without radio buttons for now as the checkmark emoji is enough for now and we do not have a design concept for radio/checkbox context menu items yet.

maybe sort "Load QR code as image" very last - these images are not very well supported on the platforms - where "Paste" has a direct pendent already on desktop

I don't care much about the ordering. Though I believe the last and first menu options are the easiest to click quickly, but that will not matter once there is a separator line both items will again be similarly easy to click.

also, the menu flows out of screen when close to the edge - this would not happen easily when using a a left-hand corner, which is maybe is nicer in general

I still favour the top left corner. maybe with a background to look like a real button.

and when on that: "Hold your camera over the QR code." is often not practical on desktop - when on that, we could add a more fitting string à la "Move the QR code to the camera" - if you agree, i can do a PR on android to get the string in

+1

[r10s]

k, if there is no separator possible currently, then it's that. as said, no blocker or so :)

I think we can do without radio buttons for now as the checkmark emoji is enough for now

agree. i was referring to "radio buttons" only for functionality, not for design

[r10s]

i think, the camera is not freed when closing the scanning dialog by tapping outside; this was not the case before (on mac, open camera is very visible by led and an icon in the menu bar)

(i am not totally sure, currently i cannot double check as i cannot build desktop for $reasons currently)

[adzialocha]

i think, the camera is not freed when closing the scanning dialog by tapping outside; this was not the case before (on mac, open camera is very visible by led and an icon in the menu bar)

(i am not totally sure, currently i cannot double check as i cannot build desktop for $reasons currently)

I couldn't reproduce this as it probably was something else than the tapping outside (that just regularly should unwind the component), but looked at the logic again and found some cases where it might have been stuck in edge error cases. I made the logic more robust now.

[Simon-Laux]

we can merge this and then do the remaining changes in a follow up pr (namely to remove blueprint usage), I think it makes sense to get more people to test the new scanner and all the serious problems that I found are already solved now.