From ae527a7f5e10ea358bb02fc9a38cc9c91e1223e6 Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Fri, 28 May 2021 22:12:12 -0230 Subject: [PATCH] Revert to old way of handling framebuffer surfaces. Still TODO is fix crash when FileListWidget is used (BrowserDialog). --- src/common/FBBackendSDL2.cxx | 2 ++ src/common/FBSurfaceSDL2.cxx | 5 +++ src/emucore/FrameBuffer.cxx | 66 +++++++++--------------------------- src/emucore/FrameBuffer.hxx | 17 ++++++++-- src/emucore/OSystem.cxx | 1 + src/emucore/OSystem.hxx | 1 + src/emucore/TIASurface.cxx | 9 ----- src/emucore/TIASurface.hxx | 10 ++---- src/gui/Dialog.cxx | 14 ++++---- src/gui/Dialog.hxx | 6 ++-- src/gui/DialogContainer.cxx | 9 ----- src/gui/DialogContainer.hxx | 7 ---- src/gui/FileListWidget.cxx | 5 ++- src/gui/FileListWidget.hxx | 5 ++- src/gui/LauncherDialog.cxx | 9 ----- src/gui/LauncherDialog.hxx | 1 - src/gui/RomInfoWidget.cxx | 7 ---- src/gui/RomInfoWidget.hxx | 4 +-- src/gui/ToolTip.cxx | 12 +++++-- src/gui/ToolTip.hxx | 6 ++-- 20 files changed, 69 insertions(+), 127 deletions(-) diff --git a/src/common/FBBackendSDL2.cxx b/src/common/FBBackendSDL2.cxx index b0b710ca4..c1204065e 100644 --- a/src/common/FBBackendSDL2.cxx +++ b/src/common/FBBackendSDL2.cxx @@ -71,6 +71,8 @@ FBBackendSDL2::~FBBackendSDL2() myWindow = nullptr; } SDL_QuitSubSystem(SDL_INIT_VIDEO | SDL_INIT_TIMER); + +cerr << "~FBBackendSDL2()" << endl; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/common/FBSurfaceSDL2.cxx b/src/common/FBSurfaceSDL2.cxx index a2b8d0166..15923a099 100644 --- a/src/common/FBSurfaceSDL2.cxx +++ b/src/common/FBSurfaceSDL2.cxx @@ -40,6 +40,8 @@ namespace { } } +static int REF_COUNT = 0; + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - FBSurfaceSDL2::FBSurfaceSDL2(FBBackendSDL2& backend, uInt32 width, uInt32 height, @@ -48,6 +50,7 @@ FBSurfaceSDL2::FBSurfaceSDL2(FBBackendSDL2& backend, : myBackend{backend}, myInterpolationMode{inter} { +REF_COUNT++; createSurface(width, height, staticData); } @@ -58,6 +61,8 @@ FBSurfaceSDL2::~FBSurfaceSDL2() if(mySurface) { +REF_COUNT--; +cerr << " ~FBSurfaceSDL2(): " << this << " " << REF_COUNT << endl; SDL_FreeSurface(mySurface); mySurface = nullptr; } diff --git a/src/emucore/FrameBuffer.cxx b/src/emucore/FrameBuffer.cxx index ee4c837e3..139848b59 100644 --- a/src/emucore/FrameBuffer.cxx +++ b/src/emucore/FrameBuffer.cxx @@ -67,6 +67,7 @@ FrameBuffer::FrameBuffer(OSystem& osystem) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - FrameBuffer::~FrameBuffer() { +cerr << "~FrameBuffer()\n"; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -885,63 +886,28 @@ void FrameBuffer::setPauseDelay() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -unique_ptr FrameBuffer::allocateSurface( +shared_ptr FrameBuffer::allocateSurface( int w, int h, ScalingInterpolation inter, const uInt32* data) { - return myBackend->createSurface(w, h, inter, data); + mySurfaceList.push_back(myBackend->createSurface(w, h, inter, data)); + return mySurfaceList.back(); +} + +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +void FrameBuffer::deallocateSurface(shared_ptr surface) +{ + if(surface) + { + cerr << "deallocateSurface: " << surface << endl; + mySurfaceList.remove(surface); + } } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void FrameBuffer::resetSurfaces() { - 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; - - #ifdef GUI_SUPPORT - case EventHandlerState::OPTIONSMENU: - myOSystem.menu().resetSurfaces(); - break; - - 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; - } + for(auto& surface: mySurfaceList) + surface->reload(); update(UpdateMode::REDRAW); // force full update } diff --git a/src/emucore/FrameBuffer.hxx b/src/emucore/FrameBuffer.hxx index a2b01364b..432d740d2 100644 --- a/src/emucore/FrameBuffer.hxx +++ b/src/emucore/FrameBuffer.hxx @@ -18,7 +18,7 @@ #ifndef FRAMEBUFFER_HXX #define FRAMEBUFFER_HXX -#include +#include class OSystem; class Console; @@ -158,13 +158,21 @@ class FrameBuffer @return A pointer to a valid surface object, or nullptr */ - unique_ptr allocateSurface( + shared_ptr allocateSurface( int w, int h, ScalingInterpolation inter = ScalingInterpolation::none, const uInt32* data = nullptr ); + /** + Deallocate a previously allocated surface. If no such surface exists, + this method does nothing. + + @param surface The surface to remove/deallocate + */ + void deallocateSurface(shared_ptr surface); + /** Set up the TIA/emulation palette. Due to the way the palette is stored, a call to this method implicitly calls setUIPalette() too. @@ -521,7 +529,7 @@ class FrameBuffer int x{0}, y{0}, w{0}, h{0}; MessagePosition position{MessagePosition::BottomCenter}; ColorId color{kNone}; - unique_ptr surface; + shared_ptr surface; bool enabled{false}; bool dirty{false}; bool showGauge{false}; @@ -540,6 +548,9 @@ class FrameBuffer // Minimum TIA zoom level that can be used for this framebuffer float myTIAMinZoom{2.F}; + // Holds a reference to all the surfaces that have been created + std::list> mySurfaceList; + // Maximum message width [chars] static constexpr int MESSAGE_WIDTH = 56; // Maximum gauge bar width [chars] diff --git a/src/emucore/OSystem.cxx b/src/emucore/OSystem.cxx index 8f11211e0..679e4ed44 100644 --- a/src/emucore/OSystem.cxx +++ b/src/emucore/OSystem.cxx @@ -110,6 +110,7 @@ OSystem::OSystem() // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - OSystem::~OSystem() { +cerr << "~OSystem()\n"; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/emucore/OSystem.hxx b/src/emucore/OSystem.hxx index 986dc5c8e..224ebfd50 100644 --- a/src/emucore/OSystem.hxx +++ b/src/emucore/OSystem.hxx @@ -103,6 +103,7 @@ class OSystem @return The frame buffer */ FrameBuffer& frameBuffer() const { return *myFrameBuffer; } + bool hasFrameBuffer() const { return myFrameBuffer.get() != nullptr; } /** Get the sound object of the system. diff --git a/src/emucore/TIASurface.cxx b/src/emucore/TIASurface.cxx index bd5f1c94b..4b2ba4952 100644 --- a/src/emucore/TIASurface.cxx +++ b/src/emucore/TIASurface.cxx @@ -541,12 +541,3 @@ bool TIASurface::correctAspect() const { return myOSystem.settings().getBool("tia.correct_aspect"); } - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void TIASurface::resetSurfaces() -{ - myTiaSurface->reload(); - mySLineSurface->reload(); - myBaseTiaSurface->reload(); - myShadeSurface->reload(); -} diff --git a/src/emucore/TIASurface.hxx b/src/emucore/TIASurface.hxx index 771eb989d..9cb73c5ba 100644 --- a/src/emucore/TIASurface.hxx +++ b/src/emucore/TIASurface.hxx @@ -49,7 +49,7 @@ class TIASurface Creates a new TIASurface object */ explicit TIASurface(OSystem& system); - virtual ~TIASurface(); + ~TIASurface(); /** Set the TIA object, which is needed for actually rendering the TIA image. @@ -183,11 +183,6 @@ 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). @@ -213,7 +208,8 @@ class TIASurface FrameBuffer& myFB; TIA* myTIA{nullptr}; - unique_ptr myTiaSurface, mySLineSurface, myBaseTiaSurface, myShadeSurface; + shared_ptr myTiaSurface, mySLineSurface, + myBaseTiaSurface, myShadeSurface; // NTSC object to use in TIA rendering mode NTSCFilter myNTSCFilter; diff --git a/src/gui/Dialog.cxx b/src/gui/Dialog.cxx index f073aa2f2..d8c5cf201 100644 --- a/src/gui/Dialog.cxx +++ b/src/gui/Dialog.cxx @@ -70,6 +70,14 @@ Dialog::Dialog(OSystem& instance, DialogContainer& parent, // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Dialog::~Dialog() { + if(instance().hasFrameBuffer()) + { + instance().frameBuffer().deallocateSurface(_surface); + instance().frameBuffer().deallocateSurface(_shadeSurface); + } + else + cerr << "!!! framebuffer not available\n"; + _myFocus.list.clear(); _myTabList.clear(); @@ -251,12 +259,6 @@ void Dialog::setDirtyChain() _dirtyChain = true; } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void Dialog::resetSurfaces() -{ - _surface->reload(); -} - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void Dialog::tick() { diff --git a/src/gui/Dialog.hxx b/src/gui/Dialog.hxx index 582956d40..f9e394230 100644 --- a/src/gui/Dialog.hxx +++ b/src/gui/Dialog.hxx @@ -63,8 +63,6 @@ class Dialog : public GuiObject virtual void saveConfig() { } virtual void setDefaults() { } - virtual void resetSurfaces(); - void setDirty() override; void setDirtyChain() override; void redraw(bool force = false); @@ -263,8 +261,8 @@ class Dialog : public GuiObject TabFocusList _myTabList; // focus for each tab (if any) WidgetArray _buttonGroup; - unique_ptr _surface; - unique_ptr _shadeSurface; + shared_ptr _surface; + shared_ptr _shadeSurface; int _tabID{0}; uInt32 _max_w{0}; // maximum wanted width diff --git a/src/gui/DialogContainer.cxx b/src/gui/DialogContainer.cxx index 3766c6383..97b555e8c 100644 --- a/src/gui/DialogContainer.cxx +++ b/src/gui/DialogContainer.cxx @@ -163,7 +163,6 @@ int DialogContainer::addDialog(Dialog* d) myDialogStack.top()->tooltip().hide(); d->setDirty(); - d->resetSurfaces(); myDialogStack.push(d); } return myDialogStack.size(); @@ -200,14 +199,6 @@ void DialogContainer::reStack() reset(); } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void DialogContainer::resetSurfaces() -{ - myDialogStack.applyAll([&](Dialog*& d) { - d->resetSurfaces(); - }); -} - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void DialogContainer::handleTextEvent(char text) { diff --git a/src/gui/DialogContainer.hxx b/src/gui/DialogContainer.hxx index 647d9eb67..8dbc438f8 100644 --- a/src/gui/DialogContainer.hxx +++ b/src/gui/DialogContainer.hxx @@ -150,13 +150,6 @@ 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 diff --git a/src/gui/FileListWidget.cxx b/src/gui/FileListWidget.cxx index b6975c246..0cce973b9 100644 --- a/src/gui/FileListWidget.cxx +++ b/src/gui/FileListWidget.cxx @@ -75,7 +75,7 @@ void FileListWidget::setLocation(const FilesystemNode& node, { progress().resetProgress(); progress().open(); - FilesystemNode::CancelCheck isCancelled = []() { + FilesystemNode::CancelCheck isCancelled = [this]() { return myProgressDialog->isCancelled(); }; @@ -154,6 +154,7 @@ ProgressDialog& FileListWidget::progress() return *myProgressDialog; } +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void FileListWidget::incProgress() { if(_includeSubDirs) @@ -269,5 +270,3 @@ string FileListWidget::getToolTip(const Common::Point& pos) const // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - uInt64 FileListWidget::_QUICK_SELECT_DELAY = 300; - -unique_ptr FileListWidget::myProgressDialog{nullptr}; diff --git a/src/gui/FileListWidget.hxx b/src/gui/FileListWidget.hxx index bd9643e92..474742104 100644 --- a/src/gui/FileListWidget.hxx +++ b/src/gui/FileListWidget.hxx @@ -94,9 +94,6 @@ class FileListWidget : public StringListWidget ProgressDialog& progress(); void incProgress(); - protected: - static unique_ptr myProgressDialog; - private: /** Very similar to setDirectory(), but also updates the history */ void setLocation(const FilesystemNode& node, const string& select); @@ -124,6 +121,8 @@ class FileListWidget : public StringListWidget uInt64 _quickSelectTime{0}; static uInt64 _QUICK_SELECT_DELAY; + unique_ptr myProgressDialog; + private: // Following constructors and assignment operators not supported FileListWidget() = delete; diff --git a/src/gui/LauncherDialog.cxx b/src/gui/LauncherDialog.cxx index f9064f790..4adba0787 100644 --- a/src/gui/LauncherDialog.cxx +++ b/src/gui/LauncherDialog.cxx @@ -353,15 +353,6 @@ void LauncherDialog::reload() myPendingReload = false; } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void LauncherDialog::resetSurfaces() -{ - if(myRomInfoWidget) - myRomInfoWidget->resetSurfaces(); - - Dialog::resetSurfaces(); -} - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void LauncherDialog::tick() { diff --git a/src/gui/LauncherDialog.hxx b/src/gui/LauncherDialog.hxx index 139ff2c19..21a9a3121 100644 --- a/src/gui/LauncherDialog.hxx +++ b/src/gui/LauncherDialog.hxx @@ -108,7 +108,6 @@ class LauncherDialog : public Dialog void loadConfig() override; void saveConfig() override; - void resetSurfaces() override; void updateUI(); /** diff --git a/src/gui/RomInfoWidget.cxx b/src/gui/RomInfoWidget.cxx index 93ba83a88..3b8927362 100644 --- a/src/gui/RomInfoWidget.cxx +++ b/src/gui/RomInfoWidget.cxx @@ -184,13 +184,6 @@ void RomInfoWidget::parseProperties(const FilesystemNode& node) setDirty(); } -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void RomInfoWidget::resetSurfaces() -{ - if(mySurface) - mySurface->reload(); -} - #ifdef PNG_SUPPORT // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - bool RomInfoWidget::loadPng(const string& filename) diff --git a/src/gui/RomInfoWidget.hxx b/src/gui/RomInfoWidget.hxx index 095c32e54..40989cea9 100644 --- a/src/gui/RomInfoWidget.hxx +++ b/src/gui/RomInfoWidget.hxx @@ -44,8 +44,6 @@ class RomInfoWidget : public Widget, public CommandSender void clearProperties(); void reloadProperties(const FilesystemNode& node); - void resetSurfaces(); - const string& getUrl() const { return myUrl; } protected: @@ -60,7 +58,7 @@ class RomInfoWidget : public Widget, public CommandSender private: // Surface pointer holding the PNG image - unique_ptr mySurface; + shared_ptr mySurface; // Whether the surface should be redrawn by drawWidget() bool mySurfaceIsValid{false}; diff --git a/src/gui/ToolTip.cxx b/src/gui/ToolTip.cxx index 4546a2285..d332ccbce 100644 --- a/src/gui/ToolTip.cxx +++ b/src/gui/ToolTip.cxx @@ -34,6 +34,12 @@ ToolTip::ToolTip(Dialog& dialog, const GUI::Font& font) setFont(font); } +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +ToolTip::~ToolTip() +{ + myDialog.instance().frameBuffer().deallocateSurface(mySurface); +} + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void ToolTip::setFont(const GUI::Font& font) { @@ -48,12 +54,12 @@ void ToolTip::setFont(const GUI::Font& font) myHeight = fontHeight * MAX_ROWS + myTextYOfs * 2; // unallocate - if(mySurface != nullptr) - mySurface.reset(); + myDialog.instance().frameBuffer().deallocateSurface(mySurface); + mySurface = nullptr; } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -const unique_ptr& ToolTip::surface() +const shared_ptr& ToolTip::surface() { if(mySurface == nullptr) mySurface = myDialog.instance().frameBuffer().allocateSurface(myWidth, myHeight); diff --git a/src/gui/ToolTip.hxx b/src/gui/ToolTip.hxx index 3938aad06..270f92aec 100644 --- a/src/gui/ToolTip.hxx +++ b/src/gui/ToolTip.hxx @@ -42,7 +42,7 @@ class ToolTip static constexpr uInt32 MAX_LEN = MAX_COLUMNS * MAX_ROWS; ToolTip(Dialog& dialog, const GUI::Font& font); - ~ToolTip() = default; + ~ToolTip(); void setFont(const GUI::Font& font); @@ -76,7 +76,7 @@ class ToolTip /** Allocate surface if required and return it */ - const unique_ptr& surface(); + const shared_ptr& surface(); void show(const string& tip); @@ -100,7 +100,7 @@ class ToolTip uInt32 myTextYOfs{0}; bool myTipShown{false}; uInt32 myScale{1}; - unique_ptr mySurface; + shared_ptr mySurface; }; #endif