Refactoring: start debugger from dispatch loop.

src/emucore/DispatchResult.cxx

@@ -0,0 +1,55 @@
+//============================================================================
+//
+// MM     MM  6666  555555  0000   2222
+// MMMM MMMM 66  66 55     00  00 22  22
+// MM MMM MM 66     55     00  00     22
+// MM  M  MM 66666  55555  00  00  22222  --  "A 6502 Microprocessor Emulator"
+// MM     MM 66  66     55 00  00 22
+// MM     MM 66  66 55  55 00  00 22
+// MM     MM  6666   5555   0000  222222
+//
+// Copyright (c) 1995-2018 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.
+//============================================================================
+
+#include "DispatchResult.hxx"
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void DispatchResult::assertStatus(Status status) const
+{
+  if (myStatus != status) throw runtime_error("invalid status for operation");
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+bool DispatchResult::isSuccess() const
+{
+  return myStatus == Status::debugger || myStatus == Status::ok;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void DispatchResult::setOk(uInt32 cycles)
+{
+  myStatus = Status::ok;
+  myCycles = cycles;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void DispatchResult::setDebugger(uInt32 cycles, const string& message, int address, bool wasReadTrap)
+{
+  myStatus = Status::debugger;
+  myCycles = cycles;
+  myMessage = message;
+  myAddress = address;
+  myWasReadTrap = wasReadTrap;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void DispatchResult::setFatal(uInt32 cycles)
+{
+  myCycles = cycles;
+
+  myStatus = Status::fatal;
+}

src/emucore/DispatchResult.hxx

@@ -0,0 +1,67 @@
+//============================================================================
+//
+// MM     MM  6666  555555  0000   2222
+// MMMM MMMM 66  66 55     00  00 22  22
+// MM MMM MM 66     55     00  00     22
+// MM  M  MM 66666  55555  00  00  22222  --  "A 6502 Microprocessor Emulator"
+// MM     MM 66  66     55 00  00 22
+// MM     MM 66  66 55  55 00  00 22
+// MM     MM  6666   5555   0000  222222
+//
+// Copyright (c) 1995-2018 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 DISPATCH_RESULT_HXX
+#define DISPATCH_RESULT_HXX
+
+#include "bspf.hxx"
+
+class DispatchResult
+{
+  public:
+    enum class Status { invalid, ok, debugger, fatal };
+
+  public:
+
+    DispatchResult() : myStatus(Status::invalid) {}
+
+    Status getStatus() const { return myStatus; }
+
+    uInt32 getCycles() const { return myCycles; }
+
+    const string& getMessage() const { assertStatus(Status::debugger); return myMessage; }
+
+    uInt16 getAddress() const { assertStatus(Status::debugger); return myAddress; }
+
+    bool wasReadTrap() const { assertStatus(Status::debugger); return myWasReadTrap; }
+
+    bool isSuccess() const;
+
+    void setOk(uInt32 cycles);
+
+    void setDebugger(uInt32 cycles, const string& message = "", int address = -1, bool wasReadTrap = -1);
+
+    void setFatal(uInt32 cycles);
+
+  private:
+
+    void assertStatus(Status status) const;
+
+  private:
+
+    Status myStatus;
+
+    uInt32 myCycles;
+
+    string myMessage;
+
+    int myAddress;
+
+    bool myWasReadTrap;
+};
+
+#endif // DISPATCH_RESULT_HXX

src/emucore/FrameBuffer.cxx

@@ -268,17 +268,8 @@ void FrameBuffer::update()
   switch(myOSystem.eventHandler().state())
   {
     case EventHandlerState::EMULATION:
-    {
+      // Do nothing; emulation mode is handled separately (see below)
-      myTIASurface->render();
+      break;
-
-      // Show frame statistics
-      if(myStatsMsg.enabled)
-        drawFrameStats();
-
-      myLastScanlines = myOSystem.console().tia().scanlinesLastFrame();
-      myPausedCount = 0;
-      break;  // EventHandlerState::EMULATION
-    }
 
     case EventHandlerState::PAUSE:
     {
@@ -340,6 +331,30 @@ void FrameBuffer::update()
   postFrameUpdate();
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameBuffer::updateInEmulationMode()
+{
+  // Determine which mode we are in (from the EventHandler)
+  // Take care of S_EMULATE mode here, otherwise let the GUI
+  // figure out what to draw
+
+  myTIASurface->render();
+
+  // Show frame statistics
+  if(myStatsMsg.enabled)
+    drawFrameStats();
+
+  myLastScanlines = myOSystem.console().tia().scanlinesLastFrame();
+  myPausedCount = 0;
+
+  // Draw any pending messages
+  if(myMsg.enabled)
+    drawMessage();
+
+  // Do any post-frame stuff
+  postFrameUpdate();
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameBuffer::showMessage(const string& message, MessagePosition position,
                               bool force)

src/emucore/FrameBuffer.hxx

@@ -118,6 +118,11 @@ class FrameBuffer
     */
     void update();
 
+    /**
+      There is a dedicated update method for emulation mode.
+     */
+    void updateInEmulationMode();
+
     /**
       Shows a message onscreen.
 

src/emucore/M6502.cxx

@@ -47,6 +47,7 @@
 
 #include "System.hxx"
 #include "M6502.hxx"
+#include "DispatchResult.hxx"
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 M6502::M6502(const Settings& settings)
@@ -208,9 +209,9 @@ inline void M6502::handleHalt()
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool M6502::execute(uInt32 number)
+void M6502::execute(uInt32 number, DispatchResult& result)
 {
-  const bool status = _execute(number);
+  _execute(number, result);
 
 #ifdef DEBUGGER_SUPPORT
   // Debugger hack: this ensures that stepping a "STA WSYNC" will actually end at the
@@ -225,12 +226,20 @@ bool M6502::execute(uInt32 number)
   // that audio samples are generated for the whole timeslice.
   mySystem->tia().updateEmulation();
   mySystem->m6532().updateEmulation();
-
-  return status;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-inline bool M6502::_execute(uInt32 cycles)
+bool M6502::execute(uInt32 number)
+{
+  DispatchResult result;
+
+  _execute(number, result);
+
+  return result.isSuccess();
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+inline void M6502::_execute(uInt32 cycles, DispatchResult& result)
 {
   // Clear all of the execution status bits except for the fatal error bit
   myExecutionStatus &= FatalErrorBit;
@@ -241,7 +250,7 @@ inline bool M6502::_execute(uInt32 cycles)
 #endif
 
   uInt64 previousCycles = mySystem->cycles();
-  uInt64 currentCycles = 0;
+  uInt32 currentCycles = 0;
 
   // Loop until execution is stopped or a fatal error occurs
   for(;;)
@@ -254,18 +263,23 @@ inline bool M6502::_execute(uInt32 cycles)
         bool read = myJustHitReadTrapFlag;
         myJustHitReadTrapFlag = myJustHitWriteTrapFlag = false;
 
-        if (startDebugger(myHitTrapInfo.message, myHitTrapInfo.address, read)) return true;
+        result.setDebugger(currentCycles, myHitTrapInfo.message, myHitTrapInfo.address, read);
+        return;
       }
 
-      if(myBreakPoints.isInitialized() && myBreakPoints.isSet(PC) && startDebugger("BP: ", PC))
+      if(myBreakPoints.isInitialized() && myBreakPoints.isSet(PC)) {
-        return true;
+        result.setDebugger(currentCycles, "BP: ", PC);
+        return;
+      }
 
       int cond = evalCondBreaks();
       if(cond > -1)
       {
         stringstream msg;
         msg << "CBP[" << Common::Base::HEX2 << cond << "]: " << myCondBreakNames[cond];
-        if (startDebugger(msg.str())) return true;
+
+        result.setDebugger(currentCycles, msg.str());
+        return;
       }
 
       cond = evalCondSaveStates();
@@ -324,21 +338,24 @@ inline bool M6502::_execute(uInt32 cycles)
     if(myExecutionStatus & StopExecutionBit)
     {
       // Yes, so answer that everything finished fine
-      return true;
+      result.setOk(currentCycles);
+      return;
     }
 
     // See if a fatal error has occured
     if(myExecutionStatus & FatalErrorBit)
     {
       // Yes, so answer that something when wrong
-      return false;
+      result.setFatal(currentCycles + icycles);
+      return;
     }
 
     // See if we've executed the specified number of instructions
     if (currentCycles >= cycles * SYSTEM_CYCLES_PER_CPU)
     {
       // Yes, so answer that everything finished fine
-      return true;
+      result.setOk(currentCycles);
+      return;
     }
   }
 }
@@ -612,15 +629,4 @@ void M6502::updateStepStateByInstruction()
                              myTrapConds.size();
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool M6502::startDebugger(const string& message, int address, bool read)
-{
-  handleHalt();
-
-  mySystem->tia().updateEmulation();
-  mySystem->m6532().updateEmulation();
-
-  return myDebugger->start(message, address, read);
-}
-
 #endif  // DEBUGGER_SUPPORT

src/emucore/M6502.hxx

@@ -22,6 +22,7 @@
 
 class Settings;
 class System;
+class DispatchResult;
 
 #ifdef DEBUGGER_SUPPORT
   class Debugger;
@@ -113,8 +114,10 @@ class M6502 : public Serializable
       @param cycles Indicates the number of cycles to execute. Not that the actual
                     granularity of the CPU is instructions, so this is only accurate up to
                     a couple of cycles
-      @return true iff execution stops normally
+      @param result A DispatchResult object that will transport the result
     */
+    void execute(uInt32 cycles, DispatchResult& result);
+
     bool execute(uInt32 cycles);
 
     /**
@@ -320,7 +323,7 @@ class M6502 : public Serializable
       This is the actual dispatch function that does the grunt work. M6502::execute
       wraps it and makes sure that any pending halt is processed before returning.
     */
-    bool _execute(uInt32 cycles);
+    void _execute(uInt32 cycles, DispatchResult& result);
 
 #ifdef DEBUGGER_SUPPORT
     /**
@@ -328,12 +331,6 @@ class M6502 : public Serializable
       with the CPU and update the flag accordingly.
     */
     void updateStepStateByInstruction();
-
-    /**
-      Make sure that the current hardware state is up to date (TIA & RIOT) and dispatch
-      debugger.
-    */
-    bool startDebugger(const string& message = "", int address = -1, bool read = true);
 #endif  // DEBUGGER_SUPPORT
 
   private:

src/emucore/OSystem.cxx

@@ -53,6 +53,7 @@
 #include "StateManager.hxx"
 #include "Version.hxx"
 #include "TIA.hxx"
+#include "DispatchResult.hxx"
 
 #include "OSystem.hxx"
 
@@ -635,6 +636,30 @@ float OSystem::frameRate() const
   return myConsole ? myConsole->getFramerate() : 0;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+double OSystem::dispatchEmulation(uInt32 cyclesPerSecond)
+{
+  if (!myConsole) return 0.;
+
+  Int64 totalCycles = 0;
+  const Int64 minCycles = myConsole->emulationTiming().minCyclesPerTimeslice();
+  const Int64 maxCycles = myConsole->emulationTiming().maxCyclesPerTimeslice();
+  DispatchResult dispatchResult;
+
+  do {
+    myConsole->tia().update(dispatchResult, totalCycles > 0 ? minCycles - totalCycles : maxCycles);
+
+    totalCycles += dispatchResult.getCycles();
+  } while (totalCycles < minCycles && dispatchResult.getStatus() == DispatchResult::Status::ok);
+
+  if (dispatchResult.getStatus() == DispatchResult::Status::debugger) myDebugger->start();
+
+  if (dispatchResult.getStatus() == DispatchResult::Status::ok && myEventHandler->frying())
+    myConsole->fry();
+
+  return static_cast<double>(totalCycles) / static_cast<double>(cyclesPerSecond);
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void OSystem::mainLoop()
 {
@@ -650,40 +675,29 @@ void OSystem::mainLoop()
     double timesliceSeconds;
 
     if (myEventHandler->state() == EventHandlerState::EMULATION) {
-      Int64 totalCycles = 0;
+      timesliceSeconds = dispatchEmulation(myConsole ? myConsole->emulationTiming().cyclesPerSecond() : 1);
-      const Int64 minCycles = myConsole ? myConsole->emulationTiming().minCyclesPerTimeslice() : 50000;
-      const Int64 maxCycles = myConsole ? myConsole->emulationTiming().maxCyclesPerTimeslice() : 0;
-      const uInt32 cyclesPerSecond = myConsole ? myConsole->emulationTiming().cyclesPerSecond() : 1;
 
-      do {
-        Int64 cycles = myConsole ? myConsole->tia().update(totalCycles > 0 ? minCycles - totalCycles : maxCycles) : 0;
-
-        totalCycles += cycles;
-      } while (myConsole && totalCycles < minCycles && myEventHandler->state() == EventHandlerState::EMULATION);
-
-      if (myEventHandler->state() == EventHandlerState::EMULATION && myEventHandler->frying())
-        myConsole->fry();
-
-      timesliceSeconds = static_cast<double>(totalCycles) / static_cast<double>(cyclesPerSecond);
-    }
-    else
-      timesliceSeconds = 1. / 30.;
-
-    if (myEventHandler->state() == EventHandlerState::EMULATION) {
       if (myConsole && myConsole->tia().newFramePending()) {
-        myFrameBuffer->update();
+        myFrameBuffer->updateInEmulationMode();
         myConsole->tia().clearNewFramePending();
       }
-    }
+    } else {
-    else
+      timesliceSeconds = 1. / 30.;
       myFrameBuffer->update();
+    }
 
     duration<double> timeslice(timesliceSeconds);
 
     virtualTime += duration_cast<high_resolution_clock::duration>(timeslice);
     time_point<high_resolution_clock> now = high_resolution_clock::now();
+    double maxLag = myConsole
+      ? (
+        static_cast<double>(myConsole->emulationTiming().cyclesPerFrame()) /
+        static_cast<double>(myConsole->emulationTiming().cyclesPerSecond())
+      )
+      : 0;
 
-    if (duration_cast<duration<double>>(now - virtualTime).count() > 0)
+    if (duration_cast<duration<double>>(now - virtualTime).count() > maxLag)
       virtualTime = now;
     else if (virtualTime > now) {
       if (busyWait && myEventHandler->state() == EventHandlerState::EMULATION) {

src/emucore/OSystem.hxx

@@ -563,6 +563,8 @@ class OSystem
     void validatePath(string& path, const string& setting,
                       const string& defaultpath);
 
+    double dispatchEmulation(uInt32 cyclesPerSecond);
+
     // Following constructors and assignment operators not supported
     OSystem(const OSystem&) = delete;
     OSystem(OSystem&&) = delete;

src/emucore/module.mk

@@ -51,6 +51,7 @@ MODULE_OBJS := \
 	src/emucore/CompuMate.o \
 	src/emucore/Console.o \
 	src/emucore/Control.o \
+	src/emucore/DispatchResult.o \
 	src/emucore/Driving.o \
 	src/emucore/EventHandler.o \
 	src/emucore/EmulationTiming.o \

src/emucore/tia/TIA.cxx

@@ -24,6 +24,7 @@
 #include "TIAConstants.hxx"
 #include "frame-manager/FrameManager.hxx"
 #include "AudioQueue.hxx"
+#include "DispatchResult.hxx"
 
 #ifdef DEBUGGER_SUPPORT
   #include "CartDebug.hxx"
@@ -814,14 +815,21 @@ bool TIA::loadDisplay(Serializer& in)
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-uInt64 TIA::update(uInt32 maxCycles)
+void TIA::update(DispatchResult& result, uInt32 maxCycles)
 {
   uInt64 timestampOld = myTimestamp;
 
-  mySystem->m6502().execute(maxCycles);
+  mySystem->m6502().execute(maxCycles, result);
 
   updateEmulation();
-  return (myTimestamp - timestampOld) / 3;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void TIA::update(uInt32 maxCycles)
+{
+  DispatchResult dispatchResult;
+
+  update(dispatchResult, maxCycles);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/emucore/tia/TIA.hxx

@@ -41,6 +41,7 @@
 #include "System.hxx"
 
 class AudioQueue;
+class DispatchResult;
 
 /**
   This class is a device that emulates the Television Interface Adaptor
@@ -199,7 +200,9 @@ class TIA : public Device
       desired frame rate to update the TIA.  Invoking this method will update
       the graphics buffer and generate the corresponding audio samples.
     */
-    uInt64 update(uInt32 maxCycles = 50000);
+    void update(DispatchResult& result, uInt32 maxCycles = 50000);
+
+    void update(uInt32 maxCycles = 50000);
 
     /**
       Did we generate a new frame?