Skip to content

adds hints trying to improve read performance #5798

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

Open
wants to merge 3 commits into
base: graphite-base/5798
Choose a base branch
from

Conversation

cmonfortep
Copy link
Contributor

@cmonfortep cmonfortep commented Mar 19, 2025

Task/Issue URL: https://app.asana.com/0/488551667048375/1209726713190993/f

Description

Adds some options trying to improve QR scan performance

Steps to test this PR

N/A just try scan QR codes

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

@cmonfortep
Copy link
Contributor Author

cmonfortep commented Mar 19, 2025

@CDRussell CDRussell changed the base branch from feature/cristian/sync/align_recovery_with_connect_flow to graphite-base/5798 March 19, 2025 17:11
@cmonfortep cmonfortep force-pushed the feature/cristian/sync/improve_scan_accuracy branch from 835f50f to b8253b8 Compare March 19, 2025 22:36
@cmonfortep cmonfortep changed the base branch from graphite-base/5798 to feature/cristian/sync/align_recovery_with_connect_flow March 19, 2025 22:36
@cmonfortep cmonfortep requested a review from CDRussell March 19, 2025 22:37
@CDRussell CDRussell changed the base branch from feature/cristian/sync/align_recovery_with_connect_flow to graphite-base/5798 March 20, 2025 13:45
@CDRussell CDRussell self-assigned this Mar 21, 2025
Comment on lines +135 to +137
if (syncFeature.qrCodeScannerHints().isEnabled()) {
if (appBuildConfig.isInternalBuild()) {
beepManager = BeepManager(getActivity())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to switch the order of these two if statements?

appBuildConfig.isInternalBuild() is cheaper than syncFeature.qrCodeScannerHints().isEnabled() so would be good to check that first

also, can you get syncFeature.qrCodeScannerHints().isEnabled() off of the main thread?

@@ -86,13 +98,22 @@ constructor(
ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[SquareDecoratedBarcodeViewModel::class.java]
}

private var beepManager: BeepManager? = null

private val cameraSettings = CameraSettings().apply {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this also be kill-switch’d ? (separate from qrCodeScannerHints)

It seems like an innocent no-brainer of a change, but you never know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants