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

Make the progress dialog look better (used for e.g. shader compiling) #7453

Merged
merged 1 commit into from
Oct 5, 2018

Conversation

amaiorano
Copy link
Contributor

  • Removed the Cancel button since the code doesn't react to it anyway.
  • Only show a window title, not the help icon (?), and disable the close button
  • Set the title to "Dolphin" instead of repeating the label text

* Removed the Cancel button since the code doesn't react to it anyway.
* Only show a window title, not the help icon (?), and disable the close button
* Set the title to "Dolphin" instead of repeating the label text
@amaiorano
Copy link
Contributor Author

This addresses issue https://bugs.dolphin-emu.org/issues/11406 (duplicate of https://bugs.dolphin-emu.org/issues/11247)

Now the dialog looks like this:
image

@shuffle2
Copy link
Contributor

shuffle2 commented Oct 4, 2018

btw, is there a way to guess if the window will appear for say, < 2secs? I think in that case it just shouldn't be shown at all, it's kind of annoying (and seeing it pop up for < 1 sec just makes it look janky imo).

@stenzek
Copy link
Contributor

stenzek commented Oct 4, 2018

@shuffle2 this is already done (although it's one second): https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/AsyncShaderCompiler.cpp#L78

edit: dependent on the resolution of sleep, querying a timer would probably be better...

@shuffle2
Copy link
Contributor

shuffle2 commented Oct 4, 2018

@stenzek Ah ok, not a huge deal either way...but in general using some other (non-modal) form of visual indicator would be nice. Of course, has nothing to do with this PR though :)

@stenzek
Copy link
Contributor

stenzek commented Oct 4, 2018

Agreed, doing this in the renderer would also work on other platforms such as Android. That said, as a stop-gap measure, I'm okay with these changes.

@ghost
Copy link

ghost commented Oct 4, 2018

I've seen it pop up twice when starting Metroid Prime 3, forgot which graphic settings but I could try recreating again, unless it's known and nothing anyone can do about it? (like if the total isn't known until the first stage completes)

@amaiorano
Copy link
Contributor Author

I've seen it pop up twice when starting Metroid Prime 3, forgot which graphic settings but I could try recreating again, unless it's known and nothing anyone can do about it? (like if the total isn't known until the first stage completes)

Yes, I also noticed this when compiling OpenGL shaders. If you comment out the code that hides the dialog for 1 second in AsyncShaderCompiler.cpp (as @stenzek mentioned above), you'll see the dialog appear whenever it compiles shaders. I noticed that for OpenGL, it opens and closes many times. I logged the calls to OnUpdateProgressDialog, and it's being called many times with a progress of 27 / 27 (100%) over and over. So this ends up showing and hiding the dialog over and over. Doesn't seem to be the case for Direct3D though.

So either there's a problem with how the OpenGL shader compile (and perhaps other renderers) reports its progress; or it's normal and it's just an artifact of how the dialog is handled. We could get rid of the flashing dialog by deciding not to hide it when progress >= total, but only when progress is set to -1 (both of these are terminating conditions at the moment).

That means changing the bottom of OnUpdateProgressDialog from:

  if (total < 0 || progress >= total)
  {
    m_progress_dialog->hide();
    m_progress_dialog->deleteLater();
    m_progress_dialog = nullptr;
  }

to

  if (total < 0)
  {
    m_progress_dialog->hide();
    m_progress_dialog->deleteLater();
    m_progress_dialog = nullptr;
  }

This means, though, that client code must pass in -1 to close the dialog, which is currently already done for shaders (see the last call):

void ShaderCache::WaitForAsyncCompiler()
{
  while (m_async_shader_compiler->HasPendingWork() || m_async_shader_compiler->HasCompletedWork())
  {
    m_async_shader_compiler->WaitUntilCompletion([](size_t completed, size_t total) {
      Host_UpdateProgressDialog(GetStringT("Compiling shaders...").c_str(),
                                static_cast<int>(completed), static_cast<int>(total));
    });
    m_async_shader_compiler->RetrieveWorkItems();
  }
  Host_UpdateProgressDialog("", -1, -1);
}

Another solution would be to not display the dialog at all if progress is already 100% when it's called, so basically adding this exit condition at the top of OnUpdateProgressDialog:

void MainWindow::OnUpdateProgressDialog(QString title, int progress, int total)
{
   if (!m_progress_dialog && progress >= total)
    return;
...
}

That is probably safer, but it means seeing no dialog for the time it gets called with 100% over and over.
Thoughts? This could and probably should be a separate PR, though.

Btw, if this PR is cool, can one of you accept it? (not merge it, just accept it so that it doesn't fail the not auto-trusted check). Thanks!

@ghost
Copy link

ghost commented Oct 4, 2018

@amaiorano I was using Vulkan backend though. Didn't notice any compile loops there.

@stenzek
Copy link
Contributor

stenzek commented Oct 5, 2018

I'll follow this up with an in-backend implementation at some point.

@stenzek stenzek merged commit 4ebf3f3 into dolphin-emu:master Oct 5, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants