Refactoring: move ystart detection logic into separate class.

src/emucore/tia/FrameManager.cxx

@@ -35,9 +35,7 @@ enum Metrics: uInt32 {
   maxUnderscan                  = 10,
   tvModeDetectionTolerance      = 20,
   initialGarbageFrames          = 10,
-  framesForModeConfirmation     = 5,
+  framesForModeConfirmation     = 5
-  maxVblankViolations           = 2,
-  minStableVblankFrames         = 1
 };
 
 static constexpr uInt32
@@ -58,9 +56,7 @@ uInt8 FrameManager::initialGarbageFrames()
 FrameManager::FrameManager()
   : myMode(TvMode::pal),
     myAutodetectTvMode(true),
-    myFixedHeight(0),
+    myFixedHeight(0)
-    myVblankMode(VblankMode::floating),
-    myYstart(0)
 {
   updateTvMode(TvMode::ntsc);
   reset();
@@ -79,24 +75,19 @@ void FrameManager::setHandlers(
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameManager::reset()
 {
+  myVblankManager.reset();
+
   myState = State::waitForVsyncStart;
   myCurrentFrameTotalLines = myCurrentFrameFinalLines = 0;
   myFrameRate = 60.0;
   myLineInState = 0;
   myVsync = false;
-  myVblank = false;
   myTotalFrames = 0;
   myFramesInMode = 0;
   myModeConfirmed = false;
-  myLastVblankLines = 0;
-  myVblankViolations = 0;
-  myStableVblankFrames = 0;
-  myVblankViolated = false;
   myVsyncLines = 0;
   myY = 0;
   myFramePending = false;
-
-  if (myVblankMode != VblankMode::fixed) myVblankMode = VblankMode::floating;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -124,7 +115,8 @@ void FrameManager::nextLine()
       break;
 
     case State::waitForFrameStart:
-      nextLineInVsync();
+      if (myVblankManager.nextLine(myTotalFrames <= Metrics::initialGarbageFrames))
+        setState(State::frame);
       break;
 
     case State::frame:
@@ -139,69 +131,6 @@ void FrameManager::nextLine()
   if (myState == State::frame && previousState == State::frame) myY++;
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameManager::nextLineInVsync()
-{
-  bool shouldTransition = myLineInState >= (myVblank ? myVblankLines : myVblankLines - Metrics::maxUnderscan);
-
-  switch (myVblankMode) {
-    case VblankMode::floating:
-
-      if (shouldTransition) {
-        if (myTotalFrames > Metrics::initialGarbageFrames && myLineInState == myLastVblankLines)
-          myStableVblankFrames++;
-        else
-          myStableVblankFrames = 0;
-
-        myLastVblankLines = myLineInState;
-
-#ifdef TIA_FRAMEMANAGER_DEBUG_LOG
-        (cout << "leaving vblank in floating mode, should transition: " << shouldTransition << "\n").flush();
-#endif
-        setState(State::frame);
-      }
-
-      if (myStableVblankFrames >= Metrics::minStableVblankFrames) {
-        myVblankMode = VblankMode::locked;
-        myVblankViolations = 0;
-      }
-
-      break;
-
-    case VblankMode::locked:
-
-      if (myLineInState == myLastVblankLines) {
-
-        if (shouldTransition && !myVblankViolated)
-          myVblankViolations = 0;
-        else {
-          if (!myVblankViolated) myVblankViolations++;
-          myVblankViolated = true;
-        }
-
-#ifdef TIA_FRAMEMANAGER_DEBUG_LOG
-        (cout << "leaving vblank in locked mode, should transition: " << shouldTransition << "\n").flush();
-#endif
-
-        setState(State::frame);
-      } else if (shouldTransition){
-        if (!myVblankViolated) myVblankViolations++;
-        myVblankViolated = true;
-      }
-
-      if (myVblankViolations > Metrics::maxVblankViolations) {
-        myVblankMode = VblankMode::floating;
-        myStableVblankFrames = 0;
-      }
-
-      break;
-
-    case VblankMode::fixed:
-      if (myLineInState > myYstart) setState(State::frame);
-      break;
-  }
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameManager::setVsync(bool vsync)
 {
@@ -249,10 +178,10 @@ void FrameManager::setState(FrameManager::State state)
     case State::waitForFrameStart:
       if (myFramePending) finalizeFrame();
       if (myOnFrameStart) myOnFrameStart();
+      myVblankManager.start();
       myFramePending = true;
 
       myVsyncLines = 0;
-      myVblankViolated = false;
       break;
 
     case State::frame:
@@ -349,32 +278,25 @@ void FrameManager::updateTvMode(TvMode mode)
   }
 
   myFrameLines = Metrics::vsync + myVblankLines + myKernelLines + myOverscanLines;
+
+  myVblankManager.setVblankLines(myVblankLines);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameManager::setYstart(uInt32 ystart)
 {
-  if (ystart == myYstart) return;
+  myVblankManager.setYstart(ystart);
-
-  myYstart = ystart;
-
-  myVblankMode = ystart ? VblankMode::fixed : VblankMode::floating;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 uInt32 FrameManager::ystart() const {
-  return myYstart;
+  return myVblankManager.ystart();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameManager::setVblank(bool vblank)
 {
-#ifdef TIA_FRAMEMANAGER_DEBUG_LOG
+  myVblankManager.setVblank(vblank);
-  if (myVblank != vblank)
-    (cout << "vblank " << myVblank << " -> " << vblank << ": state " << int(myState) << " @ " << myLineInState << "\n").flush();
-#endif
-
-  myVblank = vblank;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -427,6 +349,8 @@ bool FrameManager::save(Serializer& out) const
   {
     out.putString(name());
 
+    if (!myVblankManager.save(out)) return false;
+
     // TODO - save instance variables
   }
   catch(...)
@@ -447,6 +371,8 @@ bool FrameManager::load(Serializer& in)
     if(in.getString() != name())
       return false;
 
+    if (!myVblankManager.load(in)) return false;
+
     // TODO - load instance variables
   }
   catch(...)

src/emucore/tia/FrameManager.hxx

@@ -20,6 +20,7 @@
 
 #include <functional>
 
+#include "VblankManager.hxx"
 #include "Serializable.hxx"
 #include "TvMode.hxx"
 #include "bspf.hxx"
@@ -52,7 +53,7 @@ class FrameManager : public Serializable
 
     TvMode tvMode() const;
 
-    bool vblank() const { return myVblank; }
+    bool vblank() const { return myVblankManager.vblank(); }
 
     bool vsync() const { return myVsync; }
 
@@ -124,6 +125,8 @@ class FrameManager : public Serializable
     callback myOnFrameStart;
     callback myOnFrameComplete;
 
+    VblankManager myVblankManager;
+
     TvMode myMode;
     bool myAutodetectTvMode;
     State myState;
@@ -140,7 +143,6 @@ class FrameManager : public Serializable
     bool myModeConfirmed;
 
     bool myVsync;
-    bool myVblank;
 
     uInt32 myVblankLines;
     uInt32 myKernelLines;
@@ -148,13 +150,6 @@ class FrameManager : public Serializable
     uInt32 myFrameLines;
     uInt32 myFixedHeight;
 
-    VblankMode myVblankMode;
-    uInt32 myLastVblankLines;
-    uInt32 myYstart;
-    uInt8 myVblankViolations;
-    uInt8 myStableVblankFrames;
-    bool myVblankViolated;
-
   private:
     FrameManager(const FrameManager&) = delete;
     FrameManager(FrameManager&&) = delete;

src/emucore/tia/VblankManager.hxx

@@ -0,0 +1,87 @@
+//============================================================================
+//
+//   SSSS    tt          lll  lll
+//  SS  SS   tt           ll   ll
+//  SS     tttttt  eeee   ll   ll   aaaa
+//   SSSS    tt   ee  ee  ll   ll      aa
+//      SS   tt   eeeeee  ll   ll   aaaaa  --  "An Atari 2600 VCS Emulator"
+//  SS  SS   tt   ee      ll   ll  aa  aa
+//   SSSS     ttt  eeeee llll llll  aaaaa
+//
+// Copyright (c) 1995-2017 by Bradford W. Mott, Stephen Anthony
+// and the Stella Team
+//
+// See the file "License.txt" for information on usage and redistribution of
+// this file, and for a DISCLAIMER OF ALL WARRANTIES.
+//============================================================================
+
+#ifndef TIA_VBLANK_MANAGER
+#define TIA_VBLANK_MANAGER
+
+#include "Serializable.hxx"
+
+class VblankManager : public Serializable
+{
+  public:
+
+    VblankManager();
+
+  public:
+
+    void reset();
+
+    void start();
+
+    bool nextLine(bool isGarbageFrame);
+
+    void setVblankLines(uInt32 lines);
+
+    void setYstart(uInt32 ystart);
+
+    uInt32 ystart() const { return myYstart; }
+
+    void setVblank(bool vblank);
+
+    bool vblank() const { return myVblank; }
+
+    uInt32 currentLine() const {return myCurrentLine; };
+
+    /**
+      Serializable methods (see that class for more information).
+    */
+    bool save(Serializer& out) const override;
+    bool load(Serializer& in) override;
+    string name() const override { return "TIA_VblankManager"; }
+
+  private:
+
+    enum VblankMode {
+      locked,
+      floating,
+      fixed
+    };
+
+  private:
+
+    uInt32 myVblankLines;
+    uInt32 myMaxUnderscan;
+    uInt32 myYstart;
+    bool myVblank;
+    uInt32 myCurrentLine;
+
+    VblankMode myMode;
+    uInt32 myLastVblankLines;
+    uInt8 myVblankViolations;
+    uInt8 myStableVblankFrames;
+    bool myVblankViolated;
+
+  private:
+
+    VblankManager(const VblankManager&) = delete;
+    VblankManager(VblankManager&&) = delete;
+    VblankManager& operator=(const VblankManager&) = delete;
+    VblankManager& operator=(VblankManager&&) = delete;
+
+};
+
+#endif // TIA_VBLANK_MANAGER

src/emucore/tia/Vblankanager.cxx

@@ -0,0 +1,190 @@
+//============================================================================
+//
+//   SSSS    tt          lll  lll
+//  SS  SS   tt           ll   ll
+//  SS     tttttt  eeee   ll   ll   aaaa
+//   SSSS    tt   ee  ee  ll   ll      aa
+//      SS   tt   eeeeee  ll   ll   aaaaa  --  "An Atari 2600 VCS Emulator"
+//  SS  SS   tt   ee      ll   ll  aa  aa
+//   SSSS     ttt  eeeee llll llll  aaaaa
+//
+// Copyright (c) 1995-2017 by Bradford W. Mott, Stephen Anthony
+// and the Stella Team
+//
+// See the file "License.txt" for information on usage and redistribution of
+// this file, and for a DISCLAIMER OF ALL WARRANTIES.
+//============================================================================
+
+// #define TIA_VBLANK_MANAGER_DEBUG_LOG
+
+#include "VblankManager.hxx"
+
+enum Metrics: uInt32 {
+  maxUnderscan                  = 10,
+  maxVblankViolations           = 2,
+  minStableVblankFrames         = 1
+};
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+VblankManager::VblankManager()
+  : myVblankLines(0),
+    myMaxUnderscan(0),
+    myYstart(0),
+    myMode(VblankMode::floating)
+{
+  reset();
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void VblankManager::reset()
+{
+  myVblank = false;
+  myCurrentLine = 0;
+  myVblankViolations = 0;
+  myStableVblankFrames = 0;
+  myVblankViolated = false;
+  myLastVblankLines = 0;
+
+  if (myMode != VblankMode::fixed) myMode = VblankMode::floating;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void VblankManager::start()
+{
+  myCurrentLine = 0;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+bool VblankManager::nextLine(bool isGarbageFrame)
+{
+  myCurrentLine++;
+
+  bool shouldTransition = myCurrentLine >= (myVblank ? myVblankLines : myVblankLines - Metrics::maxUnderscan);
+  bool transition = false;
+
+  switch (myMode) {
+    case VblankMode::floating:
+
+      if (shouldTransition) {
+        if (!isGarbageFrame && myCurrentLine == myLastVblankLines)
+          myStableVblankFrames++;
+        else
+          myStableVblankFrames = 0;
+
+        myLastVblankLines = myCurrentLine;
+
+#ifdef TIA_VBLANK_MANAGER_DEBUG_LOG
+        (cout << "leaving vblank in floating mode, should transition: " << shouldTransition << "\n").flush();
+#endif
+        transition = true;
+      }
+
+      if (myStableVblankFrames >= Metrics::minStableVblankFrames) {
+        myMode = VblankMode::locked;
+        myVblankViolations = 0;
+      }
+
+      break;
+
+    case VblankMode::locked:
+
+      if (myCurrentLine == myLastVblankLines) {
+
+        if (shouldTransition && !myVblankViolated)
+          myVblankViolations = 0;
+        else {
+          if (!myVblankViolated) myVblankViolations++;
+          myVblankViolated = true;
+        }
+
+#ifdef TIA_VBLANK_MANAGER_DEBUG_LOG
+        (cout << "leaving vblank in locked mode, should transition: " << shouldTransition << "\n").flush();
+#endif
+
+        transition = true;
+      } else if (shouldTransition){
+        if (!myVblankViolated) myVblankViolations++;
+        myVblankViolated = true;
+      }
+
+      if (myVblankViolations > Metrics::maxVblankViolations) {
+        myMode = VblankMode::floating;
+        myStableVblankFrames = 0;
+      }
+
+      break;
+
+    case VblankMode::fixed:
+      if (myCurrentLine > myYstart) transition = true;
+      break;
+
+  }
+
+  return transition;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void VblankManager::setVblankLines(uInt32 vblankLines)
+{
+  myVblankLines = vblankLines;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void VblankManager::setYstart(uInt32 ystart)
+{
+  if (ystart == myYstart) return;
+
+  myYstart = ystart;
+
+  myMode = ystart ? VblankMode::fixed : VblankMode::floating;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void VblankManager::setVblank(bool vblank)
+{
+#ifdef TIA_VBLANK_MANAGER_DEBUG_LOG
+  if (myVblank != vblank)
+    (cout << "vblank " << myVblank << " -> " << vblank << ": state " << int(myState) << " @ " << myLineInState << "\n").flush();
+#endif
+
+  myVblank = vblank;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+// TODO: implement this once the class is finalized
+bool VblankManager::save(Serializer& out) const
+{
+  try
+  {
+    out.putString(name());
+
+    // TODO - save instance variables
+  }
+  catch(...)
+  {
+    cerr << "ERROR: TIA_VblankManager::save" << endl;
+    return false;
+  }
+
+  return false;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+// TODO: implement this once the class is finalized
+bool VblankManager::load(Serializer& in)
+{
+  try
+  {
+    if(in.getString() != name())
+      return false;
+
+    // TODO - load instance variables
+  }
+  catch(...)
+  {
+    cerr << "ERROR: TIA_VblankManager::load" << endl;
+    return false;
+  }
+
+  return false;
+}

src/emucore/tia/module.mk

@@ -12,7 +12,8 @@ MODULE_OBJS := \
 	src/emucore/tia/Ball.o \
 	src/emucore/tia/Background.o \
 	src/emucore/tia/LatchedInput.o \
-	src/emucore/tia/PaddleReader.o
+	src/emucore/tia/PaddleReader.o \
+	src/emucore/tia/Vblankanager.o
 
 
 MODULE_DIRS += \