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,24 +860,34 @@ void VideoThrottle()
 
 // --- Callbacks for backends / engine ---
 
-// Should be called from GPU thread when a frame is drawn
+// Called from Renderer::Swap (GPU thread) when a new (non-duplicate)
-void Callback_VideoCopiedToXFB(bool video_update)
+// frame is presented to the host screen
+void Callback_FramePresented()
 {
-  if (video_update)
   s_drawn_frame++;
+  s_stop_frame_step.store(true);
 }
 
-// Called at field boundaries in `VideoInterface::Update()`
+// Called from VideoInterface::Update (CPU thread) at emulated field boundaries
-void FrameUpdate()
+void Callback_NewField()
 {
-  Movie::FrameUpdate();
   if (s_frame_step)
+  {
+    // 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());
     }
+  }
 }
 
 void UpdateTitle()
@@ -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 Callback_FramePresented();
-void FrameUpdate();
+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,32 +817,10 @@ 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
+  // Movie's frame counter should be updated before actually rendering the frame,
-  // and/or update movie state before dealing with anything else
+  // in case frame counter display is enabled
 
-  if (s_half_line_count == 0 || s_half_line_count == GetHalfLinesPerEvenField())
+  Movie::FrameUpdate();
-    Core::FrameUpdate();
-
-  // If an SI poll is scheduled to happen on this half-line, do it!
-
-  if (s_half_line_of_next_si_poll == s_half_line_count)
-  {
-    Core::UpdateInputGate(!SConfig::GetInstance().m_BackgroundInput);
-    SerialInterface::UpdateDevices();
-    s_half_line_of_next_si_poll += 2 * SerialInterface::GetPollXLines();
-  }
-
-  // If this half-line is at the actual boundary of either field, schedule an SI poll to happen
-  // some number of half-lines in the future
-
-  if (s_half_line_count == 0)
-  {
-    s_half_line_of_next_si_poll = num_half_lines_for_si_poll;  // first results start at vsync
-  }
-  if (s_half_line_count == GetHalfLinesPerEvenField())
-  {
-    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
@@ -864,6 +843,33 @@ void Update(u64 ticks)
     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::Callback_NewField();
+
+  // If an SI poll is scheduled to happen on this half-line, do it!
+
+  if (s_half_line_of_next_si_poll == s_half_line_count)
+  {
+    Core::UpdateInputGate(!SConfig::GetInstance().m_BackgroundInput);
+    SerialInterface::UpdateDevices();
+    s_half_line_of_next_si_poll += 2 * SerialInterface::GetPollXLines();
+  }
+
+  // If this half-line is at the actual boundary of either field, schedule an SI poll to happen
+  // some number of half-lines in the future
+
+  if (s_half_line_count == 0)
+  {
+    s_half_line_of_next_si_poll = num_half_lines_for_si_poll;  // first results start at vsync
+  }
+  if (s_half_line_count == GetHalfLinesPerEvenField())
+  {
+    s_half_line_of_next_si_poll = GetHalfLinesPerEvenField() + num_half_lines_for_si_poll;
+  }
+
   // 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 @@ public:
       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.