Some refactoring of snapshot code for future reference.

src/common/PNGLibrary.cxx

@@ -142,7 +142,7 @@ void PNGLibrary::saveImage(const string& filename, const VariantList& comments)
     rows[k] = png_bytep(buffer.get() + k*width*4);
 
   // And save the image
-  saveImage(out, rows, width, height, comments);
+  saveImageToDisk(out, rows, width, height, comments);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -171,11 +171,11 @@ void PNGLibrary::saveImage(const string& filename, const FBSurface& surface,
     rows[k] = png_bytep(buffer.get() + k*width*4);
 
   // And save the image
-  saveImage(out, rows, width, height, comments);
+  saveImageToDisk(out, rows, width, height, comments);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void PNGLibrary::saveImage(ofstream& out, const unique_ptr<png_bytep[]>& rows,
+void PNGLibrary::saveImageToDisk(ofstream& out, const unique_ptr<png_bytep[]>& rows,
     png_uint_32 width, png_uint_32 height, const VariantList& comments)
 {
   #define saveImageERROR(s) { err_message = s; goto done; }
@@ -359,7 +359,7 @@ void PNGLibrary::takeSnapshot(uInt32 number)
   {
     // Make sure we have a 'clean' image, with no onscreen messages
     myOSystem.frameBuffer().enableMessages(false);
-    myOSystem.frameBuffer().tiaSurface().reRender();
+    myOSystem.frameBuffer().tiaSurface().renderForSnapshot();
 
     string message = "Snapshot saved";
     try

src/common/PNGLibrary.hxx

@@ -114,6 +114,9 @@ class PNGLibrary
     void setContinuousSnapInterval(uInt32 interval);
 
     /**
+      NOTE: This method will be made private soon, so all calls from
+            external code should be refactored
+
       Create a new snapshot based on the name of the ROM, and also
       optionally using the number given as a parameter.
 
@@ -161,7 +164,7 @@ class PNGLibrary
       @param height   The height of the PNG image
       @param comments The text comments to add to the PNG image
     */
-    void saveImage(ofstream& out, const unique_ptr<png_bytep[]>& rows,
+    void saveImageToDisk(ofstream& out, const unique_ptr<png_bytep[]>& rows,
                          png_uint_32 width, png_uint_32 height,
                          const VariantList& comments);
 

src/common/main.cxx

@@ -30,6 +30,7 @@
 #include "OSystem.hxx"
 #include "PNGLibrary.hxx"
 #include "System.hxx"
+#include "TIASurface.hxx"
 
 #include "ThreadDebugging.hxx"
 
@@ -137,12 +138,16 @@ int main(int argc, char* argv[])
       if(result != EmptyString)
         return Cleanup();
 
+#if 0
+      TODO: Fix this to use functionality from OSystem::mainLoop
       if(theOSystem->settings().getBool("takesnapshot"))
       {
         for(int i = 0; i < 30; ++i)  theOSystem->frameBuffer().update();
+//        theOSystem->frameBuffer().tiaSurface().saveSnapShot();
         theOSystem->png().takeSnapshot();
         return Cleanup();
       }
+#endif
     }
     catch(const runtime_error& e)
     {

src/emucore/EventHandler.cxx

@@ -387,7 +387,6 @@ void EventHandler::handleEvent(Event::Type event, Int32 state)
       return;
 
     case Event::TakeSnapshot:
-      //if(state) myOSystem.png().takeSnapshot();
       if(state) myOSystem.frameBuffer().tiaSurface().saveSnapShot();
       return;
 

src/emucore/TIASurface.cxx

@@ -36,7 +36,7 @@ TIASurface::TIASurface(OSystem& system)
     myPhosphorPercent(0.60f),
     myScanlinesEnabled(false),
     myPalette(nullptr),
-    saveFlag(false)
+    mySaveSnapFlag(false)
 {
   // Load NTSC filter settings
   myNTSCFilter.loadConfig(myOSystem.settings());
@@ -322,7 +322,7 @@ string TIASurface::effectsInfo() const
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-uInt32 TIASurface::averageBuffers(uInt32 bufOfs)
+inline uInt32 TIASurface::averageBuffers(uInt32 bufOfs)
 {
   uInt32 c = myRGBFramebuffer[bufOfs];
   uInt32 p = myPrevRGBFramebuffer[bufOfs];
@@ -374,7 +374,7 @@ void TIASurface::render()
       uInt8*  tiaIn = myTIA->frameBuffer();
       uInt32* rgbIn = myRGBFramebuffer;
 
-      if (saveFlag)
+      if (mySaveSnapFlag)
         memcpy(myPrevRGBFramebuffer, myRGBFramebuffer, width * height * sizeof(uInt32));
 
       uInt32 bufofs = 0, screenofsY = 0, pos;
@@ -402,7 +402,7 @@ void TIASurface::render()
 
     case Filter::BlarggPhosphor:
     {
-      if(saveFlag)
+      if(mySaveSnapFlag)
         memcpy(myPrevRGBFramebuffer, myRGBFramebuffer, height * outPitch * sizeof(uInt32));
 
       myNTSCFilter.render(myTIA->frameBuffer(), width, height, out, outPitch << 2, myRGBFramebuffer);
@@ -417,15 +417,17 @@ void TIASurface::render()
   if(myScanlinesEnabled)
     mySLineSurface->render();
 
-  if(saveFlag)
+  if(mySaveSnapFlag)
     myOSystem.png().takeSnapshot();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void TIASurface::reRender()
+void TIASurface::renderForSnapshot()
 {
-  // Note: This is currently called from PNGLibrary::takeSnapshot() only
+  // TODO: This is currently called from PNGLibrary::takeSnapshot() only
   // Therefore the code could be simplified.
+  // At some point, we will probably merge some of the functionality.
+
   uInt32 width = myTIA->width();
   uInt32 height = myTIA->height();
   uInt32 pos = 0;
@@ -433,20 +435,18 @@ void TIASurface::reRender()
 
   myTiaSurface->basePtr(outPtr, outPitch);
 
+  mySaveSnapFlag = false;
   switch (myFilter)
   {
-    // for non-phosphor modes, render the frame again
+    // For non-phosphor modes, render the frame again
     case Filter::Normal:
     case Filter::BlarggNormal:
-      saveFlag = false;
       render();
       break;
-    // for phosphor modes, copy the phosphor framebuffer
+
+    // For phosphor modes, copy the phosphor framebuffer
     case Filter::Phosphor:
     {
-      if(saveFlag) // always true
-      {
-        saveFlag = false;
       uInt32 bufofs = 0, screenofsY = 0;
       for(uInt32 y = height; y; --y)
       {
@@ -458,34 +458,14 @@ void TIASurface::reRender()
         }
         screenofsY += outPitch;
       }
-      }
-      else // never executed!
-      {
-        for(uInt32 y = height; y; --y)
-        {
-          memcpy(outPtr, myRGBFramebuffer + pos, width);
-          outPtr += outPitch;
-          pos += width;
-        }
-      }
       break;
     }
 
     case Filter::BlarggPhosphor:
-      if(saveFlag) // always true
-      {
-        saveFlag = false;
       uInt32 bufofs = 0;
       for(uInt32 y = height; y; --y)
-        {
         for(uInt32 x = outPitch; x; --x)
-          {
           outPtr[pos++] = averageBuffers(bufofs++);
-          }
-        }
-      }
-      else // never executed!
-        memcpy(outPtr, myRGBFramebuffer, height * outPitch * sizeof(uInt32));
       break;
   }
 

src/emucore/TIASurface.hxx

@@ -151,14 +151,16 @@ class TIASurface
     void render();
 
     /**
-      This method renders the current frame again.
+      This method prepares the current frame for taking a snapshot.
+      In particular, in phosphor modes the blending is adjusted slightly to
+      generate better images.
     */
-    void reRender();
+    void renderForSnapshot();
 
     /**
       Save a snapshot after rendering.
     */
-    void saveSnapShot() { saveFlag = true; }
+    void saveSnapShot() { mySaveSnapFlag = true; }
 
   private:
     /**
@@ -215,7 +217,7 @@ class TIASurface
     const uInt32* myPalette;
 
     // Flag for saving a snapshot
-    bool saveFlag;
+    bool mySaveSnapFlag;
 
   private:
     // Following constructors and assignment operators not supported