"New" Scanner decision / consideration

[M123-dev]

We switched to the new, more outside-managed scanner a few weeks before. From our limited testing, they seem to work very well.

But we have only released it to the beta testers (Around 1k persons)

Now the question is are we positive to release or are there things we want to change before the next public release?

Especially considering the drastic UI change and possible feedback from beta testers @teolemon

• [ ] Remove the ZXing and ML header in the scanner
• [ ] Make it modular again, we don't need to push both engines to every user (Not high p, the app uses 38mb with both scanning engines on my phone)

[M123-dev]

Also another thing to bring in here is the discussion to change the whole start page, do we still want to force users to start the camera at the app init? Do we need it full-screen? Is there a better alternative to the scanner?

[monsieurtanuki]

The main UI/UX change with the "new" scanners is that they are not full screen, in order to avoid technical issues regarding actual scan window. That change is compliant with minimum viable standards ("the UI is understandable" + "the scan does work"), which is good enough to me.

[teolemon]

• We need slightly more contrast in light mode between the background and the scan card
• We need to disable those labels, indeed (or make them optional and/or when DEV mode is enabled ?)

[M123-dev]

We need to disable those labels, indeed (or make them optional and/or when DEV mode is enabled ?)

Already removed in the latest dev build, together with the switch

[CoryADavis]

Performance & Stability

I think you should see good performance with mobile_scanner. We are using a similar configuration in production with ~100k users, and it's performing better than all prior implementations. We've tried many packages and implementation details over the last year and a half; barcode scanning has required more experimentation in production than anything else.

By similar configuration, I'm referring to using: same package, same package version, wrapped in a visibility detector, and not full screen.

Code

It shouldn't be fatal, but it may not be a bad idea to catch platform exceptions when calling await _controller.toggleTorch(); as it will fail with a PlatformException when the camera is stopped or the torch is unavailable.

UI/UX

With the new design that isn't full screen, the full width camera feels a little harsh to me, I wonder if a design where the camera cut-out respects the global margins would work?

Quick edit based on the last mock

[monsieurtanuki]

Thank you @CoryADavis for your feedback!

I'm about to PR the catch you're suggesting.

Regarding the UI, I've just tried what you suggested, but the scan window looks a bit too small for me (and it should probably be aligned vertically with the product card). Maybe make the scan window a bit taller?	before	after	taller

[CoryADavis]

I agree 100%. I think your alignment and height modifications are big improvements that actually make this UI variant viable; my original edit would have been too short for sure.

[monsieurtanuki]

Working on a PR about the UI change (especially in dark mode).

@CoryADavis I have to say I've found a problematic bug: I was able to scan a barcode that was NOT at all in the scan window/visible camera area. It's confusing, isn't it?

[monsieurtanuki]

@CoryADavis Should we explicitly set a scanWindow even if our scan widget only takes let's say the top half of the screen?

[CoryADavis]

Oh, interesting! Because the default scan window should be relative to the widget before being clipped, with this small clip I would have expected that scenario to be acceptably rare during usage on physical device.

If this came up quite quickly during testing though, I imagine it is indeed a problem.

Setting an explicit scan window does sound like it would work though! My only concern would be that I’m not sure if that has the potential to degrade the responsiveness of the scanner.

Edit: I modified the figma to incorporate your improvements, and added a second page with another design jumping off point in case the cut-out causes too much trouble. The primary conclusion I got from playing with the second design is that modifying padding and utilizing a fully transparent status bar really helps the full width camera transition to the cards below.

[teolemon]

@monsieurtanuki @CoryADavis Can the scanner be made full screen again, using this scanWindow attribute from the mobile_scanner library which seems to handle for us the complicated mathematics we used to do ?

[monsieurtanuki]

@teolemon I understand your point, but the other problem is the focus (beyond the fact that setting the scanWindow is not that easy). Having a scan visor not centered on the center of the screen (= camera) is problematic.

If you ask me, the UI/UX would look better with a full screen camera and a centered visor. And if you want to put data on top, I would suggest much smaller product cards, especially given the low added-value of the current carousel product card (which is basically a "click me for more info" button).

Let me try something, for argument's sake.

[monsieurtanuki]

Something like that:	now	suggestion

[teolemon]

• I think we need to dissociate a specific layout for small screen like yours, with the objective to have some explanations on screen beyond the compatibility color
• on the layout, something like that ?

[monsieurtanuki]

@teolemon I acknowledge the fact that my screen is particularly small and that not everything can be deducted from it.

That said:

1. the focus of the camera being typically on the center of the camera, we need a full screen camera and a centered viewfinder (like in my latest screenshot, like in yours), possibly with a centered scan window
2. currently there's no specific height for the carousel, just a height percentage (55%). That could be a combination of percentage with min height and max height.
3. I have to say that I'm not a big fan of your screenshot, as a barcode beneath the "welcome!" card would be scanned though it would be barely visible. The same would happen in my case, it would be just less frequent and more visible.
4. I confirm that as a user I would prefer smaller carousel product cards, as I do think that in all cases ("welcome!/search" and product) it's just a verbose button (for the product the most important being the compatibility color and the product title) (and an "info" icon button could do the trick)
5. tomorrow I'll implement both your version and mine, with DEV MODE settings that will change the carousel height percentage

[CoryADavis]

Exciting stuff! I didn't know that the card layout was a negotiable aspect of the current design specification.

The direction you are exploring seems like a closer fit with Google's recommendations, which is likely a good sign.

[monsieurtanuki]

I've just PR'ed #3886:

• full screen camera
• viewfinder and carousel with variable heights (with DEV MODE)

Oops, it looks like I've lost the flash and switch buttons in the process (actually they're under the carousel). For the record the best practice would be to put them in a semi-transparent app bar.

With the current settings (55% for the carousel) we cannot see the centered "search" icon. We could marginally make it visible with a semi-transparent carousel, but that would make the carousel harder to read.

Please experiment with this PR and send your favorite [carousel height x viewfinder height] combinations!

[M123-dev]

FYK: In yesterday's community meeting we have discussed this topic in great detail, also using #3886.

The plan we ended up with is the following:

• First @g123k tries to make the mobile_scanner full screen with the focus point moved upwards directly in the native code.

• Then we discuss possible new bigger visual changes regarding the carousel together with the community on the new upcoming project https://blog.openfoodfacts.org/en/news/towards-1-million-users-and-beyond-for-open-food-facts-mobile-app

[monsieurtanuki]

It's indeed good news if mobile_scanner gets that focus improvement thanks to @g123k's work! Of course that means the same improvement must also be coded for qr_code_scanner and the FDroid users.

[monsieurtanuki]

Meanwhile, Sentry keeps sending the same reports with the same errors:

[teolemon]

@monsieurtanuki I'm increasing the Android rollout (without the cosmetic fixes), and @g123k is going to work on both solving the iOS issue, and the cosmetic fixes.

[monsieurtanuki]

Oh, I wasn't aware there was an iOS issue.

Whatever cosmetic fix should be implemented for both mlkit and fdroid, so let me raise a red flag.

[teolemon]

Issue

• https://github.com/openfoodfacts/smooth-app/issues/3935

  Short term:

• https://github.com/juliansteenbakker/mobile_scanner/issues/629

  Long term:

• https://github.com/juliansteenbakker/mobile_scanner/issues/630

[teolemon]

Ok Julian is needing a maintainer: https://github.com/juliansteenbakker/mobile_scanner/issues/652

3 options:

• We take the mantle of maintaining the package (and that means fixing a bunch of things)
• @g123k is going to ask the maintainer of https://pub.dev/packages/camerawesome in real life tomorrow about the opportunity of a switch. Big plus: Edouard personally knows the owner of the company / one of the maintainers. @monsieurtanuki @M123-dev @g123k
• Google released a camerax revision of camera package, so we could go full vanilla

[CoryADavis]

Do you know if there has been meaningful strides in performance with the two package solutions recently?

My app used a two package solution with the Flutter Camera library and a separate Flutter MLKit library 1.5 years ago, and the performance was quite poor when compared to a one package solution that passes directly to MLKit natively without communicating back to Flutter.

The developer ergonomics of the two package solution was quite good though!

[monsieurtanuki]

@CoryADavis What I can say is that several months ago we used a custom solution for which flutter run took 30 minutes (!) on my old macbook for my old samsung smartphone. Instead of 2 minutes for our current implementations with mlkit or zxing. As I didn't code our previous barcode scanner I cannot say how it worked.

@teolemon Whatever solution would suit me, as long as in the end a reliable barcode scanner is available in pub.dev.

[M123-dev]

Do you know if there has been meaningful strides in performance with the two package solutions recently?

I think we haven't done a direct comparison, but passing an image from native to flutter back to native code is definitely something which should be avoided if possible.

Eventually we switched away due to other reasons, stored the image in storage and analyzed this one using ml kit, so:

Native -> Storage -> ML Kit

But I'd say a scanner which works on every device, is maintained is more important than avoiding unnecessary en-/decoding

[g123k]

Here is a list of the choices we have:

• Go back to old camera (2)/MLKit implementation from Google
• Use the experimental camera (X)/MLKit implementation from Google
• Write our own plugin and don't rely anymore on un-trustable dependencies
• Use camera_awesome where I, personally, know the maintainer
• Fork mobile_scanner and implement the missing features (iOS wrong camera, bring back the fullscreen scanner) AND in parallel create a PR for each of them in the official repo

[g123k]

Personally, I think the last option is better, as we have good results with the current implementation. We are just stuck by a non-maintained project.

By forking the project during the pause and still creating PR in the official repo, I think that's the best thing to do.

[monsieurtanuki]

My 2 cents:

• Don't forget ZXing
• I don't experience the enthusiasm of you guys for a fullscreen scanner
• I would rather spend time helping the release of the official repo than maintaining a fork

[teolemon]

At this point , it's just fixing the bugs, not even the layout. @g123k @monsieurtanuki let's fix mobile_scanner, I'll try to ensure our PR get merged, and we don't diverge

[monsieurtanuki]

At this point , it's just fixing the bugs, not even the layout. @g123k @monsieurtanuki let's fix mobile_scanner, I'll try to ensure our PR get merged, and we don't diverge

About #3935 (that is the only bug directly related to mobile_scanner AFAIK), we could double-check rapidly if it is actually fixed by merged https://github.com/juliansteenbakker/mobile_scanner/pull/592:

• in pubspec.yaml, link to the repo of mobile_scanner (instead of the pub.dev latest version)
• ask @tacitefood or whoever has an iPhone or experienced the same issue, and have them test if now it works

[g123k]

As the issue is now solved with iOS, I think we can close this issue and maybe reopen new ones, to eventually improve mobile_scanner