-
Notifications
You must be signed in to change notification settings - Fork 58
feat: Change OCR to multi-method model #716
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
Conversation
Refactor of TypeScript interfaces and hooks for OCR and VerticalOCR to support models that expose multiple inference methods for different input sizes. This commit simplifies current setup by allowing a single detector and recognizer source, rather than requiring separate entries for different input sizes.
…er model Adapts the C++ Recognition controllers to handle a single recognizer file that contains multiple inference methods.
This commit addapts the C++ OCR and VerticalOCR controllers to handle a single detector model with multiple inference methods
|
The cpp code and docs look good, someone needs to review the rest and test it |
IgorSwat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall a good code.
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.h
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Recognizer.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Recognizer.cpp
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Recognizer.h
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/vertical_ocr/VerticalDetector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/vertical_ocr/VerticalDetector.cpp
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/vertical_ocr/VerticalDetector.cpp
Show resolved
Hide resolved
4e3b744 to
093ecf3
Compare
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/OCR.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/vertical_ocr/VerticalDetector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/vertical_ocr/VerticalDetector.h
Outdated
Show resolved
Hide resolved
…rtical_ocr/VerticalDetector.h Co-authored-by: Jakub Chmura <[email protected]>
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.h
Outdated
Show resolved
Hide resolved
packages/react-native-executorch/common/rnexecutorch/models/ocr/Detector.cpp
Outdated
Show resolved
Hide resolved
e592b72 to
d935acb
Compare
@msluszniak
Checked your example against main branch. It's not caused by my changes
|
|
Ok, these fields with 0.000 were especially concerning. I don't think it should show any box that recognises no signs 😅 Maybe we should fix this in demo app to not show them? |
d935acb to
9b8d376
Compare
- Changed error messages in Detector classes - Added input width check in generate functions - Reverted 'export' keyword from modelUrls.ts
9b8d376 to
cbf7e36
Compare
|
I guess we released code which does not compile:
It was not visible because CI failed earlier because of cache bloating. Please fix this @benITo47
|
|
Rebasing error. On it |




Description
This PR refactors the OCR and VerticalOCR to improve efficiency.
Previously, our implementation relied on multiple instances of the same detector and recognizer models, each handling a different input size (3x Detector, 4x Recognizer instances). This approach was resource-intensive.
This update introduces a more streamlined approach by using a single detector and a single recognizer model, each with multiple forward_ methods (e.g., forward_800, forward_320). These methods handle different input widths within the same model instance, significantly reducing the number of loaded models and simplifying the API.
This change is a breaking change as it modifies the arguments for useOCR, useVerticalOCR, OCRModule, and VerticalOCRModule
Introduces a breaking change?
Type of change
Tested on
Testing instructions
Manual sanity checks.
Screenshots
Related issues
#692
Checklist
Additional notes