Hotplugging, buffer scaling and general improvements

xbmc/guilib/GraphicContext.cpp

@@ -415,33 +415,76 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat
 
   RESOLUTION_INFO info_mod = GetResInfo(res);
 
-  m_iScreenWidth  = info_mod.iWidth;
+  // FIXME Wayland windowing needs some way to "deny" resolution updates since what Kodi
+  // requests might not get actually set by the compositor.
+  // So in theory, m_iScreenWidth etc. would not need to be updated at all before the
+  // change is confirmed.
+  // But other windowing code expects these variables to be already set when
+  // SetFullScreen() is called, so set them anyway and remember the old values.
+  int origScreenWidth = m_iScreenWidth, origScreenHeight = m_iScreenHeight, origScreenId = m_iScreenId;
+  float origFPSOverride = m_fFPSOverride;
+
+  m_iScreenWidth = info_mod.iWidth;
   m_iScreenHeight = info_mod.iHeight;
-  m_iScreenId     = info_mod.iScreen;
-  m_scissors.SetRect(0, 0, (float)m_iScreenWidth, (float)m_iScreenHeight);
-  m_Resolution    = res;
-  m_fFPSOverride = 0 ;
+  m_iScreenId = info_mod.iScreen;
+  m_Resolution = res;
+  m_fFPSOverride = 0;
 
+  bool switched = true;
   if (g_advancedSettings.m_fullScreen)
   {
 #if defined (TARGET_DARWIN) || defined (TARGET_WINDOWS)
     bool blankOtherDisplays = CServiceBroker::GetSettings().GetBool(CSettings::SETTING_VIDEOSCREEN_BLANKDISPLAYS);
     g_Windowing.SetFullScreen(true,  info_org, blankOtherDisplays);
 #else
-    g_Windowing.SetFullScreen(true,  info_org, false);
+    switched = g_Windowing.SetFullScreen(true,  info_org, false);
+    // FIXME At the moment only Wayland expects the return value to be interpreted
+    // - all other windowing implementations might still assume that it does
+    // not matter what they return as it was before.
+    // This needs to get fixed when the resolution switching code is refactored.
+    if (g_Windowing.GetWinSystem() != WINDOW_SYSTEM_WAYLAND)
+    {
+      switched = true;
+    }
 #endif
   }
   else if (lastRes >= RES_DESKTOP )
     g_Windowing.SetFullScreen(false, info_org, false);
   else
     g_Windowing.ResizeWindow(info_org.iWidth, info_org.iHeight, -1, -1);
 
-  // make sure all stereo stuff are correctly setup
-  SetStereoView(RENDER_STEREO_VIEW_OFF);
+  if (switched)
+  {
+    m_scissors.SetRect(0, 0, (float)m_iScreenWidth, (float)m_iScreenHeight);
 
-  // update anyone that relies on sizing information
-  CInputManager::GetInstance().SetMouseResolution(info_org.iWidth, info_org.iHeight, 1, 1);
-  g_windowManager.SendMessage(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_WINDOW_RESIZE);
+    // make sure all stereo stuff are correctly setup
+    SetStereoView(RENDER_STEREO_VIEW_OFF);
+
+    // update anyone that relies on sizing information
+    CInputManager::GetInstance().SetMouseResolution(info_org.iWidth, info_org.iHeight, 1, 1);
+    g_windowManager.SendMessage(GUI_MSG_NOTIFY_ALL, 0, 0, GUI_MSG_WINDOW_RESIZE);
+  }
+  else
+  {
+    // Reset old state
+    m_iScreenWidth = origScreenWidth;
+    m_iScreenHeight = origScreenHeight;
+    m_iScreenId = origScreenId;
+    m_fFPSOverride = origFPSOverride;
+    if (IsValidResolution(lastRes))
+    {
+      m_Resolution = lastRes;
+    }
+    else
+    {
+      // FIXME Resolution has become invalid
+      // This happens e.g. when switching monitors and the new monitor has fewer
+      // resolutions than the old one. Fall back to RES_DESKTOP and hope that
+      // the real resolution is set soon.
+      // Again, must be fixed as part of a greater refactor.
+      m_Resolution = RES_DESKTOP;
+    }
+  }
 
   Unlock();
 }

xbmc/settings/DisplaySettings.cpp

@@ -288,14 +288,30 @@ bool CDisplaySettings::OnSettingChanging(std::shared_ptr<const CSetting> setting
     SetCurrentResolution(newRes, false);
     g_graphicsContext.SetVideoResolution(newRes);
 
+    if (m_resolutionChangeInProgress)
+    {
+      // Do not recurse into showing dialogs
+      return true;
+    }
+
     // check if the old or the new resolution was/is windowed
     // in which case we don't show any prompt to the user
     if (oldRes != RES_WINDOW && newRes != RES_WINDOW && oldRes != newRes)
     {
       if (!m_resolutionChangeAborted)
       {
-        if (HELPERS::ShowYesNoDialogText(CVariant{13110}, CVariant{13111}, CVariant{""}, CVariant{""}, 15000) !=
-          DialogResponse::YES)
+        m_resolutionChangeInProgress = true;
+#if defined(HAVE_WAYLAND)
+        // If ReloadSkin() is called when a dialog is shown, it will get aborted,
+        // so reload the skin later
+        g_Windowing.SetInhibitSkinReload(true);
+#endif
+        auto response = HELPERS::ShowYesNoDialogText(CVariant{13110}, CVariant{13111}, CVariant{""}, CVariant{""}, 15000);
+#if defined(HAVE_WAYLAND)
+        g_Windowing.SetInhibitSkinReload(false);
+#endif
+        m_resolutionChangeInProgress = false;
+        if (response != DialogResponse::YES)
         {
           m_resolutionChangeAborted = true;
           return false;
@@ -313,10 +329,26 @@ bool CDisplaySettings::OnSettingChanging(std::shared_ptr<const CSetting> setting
     SetCurrentResolution(newRes, false);
     g_graphicsContext.SetVideoResolution(newRes, true);
 
+    if (m_resolutionChangeInProgress)
+    {
+      // Do not recurse into showing dialogs
+      return true;
+    }
+
     if (!m_resolutionChangeAborted)
     {
-      if (HELPERS::ShowYesNoDialogText(CVariant{13110}, CVariant{13111}, CVariant{""}, CVariant{""}, 10000) !=
-        DialogResponse::YES)
+      m_resolutionChangeInProgress = true;
+#if defined(HAVE_WAYLAND)
+      // If ReloadSkin() is called when a dialog is shown, it will get aborted,
+      // so reload the skin later
+      g_Windowing.SetInhibitSkinReload(true);
+#endif
+      auto response = HELPERS::ShowYesNoDialogText(CVariant{13110}, CVariant{13111}, CVariant{""}, CVariant{""}, 10000);
+#if defined(HAVE_WAYLAND)
+      g_Windowing.SetInhibitSkinReload(false);
+#endif
+      m_resolutionChangeInProgress = false;
+      if (response != DialogResponse::YES)
       {
         m_resolutionChangeAborted = true;
         return false;

xbmc/settings/DisplaySettings.h

@@ -131,6 +131,7 @@ class CDisplaySettings : public ISettingCallback, public ISubSettings,
   float m_verticalShift;      // current vertical shift
   bool  m_nonLinearStretched;   // current non-linear stretch
 
+  bool m_resolutionChangeInProgress = false;
   bool m_resolutionChangeAborted;
   CCriticalSection m_critical;
 };

xbmc/utils/MathUtils.h

@@ -189,6 +189,20 @@ namespace MathUtils
     MathUtils::abs(0);
   }
 
+  /**
+   * Compare two floating-point numbers for equality and regard them
+   * as equal if their difference is below a given threshold.
+   *
+   * It is usually not useful to compare float numbers for equality with
+   * the standard operator== since very close numbers might have different
+   * representations.
+   */
+  template<typename FloatT>
+  inline bool FloatEquals(FloatT f1, FloatT f2, FloatT maxDelta)
+  {
+    return (std::abs(f2 - f1) < maxDelta);
+  }
+
 #if 0
   /*! \brief test routine for round_int and truncate_int
    Must return true on all platforms.

xbmc/windowing/wayland/Connection.cpp

@@ -62,10 +62,7 @@ CConnection::CConnection(IConnectionHandler* handler)
   HandleRegistry();
 
   CLog::Log(LOGDEBUG, "Wayland connection: Waiting for global interfaces");
-  if (m_display->roundtrip() < 0)
-  {
-    throw std::runtime_error("Wayland roundtrip failed");
-  }
+  m_display->roundtrip();
   CLog::Log(LOGDEBUG, "Wayland connection: Initial roundtrip complete");
 
   CheckRequiredGlobals();

xbmc/windowing/wayland/Output.cpp

@@ -32,6 +32,7 @@ COutput::COutput(std::uint32_t globalName, wayland::output_t const & output, std
 
   m_output.on_geometry() = [this](std::int32_t x, std::int32_t y, std::int32_t physWidth, std::int32_t physHeight, wayland::output_subpixel subpixel, std::string const& make, std::string const& model, wayland::output_transform transform)
   {
+    CSingleLock lock(m_geometryCriticalSection);
     m_x = x;
     m_y = y;
     m_physicalWidth = physWidth;
@@ -45,6 +46,7 @@ COutput::COutput(std::uint32_t globalName, wayland::output_t const & output, std
     // element and boolean information whether the element was actually added
     // which we do not need
     auto modeIterator = m_modes.emplace(width, height, refresh).first;
+    CSingleLock lock(m_iteratorCriticalSection);
     // Remember current and preferred mode
     // Current mode is the last one that was sent with current flag set
     if (flags & wayland::output_mode::current)
@@ -67,8 +69,39 @@ COutput::COutput(std::uint32_t globalName, wayland::output_t const & output, std
   };
 }
 
+COutput::~COutput()
+{
+  // Reset event handlers - someone might still hold a reference to the output_t,
+  // causing events to be dispatched. They should not go to a deleted class.
+  m_output.on_geometry() = nullptr;
+  m_output.on_mode() = nullptr;
+  m_output.on_done() = nullptr;
+  m_output.on_scale() = nullptr;
+}
+
+const COutput::Mode& COutput::GetCurrentMode() const
+{
+  CSingleLock lock(m_iteratorCriticalSection);
+  if (m_currentMode == m_modes.end())
+  {
+    throw std::runtime_error("Current mode not set");
+  }
+  return *m_currentMode;
+}
+
+const COutput::Mode& COutput::GetPreferredMode() const
+{
+  CSingleLock lock(m_iteratorCriticalSection);
+  if (m_preferredMode == m_modes.end())
+  {
+    throw std::runtime_error("Preferred mode not set");
+  }
+  return *m_preferredMode;
+}
+
 float COutput::GetPixelRatioForMode(const Mode& mode) const
 {
+  CSingleLock lock(m_geometryCriticalSection);
   if (m_physicalWidth == 0 || m_physicalHeight == 0 || mode.width == 0 || mode.height == 0)
   {
     return 1.0f;

xbmc/windowing/wayland/Output.h

@@ -19,13 +19,17 @@
  */
 #pragma once
 
+#include <atomic>
 #include <cstdint>
 #include <set>
 #include <stdexcept>
 #include <tuple>
 
 #include <wayland-client-protocol.hpp>
 
+#include "threads/CriticalSection.h"
+#include "threads/SingleLock.h"
+
 namespace KODI
 {
 namespace WINDOWING
@@ -41,7 +45,7 @@ class COutput
 {
 public:
   COutput(std::uint32_t globalName, wayland::output_t const & output, std::function<void()> doneHandler);
-  COutput(COutput&& other) = default;
+  ~COutput();
 
   wayland::output_t const& GetWaylandOutput() const
   {
@@ -57,6 +61,7 @@ class COutput
    */
   std::tuple<std::int32_t, std::int32_t> GetPosition() const
   {
+    CSingleLock lock(m_geometryCriticalSection);
     return std::make_tuple(m_x, m_y);
   }
   /**
@@ -65,14 +70,17 @@ class COutput
    */
   std::tuple<std::int32_t, std::int32_t> GetPhysicalSize() const
   {
+    CSingleLock lock(m_geometryCriticalSection);
     return std::make_tuple(m_physicalWidth, m_physicalHeight);
   }
   std::string const& GetMake() const
   {
+    CSingleLock lock(m_geometryCriticalSection);
     return m_make;
   }
   std::string const& GetModel() const
   {
+    CSingleLock lock(m_geometryCriticalSection);
     return m_model;
   }
   std::int32_t GetScale() const
@@ -113,22 +121,8 @@ class COutput
   {
     return m_modes;
   }
-  Mode const& GetCurrentMode() const
-  {
-    if (m_currentMode == m_modes.end())
-    {
-      throw std::runtime_error("Current mode not set");
-    }
-    return *m_currentMode;
-  }
-  Mode const& GetPreferredMode() const
-  {
-    if (m_preferredMode == m_modes.end())
-    {
-      throw std::runtime_error("Preferred mode not set");
-    }
-    return *m_preferredMode;
-  }
+  Mode const& GetCurrentMode() const;
+  Mode const& GetPreferredMode() const;
 
   float GetPixelRatioForMode(Mode const& mode) const;
 
@@ -139,12 +133,15 @@ class COutput
   std::uint32_t m_globalName;
   wayland::output_t m_output;
   std::function<void()> m_doneHandler;
-
+
+  CCriticalSection m_geometryCriticalSection;
+  CCriticalSection m_iteratorCriticalSection;
+
   std::int32_t m_x = 0, m_y = 0;
   std::int32_t m_physicalWidth = 0, m_physicalHeight = 0;
   std::string m_make, m_model;
-  std::int32_t m_scale = 1; // default scale of 1 if no wl_output::scale is sent
-  
+  std::atomic<std::int32_t> m_scale = {1}; // default scale of 1 if no wl_output::scale is sent
+
   std::set<Mode> m_modes;
   // For std::set, insertion never invalidates existing iterators, and modes are
   // never removed, so the usage of iterators is safe

xbmc/windowing/wayland/SeatInputProcessor.cpp

@@ -106,7 +106,7 @@ constexpr int WL_KEYBOARD_XKB_CODE_OFFSET = 8;
 }
 
 CSeatInputProcessor::CSeatInputProcessor(std::uint32_t globalName, const wayland::seat_t& seat, IInputHandler* handler)
-: m_globalName(globalName), m_seat(seat), m_handler(handler), m_keyRepeatTimer(&m_keyRepeatCallback), m_keyRepeatCallback(this)
+: m_globalName(globalName), m_seat(seat), m_handler(handler), m_keyRepeatCallback(this), m_keyRepeatTimer(&m_keyRepeatCallback)
 {
   assert(m_handler);
 
@@ -204,8 +204,8 @@ void CSeatInputProcessor::SendMouseMotion()
   event.type = XBMC_MOUSEMOTION;
   event.motion =
   {
-    .x = m_pointerX,
-    .y = m_pointerY
+    .x = static_cast<std::uint16_t> (m_pointerX * m_coordinateScale),
+    .y = static_cast<std::uint16_t> (m_pointerY * m_coordinateScale)
   };
   m_handler->OnEvent(m_globalName, InputType::POINTER, event);
 }
@@ -217,8 +217,8 @@ void CSeatInputProcessor::SendMouseButton(unsigned char button, bool pressed)
   event.button =
   {
     .button = button,
-    .x = m_pointerX,
-    .y = m_pointerY
+    .x = static_cast<std::uint16_t> (m_pointerX * m_coordinateScale),
+    .y = static_cast<std::uint16_t> (m_pointerY * m_coordinateScale)
   };
   m_handler->OnEvent(m_globalName, InputType::POINTER, event);
 }