Repository navigation
feat(scanner): scan a code from a photo in the gallery - #1509
Merged
Merged
Conversation
…size cap Two decode bugs on the gallery path, both found in review. `StaticImageAnalyzerImpl` computed its sample size as `longestSide / MAX_SOURCE_SIDE` and then rounded that down to a power of two, so the guard only fired at 2x the cap and above. A 4000px photo, which is the example in the comment right above it, divided to 1 and was decoded at full size: 12M pixels into an IntArray per tier-1 pass, and a quadratic number of windows after it. Every image between 2400 and 4800 was untouched. It now rounds up to the smallest power of two that clears the cap, with the rule pinned by a test. `StaticQrAnalyzer.detect` was `suspend` but never left the caller's dispatcher, and its only caller reaches it from `rememberCoroutineScope`. So an arbitrary user photo was decoded at full resolution on the main thread, on the slow path, after the ladder had already spent its eight seconds. It now runs on `Dispatchers.Default` and applies the same cap. Its bitmap was also recycled in a `finally`, which cancellation reaches while ML Kit's worker still holds the `InputImage` wrapping it. Recycling from the listeners instead frees it once the task settles. `Exhausted`'s doc claimed it covered cancellation; `ensureActive` throws, so it never did.
The feature module had grown a `strings.xml` of its own, the only one under `apps/flipcash/features`. Nothing localized would have picked it up. Three of its four strings were duplicates: `error_title_noCodeFound`, `error_description_noCodeFound`, `error_title_cashExpired` and `error_description_cashExpired` already exist in `ui/resources`, already translated. The gallery button's content description is genuinely new, so it goes in `apps/flipcash/core`, next to the other `content_description_*` entries.
The scanner held a boolean saying a grab from a picked photo was in flight, and showed the banner only while it was set. It was cleared in two places: a bill arriving, and the event itself. But `onCodeScan` returns silently at five points and `attemptGrab` fails for reasons other than a missing give request, so the flag survived all of them. The concrete case is a photo of a tip card. It routes to `onTipCardScanned`, which produces neither a bill nor the event, and the flag stays set for the life of the composition. The next time the camera hits `GiveRequestNotFound` the user gets "Cash Expired" over a code they are still pointing at, which is the exact false positive `CodeScanEvent`'s own doc says the camera must not produce. `onCodeScan` now takes `fromStillImage`, and the delegate raises the event only for those scans. The camera passes nothing and cannot surface a photo's error. The scanner keeps no flag, so there is no state left to leak. Also drops `scanJob` out of snapshot state. Only the cancel lambda reads it, and writing it recomposed the whole scanner.
…sition `observe` builds a fresh `stateIn(dataScope, Eagerly, ..)` per call, and `dataScope` is singleton-scoped, so each returned flow starts a DataStore collection that nothing cancels. Calling it straight from the composable body leaked one per recomposition, and `collectAsStateWithLifecycle` restarted its own collection too, since the flow identity is a remember key. This is the Scan tab, which recomposes on the camera preview state, the bill state and the still-image search, so the leak compounds. Remembering the flow against the controller is the fix at this call site; caching per flag inside the controller would fix the pattern everywhere and is a separate change.
The effect reads `useBrightness` when it applies its overrides, but keys only on `isEnabled` and the lifecycle owner, and does not wrap it in `rememberUpdatedState`. A caller that turns the boost on while `isEnabled` stays true keeps the old behaviour with no sign of it. Nothing hits this today, because every current caller dips `isEnabled` through false first. The scanner is one edit away from not doing that.
`Scanner.kt` names `KikCodeScannerImpl`, `StaticImageAnalyzerImpl` and `KikCodeAnalyzer`. It compiled only because `:ui:scanner` exposes them through `api`, so the module's own build file did not say what it uses.
github-actions
Bot
added
area: ui
The gallery entry point floated over the camera preview at the bottom-left, so
it sat on top of the scanner's own content with nothing behind it. It now sits
on the bar's row beside the pill and takes width from it.
`NavigationBar` gains a `trailing` slot in the same shape as `avatar`: the
caller supplies content, the bar supplies a modifier already carrying the
pill's size and frosted fill, so the control cannot drift from the pill next to
it. Screens reach the slot through `TabBarAction(tab) { .. }`, published into a
controller held beside the bar in `AppContent`.
The slot is keyed to the publishing tab, not to composition lifetime. A tab
press swaps the backstack but leaves the outgoing screen composed until the
route transition ends, so a dispose-driven clear left the control on the bar
for a beat after the indicator had moved. `actionFor(selectedTab)` flips on the
press itself, and the slot's enter/exit shares the indicator's spring.
The scanner drops its own haze source and its `billState` read: it borrows the
bar's blur through the slot modifier, and the bar already hides whole while a
bill is up. The glyph is outline at 25dp, which matches the ink of the
unselected tabs rather than their 30dp box; full opacity is what separates it
from them.
The button sat in a slot on the tab bar's row, which only worked because the bar is ours to lay out. iOS has no way to put a control there: no API shrinks the system tab bar for a non-tab control, and the bar spans 21.0-380.7pt of a 402pt screen on an iPhone 17 Pro, so there is no gap to sit in. `tabViewBottomAccessory` renders above the bar, and the inline beside-the-bar placement exists only alongside a scroll-collapsed bar, where collapse is scroll-driven and not settable. A layout limit, not an animation one. The corner is a placement both platforms can hold, and is where iOS already had it -- `GalleryScanButton.swift` has been top-trailing all along, so this converges the two rather than compromising between them. This reverts the `trailing` slot and the `TabBarAction` controller that fed it (52b257b). `NavigationBar` goes back to a plain pill, and nothing outside the scanner knows the gallery button exists. The button is a `CircularIconButton` again rather than a bare glyph, because the backdrop is now whatever the camera sees: a white icon alone has no contrast it can count on. The scanner keeps its own haze source for that, since the app-root source wraps `AppNavHost` and this screen is inside it -- a blur hung off it would sample a layer containing itself. The source covers the preview only, with the button outside it. Corrects the comment on the bill gate while it moves. The claim was that the glyph would sit on top of a presented bill; `NavBillOverlayEntryDecorator` draws the bill after this entry's content, so the bill covers the button. The gate stays, to avoid leaving a live control composed under a modal scrim.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Someone sends you a screenshot of a cash code instead of showing you their screen, and today there is nothing to do with it. This adds a gallery button to the Scan tab that searches a picked photo for a code, behind the
scan_from_galleryflag (default off).Where the button sits
Top-right corner of the scanner, on a frosted circle. It went through two other placements before this one: a float over the preview near the tab bar, then a slot on the tab bar's own row beside the pill. The slot only worked because the bar is ours to lay out. iOS has no way to put a control there -- no API shrinks the system tab bar for a non-tab control, and the bar spans 21.0-380.7pt of a 402pt screen on an iPhone 17 Pro, so there is no gap beside it to sit in. The corner is where iOS already had it, so this converges the two apps rather than splitting the difference.
It is a
CircularIconButtonrather than a bare glyph because the backdrop is whatever the camera happens to see, and a white icon alone has no contrast it can count on. The scanner supplies its own haze source for that: the app-root source wrapsAppNavHostand this screen is inside it, so a blur hung off it would sample a layer containing itself. The source covers the preview only, with the button outside it.The glyph is
Icons.Outlined.PhotoLibraryat 25dp in a 50dp circle. Outline rather than filled because filled is the weight the nav bar gives a selected tab, and this is an action with no selected state to claim.How the search works
A camera frame needs no searching: the user aims, so the code is roughly centred and roughly the right size. A picked photo is whatever someone captured, and
scanKikCodereads at a fixed scale, so a small code has to be cropped out and scaled up before the scanner can see it.StillImageCodeSearchenumerates those crops in three tiers — the whole image, then recursive quadrants, then a sliding window — each at the zoom levels that fit under a rendered-size cap.StaticImageAnalyzerImplwalks that list under an 8 second budget and stops at the first crop that decodes. If the ladder finds nothing, ML Kit gets one pass at the same image for a QR code.The ladder and
LuminancePlane.fromArgblive in:libs:codes:kikcodecommonMain rather than in the scanner module, because iOS reimplements both by hand. Two copies that have to produce the same crops in the same order are easier to hold together when the tests sit beside the source both sides mirror.This also gives
KikCodeAnalyzer.detect(uri)something to do. It was already there, with no binding behind it and an@Inject lateinitthat would have crashed the first caller; the analyzer now takes its dependency through the constructor.Which scanned routes are allowed
DeeplinkType.isScannablenames the allowlist in one place. Cash links, token info, tip chats and tip cards scan;/login, email verification and group chat invites do not. The scan branches already refused credential routes, but only as an emergent property of twowhens in different files, so a route added later would have inherited a default rather than a decision.What the user sees when nothing happens
A search that finds nothing says so, and so does a code that decoded but has no give request behind it. The second is a new
CodeScanEvent.CashCodeNotLive, raised only for scans that came in withfromStillImage = true. The camera must not raise it: a stale frame is one of sixty a second and the next one may be live, so a banner there would fire on a code the user is still pointing at. A picked photo is read once and cannot be re-read, which is what makes it the path that has to explain itself. Keeping that distinction in the session rather than in the scanner means the camera cannot inherit a grab it did not start.Limits
READ_MEDIA_IMAGES. The photo picker is the whole reason for the approach — the app never declares the permission and never asks for it.MIN_SECTION_SIZE,WINDOW_SIZE, the zoom levels, the budget) come from Code'sprocessBitmapRecursivelyand have never been measured against real screenshots.GalleryScanEvent.Succeededreports the tier and zoom that decoded andFailedreports how long a fruitless search took, which is what would let them be trimmed later.