Skip to content
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.

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

feat(barcode-scanning): support cornerPoints in readBarcodesFromImage(...) #100

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nurfgun
Copy link

@nurfgun nurfgun commented Nov 23, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • The changes have been tested successfully.
  • A changeset has been created (npm run changeset).

This improved two things :

  • Includes raw cornerPoints in the result data of "readBarcodesFromImage" and "scan" on iOS instead of empty array.
  • Returns empty data when there is no detection on readBarcodesFromImage instead of having a call hanging.

Close #39 and for my personal usage

@nurfgun nurfgun changed the title fix empty cornerPoints and a never-ending call when image has no barcode on iOS fix empty cornerPoints and a never-ending call when image has no barcode on iOS and android Nov 24, 2023
@nurfgun
Copy link
Author

nurfgun commented Nov 24, 2023

did the same for android for missing corner points.

@robingenz robingenz changed the title fix empty cornerPoints and a never-ending call when image has no barcode on iOS and android feat(barcode-scanning): support cornerPoints in readBarcodesFromImage(...) Nov 24, 2023
Copy link
Member

@robingenz robingenz left a comment

Choose a reason for hiding this comment

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

Please run npm run changeset to create a changeset.

@@ -1,6 +1,6 @@
{
"name": "@capacitor-mlkit/barcode-scanning",
"version": "5.3.0",
"version": "5.3.1",
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this change.

@@ -30,7 +30,14 @@ public static JSObject createBarcodeResultForBarcode(@NonNull Barcode barcode, @
cornerPointsResult.put(cornerPointResult);
}
}

else if (cornerPoints != null) {
Copy link
Member

Choose a reason for hiding this comment

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

This also affects the scan(...) method and leads to invalid values there, as these would have to be normalized.
For now, my suggestion is to make this dependent on the screenSize parameter. If this parameter is set, corner points may only be returned normalized. I will improve this later.

Suggested change
else if (cornerPoints != null) {
else if (cornerPoints != null && screenSize == null) {

Make sure that the screenSize parameter is null only for the readBarcodesFromImage(...) method.
Please also apply this to iOS.

@robingenz
Copy link
Member

@nurfgun Thank you for your contribution!

This improved two things :

Please create two seperate PRs. I will review this PR for #39.

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.

feat(barcode-scanning): support cornerPoints in readBarcodesFromImage(...)
2 participants