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

VideoCommon: Move imgui-related functions/data to ImGuiManager class #8009

Open
wants to merge 1 commit into
base: master
from

Conversation

4 participants
@stenzek
Copy link
Contributor

commented Apr 19, 2019

This PR moves the imgui members/functions out of Renderer and into a separate class.

@iwubcode some of your pause screen logic could likely be moved within here, and tied into the Draw() function for different states.

@stenzek stenzek force-pushed the stenzek:imgui-manager branch from 6862369 to b005534 Apr 19, 2019

@stenzek stenzek force-pushed the stenzek:imgui-manager branch from b005534 to 60112e7 Apr 19, 2019

io.DeltaTime = time_diff_secs;
}

void ImGuiManager::Draw()

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN Apr 19, 2019

Member

Would there be any benefit in writing this in a way that other places can register themselves to be rendered here (like, for (auto* drawable : m_drawables) drawable->Draw()), instead of a long if (drawA) renderA(); chain?

This comment has been minimized.

Copy link
@iwubcode

iwubcode Apr 20, 2019

Contributor

I like this idea, if it's done, it'd be nice to have a way to unregister as well (thinking of pause screen)

This comment has been minimized.

Copy link
@stenzek

stenzek Apr 20, 2019

Author Contributor

How about a bitmask of states where it should be displayed?

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN Apr 20, 2019

Member

You could just pass that in, if its the same method for all states. Or just have methods for individual states (not sure which one makes more sense).
Having a Register/Unregister mechanism here would make it a lot nicer to plug new things into the ImGUI UI without having to dig around this manager and related files.

This comment has been minimized.

Copy link
@stenzek

stenzek Apr 21, 2019

Author Contributor

Yeah, sorry, I meant using a bitmask in combination with the callback registration idea. This would save having to check which state the UI was in to display a window or not.


#include "imgui.h"

std::unique_ptr<VideoCommon::ImGuiManager> g_imgui_manager;

This comment has been minimized.

Copy link
@lioncash

lioncash Apr 20, 2019

Member

Would this not belong within the renderer class instead of having separate global state for it?

This comment has been minimized.

Copy link
@stenzek

stenzek Apr 21, 2019

Author Contributor

The long-term plan is to make the Renderer class more of a "GPU device" object. I know that currently it still has other stuff in there, but I've been slowly moving things out.

So, the UI-related data isn't really part of the GPU device, so IMO should be separate. If anything, it should be part of the overall "video backend" state. That said, things like netplay chat should be moved within the manager class, removing those globals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.