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

ThreadedProgressDialog #23391

Closed
fire opened this issue Oct 29, 2018 · 12 comments
Closed

ThreadedProgressDialog #23391

fire opened this issue Oct 29, 2018 · 12 comments

Comments

@fire
Copy link
Member

fire commented Oct 29, 2018

Godot version:
master

Issue description:

Create a threaded progress dialog with 2 modes / 4 combinations.

  1. Blocking or not blocking. Prevents the editor from being used but doesn't stop ui.
  2. Background or foreground. Display in the middle of the screen or as a progress bar on the top right corner.

@neikeq Design review?

<neikeq> yes, i want it to block in this case

<neikeq> reduz: what's the use of BackgroundProgress if it does not block? how is it displayed in the UI?

<reduz> i think on the top right region a small progress is displayed

<reduz> its for background processes, Filesystem Scan uses it

<reduz> not very useful in general, though

<reduz> i guess what you want is a ProgressDialog that does not block and can be used from a thread

<reduz> maybe ThreadedProgressDialog

@neikeq
Copy link
Contributor

neikeq commented Oct 29, 2018

  1. Background or foreground. Display in the middle of the screen or as a progress bar on the top right corner.

We already have BackgroundProgress for a background progress dialog that doesn't block the main thread. What's the point of one that blocks the main thread?

@fire
Copy link
Member Author

fire commented Oct 29, 2018

Example case: converting a scene to one mesh instance.

  1. Not be able to modify the scene
  2. Not block the main thread
  3. Make it background so that the developer can see the editor 3d window but not actually edit
  4. Make it background and allow the developer to edit the scene and when the system is done replace the scene. (I have an implementation of this), but it modifies a label to display progress rather than a dialog.

@fire
Copy link
Member Author

fire commented Oct 29, 2018

I don't see a BackgroundProgress in the latest docs on https://godot.readthedocs.io/en/latest/index.html

@fire fire closed this as completed Oct 29, 2018
@neikeq
Copy link
Contributor

neikeq commented Oct 29, 2018

I'll use the term exclusive dialog to describe a dialog that captures all input, hence it "prevents the editor from being used".

What we have right now:

  • ProgressDialog. The way it's designed is for tasks that block the main thread. It's an exclusive dialog. It will rely on a call to Main::iteration() when progressing for redrawing the ui. This is quite an ugly hack and it can cause serious trouble (like infinite recursion) when using MessageQueue/call_deferred. It also cannot be used from a function that would execute on every iteration (like _process and _fixed_process) unless you have some recursion guard (example).

  • BackgroundProgress. It's being used by the FS scan thread. I don't know what it looks like. @reduz says it's a small progress dialog in a corner. It's not an exclusive dialog.

What we need: An equivalent of ProgressDialog that doesn't block the main thread but still acts like an exclusive dialog.

@fire fire reopened this Oct 29, 2018
@fire
Copy link
Member Author

fire commented Oct 29, 2018

Design 2

An exclusive dialog ("prevents the editor from being used"):

  • do not block the main thread
  • blocks user input
  • Show ui in the middle of the screen.

A non-exclusive dialog ("does not prevent the editor from being used"):

  • do not block the main thread
  • does not block user input
  • Show the same ui as BackgroundProgress as a small progress dialog in a corner

The desired implementation will take the non-exclusive dialog and make it exclusive.

@fire
Copy link
Member Author

fire commented Oct 29, 2018

To allow threaded asset import, every instance of ProgressDialog should be replaced by the exclusive progress dialog that doesn't block the main thread.

Edit: Another part is to solve the dependencies of assets, but that's a different issue.

@Calinou
Copy link
Member

Calinou commented Apr 8, 2020

What's the status on this now that #36640 was merged?

@fire
Copy link
Member Author

fire commented Apr 11, 2020

I have not looked into this. Feel free to take this task.

@fire
Copy link
Member Author

fire commented May 25, 2020

I have forgotten about issue. I think a new issue should be opening in the proposals for it.

@akien-mga
Copy link
Member

I think this can be considered implemented by #36640.

@akien-mga akien-mga added this to the 4.0 milestone May 25, 2020
@neikeq
Copy link
Contributor

neikeq commented May 26, 2020

@akien-mga This is about the progress dialog, not resource loading. I don't see the relation between with that PR, unless I'm missing something.

@akien-mga
Copy link
Member

Then this should be moved to a proposal on https://github.com/godotengine/godot-proposals, which can now use the threaded resource loader for a proposed implementation.

@akien-mga akien-mga removed this from the 4.0 milestone May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants