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

loading spinner #232

Merged
merged 7 commits into from
Jan 26, 2024
Merged

loading spinner #232

merged 7 commits into from
Jan 26, 2024

Conversation

AlperK61
Copy link
Contributor

closes #208

@AlperK61 AlperK61 requested review from a team and a-miscellaneous and removed request for a team January 22, 2024 18:29
@a-miscellaneous
Copy link
Contributor

generating differences is sadly not cancelable, job.cancel does not work.
parentJob lauch generation and then parentjob.cancel also does not work

@akriese
Copy link
Contributor

akriese commented Jan 23, 2024

What do we do with this then? @a-miscellaneous @AlperK61

@a-miscellaneous
Copy link
Contributor

What do we do with this then? @a-miscellaneous @AlperK61

next sprint, conflicts left

@a-miscellaneous a-miscellaneous marked this pull request as draft January 24, 2024 14:32
@akriese
Copy link
Contributor

akriese commented Jan 24, 2024

We have to make the difference generation cooperative for cancelling. Then, job.cancel() will work. I'll look into that now: https://kotlinlang.org/docs/cancellation-and-timeouts.html#making-computation-code-cancellable

@akriese
Copy link
Contributor

akriese commented Jan 24, 2024

After doing my research, it is pretty clear, that we can only cancel the job by injecting some kind of suspend function into the Algorithm.run() or some check if the job is still wanted. That would mean needing to refactor lib2 quite a bit so that it is cancel-aware... Unfortunately, the builtin Threat.yield() does not work as kotlinx.coroutines.yield and thus, lib2 must include kotlinx somehow.
Or... We redefine the algorithm to run in steps, like a ML training. Then, we could check if the job was cancelled between each step.
Some links I checked:
https://stackoverflow.com/questions/69097765/how-to-stop-or-cancel-a-kotlin-coroutine-stop-currently-running-code-block-inst
https://stackoverflow.com/questions/74065990/properly-cancelling-kotlin-coroutine-job

AlperK61 and others added 6 commits January 25, 2024 18:06
This introduces a singleton that can be used to signal
the premature cancellation of a running algorithm.
The algorithm checks if it is still alive regularly
while the user can signal from the outside that the
algorithm should finish itself.

This is similar to how one would cancel a coroutine
in compose (with `yield`), but we wanted to avoid having
kotlinx as a dependency for lib2.

Signed-off-by: Anton Kriese <[email protected]>
Catches the exception thrown by a cancelled algorithm,
to not switch to the DiffScreen after the computation.

This change leads to the algorithm actually being cancelled
instead of silently run in the background like previously.

Signed-off-by: Anton Kriese <[email protected]>
@akriese akriese force-pushed the gui/feature/loading-spinner branch from 8ddcfe7 to 4e7c96f Compare January 25, 2024 17:07
@akriese
Copy link
Contributor

akriese commented Jan 25, 2024

Rebased onto main and added singleton to cancel running algorithm instance.

@akriese akriese marked this pull request as ready for review January 26, 2024 11:11
Copy link
Contributor

@zino212 zino212 left a comment

Choose a reason for hiding this comment

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

LGTM and seems to work nicely. I only request restyling the AlertDialog to match our new style

@akriese akriese self-requested a review January 26, 2024 16:08
@akriese akriese merged commit 2238981 into main Jan 26, 2024
8 checks passed
@zino212 zino212 deleted the gui/feature/loading-spinner branch January 26, 2024 16:33
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.

Show loading spinner while frame differences are computed (Library 3)
4 participants