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

Core: Remove FPS, VPS and speed percentage from window title #11467

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

Simonx22
Copy link
Member

@Simonx22 Simonx22 commented Jan 20, 2023

Why?

  • There's already an FPS widget that the user can enable.
  • I don't think constantly updating values such as the FPS count should be in the title bar.

I can also further clean it up while I'm already touching this code. For example, I could remove the DSP section (which displays either HLE or LLE).
EDIT: This will be done separately in another PR in the future. I want to keep it simple.

Screenshots
Before:
image
After:
image

@MayImilae
Copy link
Contributor

MayImilae commented Jan 21, 2023

OBS doesn't care for our title bar shenanigans, so I'm sure everyone who wants to use Dolphin for streaming or recording will be quite happy with this change. I think this is a good move.

The DSP and other settings indicators are for support reasons, as it makes it easier to spot user settings in screenshots. That's less relevant than before due to Android and what not. It may be a good idea to make a "settings display" with our IMGUI HUD that the user can toggle on that just lists a ton of settings for us to inspect. Hmm, maybe we could do it in a fancy compact way, like a QR code or an interpretable string? That way it could show us ALL settings while still being compact? What do you think @Sam-Belliveau ? IMO the settings bits of the title bar shouldn't go until there's a replacement.

@Zopolis4
Copy link
Contributor

Given that we have a obviously superior way of displaying the FPS and VPS information, this is a great change.

Source/Core/Core/Core.cpp Show resolved Hide resolved
@Simonx22 Simonx22 changed the title [RFC] Core: Remove FPS from title bar Core: Remove FPS, VPS and speed percentage from window title Jan 21, 2023
Copy link
Contributor

@MayImilae MayImilae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JMC47
Copy link
Contributor

JMC47 commented Jan 22, 2023

I'll be honest. I'm going to miss this as it's made seeing user's settings a lot easier. But I think this is a good, necessary choice regardless. It'll fix OBS freaking out, and we can come up with more solutions later.

@BhaaLseN
Copy link
Member

This doesn't remove the settings (yet), only the FPS/VPS update. And the distinction between playing and movie playback I guess.

@JMC47
Copy link
Contributor

JMC47 commented Jan 22, 2023

Yeah, but this is definitely the start of that direction. Seeing FPS/VPS desyncs in the title bar was a thing back in the day too. Thankfully the imgui stuff now shows it too.

@Simonx22 Simonx22 marked this pull request as draft January 23, 2023 10:57
@Simonx22 Simonx22 marked this pull request as ready for review January 23, 2023 20:39
@Simonx22
Copy link
Member Author

This PR is ready to review now. Build failures are not related to my changes.

@CasualPokePlayer
Copy link
Contributor

There's already an FPS widget that the user can enable.

This actually is not suitable in one edge case: the Null backend, as there is no OSD with the Null backend. This means using the Null backend to judge how much a game is bottlenecked by the GPU is not really possible with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants