Code cleanup and added metadata for ML Vision #631

bparrishMines · 2018-06-21T21:34:16Z

No description provided.

…xt_dart

…xt_android

…ext_iOS

bparrishMines · 2018-06-25T17:36:20Z

This PR is also missing a rebase. I'm working on rebasing it correctly. The HEAD for this branch is 9c2af92.

…eanup

mravn-google

LGTM

mravn-google · 2018-06-26T07:42:29Z

FlutterFire.md

@@ -26,6 +26,7 @@ The plugins are still under development, and some APIs might not be available ye
 | [firebase_performance][performance_pub] | ![pub package][performance_badge] | [Firebase Performance Monitoring][performance_product] | [`packages/firebase_performance`][performance_code] |
 | [firebase_remote_config][remote_config_pub] | ![pub package][remote_config_badge] | [Firebase Remote Config][remote_config_product] | [`packages/firebase_remote_config`][remote_config_code] |
 | [cloud_functions][functions_pub] | ![pub package][functions_badge] | [Cloud Functions][functions_product] | [`packages/cloud_functions`][functions_code] |
+| [firebase_ml_vision][ml_vision_pub] | ![pub package][ml_vision_badge] | [Firebase ML Kit][ml_vision_product] | [`packages/firebase_ml_vision`][ml_vision_code] |


This list is getting long. Perhaps alphabetize it?

mravn-google · 2018-06-26T07:42:39Z

README.md

@@ -70,5 +70,6 @@ These are the available plugins in this repository.
 | [firebase_remote_config](./packages/firebase_remote_config/) | [![pub package](https://img.shields.io/pub/v/firebase_remote_config.svg)](https://pub.dartlang.org/packages/firebase_remote_config) |
 | [firebase_dynamic_links](./packages/firebase_dynamic_links/) | [![pub package](https://img.shields.io/pub/v/firebase_dynamic_links.svg)](https://pub.dartlang.org/packages/firebase_dynamic_links) |
 | [cloud_functions](./packages/cloud_functions/) | [![pub package](https://img.shields.io/pub/v/cloud_functions.svg)](https://pub.dartlang.org/packages/cloud_functions) |
+| [firebase_ml_vision](./packages/firebase_ml_vision/) | [![pub package](https://img.shields.io/pub/v/firebase_ml_vision.svg)](https://pub.dartlang.org/packages/firebase_ml_vision) |


mravn-google · 2018-06-26T07:43:51Z

packages/firebase_ml_vision/README.md

+
+## Usage
+
+To use this plugin, add `firebase_ml_vision` as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/). You must also configure firebase for each platform project: Android and iOS (see the example folder or https://codelabs.developers.google.com/codelabs/flutter-firebase/#4 for step by step details).


"configure firebase" => "configure Firebase"?

mravn-google · 2018-06-26T07:45:06Z

packages/firebase_ml_vision/README.md

+
+To use the on-device text recognition model, run the text detector as described below:
+
+1. Create a FirebaseVisionImage object from your image.


FirebaseVisionImage should be code font for consistency.

mravn-google · 2018-06-26T07:45:42Z

packages/firebase_ml_vision/README.md

+  final String text = block.text;
+
+  for (TextLine line in block.lines) {
+    // ....


An ellipsis is usually three dots: // ...

mravn-google · 2018-06-26T07:49:13Z

...vision/android/src/main/java/io/flutter/plugins/firebasemlvision/FirebaseMlVisionPlugin.java

+          try {
+            textDetector.close();
+          } catch (IOException exception) {
+            result.error("textDetectorError", exception.getLocalizedMessage(), null);


If we get to this code, we'll be invoking both result.error and result.success.

mravn-google · 2018-06-26T07:51:26Z

packages/firebase_ml_vision/lib/src/text_detector.dart

+  /// Due to the possible perspective distortions, this is not necessarily a
+  /// rectangle. Parts of the region could be outside of the image.
+  ///
+  /// Could be empty even if text is found.


Any idea what that means or when that happens?

The android implementation has the @Nullable annotation for boundingBox() and cornerPoints(). I haven't found anywhere that specifies why or had it actually return null for any of the test images. However, I added code and a comment to handle it just in case it does because I didn't want it to accidentally crash an app. Also, I get warnings from android studio to handle a null situation.

What's interesting though, is that the iOS side marks cornerPoints() as _NonNull.
https://firebase.google.com/docs/reference/ios/firebasemlvision/api/reference/Protocols/FIRVisionText

mravn-google · 2018-06-26T07:53:17Z

packages/firebase_ml_vision/lib/src/text_detector.dart

            .map<Point<num>>((dynamic item) => Point<num>(
                  item[0],
                  item[1],
                ))
            .toList(),
        text = data['text'];

-  /// Axis-aligned bounding rectangle of the detected text.
-  final Rectangle<num> boundingBox;
+  final List<Point<num>> _cornerPoints;


Could we avoid num here and just use double?

tl;dr
num => int

I decided to use num because iOS returns doubles while android returns ints. With further investigating though, iOS only returns doubles for bounding box because CGRect uses doubles. So, I changed num to int to make it clearer to the user that the values will be whole numbers.

mravn-google · 2018-06-26T07:53:29Z

packages/firebase_ml_vision/lib/src/text_detector.dart

-  /// rectangle. Parts of the region could be outside of the image.
-  final List<Point<num>> cornerPoints;
+  /// Could be null even if text is found.
+  final Rectangle<num> boundingBox;


mravn-google · 2018-06-26T07:55:33Z

packages/firebase_ml_vision/test/firebase_ml_vision_test.dart

+          ),
+        ]);
+
+        expect(blocks, <TextBlock>[]);


Or expect(blocks, isEmpty);

bparrishMines added 30 commits June 11, 2018 15:56

Initial 'fluter create' for Firebase ML Vision plugin

c0303b3

Formatting

cbe3715

More Formatting

d6c20cd

Import ordering

e002953

Added android app to firebase

79c085f

Added ios app to firebase

a1775e8

Added ml vision to android and ios

a6734f1

Merge branch 'master' of github.com:flutter/plugins into ml_vision

debb1c4

WIP dart side of ml vision

84ac21f

finish dart side of ml vision and text detector

e193edf

Added TextDetector tests

ef0a43f

Added some comments

1b098f7

Finished comments

3475f17

Removed usused dependency

fe05e34

Merge branch 'master' of github.com:flutter/plugins into ml_vision_te…

a958e5b

…xt_dart

Formatting and unused imports

e8faf79

Changed variable name to restart WIP test

a634645

Start of android text detection

bb2c0b5

Added text recognition to android channel

76e7917

Create dart test app

636cdc6

Added to test app

418f646

Merge branch 'master' of github.com:flutter/plugins into ml_vision_te…

29ec78d

…xt_android

Finished example ml vision app

e3f5a02

Merge branch 'master' of github.com:flutter/plugins into ml_vision_te…

ed3ec07

…xt_android

Formatting

2a5b223

Fixed crash

e91584a

Add permssions

98de4cf

Setup iOS side of text detector

c1b7114

Formatting and comments

0bcc44b

More formattign

c611304

bparrishMines added 2 commits June 19, 2018 11:03

More More Formatting

501b85a

Merge branch 'master' of github.com:flutter/plugins into ml_visioin_t…

ad6b96d

…ext_iOS

googlebot added the cla: yes label Jun 21, 2018

bparrishMines changed the title ~~Code cleanup and added metadat for ML Vision~~ Code cleanup and added metadata for ML Vision Jun 22, 2018

bparrishMines requested a review from mravn-google June 25, 2018 16:38

Code cleanup and added necessary metadata

2aa05b8

Merge branch 'master' of github.com:flutter/plugins into ml_vision_cl…

f6c3bc5

…eanup

mravn-google approved these changes Jun 26, 2018

View reviewed changes

bparrishMines added 6 commits June 26, 2018 11:27

Alphabetize

df5055e

Small code fixes and updated README

6427a5b

Return only ints

822bc38

Formatting

ae3d887

Simplified conversion

c411458

Formatting

00cf836

bparrishMines merged commit f90755e into flutter:master Jun 28, 2018

bparrishMines deleted the ml_vision_cleanup branch June 28, 2018 16:34

julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020

Code cleanup and added metadata for ML Vision (flutter#631)

3189324

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code cleanup and added metadata for ML Vision #631

Code cleanup and added metadata for ML Vision #631

bparrishMines commented Jun 21, 2018

bparrishMines commented Jun 25, 2018

mravn-google left a comment

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018

bparrishMines Jun 26, 2018

mravn-google Jun 26, 2018

bparrishMines Jun 26, 2018

mravn-google Jun 26, 2018

mravn-google Jun 26, 2018


		## Usage

		To use this plugin, add `firebase_ml_vision` as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/). You must also configure firebase for each platform project: Android and iOS (see the example folder or https://codelabs.developers.google.com/codelabs/flutter-firebase/#4 for step by step details).


		To use the on-device text recognition model, run the text detector as described below:

		1. Create a FirebaseVisionImage object from your image.

Code cleanup and added metadata for ML Vision #631

Code cleanup and added metadata for ML Vision #631

Conversation

bparrishMines commented Jun 21, 2018

bparrishMines commented Jun 25, 2018

mravn-google left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment