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

UX: Add Overclock setting and Override CPU clock-speed setting #1467

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Spidy123222
Copy link

@Spidy123222 Spidy123222 commented Jun 29, 2023

Fixes #1429

This adds the ability to override the current clock speed and adjust it to help optimize a game.

Changing this value like other emulators when increasing the value increases the clock-speed cpu timing which can benefit variable-refresh rate games or mods that require higher clocks and or lower can trigger a games frame skip feature if it has one to increase performance.

Adjusting clock-speed can sometimes cause issues in some games so is to be warned about doing it and maybe shouldn’t be used in compatibility reports.

Increasing clock-speed seems to get some games to allow to go over 60 fps which could be useful if the game properly handles it.

A game I find improvements on personally is in Burnout 3 turning it down makes it more usable in menus on steamdeck from 4fps to 23fps adjusting to 47% and a bit ingame. In Jet Set Radio future with a 10 fps increase in more demanding areas.

IMG_7386

@Spidy123222 Spidy123222 changed the title UI: Add Overclock setting and Override CPU clock-speed setting UX: Add Overclock setting and Override CPU clock-speed setting Jun 29, 2023
Copy link
Contributor

@antangelo antangelo left a comment

Choose a reason for hiding this comment

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

We should prevent compatibility reports from being submitted if the clock speed is not stock.

Any fixes to games as a result of adjusting clock speeds are (most probably) hacks. I imagine the only non-hacky utility this change adds is in testing CPU overclocking and replacements.

hw/i386/x86.c Outdated Show resolved Hide resolved
hw/i386/x86.c Outdated Show resolved Hide resolved
hw/i386/x86.c Outdated Show resolved Hide resolved
hw/i386/x86.c Outdated Show resolved Hide resolved
hw/i386/x86.c Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Show resolved Hide resolved
@Spidy123222
Copy link
Author

We should prevent compatibility reports from being submitted if the clock speed is not stock.

I am not quite sure how to do that as i am new to this codebase and also to C. But to be able to know it's enabled is as simple as using the override toggle's state as it's required for it to be enabled for it to work.

Any fixes to games as a result of adjusting clock speeds are (most probably) hacks. I imagine the only non-hacky utility this change adds is in testing CPU overclocking and replacements.

Yeah that's reasonable and makes sense as the report system is meant for mimicking the actual hardware as close as possible but having this can help optimize game and test overclocks also. On dolphin's one there is more detail on it about not reporting compatibility using their enabled.

I couldn't fit more text than a few words so I would of added more detail on how it works but that's not entirely important to have and it can be on the site.

@antangelo
Copy link
Contributor

antangelo commented Jun 29, 2023

See ui/xui/reporting.cc and ui/xui/compat.cc for the reporting system.

My main point there is that "optimizing" games by altering the emulated clock speed is not correct. Issues should be resolved by moving the emulation closer to that of original hardware, not farther.

@binarymaster
Copy link
Contributor

Would this setting be affected by a request coming from the guest code / software side, such as https://github.com/GXTX/XboxOverclock ?

@Spidy123222
Copy link
Author

Would this setting be affected by a request coming from the guest code / software side, such as https://github.com/GXTX/XboxOverclock ?

I haven’t tried it but you can try it out. It might just default back to what xemu says it should be.

config_spec.yml Outdated Show resolved Hide resolved
ui/xui/compat.cc Outdated Show resolved Hide resolved
ui/xui/compat.cc Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
ui/xui/widgets.hh Outdated Show resolved Hide resolved
snprintf(buf, sizeof(buf), "Clock Speed %d%% (%.2f MHz)", (int)(g_config.perf.cpu_clockspeed * 100), (733333333 * g_config.perf.cpu_clockspeed)/1000000);
Slider("Virtual CPU clock", &g_config.perf.cpu_clockspeed, buf,0.01f , 2.f, 0.01f);

if ((g_config.perf.cpu_clockspeed-0.999)*(g_config.perf.cpu_clockspeed-1.009) <= 0) {g_config.perf.cpu_clockspeed = 1;}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of this?

Copy link
Author

@Spidy123222 Spidy123222 Jul 20, 2023

Choose a reason for hiding this comment

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

To keep accuracy when your at 100% that it will be 733.33….MHz. I feel it was a better alternative than doing a reset button. It will snap to that value making it easy to go back to default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this a bit more clear? Maybe change the check to std::abs(g_config.perf...- 1.f) <= 0.005f and fix the brace formatting.
Also, should make sure this can be escaped with a controller, as if the increment is too low you will be stuck at 1.

Copy link
Author

@Spidy123222 Spidy123222 Jul 22, 2023

Choose a reason for hiding this comment

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

Can you make this a bit more clear? Maybe change the check to std::abs(g_config.perf...- 1.f) <= 0.005f and fix the brace formatting.
Also, should make sure this can be escaped with a controller, as if the increment is too low you will be stuck at 1.

So I managed to get abs working in some form at some point. Using fabs seemed to work better for this and now is working. It’s in latest commit and I rebased.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, in what way did fabs work better? They generate the same code on my machine.

There's a minor edge case here in that if the config is edited manually to set the clock override to a value within this interval, that value will be overwritten to snap back to 1 despite the user not touching the slider (and it will not snap to one until this screen is opened). I personally don't think this is worth the complexity to solve though.

Copy link
Author

Choose a reason for hiding this comment

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

Just curious, in what way did fabs work better? They generate the same code on my machine.

There's a minor edge case here in that if the config is edited manually to set the clock override to a value within this interval, that value will be overwritten to snap back to 1 despite the user not touching the slider (and it will not snap to one until this screen is opened). I personally don't think this is worth the complexity to solve though.

So when I was testing using normal abs it would some reason not allow to remove slider but changing to fabs worked. Maybe could of been a bug during compile, Not sure. Yeah doing it this way does have effect that it will do it directly to the value.

ui/xui/main-menu.cc Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
ui/xui/compat.cc Outdated Show resolved Hide resolved
@Spidy123222 Spidy123222 requested a review from antangelo July 23, 2023 17:03
hw/i386/x86.c Outdated Show resolved Hide resolved
ui/xui/widgets.cc Outdated Show resolved Hide resolved
config_spec.yml Outdated Show resolved Hide resolved
ui/xui/main-menu.cc Outdated Show resolved Hide resolved
@Spidy123222 Spidy123222 requested a review from antangelo August 23, 2023 05:04
antangelo
antangelo previously approved these changes Aug 23, 2023
@antangelo antangelo dismissed their stale review October 16, 2023 01:06

Deferring approval to Matt

Spidy123222 and others added 11 commits July 9, 2024 23:25
* Adds Overclock setting inside x86.c

* Add clockspeed and override toggle in config_spec.yml

* Implement Override Toggle

* Add clock speed value to cpu

* Add Changes to UI
This adds a disable feature for when having the override toggle enabled to prevent bad reports. This also grey’s out the send button and tells a message that it’s not allowed.
Co-authored-by: antangelo <antonio.abbatangelo@gmail.com>
Some reason before print was added it wasn’t working on my end and apparently it started working some reason. Might of been a compile issue.
* Add MHz to slider to be able to see the frequency.
This snaps the slider to use default clockspeed if it is in 100% or very close to it.
Spidy123222 and others added 9 commits July 9, 2024 23:25
This adds a minimum and maximum and gamepad speed values to change the behavior of the slider. 

This change included separating slider value and the properties value for the actual setting because increasing the sliders clamp also keeps the slider handle in the right position and if increased it will allow outside of the slider range. So using some math I was able to make the slider keep its range while also making the value change appropriately. 

Adding gamepad speed allowed for better fine control of the slider for certain sliders if needed. Minimum and maximum is value ranges that can be set when making a slider. In this commit I also slightly adjusted the notch when hitting 100% so that the gamepad can escape it. This also has been tested with audio to work like before and can even increase the range it can do even if audio won’t go higher but just proves that it Infact changes the range.
Co-authored-by: Stanislav Motylkov <x86corez@gmail.com>
Co-authored-by: antangelo <antonio.abbatangelo@gmail.com>
user18081972 added a commit to user18081972/xemu-canary that referenced this pull request Nov 11, 2024
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.

[Feature Request] virtual cpu clock control
4 participants