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: New Feature Document Scanner #605

Merged
merged 29 commits into from
Apr 16, 2024
Merged

feat: New Feature Document Scanner #605

merged 29 commits into from
Apr 16, 2024

Conversation

bensonarafat
Copy link
Collaborator

Document Scanner feature based the new feature request #581

Key point to note:

  • Low binary size impact (all ML models and large resources are downloaded centrally in Google Play services).
  • No camera permission is required - the document scanner leverages the Google Play services' camera permission, and users are in control of which files to share back with your app.

Document Scanner is currently in Beta, and not currently available for iOS (I will stay updated and update this feature when available)
Read more on the Google ML doc

Thanks ablause on the onAttachedToActivity issue

Test Sample

2.mp4

@patassel
Copy link

Great result ! Good job guys

@fbernaly
Copy link
Collaborator

I will review, after that I will merge and release a new version. I the meantime use @bensonarafat fork if you need the feature.

@fbernaly
Copy link
Collaborator

@bensonarafat : could address the comments and also make sure that the link job passes.

@bensonarafat
Copy link
Collaborator Author

bensonarafat commented Apr 15, 2024

@bensonarafat : could address the comments and also make sure that the link job passes.
@fbernaly
The link job failed because, the package google_mlkit_document_scanner is not found in pub_spec. should i just use relative path instead ?

@fbernaly
Copy link
Collaborator

use path to pass it, and also make sure the job is added to this file:

https://github.com/flutter-ml/google_ml_kit_flutter/blob/develop/.github/workflows/flutter.yml

they are arrange in alphabetical order.

@bensonarafat
Copy link
Collaborator Author

bensonarafat commented Apr 15, 2024

use path to pass it, and also make sure the job is added to this file:

https://github.com/flutter-ml/google_ml_kit_flutter/blob/develop/.github/workflows/flutter.yml

they are arrange in alphabetical order.

Yeah, i finally got it.
Thanks @fbernaly

Update:
@fbernaly all link now pass.

@fbernaly fbernaly changed the base branch from master to develop April 15, 2024 23:59
@@ -0,0 +1,238 @@
package com.google_mlkit_document_scanner;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Follow the same design we have for the other plugins. You should have 2 files: DocumentScanner.java and GoogleMlKitDocumentScannerPlugin.java

Copy link
Collaborator Author

@bensonarafat bensonarafat Apr 16, 2024

Choose a reason for hiding this comment

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

@fbernaly
Yeah! About that.
Separating the file won't work.
I did that initially, but we need to have the FlutterPlugin and ActivityAware on the same abstract class for the onAttachedToActivity to work for some reason. I spent sometime on it.

@fbernaly
Copy link
Collaborator

@bensonarafat : I have more comments.

@bensonarafat
Copy link
Collaborator Author

bensonarafat commented Apr 16, 2024

@bensonarafat : I have more comments.

Updated.

My comment on separating the class

#605 (comment)

@@ -0,0 +1,4 @@
#import <Flutter/Flutter.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Name of this file should be:
GoogleMlKitDocumentScannerPlugin.h

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay, i thought i changed this.

@@ -0,0 +1,20 @@
#import "GoogleMlKitDocumentScannerPlugin.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Name of this file should be:
GoogleMlKitDocumentScannerPlugin.m

@fbernaly
Copy link
Collaborator

@bensonarafat : I added some more comments, and branch is out-of date, update branch.

@bensonarafat
Copy link
Collaborator Author

@bensonarafat : I added some more comments, and branch is out-of date, update branch.

@fbernaly Branch updated and review updated

@fbernaly fbernaly changed the base branch from develop to feature/doc_scanner April 16, 2024 17:11
@fbernaly
Copy link
Collaborator

@bensonarafat : thanks for your contribution, I am merging this into feature/doc_scanner and play with it, once I confirm the feature is working as expected I will merge into master and cut a release.

@fbernaly fbernaly merged commit 970ac7e into flutter-ml:feature/doc_scanner Apr 16, 2024
3 checks passed
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.

4 participants