duckstation icon indicating copy to clipboard operation
duckstation copied to clipboard

fix throttler and vsync related issues

Open Guwange opened this issue 1 year ago • 1 comments

This commit may fix issues like below:

  • VSYNC will be disabled after pause (including disc change, etc.)
  • 'Unlimited' setting for fast-forward / turbo not working. (And it seems other speed setting also not working..)

This commit fixes below functions:

  • System::UpdateSpeedLimiterState()

    • m_throttler_enabled should be set if target_speed != 1.0f, not 0.0f.
      • m_throttler_enabled was set to true even in case emulator is running at 100%.
      • m_throttler_enabled was set to false in case fast_forward/turbo was enabled but set to 'Unlimited'
    • ShouldUseVSync() should be called after s_target_speed is updated with target_speed.
      • ShouldUseVSync() uses IsRunningAtNonStandardSpeed() internally, and it refers s_target_speed.
  • System::ShouldUseVSync()

    • I guess this function should return 'true' in case 'g_settings.video_sync_enabled == true' && 'm_throttler_enabled == false'. but current implementation returns false.

Guwange avatar Aug 05 '22 18:08 Guwange

I updated the commit and force pushed. Could you review it again ? Thanks.

This commit fixes below functions:

  • System::UpdateSpeedLimiterState() ShouldUseVSync() should be called after s_target_speed is updated with target_speed. (ShouldUseVSync() uses IsRunningAtNonStandardSpeed() internally, and it refers s_target_speed.)

  • System::ShouldUseVSync() This function should return 'true' in case 'g_settings.video_sync_enabled == true' and it is running at normal speed, regardless of m_throttler_enabled. (m_throttler_enabled maybe true in case it is called from UpdateSpeedLimiterState(), and false after UpdateSpeedLimiterState() exits)

Guwange avatar Aug 06 '22 16:08 Guwange

Might want to re-check this on the latest master, I ended up redoing part of this logic anyway (https://github.com/stenzek/duckstation/commit/54c2447ff3b66b8636748ee0b68c1c0c50304b87).

stenzek avatar Aug 10 '22 03:08 stenzek

Might want to re-check this on the latest master, I ended up redoing part of this logic anyway (54c2447).

I checked the latest master. It looks fine and the built binary also seems working fine. I think now this PR can be closed. Thanks !

Guwange avatar Aug 10 '22 12:08 Guwange