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

add busy cursors to history paste/compress/discard #17301

Closed
wants to merge 3 commits into from

Conversation

ralfbrown
Copy link
Collaborator

@ralfbrown ralfbrown commented Aug 9, 2024

Also add a verification popup if the operation is being applied to 1000 or more images.

Simpler alternative to #17207 which doesn't have any of the complications caused by background execution of history operations.

@ralfbrown ralfbrown added feature: enhancement current features to improve scope: UI user interface and interactions labels Aug 9, 2024
@TurboGit
Copy link
Member

TurboGit commented Aug 9, 2024

Only to be merged if #17207 is not solved over the week-end, right? I may also take over #17207 if you don't mind as there is no urgency on this and #17207 is a better proposal to me.

@TurboGit TurboGit added this to the 5.0 milestone Aug 9, 2024
@ralfbrown
Copy link
Collaborator Author

Yes, this is Plan B.

@ralfbrown
Copy link
Collaborator Author

Add a progress-bar modal dialog window in place of the busy cursor when applying a history operation to ten or more images. The dialog has a cancel button which will abort the bulk operation before completion.

@TurboGit
Copy link
Member

Add a progress-bar modal dialog window in place of the busy cursor when applying a history operation to ten or more images. The dialog has a cancel button which will abort the bulk operation before completion.

I'm not sure this is a good move, it introduces for the first time a busy-modal-dialog.

@ralfbrown
Copy link
Collaborator Author

True, but the existing behavior is effectively modal anyway, since it completely freezes the GUI until the bulk operation completes. At least this way, the user can cancel the operation before it completes, and implementation-wise we don't have the synchronization headaches encountered on the other PR.

@TurboGit
Copy link
Member

But then I'd prefer having the current progress status bar (bottom left of UI) with a busy cursor icon and making the UI non responsive except for cancelling the job using bottom left cross.

@ralfbrown
Copy link
Collaborator Author

I don't know how to do that, since the job system's "foreground" queues aren't actually blocking, they're just higher priority than the background jobs.... Already tried that approach.

@TurboGit
Copy link
Member

Then why not just adding a busy cursor and not make the job in background... So what we have in master but just with a busy cursor?

@ralfbrown
Copy link
Collaborator Author

The progress bar added in the last commit is running in the foreground -- making the dialog containing it modal prevents other things from running as a result of the GUI thread reacting to user actions, which might then cause interference such as we see in the other PR where the bulk processing is a background task.

Unless there's some trick I haven't learned yet, if you want to handle interactions with a particuar widget (or even just update the screen), you still have to process events from all widgets. Making the one you are interested in (or one of its ancestors) modal causes the unwanted events to be ignored or blocked until later, so that only the desired events get processed.

@TurboGit
Copy link
Member

TurboGit commented Sep 1, 2024

Is having a busy cursor, a background job started and void dt_control_job_wait(dt_job_t *job); called to wait it to end would work here? I'm still trying to see if we can avoid the new modal dialog...

@ralfbrown
Copy link
Collaborator Author

@TurboGit
I tried dt_control_job_wait on PR 17207, since that already has things rearranged to start an independent job for the bulk operation together with enabling the busy cursor while waiting. I had to fix dt_control_job_wait to actually wait (it was getting in before the scheduler actually started the job and thus returned immediately). Once I did that, there were no more GUI updates - the progress meter stayed at 0 until complete and didn't allow cancellation.

I'm pretty sure that if I modify dt_control_job_wait to use a trylock loop with dt_gui_process_events() inside instead of simply requesting to lock the mutex, we'll be back to all the issues of unwanted operations starting when the user clicks on something while the bulk history operation is in progress....

@TurboGit
Copy link
Member

TurboGit commented Sep 7, 2024

@ralfbrown : Just checking the status here, have you tested the @dterrahe PR on top of yours which should enhance the behavior as it was discussed? Let me know, I don't want to let this important PR not merged for too long to ensure lot of field testing.

@ralfbrown
Copy link
Collaborator Author

Haven't had time yet - probably tomorrow.

@ralfbrown
Copy link
Collaborator Author

ralfbrown commented Sep 8, 2024

@TurboGit Took a look, seems good to me. It's on top of the other PR, #17207, which should now be the preferred one with other user interactions blocked while the bulk action is in progress (since we get the progress bar with cancellation without a separate popup window and keep the busy cursor except while on the progress bar).

@ralfbrown
Copy link
Collaborator Author

Mooted by #17207.

@ralfbrown ralfbrown closed this Sep 8, 2024
@ralfbrown ralfbrown deleted the more_busy_cursors branch September 8, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: enhancement current features to improve scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants