StevensSEC / monocle

A mobile app to transcribe images of printed text from a page.
GNU General Public License v3.0
3 stars 0 forks source link

Add react-navigation, move processing logic from CameraView to multiple files, created modal for results view, added better debugging experience #52

Closed rmonaghanjr closed 2 years ago

rmonaghanjr commented 2 years ago

I broke up the image processing logic into multiple files:

I added react-navigation to serve as the primary way of managing multiple screens in the app. It can do modals, drawers, and stacks. It can also do bottom tabs, top tabs, and requires no native module compiling. You can learn more about it here.

I also added an app lock until the tf/object detection model is loaded to prevent errors.

dcarpenter31 commented 2 years ago

Also got these errors when trying to run the app:

Non-serializable values were found in the navigation state. Check:

Camera > params.objectModel.model.resourceManager.addHashTable (Function)

This can break usage such as persisting and restoring state. This might happen if you passed non-serializable values such as function, class instances etc. in params. If you need to use components with callbacks in your options, you can use 'navigation.setOptions' instead. See https://reactnavigation.org/docs/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state for more details.
at node_modules/@react-navigation/core/src/BaseNavigationContainer.tsx:350:14 in React.useEffect$argument_0
at App.tsx:38:19 in load

[Unhandled promise rejection: Error: Out of memory]
at node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.node.js:2718:24 in uploadDenseMatrixToTexture
at node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.node.js:2934:34 in GPGPUContext.prototype.uploadDenseMatrixToTexture
at node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.node.js:6449:12 in MathBackendWebGL.prototype.uploadToGPU
at node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.node.js:9137:4 in slice
at node_modules/@tensorflow/tfjs-backend-webgl/dist/tf-backend-webgl.node.js:16623:26 in unpack
at node_modules/@tensorflow/tfjs-core/dist/tf-core.node.js:4539:22 in kernelFunc
at node_modules/@tensorflow/tfjs-core/dist/tf-core.node.js:4600:36 in scopedRun$argument_2
at node_modules/@tensorflow/tfjs-core/dist/tf-core.node.js:4404:23 in Engine.prototype.scopedRun
at node_modules/@tensorflow/tfjs-core/dist/tf-core.node.js:4596:8 in Engine.prototype.runKernelFunc
at node_modules/@tensorflow/tfjs-core/dist/tf-core.node.js:5406:25 in f2
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:4603:32 in TensorArray.prototype.scatter
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:5197:20 in __generator$argument_1
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:81:17 in step
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:55:13 in <anonymous>
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:51:11 in __awaiter
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:6607:16 in executeOp$j
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:7418:41 in _loop_1
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:7444:19 in GraphExecutor.prototype.processStack
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:7370:35 in __generator$argument_1
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:81:17 in step
at node_modules/@tensorflow/tfjs-converter/dist/tf-converter.node.js:52:47 in fulfilled

Looks like I'm running out of memory same as @dyc3 . the first error seems to be related to line 21 of CameraView, is there any way we can pass less data through that method?

Otherwise, I like that it auto-navigates you to the other view! that's exactly what I imagined while writing the use cases.

dyc3 commented 2 years ago

Maybe we could pre-convert the model to tensorflow js, since it seems to be doing that automatically on startup. also, see #45 But I think that should be in a separate PR tho.

rmonaghanjr commented 2 years ago

Well for organization I would say yes we do, so that we can distinguish between components of a screen (button, maybe a custom text view, a loading indicator, basically things that can be used in multiple places) from a screen (a thing for usage in one place only).

dcarpenter31 commented 2 years ago

I'm not sure I understand. A View component in this project is meant to be one that takes up the whole screen. we have View components in both /components, and in /screens. If a component is not a view (does not take up the whole screen), it should not have the View suffix.

rmonaghanjr commented 2 years ago

Yeah...I overlooked this in my naming convention. I'll move the files.

dcarpenter31 commented 2 years ago

we could also consider switching the suffix to Screen if that is less confusing

rmonaghanjr commented 2 years ago

we could also consider switching the suffix to Screen if that is less confusing

That is originally what I had, dyc3 requested a change for that.

dcarpenter31 commented 2 years ago

we could also consider switching the suffix to Screen if that is less confusing

That is originally what I had, dyc3 requested a change for that.

I'll open up an issue and we can discuss there

rmonaghanjr commented 2 years ago

we could also consider switching the suffix to Screen if that is less confusing

That is originally what I had, dyc3 requested a change for that.

I'll open up an issue and we can discuss there

Sounds good.