Merge pull request #8730 from JosJuice/frame-advance-duplicate-frame

Core: Skip duplicate frames when using frame advance

Source/Core/Core/Core.cpp

@@ -77,6 +77,7 @@
 #include "InputCommon/ControllerInterface/ControllerInterface.h"
 #include "InputCommon/GCAdapter.h"
 
+#include "VideoCommon/AsyncRequests.h"
 #include "VideoCommon/Fifo.h"
 #include "VideoCommon/OnScreenDisplay.h"
 #include "VideoCommon/RenderBase.h"
@@ -104,6 +105,7 @@ static std::thread s_cpu_thread;
 static bool s_request_refresh_info = false;
 static bool s_is_throttler_temp_disabled = false;
 static bool s_frame_step = false;
+static std::atomic<bool> s_stop_frame_step;
 
 #ifdef USE_MEMORYWATCHER
 static std::unique_ptr<MemoryWatcher> s_memory_watcher;
@@ -858,23 +860,33 @@ void VideoThrottle()
 
 // --- Callbacks for backends / engine ---
 
-// Should be called from GPU thread when a frame is drawn
-void Callback_VideoCopiedToXFB(bool video_update)
+// Called from Renderer::Swap (GPU thread) when a new (non-duplicate)
+// frame is presented to the host screen
+void Callback_FramePresented()
 {
-  if (video_update)
-    s_drawn_frame++;
+  s_drawn_frame++;
+  s_stop_frame_step.store(true);
 }
 
-// Called at field boundaries in `VideoInterface::Update()`
-void FrameUpdate()
+// Called from VideoInterface::Update (CPU thread) at emulated field boundaries
+void Callback_NewField()
 {
-  Movie::FrameUpdate();
   if (s_frame_step)
   {
-    s_frame_step = false;
-    CPU::Break();
-    if (s_on_state_changed_callback)
-      s_on_state_changed_callback(Core::GetState());
+    // To ensure that s_stop_frame_step is up to date, wait for the GPU thread queue to empty,
+    // since it is may contain a swap event (which will call Callback_FramePresented). This hurts
+    // the performance a little, but luckily, performance matters less when using frame stepping.
+    AsyncRequests::GetInstance()->WaitForEmptyQueue();
+
+    // Only stop the frame stepping if a new frame was displayed
+    // (as opposed to the previous frame being displayed for another frame).
+    if (s_stop_frame_step.load())
+    {
+      s_frame_step = false;
+      CPU::Break();
+      if (s_on_state_changed_callback)
+        s_on_state_changed_callback(Core::GetState());
+    }
   }
 }
 
@@ -1045,6 +1057,7 @@ void DoFrameStep()
   if (GetState() == State::Paused)
   {
     // if already paused, frame advance for 1 frame
+    s_stop_frame_step = false;
     s_frame_step = true;
     RequestRefreshInfo();
     SetState(State::Running);

Source/Core/Core/Core.h

@@ -25,8 +25,8 @@ namespace Core
 bool GetIsThrottlerTempDisabled();
 void SetIsThrottlerTempDisabled(bool disable);
 
-void Callback_VideoCopiedToXFB(bool video_update);
-void FrameUpdate();
+void Callback_FramePresented();
+void Callback_NewField();
 
 enum class State
 {

Source/Core/Core/HW/VideoInterface.cpp

@@ -24,6 +24,7 @@
 #include "Core/HW/ProcessorInterface.h"
 #include "Core/HW/SI/SI.h"
 #include "Core/HW/SystemTimers.h"
+#include "Core/Movie.h"
 
 #include "DiscIO/Enums.h"
 
@@ -816,11 +817,37 @@ static void EndField()
 // Run when: When a frame is scanned (progressive/interlace)
 void Update(u64 ticks)
 {
-  // If this half-line is at a field boundary, potentially deal with frame-stepping
-  // and/or update movie state before dealing with anything else
+  // Movie's frame counter should be updated before actually rendering the frame,
+  // in case frame counter display is enabled
+
+  Movie::FrameUpdate();
+
+  // If this half-line is at some boundary of the "active video lines" in either field, we either
+  // need to (a) send a request to the GPU thread to actually render the XFB, or (b) increment
+  // the number of frames we've actually drawn
+
+  if (s_half_line_count == s_even_field_first_hl)
+  {
+    BeginField(FieldType::Even, ticks);
+  }
+  else if (s_half_line_count == s_odd_field_first_hl)
+  {
+    BeginField(FieldType::Odd, ticks);
+  }
+  else if (s_half_line_count == s_even_field_last_hl)
+  {
+    EndField();
+  }
+  else if (s_half_line_count == s_odd_field_last_hl)
+  {
+    EndField();
+  }
+
+  // If this half-line is at a field boundary, deal with updating movie state before potentially
+  // dealing with SI polls, but after potentially sending a swap request to the GPU thread
 
   if (s_half_line_count == 0 || s_half_line_count == GetHalfLinesPerEvenField())
-    Core::FrameUpdate();
+    Core::Callback_NewField();
 
   // If an SI poll is scheduled to happen on this half-line, do it!
 
@@ -843,27 +870,6 @@ void Update(u64 ticks)
     s_half_line_of_next_si_poll = GetHalfLinesPerEvenField() + num_half_lines_for_si_poll;
   }
 
-  // If this half-line is at some boundary of the "active video lines" in either field, we either
-  // need to (a) send a request to the GPU thread to actually render the XFB, or (b) increment
-  // the number of frames we've actually drawn
-
-  if (s_half_line_count == s_even_field_first_hl)
-  {
-    BeginField(FieldType::Even, ticks);
-  }
-  else if (s_half_line_count == s_odd_field_first_hl)
-  {
-    BeginField(FieldType::Odd, ticks);
-  }
-  else if (s_half_line_count == s_even_field_last_hl)
-  {
-    EndField();
-  }
-  else if (s_half_line_count == s_odd_field_last_hl)
-  {
-    EndField();
-  }
-
   // Move to the next half-line and potentially roll-over the count to zero. If we've reached
   // the beginning of a new full-line, update the timer
 

Source/Core/VideoCommon/AsyncRequests.cpp

@@ -97,6 +97,12 @@ void AsyncRequests::PushEvent(const AsyncRequests::Event& event, bool blocking)
   }
 }
 
+void AsyncRequests::WaitForEmptyQueue()
+{
+  std::unique_lock<std::mutex> lock(m_mutex);
+  m_cond.wait(lock, [this] { return m_queue.empty(); });
+}
+
 void AsyncRequests::SetEnable(bool enable)
 {
   std::unique_lock<std::mutex> lock(m_mutex);

Source/Core/VideoCommon/AsyncRequests.h

@@ -82,6 +82,7 @@ class AsyncRequests
       PullEventsInternal();
   }
   void PushEvent(const Event& event, bool blocking = false);
+  void WaitForEmptyQueue();
   void SetEnable(bool enable);
   void SetPassthrough(bool enable);
 

Source/Core/VideoCommon/RenderBase.cpp

@@ -1309,7 +1309,7 @@ void Renderer::Swap(u32 xfb_addr, u32 fb_width, u32 fb_stride, u32 fb_height, u6
       {
         // Remove stale EFB/XFB copies.
         g_texture_cache->Cleanup(m_frame_count);
-        Core::Callback_VideoCopiedToXFB(true);
+        Core::Callback_FramePresented();
       }
 
       // Handle any config changes, this gets propogated to the backend.