Make windows positioning independent for launcher, emulator and debugger (resolves #615)

docs/index.html

@@ -1999,6 +1999,16 @@
       <td>Centers game window (if possible).</td>
     </tr>
 
+    <tr>
+      <td><pre>-windowedpos &lt;WxH&gt;</pre></td>
+      <td>Sets the window position in windowed emulator mode.</td>
+    </tr>
+
+    <tr>
+      <td><pre>-display &lt;number&gt;</pre></td>
+      <td>Sets the display for Stella's emulator.</td>
+    </tr>
+
     <tr>
       <td><pre>-palette &lt;standard|z26|user&gt;</pre></td>
       <td>Set the palette to either normal Stella, the one used in the z26
@@ -2376,6 +2386,16 @@
         launcher only happens when started with the launcher).</td>
     </tr>
 
+    <tr>
+      <td><pre>-launcherpos &lt;WxH&gt;</pre></td>
+      <td>Sets the window position in windowed ROM launcher mode.</td>
+    </tr>
+
+    <tr>
+      <td><pre>-launcherdisplay &lt;number&gt;</pre></td>
+      <td>Sets the display for the ROM launcher.</td>
+    </tr>
+
     <tr>
       <td><pre>-launcherres &lt;WxH&gt;</pre></td>
       <td>Set the size of the ROM launcher.</td>
@@ -2522,6 +2542,16 @@
       <td>Relocate calls out of address range in the disassembler.</td>
     </tr>
 
+    <tr>
+      <td><pre>-dbg.pos &lt;WxH&gt;</pre></td>
+      <td>Sets the window position in windowed debugger mode.</td>
+    </tr>
+
+    <tr>
+      <td><pre>-dbg.display &lt;number&gt;</pre></td>
+      <td>Sets the display for the debugger.</td>
+    </tr>
+
     <tr>
       <td><pre>-dbg.res &lt;WxH&gt;</pre></td>
       <td>Set the size of the debugger window.</td>

src/common/FrameBufferSDL2.cxx

@@ -48,8 +48,6 @@ FrameBufferSDL2::FrameBufferSDL2(OSystem& osystem)
   // since the structure may be needed before any FBSurface's have
   // been created
   myPixelFormat = SDL_AllocFormat(SDL_PIXELFORMAT_ARGB8888);
-
-  myWindowedPos = myOSystem.settings().getPoint("windowedpos");
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -183,27 +181,34 @@ void FrameBufferSDL2::queryHardware(vector<Common::Size>& fullscreenRes,
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-Int32 FrameBufferSDL2::getCurrentDisplayIndex()
+bool FrameBufferSDL2::isCurrentWindowPositioned() const
+{
+  ASSERT_MAIN_THREAD;
+
+  return !myCenter
+    && myWindow && !(SDL_GetWindowFlags(myWindow) & SDL_WINDOW_FULLSCREEN_DESKTOP);
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+Common::Point FrameBufferSDL2::getCurrentWindowPos() const
+{
+  ASSERT_MAIN_THREAD;
+
+  Common::Point pos;
+
+  SDL_GetWindowPosition(myWindow, &pos.x, &pos.y);
+
+  return pos;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+Int32 FrameBufferSDL2::getCurrentDisplayIndex() const
 {
   ASSERT_MAIN_THREAD;
 
   return SDL_GetWindowDisplayIndex(myWindow);
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void FrameBufferSDL2::updateWindowedPos()
-{
-  ASSERT_MAIN_THREAD;
-
-  // only save if the window is not centered and not in full screen mode
-  if (!myCenter && myWindow && !(SDL_GetWindowFlags(myWindow) & SDL_WINDOW_FULLSCREEN_DESKTOP))
-  {
-    // save current windowed position
-    SDL_GetWindowPosition(myWindow, &myWindowedPos.x, &myWindowedPos.y);
-    myOSystem.settings().setValue("windowedpos", myWindowedPos);
-  }
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
 {
@@ -213,24 +218,13 @@ bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
   if(SDL_WasInit(SDL_INIT_VIDEO) == 0)
     return false;
 
-  Int32 displayIndex = mode.fsIndex;
+  // TODO: On multiple displays, switching from centered mode, does not respect
-  if (displayIndex == -1)
+  //  current window's display (which many not be centered anymore)
-  {
-    // windowed mode
-    if (myWindow)
-    {
-      // Show it on same screen as the previous window
-      displayIndex = SDL_GetWindowDisplayIndex(myWindow);
-    }
-    if (displayIndex < 0)
-    {
-      // fallback to the last used screen if still existing
-      displayIndex = std::min(myNumDisplays, myOSystem.settings().getInt("display"));
-    }
-  }
 
-  // save and get last windowed window's position
+  // Get windowed window's last display
-  updateWindowedPos();
+  Int32 displayIndex = std::min(myNumDisplays, myOSystem.settings().getInt(getDisplayKey()));
+  // Get windowed window's last position
+  myWindowedPos = myOSystem.settings().getPoint(getPositionKey());
 
   // Always recreate renderer (some systems need this)
   if(myRenderer)
@@ -249,7 +243,7 @@ bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
     posX = myWindowedPos.x;
     posY = myWindowedPos.y;
 
-    // make sure the window is at least partially visibile
+    // Make sure the window is at least partially visibile
     int x0 = 0, y0 = 0, x1 = 0, y1 = 0;
 
     for (int display = SDL_GetNumVideoDisplays() - 1; display >= 0; display--)
@@ -276,13 +270,15 @@ bool FrameBufferSDL2::setVideoMode(const string& title, const VideoMode& mode)
   // toggling fullscreen windowed mode
   // So we have a special case for macOS
 #ifndef BSPF_MACOS
-  // Don't re-create the window if its size hasn't changed, as it's not
+  // Don't re-create the window if its display and size hasn't changed,
-  // necessary, and causes flashing in fullscreen mode
+  // as it's not necessary, and causes flashing in fullscreen mode
   if(myWindow)
   {
+    int d = SDL_GetWindowDisplayIndex(myWindow);
     int w, h;
+
     SDL_GetWindowSize(myWindow, &w, &h);
-    if(uInt32(w) != mode.screen.w || uInt32(h) != mode.screen.h)
+    if(d != displayIndex || uInt32(w) != mode.screen.w || uInt32(h) != mode.screen.h)
     {
       SDL_DestroyWindow(myWindow);
       myWindow = nullptr;

src/common/FrameBufferSDL2.hxx

@@ -95,6 +95,21 @@ class FrameBufferSDL2 : public FrameBuffer
     */
     void readPixels(uInt8* buffer, uInt32 pitch, const Common::Rect& rect) const override;
 
+    /**
+      This method is called to query if the current window is not centered
+      or fullscreen.
+
+      @return  True, if the current window is positioned
+    */
+    bool isCurrentWindowPositioned() const override;
+
+    /**
+      This method is called to query the video hardware for position of
+      the current window
+
+      @return  The position of the currently displayed window
+    */
+    Common::Point getCurrentWindowPos() const override;
     /**
       This method is called to query the video hardware for the index
       of the display the current window is displayed on
@@ -102,12 +117,7 @@ class FrameBufferSDL2 : public FrameBuffer
       @return  the current display index or a negative value if no
                window is displayed
     */
-    Int32 getCurrentDisplayIndex() override;
+    Int32 getCurrentDisplayIndex() const override;
-
-    /**
-      This method is called to preserve the last current windowed position.
-    */
-    void updateWindowedPos() override;
 
     /**
       Clear the frame buffer.

src/debugger/Debugger.cxx

@@ -115,7 +115,8 @@ void Debugger::initialize()
 FBInitStatus Debugger::initializeVideo()
 {
   string title = string("Stella ") + STELLA_VERSION + ": Debugger mode";
-  return myOSystem.frameBuffer().createDisplay(title, myWidth, myHeight);
+  return myOSystem.frameBuffer().createDisplay(title, FrameBuffer::BufferType::Debugger,
+                                               myWidth, myHeight);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/emucore/Console.cxx

@@ -574,7 +574,7 @@ void Console::toggleTurbo()
   initializeAudio();
 
   // update VSync
-  myOSystem.createFrameBuffer();
+  initializeVideo();
 
   ostringstream ss;
   ss << "Turbo mode " << (!enabled ? "enabled" : "disabled");
@@ -651,7 +651,7 @@ FBInitStatus Console::initializeVideo(bool full)
     bool devSettings = myOSystem.settings().getBool("dev.settings");
     const string& title = string("Stella ") + STELLA_VERSION +
                    ": \"" + myProperties.get(PropType::Cart_Name) + "\"";
-    fbstatus = myOSystem.frameBuffer().createDisplay(title,
+    fbstatus = myOSystem.frameBuffer().createDisplay(title, FrameBuffer::BufferType::Emulator,
         TIAConstants::viewableWidth, TIAConstants::viewableHeight, false);
     if(fbstatus != FBInitStatus::Success)
       return fbstatus;

src/emucore/FrameBuffer.cxx

@@ -203,10 +203,14 @@ FontDesc FrameBuffer::getFontDesc(const string& name) const
 #endif
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-FBInitStatus FrameBuffer::createDisplay(const string& title,
+FBInitStatus FrameBuffer::createDisplay(const string& title, BufferType type,
                                         uInt32 width, uInt32 height,
                                         bool honourHiDPI)
 {
+  // always save, maybe only the mode of the window has changed
+  saveCurrentWindowPosition();
+  myBufferType = type;
+
   ++myInitializedCount;
   myScreenTitle = title;
 
@@ -789,6 +793,58 @@ void FrameBuffer::stateChanged(EventHandlerState state)
   update(true); // force full update
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+string FrameBuffer::getDisplayKey()
+{
+  // save current window's display and position
+  switch(myBufferType)
+  {
+    case BufferType::Launcher:
+      return "launcherdisplay";
+
+    case BufferType::Emulator:
+      return "display";
+
+  #ifdef DEBUGGER_SUPPORT
+    case BufferType::Debugger:
+      return "dbg.display";
+  #endif
+
+    default:
+      return "";
+  }
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+string FrameBuffer::getPositionKey()
+{
+  // save current window's display and position
+  switch(myBufferType)
+  {
+    case BufferType::Launcher:
+      return "launcherpos";
+
+    case BufferType::Emulator:
+      return  "windowedpos";
+
+  #ifdef DEBUGGER_SUPPORT
+    case BufferType::Debugger:
+      return "dbg.pos";
+  #endif
+
+    default:
+      return "";
+  }
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void FrameBuffer::saveCurrentWindowPosition()
+{
+  myOSystem.settings().setValue(getDisplayKey(), getCurrentDisplayIndex());
+  if(isCurrentWindowPositioned())
+    myOSystem.settings().setValue(getPositionKey(), getCurrentWindowPos());
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void FrameBuffer::setFullscreen(bool enable)
 {

src/emucore/FrameBuffer.hxx

@@ -81,6 +81,13 @@ class FrameBuffer
       }
     };
 
+    enum class BufferType {
+      None,
+      Launcher,
+      Emulator,
+      Debugger
+    };
+
     enum class ScalingInterpolation {
       none,
       sharp,
@@ -115,7 +122,8 @@ class FrameBuffer
 
       @return  Status of initialization (see FBInitStatus 'enum')
     */
-    FBInitStatus createDisplay(const string& title, uInt32 width, uInt32 height,
+    FBInitStatus createDisplay(const string& title, BufferType type,
+                               uInt32 width, uInt32 height,
                                bool honourHiDPI = true);
 
     /**
@@ -299,6 +307,13 @@ class FrameBuffer
     bool hidpiEnabled() const { return myHiDPIEnabled; }
     uInt32 hidpiScaleFactor() const { return myHiDPIEnabled ? 2 : 1; }
 
+    /**
+      These methods are used to load/save position and display of the current window.
+    */
+    string getPositionKey();
+    string getDisplayKey();
+    void saveCurrentWindowPosition();
+
   #ifdef GUI_SUPPORT
     /**
       Get the font object(s) of the framebuffer
@@ -371,6 +386,21 @@ class FrameBuffer
     */
     virtual void readPixels(uInt8* buffer, uInt32 pitch, const Common::Rect& rect) const = 0;
 
+    /**
+      This method is called to query if the current window is not centered or fullscreen.
+
+      @return  True, if the current window is positioned
+    */
+    virtual bool isCurrentWindowPositioned() const = 0;
+
+    /**
+      This method is called to query the video hardware for position of
+      the current window
+
+      @return  The position of the currently displayed window
+    */
+    virtual Common::Point getCurrentWindowPos() const = 0;
+
     /**
       This method is called to query the video hardware for the index
       of the display the current window is displayed on
@@ -378,12 +408,7 @@ class FrameBuffer
       @return  the current display index or a negative value if no
                window is displayed
     */
-    virtual Int32 getCurrentDisplayIndex() = 0;
+    virtual Int32 getCurrentDisplayIndex() const = 0;
-
-    /**
-      This method is called to preserve the last current windowed position.
-    */
-    virtual void updateWindowedPos() = 0;
 
     /**
       Clear the framebuffer.
@@ -554,6 +579,9 @@ class FrameBuffer
     // Title of the main window/screen
     string myScreenTitle;
 
+    // Type of the frame buffer
+    BufferType myBufferType{BufferType::None};
+
     // Number of displays
     int myNumDisplays{1};
 

src/emucore/OSystem.cxx

@@ -239,8 +239,7 @@ void OSystem::saveConfig()
   if(myFrameBuffer)
   {
     // Save the last windowed position and display on system shutdown
-    myFrameBuffer->updateWindowedPos();
+    myFrameBuffer->saveCurrentWindowPosition();
-    settings().setValue("display", myFrameBuffer->getCurrentDisplayIndex());
 
     Logger::debug("Saving TV effects options ...");
     myFrameBuffer->tiaSurface().ntsc().saveConfig(settings());

src/emucore/Settings.cxx

@@ -126,6 +126,8 @@ Settings::Settings()
   // ROM browser options
   setPermanent("exitlauncher", "false");
   setPermanent("followlauncher", "false");
+  setPermanent("launcherpos", Common::Point(50, 50));
+  setPermanent("launcherdisplay", 0);
   setPermanent("launcherres", Common::Size(900, 600));
   setPermanent("launcherfont", "medium");
   setPermanent("launcherroms", "true");
@@ -137,6 +139,8 @@ Settings::Settings()
   setPermanent("dbg.res",
     Common::Size(DebuggerDialog::kMediumFontMinW,
                  DebuggerDialog::kMediumFontMinH));
+  setPermanent("dbg.pos", Common::Point(50, 50));
+  setPermanent("dbg.display", 0);
 #endif
   setPermanent("uipalette", "standard");
   setPermanent("hidpi", "false");
@@ -396,8 +400,8 @@ void Settings::usage() const
     << "  -vsync        <1|0>          Enable 'synchronize to vertical blank interrupt'\n"
     << "  -fullscreen   <1|0>          Enable fullscreen mode\n"
     << "  -center       <1|0>          Centers game window in windowed modes\n"
-    << "  -windowedpos  <XxY>          Sets the window position in windowed modes\n"
+    << "  -windowedpos  <XxY>          Sets the window position in windowed emulator mode\n"
-    << "  -display      <number>       Sets the display for Stella\n"
+    << "  -display      <number>       Sets the display for Stella's emulator\n"
     << "  -palette      <standard|     Use the specified color palette\n"
     << "                 z26|\n"
     << "                 user>\n"
@@ -493,8 +497,10 @@ void Settings::usage() const
     << "  -rominfo      <rom>          Display detailed information for the given ROM\n"
     << "  -listrominfo                 Display contents of stella.pro, one line per ROM\n"
     << "                                entry\n"
-      << endl
+    << endl
     << "  -exitlauncher <1|0>          On exiting a ROM, go back to the ROM launcher\n"
+    << "  -launcherpos  <XxY>          Sets the window position in windowed EOM launcher mode\n"
+    << "  -launcherdisplay <number>    Sets the display for the ROM launcher\n"
     << "  -launcherres  <WxH>          The resolution to use in ROM launcher mode\n"
     << "  -launcherfont <small|        Use the specified font in the ROM launcher\n"
     << "                 low_medium|\n"
@@ -557,6 +563,8 @@ void Settings::usage() const
     << "   -dis.relocate  <1|0>        Relocate calls out of address range in\n"
     << "                                disassembler\n"
     << endl
+    << "   -dbg.pos       <XxY>          Sets the window position in windowed debugger mode\n"
+    << "   -dbg.display   <number>       Sets the display for the debugger\n"
     << "   -dbg.res       <WxH>          The resolution to use in debugger mode\n"
     << "   -dbg.fontsize  <small|medium| Font size to use in debugger window\n"
     << "                  large>\n"

src/gui/Launcher.cxx

@@ -59,7 +59,8 @@ Launcher::~Launcher()
 FBInitStatus Launcher::initializeVideo()
 {
   string title = string("Stella ") + STELLA_VERSION;
-  return myOSystem.frameBuffer().createDisplay(title, myWidth, myHeight);
+  return myOSystem.frameBuffer().createDisplay(title, FrameBuffer::BufferType::Launcher,
+                                               myWidth, myHeight);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -