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

Add CPU Clock Frequency slider #5025

Merged
merged 1 commit into from Mar 28, 2020
Merged

Conversation

jroweboy
Copy link
Contributor

@jroweboy jroweboy commented Dec 16, 2019

This slider affects the number of cycles that the guest cpu emulation reports that have passed since the last time slice. This option scales the result returned by a percentage that the user selects. In some games underclocking the CPU can give a major speedup. Exposing this as an option will give users something to toy with for performance, while also potentially enhancing games that experience lag on the real console

For those that have seen "Custom Tick Hacks" this is that, but now with 100% less hardcoded-per-game values πŸ‘

Screenshot of UI change:
https://i.imgur.com/FsNQVod.png

I don't like having it on the first page in the settings as this is an advanced configuration option. I thinking about moving it to system if people agree with me (even though technically i think it fits better in emulation)


This change is Reviewable

@flamesage
Copy link
Member

flamesage commented Dec 16, 2019

Some sort of label explaining what this feature is may help guide some users.

If I want this speedup, do I raise the CPU Clock speed?

@jroweboy jroweboy force-pushed the tomoscrewme branch 3 times, most recently from c902452 to 18f80a7 Compare Dec 16, 2019
@jroweboy
Copy link
Contributor Author

jroweboy commented Dec 16, 2019

@chris062689 updated the picture to show the tooltip

@jroweboy
Copy link
Contributor Author

jroweboy commented Dec 16, 2019

As it currently works, this breaks the core timing test cases. I'll try to think of a good way to fix this.

@@ -47,7 +48,7 @@ u64 Timing::GetTicks() const {
}

void Timing::AddTicks(u64 ticks) {
downcount -= ticks;
downcount -= static_cast<u64>(ticks * 100.0 / Settings ::values.cpu_clock_percentage);
Copy link
Contributor

@BreadFish64 BreadFish64 Dec 16, 2019

Choose a reason for hiding this comment

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

The results here are going to be the same as integer ops because the multiplication is first.

Copy link
Contributor Author

@jroweboy jroweboy Dec 16, 2019

Choose a reason for hiding this comment

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

Since there is a float in there it gets promoted to a float. https://stackoverflow.com/a/5563063/745719

Copy link
Member

@zhaowenlan1779 zhaowenlan1779 left a comment

Codewise LGTM.

However I don't think putting this information in tooltips is too great an idea. Users (even experienced ones) hardly can see tooltips. For important information (which I think this one is) I would personally perfer just making a brief, clear label.

@robzombie91
Copy link

robzombie91 commented Feb 10, 2020

Works around #2436 and #3330 if set to 55%, no higher.

@B3n30
Copy link
Contributor

B3n30 commented Feb 21, 2020

remove from canary due to conflicts

This slider affects the number of cycles that the guest cpu emulation
reports that have passed since the last time slice. This option scales
the result returned by a percentage that the user selects. In some games
underclocking the CPU can give a major speedup. Exposing this as an
option will give users something to toy with for performance, while also
potentially enhancing games that experience lag on the real console
@jroweboy
Copy link
Contributor Author

jroweboy commented Feb 21, 2020

Rebased on master
Moved it to System settings
Changed the hover text to just be in a label below
changed the minimum to 5%

@Owen404
Copy link

Owen404 commented Feb 23, 2020

the cpu clock is breaking luig's mansion dark moon the audio is slowed down to 30% speed but the video output you see is over 100% i would suspect this broke other games that rely on the cpu clock being changed this happened when the cpu slider was able to go to 5%
here is a video showing the problem
https://streamable.com/06ds0

@@ -7,7 +7,7 @@
<x>0</x>
<y>0</y>
<width>345</width>
<height>357</height>
<height>358</height>
Copy link
Contributor

@FearlessTobi FearlessTobi Mar 1, 2020

Choose a reason for hiding this comment

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

Maybe you should revert all your changes to this file now?

Copy link
Contributor

@FearlessTobi FearlessTobi left a comment

Other than the comment above, LGTM

@HeriKuen
Copy link

HeriKuen commented Mar 20, 2020

That's what I hoped for

@hamish-milne
Copy link
Contributor

hamish-milne commented Mar 28, 2020

Merging this so we can resolve some conflicts (and everyone seems happy)

@hamish-milne hamish-milne merged commit 1ff8d00 into citra-emu:master Mar 28, 2020
3 checks passed
@ghost
Copy link

ghost commented Mar 28, 2020

@hamish-milne no one is happy because this breaks Luigi's Mansion Dark Moon's audio.
https://streamable.com/06ds0

@Owen404
Copy link

Owen404 commented Mar 28, 2020

if you meant that you were merging a fix its still broken

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

Successfully merging this pull request may close these issues.

None yet