Skip to content

Conversation

navaronbracke
Copy link
Collaborator

@navaronbracke navaronbracke commented Sep 4, 2024

This PR fixes a discrepancy between what is meant with the bounding box of a scanned barcode and the size of the barcode capture output image.

It also adds the following changes:

  • Added support for returnImage on MacOS
  • Fixes a bug where returnImage set to false would not do anything on iOS
  • Cleans up some unused code and refactors some control flow to use early returns

Fixes #1162

cc @fulstadev

## 5.2.2

Improvements:
* [MacOS] Adds Swift Package Manager support.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was added by a previous PR, but wasn't yet in the changelog.

"name" to "barcode",
"data" to barcodes,
"image" to image,
"width" to width!!.toDouble(),
Copy link
Collaborator Author

@navaronbracke navaronbracke Sep 4, 2024

Choose a reason for hiding this comment

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

Since the width/height is only for the output image data, I put them in a Map for the image data.

I also added safe access to the width / height. The method channel implementation handles null width/height by falling back to Size.zero

"driverLicense" to driverLicense?.data, "email" to email?.data,
"geoPoint" to geoPoint?.data, "phone" to phone?.data, "sms" to sms?.data,
"url" to url?.data, "wifi" to wifi?.data, "displayValue" to displayValue
"calendarEvent" to calendarEvent?.data,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I sorted these properties alphabetically, to make it easier to follow. The line "size" to boundingBox?.size, is new.


private val Rect.size: Map<String, Any?>
get() {
// Rect.isValid can't be accessed for some reason, so just do the check manually.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When inspecting the source code of android.graphics.Rect, there was an isValid() method, but I could not access it. So I just took the check from there.

var scanner = BarcodeScanner.barcodeScanner()

/// Return image buffer with the Barcode event
var returnImage: Bool = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was moved

/// Gets called when a new image is added to the buffer
public func captureOutput(_ output: AVCaptureOutput, didOutput sampleBuffer: CMSampleBuffer, from connection: AVCaptureConnection) {
guard let imageBuffer = CMSampleBufferGetImageBuffer(sampleBuffer) else {
print("Failed to get image buffer from sample buffer.")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This print doesn't add any value

if (error == nil && barcodesString != nil && newScannedBarcodes != nil && barcodesString!.elementsEqual(newScannedBarcodes!)) {
return
} else if (newScannedBarcodes?.isEmpty == false) {
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Early return instead of else


var barcodesString: Array<String?>?

// /// Convert image buffer to jpeg
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused

}

/// Start scanning for barcodes
func start(barcodeScannerOptions: BarcodeScannerOptions?, returnImage: Bool, cameraPosition: AVCaptureDevice.Position, torch: Bool, detectionSpeed: DetectionSpeed, completion: @escaping (MobileScannerStartParameters) -> ()) throws {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This parameter is no longer needed

} else {
return barcode.data
}
if error != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rewriting to use early returns

return
}

if (!MobileScannerPlugin.returnImage) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is new. Previously we would always send the image data, which is incorrect.

DispatchQueue.main.async {
result(["name": "barcode", "data": barcodesMap])
}
return
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Early return

let corners = cornerPoints?.map({$0.cgPointValue.data})
return ["corners": corners, "format": format.rawValue, "rawBytes": rawData, "rawValue": rawValue, "type": valueType.rawValue, "calendarEvent": calendarEvent?.data, "contactInfo": contactInfo?.data, "driverLicense": driverLicense?.data, "email": email?.data, "geoPoint": geoPoint?.data, "phone": phone?.data, "sms": sms?.data, "url": url?.data, "wifi": wifi?.data, "displayValue": displayValue]
return [
"calendarEvent": calendarEvent?.data,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Like on Android, I sorted these on alphabetical keys, and added the size.

final List<Map<Object?, Object?>> barcodes =
data.cast<Map<Object?, Object?>>();

if (defaultTargetPlatform == TargetPlatform.macOS) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the MacOS implementation to use the same keys / datatypes as iOS/Android. So this can be removed.

final double? width = event['width'] as double?;
final double? height = event['height'] as double?;
defaultTargetPlatform == TargetPlatform.iOS ||
defaultTargetPlatform == TargetPlatform.macOS) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MacOS is added here, as the format has been updated to match the other platforms.


return BarcodeCapture(
raw: data,
raw: event,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MacOS used the event Map as raw data. This wasn't true for iOS/Android, so I updated it to match MacOS.

Unlikely that a lot of people need the raw data anyway.

factory Barcode.fromNative(Map<Object?, Object?> data) {
final Map<Object?, Object?>? calendarEvent =
data['calendarEvent'] as Map<Object?, Object?>?;
final List<Object?>? corners = data['corners'] as List<Object?>?;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved down, to be sorted on alphabetical names


var symbologies:[VNBarcodeSymbology] = []

// var analyzeMode: Int = 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was unused

setScale(call, result)
case "resetScale":
resetScale(call, result)
// case "analyze":
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused

return
}
guard let imageBuffer = CMSampleBufferGetImageBuffer(sampleBuffer) else {
print("Failed to get image buffer from sample buffer.")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again, a print that didn't add any value.

}
} else {

if error != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored to use early returns.

return
}

let barcodes: [VNBarcodeObservation] = results.compactMap({ barcode in
Copy link
Collaborator Author

@navaronbracke navaronbracke Sep 4, 2024

Choose a reason for hiding this comment

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

There was actually a bug here, too. MacOS would call DispatchQueue.main.async {} for every single barcode, instead of once, for all of them together. This has been corrected, by collecting the barcodes - that match the scan window - and passing them as a list.

})

DispatchQueue.main.async {
if (!MobileScannerPlugin.returnImage) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is new, and similarly to iOS, we only pass the image data if requested.

result(nil)
}

// func switchAnalyzeMode(_ call: FlutterMethodCall, _ result: @escaping FlutterResult) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unused

}

extension VNBarcodeObservation {
public func toMap() -> [String: Any?] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is similar to the extension in the iOS implementation

}

extension CGImage {
public func jpegData(compressionQuality: CGFloat) -> Data? {
Copy link
Collaborator Author

@navaronbracke navaronbracke Sep 4, 2024

Choose a reason for hiding this comment

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

I needed this extension to get the (compressed) JPEG data from the input image.

The signature matches what the iOS implementation uses.

/// The list of scanned barcodes.
final List<Barcode> barcodes;

/// The bytes of the image that is embedded in the barcode.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Clarified a bunch of docs, that were incorrect.

@@ -1,3 +1,6 @@
/// @docImport 'package:mobile_scanner/src/mobile_scanner_controller.dart';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New doc import annotation, for line 27, which has a doc reference to a different class.

"data": barcodes.map({ $0.toMap() }),
"image": cgImage == nil ? nil : [
"bytes": FlutterStandardTypedData(bytes: cgImage!.jpegData(compressionQuality: 0.8)!),
"width": Double(cgImage!.width),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The image on iOS uses doubles for width/height. On Android it uses integers, but we did convert them into doubles.

On MacOS, CGImage also uses integers, so I added a double conversion here.

import UIKit
import Flutter

@UIApplicationMain
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

UIApplicationMain is deprecated. This was updated by the Flutter tool project migrators.

@navaronbracke navaronbracke merged commit c4bf426 into juliansteenbakker:master Sep 5, 2024
5 checks passed
@navaronbracke navaronbracke deleted the size_fix_android branch September 5, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

barcodeCapture.size.isEmpty is always true on Android
1 participant