Merge branch 'feature/fix-surfaces'

src/common/FBSurfaceSDL2.cxx

@@ -200,17 +200,10 @@ void FBSurfaceSDL2::invalidateRect(uInt32 x, uInt32 y, uInt32 w, uInt32 h)
   SDL_FillRect(mySurface, &tmp, 0);
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FBSurfaceSDL2::free()
-{
-  myBlitter.reset();
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FBSurfaceSDL2::reload()
 {
-  free();
-  reinitializeBlitter();
+  reinitializeBlitter(true);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -221,8 +214,6 @@ void FBSurfaceSDL2::resize(uInt32 width, uInt32 height)
   if(mySurface)
     SDL_FreeSurface(mySurface);
 
-  free();
-
   // NOTE: Currently, a resize changes a 'static' surface to 'streaming'
   //       No code currently does this, but we should at least check for it
   if(myIsStatic)
@@ -260,12 +251,15 @@ void FBSurfaceSDL2::createSurface(uInt32 width, uInt32 height,
   if(myIsStatic)
     SDL_memcpy(mySurface->pixels, data, mySurface->w * mySurface->h * 4);
 
-  reinitializeBlitter();
+  reload();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FBSurfaceSDL2::reinitializeBlitter()
+void FBSurfaceSDL2::reinitializeBlitter(bool force)
 {
+  if (force)
+    myBlitter.reset();
+
   if (!myBlitter && myBackend.isInitialized())
     myBlitter = BlitterFactory::createBlitter(
         myBackend, scalingAlgorithm(myInterpolationMode));

src/common/FBSurfaceSDL2.hxx

@@ -60,7 +60,6 @@ class FBSurfaceSDL2 : public FBSurface
     void invalidate() override;
     void invalidateRect(uInt32 x, uInt32 y, uInt32 w, uInt32 h) override;
 
-    void free() override;
     void reload() override;
     void resize(uInt32 width, uInt32 height) override;
 
@@ -109,7 +108,7 @@ class FBSurfaceSDL2 : public FBSurface
 
     void createSurface(uInt32 width, uInt32 height, const uInt32* data);
 
-    void reinitializeBlitter();
+    void reinitializeBlitter(bool force = false);
 
     // Following constructors and assignment operators not supported
     FBSurfaceSDL2() = delete;

src/emucore/FBSurface.hxx

@@ -350,16 +350,8 @@ class FBSurface
     */
     virtual void invalidateRect(uInt32 x, uInt32 y, uInt32 w, uInt32 h) = 0;
 
-
-    /**
-      This method should be called to free any resources being used by
-      the surface.
-    */
-    virtual void free() = 0;
-
     /**
       This method should be called to reload the surface data/state.
-      It will normally be called after free().
     */
     virtual void reload() = 0;
 

src/emucore/FrameBuffer.cxx

@@ -67,11 +67,6 @@ FrameBuffer::FrameBuffer(OSystem& osystem)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 FrameBuffer::~FrameBuffer()
 {
-  // Make sure to free surfaces/textures before destroying the backend itself
-  // Most platforms are fine with doing this in either order, but it seems
-  // that OpenBSD in particular crashes when attempting to destroy textures
-  // *after* the renderer is already destroyed
-  freeSurfaces();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -256,21 +251,7 @@ FBInitStatus FrameBuffer::createDisplay(const string& title, BufferType type,
     return FBInitStatus::FailTooLarge;
 #endif
 
-  // Initialize video mode handler, so it can know what video modes are
-  // appropriate for the requested image size
-  myVidModeHandler.setImageSize(size);
-
-  // Always save, maybe only the mode of the window has changed
-  saveCurrentWindowPosition();
-  myBufferType = type;
-
-  // Initialize video subsystem
-  string pre_about = myBackend->about();
-  FBInitStatus status = applyVideoMode();
-  if(status != FBInitStatus::Success)
-    return status;
-
-#ifdef GUI_SUPPORT
+#ifdef GUI_SUPPORT  // TODO: put message stuff in its own class
   // Erase any messages from a previous run
   myMsg.enabled = false;
 
@@ -297,6 +278,20 @@ FBInitStatus FrameBuffer::createDisplay(const string& title, BufferType type,
   }
 #endif
 
+  // Initialize video mode handler, so it can know what video modes are
+  // appropriate for the requested image size
+  myVidModeHandler.setImageSize(size);
+
+  // Always save, maybe only the mode of the window has changed
+  saveCurrentWindowPosition();
+  myBufferType = type;
+
+  // Initialize video subsystem
+  string pre_about = myBackend->about();
+  FBInitStatus status = applyVideoMode();
+  if(status != FBInitStatus::Success)
+    return status;
+
   // Print initial usage message, but only print it later if the status has changed
   if(myInitializedCount == 1)
   {
@@ -553,6 +548,7 @@ void FrameBuffer::updateInEmulationMode(float framesPerSecond)
 }
 
 #ifdef GUI_SUPPORT
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameBuffer::createMessage(const string& message, MessagePosition position, bool force)
 {
   // Only show messages if they've been enabled
@@ -866,42 +862,63 @@ void FrameBuffer::setPauseDelay()
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-shared_ptr<FBSurface> FrameBuffer::allocateSurface(
-    int w, int h, ScalingInterpolation inter, const uInt32* data
-)
+unique_ptr<FBSurface> FrameBuffer::allocateSurface(
+    int w, int h, ScalingInterpolation inter, const uInt32* data)
 {
-  // Add new surface to the list
-  mySurfaceList.push_back(myBackend->createSurface(w, h, inter, data));
-
-  // And return a pointer to it (pointer should be treated read-only)
-  return mySurfaceList.at(mySurfaceList.size() - 1);
+  return myBackend->createSurface(w, h, inter, data);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameBuffer::freeSurfaces()
+void FrameBuffer::resetSurfaces()
 {
-  for(auto& s: mySurfaceList)
-    s->free();
-}
+  switch(myOSystem.eventHandler().state())
+  {
+    case EventHandlerState::NONE:
+    case EventHandlerState::EMULATION:
+    case EventHandlerState::PAUSE:
+    case EventHandlerState::PLAYBACK:
+    #ifdef GUI_SUPPORT
+      myMsg.surface->reload();
+      myStatsMsg.surface->reload();
+    #endif
+      myTIASurface->resetSurfaces();
+      break;
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameBuffer::reloadSurfaces()
-{
-  for(auto& s: mySurfaceList)
-    s->reload();
-}
+  #ifdef GUI_SUPPORT
+    case EventHandlerState::OPTIONSMENU:
+      myOSystem.menu().resetSurfaces();
+      break;
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameBuffer::resetSurfaces()
-{
-  // Free all resources for each surface, then reload them
-  // Due to possible timing and/or synchronization issues, all free()'s
-  // are done first, then all reload()'s
-  // Any derived FrameBuffer classes that call this method should be
-  // aware of these restrictions, and act accordingly
-
-  freeSurfaces();
-  reloadSurfaces();
+    case EventHandlerState::CMDMENU:
+      myOSystem.commandMenu().resetSurfaces();
+      break;
+
+    case EventHandlerState::HIGHSCORESMENU:
+      myOSystem.highscoresMenu().resetSurfaces();
+      break;
+
+    case EventHandlerState::MESSAGEMENU:
+      myOSystem.messageMenu().resetSurfaces();
+      break;
+
+    case EventHandlerState::TIMEMACHINE:
+      myOSystem.timeMachine().resetSurfaces();
+      break;
+
+    case EventHandlerState::LAUNCHER:
+      myOSystem.launcher().resetSurfaces();
+      break;
+  #endif
+
+  #ifdef DEBUGGER_SUPPORT
+    case EventHandlerState::DEBUGGER:
+      myOSystem.debugger().resetSurfaces();
+      break;
+  #endif
+
+    default:
+      break;
+  }
 
   update(UpdateMode::REDRAW); // force full update
 }

src/emucore/FrameBuffer.hxx

@@ -158,7 +158,7 @@ class FrameBuffer
 
       @return  A pointer to a valid surface object, or nullptr
     */
-    shared_ptr<FBSurface> allocateSurface(
+    unique_ptr<FBSurface> allocateSurface(
       int w,
       int h,
       ScalingInterpolation inter = ScalingInterpolation::none,
@@ -384,16 +384,6 @@ class FrameBuffer
     string getDisplayKey() const;
     void saveCurrentWindowPosition() const;
 
-    /**
-      Calls 'free()' on all surfaces that the framebuffer knows about.
-    */
-    void freeSurfaces();
-
-    /**
-      Calls 'reload()' on all surfaces that the framebuffer knows about.
-    */
-    void reloadSurfaces();
-
     /**
       Frees and reloads all surfaces that the framebuffer knows about.
     */
@@ -520,7 +510,7 @@ class FrameBuffer
       int x{0}, y{0}, w{0}, h{0};
       MessagePosition position{MessagePosition::BottomCenter};
       ColorId color{kNone};
-      shared_ptr<FBSurface> surface;
+      unique_ptr<FBSurface> surface;
       bool enabled{false};
       bool dirty{false};
       bool showGauge{false};
@@ -541,9 +531,6 @@ class FrameBuffer
     // Maximum TIA zoom level that can be used for this framebuffer
     float myTIAMaxZoom{1.F};
 
-    // Holds a reference to all the surfaces that have been created
-    vector<shared_ptr<FBSurface>> mySurfaceList;
-
     // Maximum message width [chars]
     static constexpr int MESSAGE_WIDTH = 56;
     // Maximum gauge bar width [chars]

src/emucore/TIASurface.cxx

@@ -541,3 +541,12 @@ bool TIASurface::correctAspect() const
 {
   return myOSystem.settings().getBool("tia.correct_aspect");
 }
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void TIASurface::resetSurfaces()
+{
+  myTiaSurface->reload();
+  mySLineSurface->reload();
+  myBaseTiaSurface->reload();
+  myShadeSurface->reload();
+}

src/emucore/TIASurface.hxx

@@ -183,6 +183,11 @@ class TIASurface
      */
     void updateSurfaceSettings();
 
+    /**
+      Issue a 'reload' to each surface.
+    */
+    void resetSurfaces();
+
   private:
     /**
       Average current calculated buffer's pixel with previous calculated buffer's pixel (50:50).
@@ -208,7 +213,7 @@ class TIASurface
     FrameBuffer& myFB;
     TIA* myTIA{nullptr};
 
-    shared_ptr<FBSurface> myTiaSurface, mySLineSurface, myBaseTiaSurface, myShadeSurface;
+    unique_ptr<FBSurface> myTiaSurface, mySLineSurface, myBaseTiaSurface, myShadeSurface;
 
     // NTSC object to use in TIA rendering mode
     NTSCFilter myNTSCFilter;

src/gui/ContextMenu.cxx

@@ -583,7 +583,6 @@ void ContextMenu::setArrows()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void ContextMenu::drawDialog()
 {
-
   // Normally we add widgets and let Dialog::draw() take care of this
   // logic.  But for some reason, this Dialog was written differently
   // by the ScummVM guys, so I'm not going to mess with it.

src/gui/Dialog.cxx

@@ -50,23 +50,12 @@ Dialog::Dialog(OSystem& instance, DialogContainer& parent, const GUI::Font& font
                const string& title, int x, int y, int w, int h)
   : GuiObject(instance, parent, *this, x, y, w, h),
     _font{font},
-    _title{title}
+    _title{title},
+    _renderCallback{[]() { return; }}
 {
   _flags = Widget::FLAG_ENABLED | Widget::FLAG_BORDER | Widget::FLAG_CLEARBG;
   setTitle(title);
 
-  // Create shading surface
-  uInt32 data = 0xff000000;
-
-  _shadeSurface = instance.frameBuffer().allocateSurface(
-      1, 1, ScalingInterpolation::sharp, &data);
-
-  FBSurface::Attributes& attr = _shadeSurface->attributes();
-
-  attr.blending = true;
-  attr.blendalpha = 25; // darken background dialogs by 25%
-  _shadeSurface->applyAttributes();
-
   _toolTip = make_unique<ToolTip>(*this, font);
 }
 
@@ -173,6 +162,12 @@ void Dialog::setDirtyChain()
   _dirtyChain = true;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Dialog::resetSurfaces()
+{
+  _surface->reload();
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void Dialog::tick()
 {
@@ -251,11 +246,7 @@ void Dialog::render()
   // Update dialog surface; also render any extra surfaces
   // Extra surfaces must be rendered afterwards, so they are drawn on top
   if(_surface->render())
-  {
-    mySurfaceStack.applyAll([](shared_ptr<FBSurface>& surface) {
-      surface->render();
-    });
-  }
+    _renderCallback();
 
   // A dialog is still on top if a non-shading dialog (e.g. ContextMenu)
   // is opended above it.
@@ -265,6 +256,20 @@ void Dialog::render()
 
   if(!onTop)
   {
+    if(_shadeSurface == nullptr)
+    {
+      // Create shading surface
+      uInt32 data = 0xff000000;
+
+      _shadeSurface = instance().frameBuffer().allocateSurface(
+        1, 1, ScalingInterpolation::sharp, &data);
+
+      FBSurface::Attributes& attr = _shadeSurface->attributes();
+
+      attr.blending = true;
+      attr.blendalpha = 25; // darken background dialogs by 25%
+      _shadeSurface->applyAttributes();
+    }
     _shadeSurface->setDstRect(_surface->dstRect());
     _shadeSurface->render();
   }
@@ -418,9 +423,9 @@ void Dialog::buildCurrentFocusList(int tabID)
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void Dialog::addSurface(const shared_ptr<FBSurface>& surface)
+void Dialog::addRenderCallback(const std::function<void()>& callback)
 {
-  mySurfaceStack.push(surface);
+  _renderCallback = callback;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/gui/Dialog.hxx

@@ -28,7 +28,6 @@ class TabWidget;
 class CommandSender;
 class ToolTip;
 
-#include "Stack.hxx"
 #include "Widget.hxx"
 #include "GuiObject.hxx"
 #include "StellaKeys.hxx"
@@ -45,6 +44,8 @@ class Dialog : public GuiObject
   friend class DialogContainer;
 
   public:
+    using RenderCallback = std::function<void()>;
+
     Dialog(OSystem& instance, DialogContainer& parent,
            int x = 0, int y = 0, int w = 0, int h = 0);
     Dialog(OSystem& instance, DialogContainer& parent, const GUI::Font& font,
@@ -62,6 +63,8 @@ class Dialog : public GuiObject
     virtual void saveConfig()  { }
     virtual void setDefaults() { }
 
+    virtual void resetSurfaces();
+
     void setDirty() override;
     void setDirtyChain() override;
     void redraw(bool force = false);
@@ -85,12 +88,11 @@ class Dialog : public GuiObject
     FBSurface& surface() const { return *_surface; }
 
     /**
-      Adds a surface to this dialog, which is rendered on top of the
-      base surface whenever the base surface is re-rendered.  Since
-      the surface render() call will always occur in such a case, the
-      surface should call setVisible() to enable/disable its output.
+      This method is called each time the main Dialog::render is called.
+      It is called *after* the dialog has been rendered, so it can be
+      used to render another surface on top of it, among other things.
     */
-    void addSurface(const shared_ptr<FBSurface>& surface);
+    void addRenderCallback(const RenderCallback& callback);
 
     void setTitle(const string& title);
     bool hasTitle() { return !_title.empty(); }
@@ -220,8 +222,6 @@ class Dialog : public GuiObject
     int     _layer{0};
     unique_ptr<ToolTip> _toolTip;
 
-    Common::FixedStack<shared_ptr<FBSurface>> mySurfaceStack;
-
   private:
     struct Focus {
       Widget* widget{nullptr};
@@ -248,13 +248,15 @@ class Dialog : public GuiObject
     TabFocusList _myTabList;  // focus for each tab (if any)
 
     WidgetArray _buttonGroup;
-    shared_ptr<FBSurface> _surface;
-    shared_ptr<FBSurface> _shadeSurface;
+    unique_ptr<FBSurface> _surface;
+    unique_ptr<FBSurface> _shadeSurface;
 
     int _tabID{0};
     uInt32 _max_w{0}; // maximum wanted width
     uInt32 _max_h{0}; // maximum wanted height
 
+    RenderCallback _renderCallback;
+
   private:
     // Following constructors and assignment operators not supported
     Dialog() = delete;

src/gui/DialogContainer.cxx

@@ -199,6 +199,14 @@ void DialogContainer::reStack()
   reset();
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void DialogContainer::resetSurfaces()
+{
+  myDialogStack.applyAll([&](Dialog*& d) {
+    d->resetSurfaces();
+  });
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void DialogContainer::handleTextEvent(char text)
 {

src/gui/DialogContainer.hxx

@@ -150,6 +150,13 @@ class DialogContainer
     */
     void reStack();
 
+    /**
+      Issue a 'reload' event to each dialog surface in the stack.  This
+      is typically used when interpolation or attributes for a dialog
+      have changed.
+    */
+    void resetSurfaces();
+
     /**
       Inform the container that it should resize according to the current
       screen dimensions.  We make this virtual, since the container may or

src/gui/EditableWidget.cxx

@@ -40,9 +40,6 @@ EditableWidget::EditableWidget(GuiObject* boss, const GUI::Font& font,
   _bgcolorlo = kDlgColor;
   _textcolor = kTextColor;
   _textcolorhi = kTextColor;
-
-  // add mouse context menu
-  myMouseMenu = make_unique<ContextMenu>(this, font, EmptyVarList);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -143,7 +140,7 @@ int EditableWidget::toCaretPos(int x) const
 void EditableWidget::handleMouseDown(int x, int y, MouseButton b, int clickCount)
 {
   // Grab right mouse button for context menu, send left to base class
-  if(b == MouseButton::RIGHT && isEnabled() && !myMouseMenu->isVisible())
+  if(b == MouseButton::RIGHT && isEnabled() && !mouseMenu().isVisible())
   {
     VariantList items;
   #ifndef BSPF_MACOS
@@ -159,10 +156,10 @@ void EditableWidget::handleMouseDown(int x, int y, MouseButton b, int clickCount
     if(isEditable())
       VarList::push_back(items, " Paste    Cmd+V ", "paste");
   #endif
-    myMouseMenu->addItems(items);
+    mouseMenu().addItems(items);
 
     // Add menu at current x,y mouse location
-    myMouseMenu->show(x + getAbsX(), y + getAbsY(), dialog().surface().dstRect());
+    mouseMenu().show(x + getAbsX(), y + getAbsY(), dialog().surface().dstRect());
     return;
   }
   else if(b == MouseButton::LEFT && isEnabled())
@@ -179,6 +176,16 @@ void EditableWidget::handleMouseDown(int x, int y, MouseButton b, int clickCount
   Widget::handleMouseDown(x, y, b, clickCount);
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+ContextMenu& EditableWidget::mouseMenu()
+{
+  // add mouse context menu
+  if(myMouseMenu == nullptr)
+    myMouseMenu = make_unique<ContextMenu>(this, _font, EmptyVarList);
+
+  return *myMouseMenu;
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void EditableWidget::handleMouseUp(int x, int y, MouseButton b, int clickCount)
 {
@@ -205,7 +212,7 @@ void EditableWidget::handleCommand(CommandSender* sender, int cmd, int data, int
 {
   if(cmd == ContextMenu::kItemSelectedCmd)
   {
-    const string& rmb = myMouseMenu->getSelectedTag().toString();
+    const string& rmb = mouseMenu().getSelectedTag().toString();
 
     if(rmb == "cut")
     {

src/gui/EditableWidget.hxx

@@ -117,6 +117,8 @@ class EditableWidget : public Widget, public CommandSender
     // internal buffer
     bool tryInsertChar(char c, int pos);
 
+    ContextMenu& mouseMenu();
+
   private:
     unique_ptr<ContextMenu> myMouseMenu;
     bool    _isDragging{false};

src/gui/EventMappingWidget.cxx

@@ -128,7 +128,7 @@ EventMappingWidget::EventMappingWidget(GuiObject* boss, const GUI::Font& font,
     addFocusWidget(myComboButton);
 
     VariantList combolist = instance().eventHandler().getComboList(mode);
-    myComboDialog = new ComboDialog(boss, font, combolist);
+    myComboDialog = make_unique<ComboDialog>(boss, font, combolist);
   }
 
   // Show message for currently selected event

src/gui/EventMappingWidget.hxx

@@ -90,7 +90,7 @@ class EventMappingWidget : public Widget, public CommandSender
     StringListWidget* myActionsList{nullptr};
     EditTextWidget*   myKeyMapping{nullptr};
 
-    ComboDialog* myComboDialog{nullptr};
+    unique_ptr<ComboDialog> myComboDialog;
 
     // Since this widget can be used for different collections of events,
     // we need to specify exactly which group of events we are remapping

src/gui/LauncherDialog.cxx

@@ -295,9 +295,6 @@ LauncherDialog::LauncherDialog(OSystem& osystem, DialogContainer& parent,
 
   addToFocusList(wid);
 
-  // Create (empty) context menu for ROM list options
-  myMenu = make_unique<ContextMenu>(this, _font, EmptyVarList);
-
   // since we cannot know how many files there are, use are really high value here
   myList->progress().setRange(0, 50000, 5);
   myList->progress().setMessage("        Filtering files" + ELLIPSIS + "        ");
@@ -353,6 +350,15 @@ void LauncherDialog::reload()
   myPendingReload = false;
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void LauncherDialog::resetSurfaces()
+{
+  if(myRomInfoWidget)
+    myRomInfoWidget->resetSurfaces();
+
+  Dialog::resetSurfaces();
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void LauncherDialog::tick()
 {
@@ -608,7 +614,7 @@ void LauncherDialog::loadRomInfo()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void LauncherDialog::handleContextMenu()
 {
-  const string& cmd = myMenu->getSelectedTag().toString();
+  const string& cmd = menu().getSelectedTag().toString();
 
   if(cmd == "override")