Skip to content
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

QR image generating taking too long #278

Closed
yellowHatpro opened this issue Mar 25, 2023 · 6 comments · Fixed by #279
Closed

QR image generating taking too long #278

yellowHatpro opened this issue Mar 25, 2023 · 6 comments · Fixed by #279

Comments

@yellowHatpro
Copy link
Contributor

  • IMO, The QR generation is happening on the main thread. Therefore it's too slow. Because of it, clicking on the Recieve button seems buggy, as visible in the recording:
f2.mp4

Fixes:

  • We can switch the context using withContext(Dispatchers.IO) and call the corresponding qr generation function there.
  • Visible changes:
firstissue.mp4
@yellowHatpro
Copy link
Contributor Author

@thunderbiscuit is the issue there on other devices??
Can you confirm if it persists in your device/emulator?

@thunderbiscuit
Copy link
Owner

Yes I think there are a few things at play here. This is a good place to discuss maybe what's wrong with this screen.

One of the issues with that screen (I have not deeply investigated this yet) is that it tends to trigger a lot of recompositions of the composables. I'm not sure why.

In theory, the main thread at that point does nothing else than get this address from the wallet. I don't know that sending this work to a background thread should "speed it up" per se. I'm also not sure which part is actually the slowest part of this process. Is it the call to the Wallet object? Or the addressToQR() function? We could potentially pre-compute these things on a background thread as the user opens the screen and have it ready for them before they even click the button, which would then make it seem like the process was really fast. But even that feels a bit like over-optimization to me at the moment. Any thoughts on that?

On my devices and emulators it's not very slow to load this address (as opposed to navigating to the screen itself, which tends to be a bit slow I find).

@yellowHatpro
Copy link
Contributor Author

I think generating the address is not a problem, but generating an image without thread switching will block the main thread. So what I am doing is, that I am calling the addressToQR() function within a LaunchedEffect, which depends on the address itself, so the image generation is now bound to the address, which I don't think changes frequently or on its own.

@yellowHatpro
Copy link
Contributor Author

I will try to check if there are other recompositions in the mentioned screen, but I suppose generating the QR in the background should be a safe call. Although, it might be noticeable to me because my device is a slow potato >﹏<.

@thunderbiscuit
Copy link
Owner

thunderbiscuit commented Apr 11, 2023

Generating the image in the background is a good way to keep the UI from freezing, but I don't know how it can address your issue of the slowness of it directly.

@yellowHatpro
Copy link
Contributor Author

Yeah slow won't be an appropriate word here. I was referring to the delay it causes.

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 a pull request may close this issue.

2 participants