RMET-2912 OSBarcodeLib-Android - Screen UI for Tablet and only scan frame area #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: According to Android's documentation, we should use the Window size classes to determine if the device the app is running on is a phone or a tablet. When in Portrait, we should use the window width to determine this (99.96% of phones will have a compact width). When in Landscape, the height of the window should be used (99.78% of phones will have a compact height). More info here: https://developer.android.com/guide/topics/large-screens/support-different-screen-sizes#window_size_classes, and here: https://developer.android.com/jetpack/compose/layouts/adaptive References: https://outsystemsrd.atlassian.net/browse/RMET-2912
Context: According to Android's documentation, we should use the Window size classes to determine if the device the app is running on is a phone or a tablet. When in Portrait, we should use the window width to determine this (99.96% of phones will have a compact width). When in Landscape, the height of the window should be used (99.78% of phones will have a compact height). More info here: https://developer.android.com/guide/topics/large-screens/support-different-screen-sizes#window_size_classes, and here: https://developer.android.com/jetpack/compose/layouts/adaptive Why we are using the landscape UI (ScanScreenUILandscape) for tablets for both orientations, portrait and landscape: because for tablets, the arrangement of the elements in the screen is always the same, in a row, where the buttons are to the right of the scan area (rectangle). This way, we can avoid code repetition. References: https://outsystemsrd.atlassian.net/browse/RMET-2912
Context: Since we want to "crop" the image passed to analyze, we need to use a Bitmap object to represent the image, so that we can "crop" it. This wouldn't be possible using the ImageProxy object directly. References: https://outsystemsrd.atlassian.net/browse/RMET-2912
Context: Since we want to "crop" the image passed to analyze, we need to use a Bitmap object to represent the image, so that we can "crop" it. This wouldn't be possible using the ImageProxy object directly. Since both libraries (ZXing and MLKit) now use a Bitmap, we might as well refactor the code and avoid code repetition. References: https://outsystemsrd.atlassian.net/browse/RMET-2912
Context: Since we only want to look for barcodes within the frame, we need to crop the image (bitmap) to analyze, before passing it to MLKit or ZXing. References: https://outsystemsrd.atlassian.net/browse/RMET-2912https://outsystemsrd.atlassian.net/browse/RMET-2912
alexgerardojacinto
requested review from
nflsilva,
hectorjdgdev and
OS-ricardomoreirasilva
alexgerardojacinto
changed the title
hectorjdgdev
suggested changes
Dec 7, 2023
| val image = InputImage.fromMediaImage( | ||
| mediaImage, | ||
| val image = InputImage.fromBitmap( | ||
| imageBitmap, | ||
| imageProxy.imageInfo.rotationDegrees | ||
| ) | ||
| runBlocking { |
There was a problem hiding this comment.
Is needed to use runBlocking here when MLKit already provides the async work?
There was a problem hiding this comment.
You're right, nice catch! We don't need to use runBlocking in this case. I think we used to have more code after scanner.process(image) and so that's why it had the runBlocking.
Context: The call to scanner.process is asynchronous, as it should be. As such, we don't need to use runBlocking here. References: https://outsystemsrd.atlassian.net/browse/RMET-2912
nflsilva
approved these changes
Dec 7, 2023
OS-ricardomoreirasilva
approved these changes
Dec 7, 2023
|
Kudos, SonarCloud Quality Gate passed! |
hectorjdgdev
approved these changes
Dec 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Bitmapobjects for both libraries. Before, we were only using bitmaps for ZXing, because MLKit can deal withImageProxyobjects. Since we want to crop the image, we need to use aBitmap. This way, we also avoid code repetition.Context
References: https://outsystemsrd.atlassian.net/browse/RMET-2912
Type of changes
Platforms affected
Tests
MABS 9 build: https://intranet.outsystems.net/MABS/BuildDetail.aspx?BuildId=ed8778ad27785e820008bbd3ffcc4ea52e3e0f62
MABS 10 build: https://intranet.outsystems.net/MABS/BuildDetail.aspx?BuildId=5c41618abb4ebfa0044ef94a00a88bfa971e5153
Tested in Pixel 7 with Android 14, and Pixel C tablet emulator with Android 14.
Screenshots (if appropriate)
UI for Tablets:
Portrait
Landscape
Only scan the frame area:
screen-20231206-180638.mp4
Checklist
RNMT-XXXX <title>