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

Reset Throttle on savestate load #11449

Merged
merged 1 commit into from Jan 16, 2023
Merged

Conversation

phire
Copy link
Member

@phire phire commented Jan 16, 2023

Fixes regression from #11348, where loading a save state from the past would cause issues.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

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

Code LGTM, untested!!!

@JMC47 JMC47 merged commit c4f4ecb into dolphin-emu:master Jan 16, 2023
11 checks passed
@@ -101,8 +101,7 @@ void CoreTimingManager::Init()
m_is_global_timer_sane = true;

// Reset data used by the throttling system
m_throttle_last_cycle = 0;
m_throttle_deadline = Clock::now();
ResetThrottle(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

this should reset to m_globals.global_timer, which is zero, but i dont think that it should be passed in, because there is never a situation where the throttle clock should be set to anything different then the global timer when resetting.

@@ -382,6 +387,12 @@ void CoreTimingManager::Throttle(const s64 target_cycle)
}
}

void CoreTimingManager::ResetThrottle(s64 cycle)
{
m_throttle_last_cycle = cycle;
Copy link
Contributor

Choose a reason for hiding this comment

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

you should probably remove the cycle argument and make it reset to GetTicks()

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point.

But that makes me wonder about what happens to m_is_global_timer_sane during savestates. Is there a bug there?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the save-state code can only be triggered on the cpu thread from within an event, so it should always be true when the savestate code runs (and there would be more bugs if we were saving or loading with a non-sane global timer)

@phire phire deleted the resetThrottle branch January 16, 2023 03:51
@shuffle2
Copy link
Contributor

shuffle2 commented Jan 16, 2023

I had throttle set to "unlimited", and on first emulation start (not using save states) after updating to a build including #11348 , I only got a black screen. It didn't occur again, but I wonder if it would have been fixed by this PR?

@Sam-Belliveau
Copy link
Contributor

the fallback code was supposed to stop this from happening, and it did, but there was a last second fix that introduced a lot of stuff when the cycles get changed. I cant guarantee that this will fix that issue you had, but it does fix the save states one.

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