Skip to content

Progressbar updates in main thread#1586

Merged
Lestropie merged 4 commits intodevfrom
progressbar_updates_in_main_thread
Aug 24, 2019
Merged

Progressbar updates in main thread#1586
Lestropie merged 4 commits intodevfrom
progressbar_updates_in_main_thread

Conversation

@jdtournier
Copy link
Copy Markdown
Member

Took quite a bit of experimentation, but I reckon this may finally fix #256 and #378 (and probably others besides). Commit a87abc3 is the relevant one here. I'll need all MRView users who've reported issues when the progressbar is shown to give this a spin and see whether the issue persists...

@jdtournier jdtournier added this to the 3.0_RC4 release milestone Mar 22, 2019
@jdtournier jdtournier self-assigned this Mar 22, 2019
@Lestropie
Copy link
Copy Markdown
Member

Does not fix #1578.

This is a split of original commit 446630c, that removes changes unrelated to the ProgressBar.
@Lestropie Lestropie force-pushed the progressbar_updates_in_main_thread branch from 446630c to cdb9133 Compare July 31, 2019 04:53
@Lestropie
Copy link
Copy Markdown
Member

Extracted some unrelated code and moved to #1663.

The actual technique used here, I'd need to spend a fair bit of time on to wrap my head around.

I suppose that as long as it does not regress for GUI / non-GUI on any of the three OS's, it should be safe to merge, even if it fails to exhaustively solve all issues?

It also looks like it may only fix the issue if ThreadedLoop is utilised; so for any GUI tool that uses custom multi-threading code with a progress bar, ProgressBar::run_update_thread() won't be utilised?

@jdtournier
Copy link
Copy Markdown
Member Author

Yes, as far as I can tell, there's no regression. And yes, it would only solve issues when using our own ThreadedLoop class - hopefully there won't be any other instances of this going on within MRView...

Most of the changes are actually cosmetic, even though the diff looks bigger than it needs to. As mentioned earlier, the bulk of the logic is contained within a87abc3: basically set up condition variable that can be notified in the main thread, whose job is then to update the progressbar or exit if all worker threads have actually completed.

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