Use takeSnapshot instead of takePhoto on Android#84951
Conversation
|
🚧 @marcaaron has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🚧 @marcaaron has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
Looks good overall, but I see 2 potential problems
|
Good question. It might be. I was thinking if we defer it until the camera is ready then by that point the user is already scanning something and the re-renders would not create a negative impact. But think I will do some more E2E timing and compare things side by side. After testing this a bit - it does seem better on low end Android, but kind of worse on higher end iOS. |
…VisionCameraImprovements
Julesssss
left a comment
There was a problem hiding this comment.
Sneaky, I love it. With a noticable improvements lets do this just for Android?
After testing this a bit - it does seem better on low end Android, but kind of worse on higher end iOS.
Yeah, I think also we should test on a higher end Android device to see if it still helps there. I rolled back the deferred rendering idea since it absolutely made performance worse on an iPhone 16 Pro. |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
# Conflicts: # src/pages/iou/request/step/IOURequestStepScan/hooks/useReceiptScan.ts # src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
|
🚧 @marcaaron has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Julesssss
left a comment
There was a problem hiding this comment.
Testing well on Android 👍
QichenZhu
left a comment
There was a problem hiding this comment.
Sorry for the last minute request, just found a small style issue.
| // This cuts capture time roughly in half while maintaining the same output photo resolution. | ||
| const format = useCameraFormat(device, [{photoAspectRatio: 4 / 3}, {photoResolution: {width: 4032, height: 3024}}, {videoResolution: 'max'}]); | ||
| // Use screen dimensions for video resolution since we only need enough for the preview. | ||
| const format = useCameraFormat(device, [{photoAspectRatio: 4 / 3}, {photoResolution: {width: 4032, height: 3024}}, {videoResolution: {width: windowHeight, height: windowWidth}}]); |
There was a problem hiding this comment.
Can we make these magic numbers constants?
…onCameraImprovements # Conflicts: # src/pages/iou/request/step/IOURequestStepScan/index.native.tsx
|
@AndrewGable Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Looking into why Melvin keeps trying to assign random people to this review 😄 @Julesssss ready for you whenever 🙇 |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.51-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. I reviewed all relevant help site articles under Articles reviewed:
Why no changes are needed: This PR is a pure performance optimization with no user-facing behavioral changes:
The existing help site instructions ("tap the camera icon and snap a photo of a receipt") remain accurate. No feature names, button labels, workflows, settings, or user-facing behaviors are affected. |
|
Deploy Blocker #86852 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.3.51-10 🚀
|
Explanation of Change
After taking a receipt photo, the transition to the confirmation page felt sluggish due to slow photo capture on Android, synchronous Onyx writes blocking navigation, and a heavy component tree rendering during the navigation animation.
This PR addresses:
Faster Android capture (
captureReceipt/index.android.ts): On Android, usetakeSnapshotinstead oftakePhotowhen flash is off.takeSnapshotgrabs the preview buffer directly (~16ms), avoiding the full camera capture pipeline. When flash is on, fall back totakePhoto. iOS keepstakePhotofor both cases.iOS camera format optimization (
index.native.tsx): ReorderuseCameraFormatpriorities sophotoResolutiontakes precedence overvideoResolution. PreviouslyvideoResolution:'max'was selecting a 5712x4284 (24.5MP) format when photos only output at 4032x3024. The new ordering selects a 4032x3024 format directly, cutting iOS capture time roughly in half (~400ms → ~200ms). Capfpsat 30 to prevent the "soap opera effect" that lower-resolution formats exhibit at higher native frame rates.Camera quality/speed tuning (
Camera.tsx): ChangephotoQualityBalancefrom"speed"to"balanced"— benchmarking showed only ~15ms difference but"balanced"applies noise reduction that produces visually better photos with smaller file sizes (2.3MB → 1.5MB for the same scene). DisablephotoHdrandvideoStabilizationModeper VisionCamera performance tips. Remove redundant props (enableBufferCompression,enableDepthData,enablePortraitEffectsMatteDelivery) that already match their defaults.Deferred navigation (
index.native.tsx): UserequestAnimationFramebefore navigating to the confirmation step so React renders the frozen camera state (didCapturePhoto=true) before the screen transitions away.Benchmarks (iOS, iPhone — same subject across tests)
"speed")"balanced", fps=30)Fixed Issues
$ #84793
$ #85006
Tests
Repeat with track expense flow (FAB → Create expense → photo)
General testing of flash
Test multi-scan mode (take multiple photos)
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.