Refactored phosphor calculations into a new PhosphorHandler class.

This class is accessible from both TIASurface and AtariNTSC, so there's no more code duplication.
Testing is required to see if this results in any slowdowns.  It shouldn't, since the relevant code is inlined ...

src/common/PhosphorHandler.cxx

@@ -0,0 +1,56 @@
+//============================================================================
+//
+//   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-2019 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 "PhosphorHandler.hxx"
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+PhosphorHandler::PhosphorHandler()
+  : myUsePhosphor(false),
+    myPhosphorPercent(0.60F)
+{
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+bool PhosphorHandler::initialize(bool enable, int blend)
+{
+  if(myUsePhosphor == enable && myPhosphorPercent == blend / 100.F)
+    return false;
+
+  myUsePhosphor = enable;
+  if(blend >= 0 && blend <= 100)
+    myPhosphorPercent = blend / 100.F;
+
+  // Used to calculate an averaged color for the 'phosphor' effect
+  auto getPhosphor = [&] (const uInt8 c1, uInt8 c2) -> uInt8 {
+    // Use maximum of current and decayed previous values
+    c2 = static_cast<uInt8>(c2 * myPhosphorPercent);
+    if(c1 > c2)  return c1; // raise (assumed immediate)
+    else         return c2; // decay
+  };
+
+  // Precalculate the average colors for the 'phosphor' effect
+  if(myUsePhosphor)
+  {
+    for(int c = 255; c >= 0; --c)
+      for(int p = 255; p >= 0; --p)
+        ourPhosphorLUT[c][p] = getPhosphor(uInt8(c), uInt8(p));
+  }
+  return true;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+PhosphorHandler::PhosphorLUT PhosphorHandler::ourPhosphorLUT;

src/common/PhosphorHandler.hxx

@@ -0,0 +1,73 @@
+//============================================================================
+//
+//   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-2019 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 PHOSPHOR_HANDLER_HXX
+#define PHOSPHOR_HANDLER_HXX
+
+#include "FrameBufferConstants.hxx"
+#include "bspf.hxx"
+
+class PhosphorHandler
+{
+  public:
+    PhosphorHandler();
+
+    bool initialize(bool enable, int blend);
+
+    bool phosphorEnabled() const { return myUsePhosphor; }
+
+    /**
+      Used to calculate an averaged color pixel for the 'phosphor' effect.
+
+      @param c  RGB Color 1 (current frame)
+      @param p  RGB Color 2 (previous frame)
+
+      @return  Averaged value of the two RGB colors
+    */
+    static inline uInt32 getPixel(const uInt32 c, const uInt32 p)
+    {
+      // Mix current calculated frame with previous displayed frame
+      const uInt8 rc = static_cast<uInt8>(c >> 16),
+                  gc = static_cast<uInt8>(c >> 8),
+                  bc = static_cast<uInt8>(c),
+                  rp = static_cast<uInt8>(p >> 16),
+                  gp = static_cast<uInt8>(p >> 8),
+                  bp = static_cast<uInt8>(p);
+
+      return (ourPhosphorLUT[rc][rp] << 16) | (ourPhosphorLUT[gc][gp] << 8) |
+              ourPhosphorLUT[bc][bp];
+    }
+
+  private:
+    // Use phosphor effect
+    bool myUsePhosphor;
+
+    // Amount to blend when using phosphor effect
+    float myPhosphorPercent;
+
+    // Precalculated averaged phosphor colors
+    using PhosphorLUT = std::array<std::array<uInt8, kColor>, kColor>;
+    static PhosphorLUT ourPhosphorLUT;
+
+  private:
+    PhosphorHandler(const PhosphorHandler&) = delete;
+    PhosphorHandler(PhosphorHandler&&) = delete;
+    PhosphorHandler& operator=(const PhosphorHandler&) = delete;
+    PhosphorHandler& operator=(const PhosphorHandler&&) = delete;
+};
+
+#endif // PHOSPHOR_HANDLER_HXX

src/common/module.mk

@@ -11,6 +11,7 @@ MODULE_OBJS := \
 	src/common/Logger.o \
 	src/common/main.o \
 	src/common/MouseControl.o \
+	src/common/PhosphorHandler.o \
 	src/common/PhysicalJoystick.o \
 	src/common/PJoystickHandler.o \
 	src/common/PKeyboardHandler.o \

src/common/tv_filters/AtariNTSC.cxx

@@ -17,6 +17,7 @@
 
 #include <thread>
 #include "AtariNTSC.hxx"
+#include "PhosphorHandler.hxx"
 
 // blitter related
 #ifndef restrict
@@ -50,12 +51,6 @@ void AtariNTSC::setPalette(const PaletteArray& palette)
   generateKernels();
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void AtariNTSC::setPhosphorTable(const PhosphorLUT& table)
-{
-  myPhosphorLUT = table;
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void AtariNTSC::generateKernels()
 {
@@ -281,38 +276,38 @@ void AtariNTSC::renderWithPhosphorThread(const uInt8* atari_in, const uInt32 in_
     for (uInt32 x = AtariNTSC::outWidth(in_width) / 8; x; --x)
     {
       // Store back into displayed frame buffer (for next frame)
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
-      rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+      rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
       ++bufofs;
     }
     // finish final 565 % 8 = 5 pixels
-    /*rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    /*rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;*/
 #if 0
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
-    rgb_in[bufofs] = getRGBPhosphor(out[bufofs], rgb_in[bufofs]);
+    rgb_in[bufofs] = PhosphorHandler::getPixel(out[bufofs], rgb_in[bufofs]);
     ++bufofs;
 #endif
 
@@ -321,21 +316,6 @@ void AtariNTSC::renderWithPhosphorThread(const uInt8* atari_in, const uInt32 in_
   }
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-inline uInt32 AtariNTSC::getRGBPhosphor(const uInt32 c, const uInt32 p) const
-{
-  // Mix current calculated frame with previous displayed frame
-  const uInt8 rc = static_cast<uInt8>(c >> 16),
-              gc = static_cast<uInt8>(c >> 8),
-              bc = static_cast<uInt8>(c),
-              rp = static_cast<uInt8>(p >> 16),
-              gp = static_cast<uInt8>(p >> 8),
-              bp = static_cast<uInt8>(p);
-
-  return (myPhosphorLUT[rc][rp] << 16) | (myPhosphorLUT[gc][gp] << 8) |
-          myPhosphorLUT[bc][bp];
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void AtariNTSC::init(init_t& impl, const Setup& setup)
 {

src/common/tv_filters/AtariNTSC.hxx

@@ -83,8 +83,6 @@ class AtariNTSC
 
     // Set palette for normal Blarrg mode
     void setPalette(const PaletteArray& palette);
-    // Set phosphor table, for use in calculating phosphor palette
-    void setPhosphorTable(const PhosphorLUT& table);
 
     // Set up threading
     void enableThreading(bool enable);
@@ -96,13 +94,6 @@ class AtariNTSC
     void render(const uInt8* atari_in, const uInt32 in_width, const uInt32 in_height,
                 void* rgb_out, const uInt32 out_pitch, uInt32* rgb_in = nullptr);
 
-    // Number of input pixels that will fit within given output width.
-    // Might be rounded down slightly; use outWidth() on result to find
-    // rounded value.
-    /*static constexpr uInt32 inWidth( uInt32 out_width ) {
-      return (((out_width - 8) / PIXEL_out_chunk - 1) * PIXEL_in_chunk + 1);
-    }*/
-
     // Number of output pixels written by blitter for given input width.
     // Width might be rounded down slightly; use inWidth() on result to
     // find rounded value. Guaranteed not to round 160 down at all.
@@ -120,16 +111,6 @@ class AtariNTSC
     void renderWithPhosphorThread(const uInt8* atari_in, const uInt32 in_width,
       const uInt32 in_height, const uInt32 numThreads, const uInt32 threadNum, uInt32* rgb_in, void* rgb_out, const uInt32 out_pitch);
 
-    /**
-      Used to calculate an averaged color for the 'phosphor' effect.
-
-      @param c  RGB Color 1 (current frame)
-      @param cp RGB Color 2 (previous frame)
-
-      @return  Averaged value of the two RGB colors
-    */
-    uInt32 getRGBPhosphor(const uInt32 c, const uInt32 cp) const;
-
   private:
     static constexpr Int32
       PIXEL_in_chunk  = 2,   // number of input pixels read per chunk
@@ -172,7 +153,6 @@ class AtariNTSC
 
     std::array<uInt8, palette_size*3> myRGBPalette;
     std::array<std::array<uInt32, entry_size>, palette_size> myColorTable;
-    PhosphorLUT myPhosphorLUT;
 
     // Rendering threads
     unique_ptr<std::thread[]> myThreads;
@@ -277,6 +257,13 @@ class AtariNTSC
     }
 
   #if 0  // DEAD CODE
+    // Number of input pixels that will fit within given output width.
+    // Might be rounded down slightly; use outWidth() on result to find
+    // rounded value.
+    static constexpr uInt32 inWidth( uInt32 out_width ) {
+      return (((out_width - 8) / PIXEL_out_chunk - 1) * PIXEL_in_chunk + 1);
+    }
+
     #define ROTATE_IQ( i, q, sin_b, cos_b ) {\
       float t;\
       t = i * cos_b - q * sin_b;\

src/common/tv_filters/NTSCFilter.hxx

@@ -63,9 +63,6 @@ class NTSCFilter
     void setPalette(const PaletteArray& palette) {
       myNTSC.setPalette(palette);
     }
-    void setPhosphorTable(const PhosphorLUT& table) {
-      myNTSC.setPhosphorTable(table);
-    }
 
     // The following are meant to be used strictly for toggling from the GUI
     string setPreset(Preset preset);

src/emucore/FrameBufferConstants.hxx

@@ -107,9 +107,6 @@ using PaletteArray = std::array<uInt32, kColor>;
 using UIPaletteArray = std::array<uInt32, kNumColors-kColor>;
 using FullPaletteArray = std::array<uInt32, kNumColors>;
 
-// Lookup table for phosphor mode, for generating corresponding palette
-using PhosphorLUT = std::array<std::array<uInt8, kColor>, kColor>;
-
 // Text alignment modes for drawString()
 enum class TextAlign {
   Left,

src/emucore/TIASurface.cxx

@@ -40,8 +40,6 @@ TIASurface::TIASurface(OSystem& system)
     myFB(system.frameBuffer()),
     myTIA(nullptr),
     myFilter(Filter::Normal),
-    myUsePhosphor(false),
-    myPhosphorPercent(0.60f),
     myScanlinesEnabled(false),
     mySaveSnapFlag(false)
 {
@@ -213,42 +211,13 @@ uInt32 TIASurface::enableScanlines(int relative, int absolute)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void TIASurface::enablePhosphor(bool enable, int blend)
 {
-  if(myUsePhosphor == enable && myPhosphorPercent == blend / 100.0f)
+  if(myPhosphorHandler.initialize(enable, blend))
-    return;
-
-  myUsePhosphor = enable;
-  if(blend >= 0)
-    myPhosphorPercent = blend / 100.0f;
-  myFilter = Filter(enable ? uInt8(myFilter) | 0x01 : uInt8(myFilter) & 0x10);
-
-  myRGBFramebuffer.fill(0);
-
-  // Precalculate the average colors for the 'phosphor' effect
-  if(myUsePhosphor)
   {
-    for(int c = 255; c >= 0; --c)
+    myFilter = Filter(enable ? uInt8(myFilter) | 0x01 : uInt8(myFilter) & 0x10);
-      for(int p = 255; p >= 0; --p)
+    myRGBFramebuffer.fill(0);
-        myPhosphorLUT[c][p] = getPhosphor(uInt8(c), uInt8(p));
-
-    myNTSCFilter.setPhosphorTable(myPhosphorLUT);
   }
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-inline uInt32 TIASurface::getRGBPhosphor(const uInt32 c, const uInt32 p) const
-{
-  // Mix current calculated frame with previous displayed frame
-  const uInt8 rc = static_cast<uInt8>(c >> 16),
-              gc = static_cast<uInt8>(c >> 8),
-              bc = static_cast<uInt8>(c),
-              rp = static_cast<uInt8>(p >> 16),
-              gp = static_cast<uInt8>(p >> 8),
-              bp = static_cast<uInt8>(p);
-
-  return (myPhosphorLUT[rc][rp] << 16) | (myPhosphorLUT[gc][gp] << 8) |
-          myPhosphorLUT[bc][bp];
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void TIASurface::enableNTSC(bool enable)
 {
@@ -364,9 +333,9 @@ void TIASurface::render()
         for(uInt32 x = width / 2; x ; --x)
         {
           // Store back into displayed frame buffer (for next frame)
-          rgbIn[bufofs] = out[pos++] = getRGBPhosphor(myPalette[tiaIn[bufofs]], rgbIn[bufofs]);
+          rgbIn[bufofs] = out[pos++] = PhosphorHandler::getPixel(myPalette[tiaIn[bufofs]], rgbIn[bufofs]);
           ++bufofs;
-          rgbIn[bufofs] = out[pos++] = getRGBPhosphor(myPalette[tiaIn[bufofs]], rgbIn[bufofs]);
+          rgbIn[bufofs] = out[pos++] = PhosphorHandler::getPixel(myPalette[tiaIn[bufofs]], rgbIn[bufofs]);
           ++bufofs;
         }
         screenofsY += outPitch;
@@ -457,7 +426,7 @@ void TIASurface::renderForSnapshot()
       break;
   }
 
-  if(myUsePhosphor)
+  if(myPhosphorHandler.phosphorEnabled())
   {
     // Draw TIA image
     myTiaSurface->render();
@@ -472,5 +441,7 @@ void TIASurface::renderForSnapshot()
 void TIASurface::updateSurfaceSettings()
 {
   myTiaSurface->setScalingInterpolation(interpolationModeFromSettings(myOSystem.settings()));
-  mySLineSurface->setScalingInterpolation(interpolationModeFromSettings(myOSystem.settings()));
+  mySLineSurface->setScalingInterpolation(
+      interpolationModeFromSettings(myOSystem.settings())
+  );
 }

src/emucore/TIASurface.hxx

@@ -28,6 +28,7 @@ class FBSurface;
 #include "Rect.hxx"
 #include "FrameBuffer.hxx"
 #include "NTSCFilter.hxx"
+#include "PhosphorHandler.hxx"
 #include "bspf.hxx"
 #include "TIAConstants.hxx"
 
@@ -105,32 +106,7 @@ class TIASurface
       Enable/disable/query phosphor effect.
     */
     void enablePhosphor(bool enable, int blend = -1);
-    bool phosphorEnabled() const { return myUsePhosphor; }
+    bool phosphorEnabled() const { return myPhosphorHandler.phosphorEnabled(); }
-
-    /**
-      Used to calculate an averaged color for the 'phosphor' effect.
-
-      @param c1  Color 1
-      @param c2  Color 2
-
-      @return  Averaged value of the two colors
-    */
-    inline uInt8 getPhosphor(const uInt8 c1, uInt8 c2) const {
-      // Use maximum of current and decayed previous values
-      c2 = uInt8(c2 * myPhosphorPercent);
-      if(c1 > c2)  return c1; // raise (assumed immediate)
-      else         return c2; // decay
-    }
-
-    /**
-      Used to calculate an averaged color for the 'phosphor' effect.
-
-      @param c  RGB Color 1 (current frame)
-      @param cp RGB Color 2 (previous frame)
-
-      @return  Averaged value of the two RGB colors
-    */
-    uInt32 getRGBPhosphor(const uInt32 c, const uInt32 cp) const;
 
     /**
       Enable/disable/query NTSC filtering effects.
@@ -190,19 +166,12 @@ class TIASurface
     /////////////////////////////////////////////////////////////
     // Phosphor mode items (aka reduced flicker on 30Hz screens)
     // RGB frame buffer
+    PhosphorHandler myPhosphorHandler;
+
     std::array<uInt32, AtariNTSC::outWidth(TIAConstants::frameBufferWidth) *
         TIAConstants::frameBufferHeight> myRGBFramebuffer;
     std::array<uInt32, AtariNTSC::outWidth(TIAConstants::frameBufferWidth) *
         TIAConstants::frameBufferHeight> myPrevRGBFramebuffer;
-
-    // Use phosphor effect
-    bool myUsePhosphor;
-
-    // Amount to blend when using phosphor effect
-    float myPhosphorPercent;
-
-    // Precalculated averaged phosphor colors
-    PhosphorLUT myPhosphorLUT;
     /////////////////////////////////////////////////////////////
 
     // Use scanlines in TIA rendering mode

src/libretro/Makefile.common

@@ -23,6 +23,7 @@ SOURCES_CXX := \
 	$(CORE_DIR)/common/KeyMap.cxx \
 	$(CORE_DIR)/common/Logger.cxx \
 	$(CORE_DIR)/common/MouseControl.cxx \
+	$(CORE_DIR)/common/PhosphorHandler.cxx \
 	$(CORE_DIR)/common/PhysicalJoystick.cxx \
 	$(CORE_DIR)/common/PJoystickHandler.cxx \
 	$(CORE_DIR)/common/PKeyboardHandler.cxx \