From 0f607f19a018c75da8c8c9069fdebc813215880f Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Thu, 15 Jun 2017 16:44:54 +0200 Subject: [PATCH 01/22] Make COutput unmovable Using the default move constructor does not it work since it does not re-register the event handlers with the new this-pointer. Instead of implementing a custom move constructor, using std::unique_ptr<> is the easier solution. --- xbmc/windowing/wayland/Output.cpp | 11 ++++++ xbmc/windowing/wayland/Output.h | 2 +- xbmc/windowing/wayland/WinSystemWayland.cpp | 37 ++++++++++----------- xbmc/windowing/wayland/WinSystemWayland.h | 2 +- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/xbmc/windowing/wayland/Output.cpp b/xbmc/windowing/wayland/Output.cpp index bcdc1e53c9e0b..c300284f2f754 100644 --- a/xbmc/windowing/wayland/Output.cpp +++ b/xbmc/windowing/wayland/Output.cpp @@ -67,6 +67,17 @@ 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; +} + + float COutput::GetPixelRatioForMode(const Mode& mode) const { if (m_physicalWidth == 0 || m_physicalHeight == 0 || mode.width == 0 || mode.height == 0) diff --git a/xbmc/windowing/wayland/Output.h b/xbmc/windowing/wayland/Output.h index 369a0d87cf895..66b326a17643f 100644 --- a/xbmc/windowing/wayland/Output.h +++ b/xbmc/windowing/wayland/Output.h @@ -41,7 +41,7 @@ class COutput { public: COutput(std::uint32_t globalName, wayland::output_t const & output, std::function doneHandler); - COutput(COutput&& other) = default; + ~COutput(); wayland::output_t const& GetWaylandOutput() const { diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 64448eaa76426..6affe210123d8 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -70,17 +70,13 @@ bool CWinSystemWayland::InitWindowSystem() CLog::Log(LOGWARNING, "Wayland compositor did not announce a wl_seat - you will not have any input devices for the time being"); } // Do another roundtrip to get initial wl_output information - int tries = 0; - while (m_outputs.empty()) + if (m_connection->GetDisplay().roundtrip() < 0) { - if (tries++ > 5) - { - throw std::runtime_error("No outputs received from compositor"); - } - if (m_connection->GetDisplay().roundtrip() < 0) - { - throw std::runtime_error("Wayland roundtrip failed"); - } + throw std::runtime_error("Wayland roundtrip failed"); + } + if (m_outputs.empty()) + { + throw std::runtime_error("No outputs received from compositor"); } // Event loop is started in CreateWindow @@ -105,6 +101,7 @@ bool CWinSystemWayland::DestroyWindowSystem() m_cursorImage = wayland::cursor_image_t(); m_cursorTheme = wayland::cursor_theme_t(); m_seatProcessors.clear(); + m_outputsInPreparation.clear(); m_outputs.clear(); m_connection.reset(); @@ -235,7 +232,7 @@ void CWinSystemWayland::GetConnectedOutputs(std::vector* outputs) std::transform(m_outputs.cbegin(), m_outputs.cend(), std::back_inserter(*outputs), [this](decltype(m_outputs)::value_type const& pair) { - return UserFriendlyOutputName(pair.second); }); + return UserFriendlyOutputName(*pair.second); }); } void CWinSystemWayland::UpdateResolutions() @@ -265,13 +262,12 @@ void CWinSystemWayland::UpdateResolutions() if (!output) { // Well just use the first one - output = &m_outputs.begin()->second; + output = m_outputs.begin()->second.get(); } std::string outputName = UserFriendlyOutputName(*output); auto const& modes = output->GetModes(); - // TODO wait until output has all information auto const& currentMode = output->GetCurrentMode(); auto physicalSize = output->GetPhysicalSize(); CLog::Log(LOGINFO, "User wanted output \"%s\", we now have \"%s\" size %dx%d mm with %zu mode(s):", userOutput.c_str(), outputName.c_str(), std::get<0>(physicalSize), std::get<1>(physicalSize), modes.size()); @@ -309,16 +305,21 @@ bool CWinSystemWayland::ResizeWindow(int newWidth, int newHeight, int newLeft, i return false; } +/** + * Find output by name + * + * Every caller of this function must hold a lock \ref m_outputsMutex + * for the duration of the call and as long as it uses the returned COutput. + */ COutput* CWinSystemWayland::FindOutputByUserFriendlyName(const std::string& name) { - CSingleLock lock(m_outputsMutex); auto outputIt = std::find_if(m_outputs.begin(), m_outputs.end(), [this, &name](decltype(m_outputs)::value_type const& entry) { - return (name == UserFriendlyOutputName(entry.second)); + return (name == UserFriendlyOutputName(*entry.second)); }); - return (outputIt == m_outputs.end() ? nullptr : &outputIt->second); + return (outputIt == m_outputs.end() ? nullptr : outputIt->second.get()); } bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, bool blankOtherDisplays) @@ -530,9 +531,7 @@ void CWinSystemWayland::OnSeatAdded(std::uint32_t name, wayland::seat_t& seat) void CWinSystemWayland::OnOutputAdded(std::uint32_t name, wayland::output_t& output) { // This is not accessed from multiple threads - m_outputsInPreparation.emplace(std::piecewise_construct, - std::forward_as_tuple(name), - std::forward_as_tuple(name, output, std::bind(&CWinSystemWayland::OnOutputDone, this, name))); + m_outputsInPreparation.emplace(name, new COutput(name, output, std::bind(&CWinSystemWayland::OnOutputDone, this, name))); } void CWinSystemWayland::OnOutputDone(std::uint32_t name) diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 69bef3f6ef713..18dfd438cb49b 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -111,7 +111,7 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC std::map m_seatProcessors; CCriticalSection m_seatProcessorsMutex; // m_outputsInPreparation did not receive their done event yet - std::map m_outputs, m_outputsInPreparation; + std::map> m_outputs, m_outputsInPreparation; CCriticalSection m_outputsMutex; bool m_osCursorVisible = true; From 85c9432d027fe0227b508bb97efa9f03b2ef9974 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Thu, 15 Jun 2017 20:05:51 +0200 Subject: [PATCH 02/22] Try to switch resolution on output hotplug Closes #8 --- xbmc/windowing/wayland/WinSystemWayland.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 6affe210123d8..fa79102ddf825 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -404,6 +404,10 @@ void CWinSystemWayland::HandleSurfaceConfigure(std::int32_t width, std::int32_t m_nWidth = width; m_nHeight = height; + } + + { + CSingleLock lock(g_graphicsContext); // Update desktop resolution auto& res = CDisplaySettings::GetInstance().GetCurrentResolutionInfo(); res.iWidth = width; @@ -546,6 +550,17 @@ void CWinSystemWayland::OnOutputDone(std::uint32_t name) // Move from m_outputsInPreparation to m_outputs m_outputs.emplace(std::move(*it)); m_outputsInPreparation.erase(it); + + // Maybe the output that was added was the one we should be on? + if (m_bFullScreen) + { + CSingleLock lock(g_graphicsContext); + UpdateResolutions(); + // This will call SetFullScreen(), which will match the output against + // the information from the resolution and call set_fullscreen on the + // surface if it changed. + g_graphicsContext.SetVideoResolution(g_graphicsContext.GetVideoResolution(), true); + } } void CWinSystemWayland::OnGlobalRemoved(std::uint32_t name) @@ -559,7 +574,9 @@ void CWinSystemWayland::OnGlobalRemoved(std::uint32_t name) CSingleLock lock(m_outputsMutex); if (m_outputs.erase(name) != 0) { - // TODO Handle: Update resolution etc. + // Theoretically, the compositor should automatically put us on another + // (visible and connected) output if the output we were on is lost, + // so there is nothing in particular to do here } } } From 53443e585ed0683b902858c8fa0460e0a6050f14 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Thu, 15 Jun 2017 20:21:12 +0200 Subject: [PATCH 03/22] Release outputs lock during resolution change --- xbmc/windowing/wayland/WinSystemWayland.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index fa79102ddf825..dc2d92aabe79c 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -546,10 +546,12 @@ void CWinSystemWayland::OnOutputDone(std::uint32_t name) return; } - CSingleLock lock(m_outputsMutex); - // Move from m_outputsInPreparation to m_outputs - m_outputs.emplace(std::move(*it)); - m_outputsInPreparation.erase(it); + { + CSingleLock lock(m_outputsMutex); + // Move from m_outputsInPreparation to m_outputs + m_outputs.emplace(std::move(*it)); + m_outputsInPreparation.erase(it); + } // Maybe the output that was added was the one we should be on? if (m_bFullScreen) From 3820cd7cfde1cdbab92376754b9464f4d48cbc28 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Mon, 19 Jun 2017 10:05:47 +0200 Subject: [PATCH 04/22] Reorder SeatInputProcessor member initialization --- xbmc/windowing/wayland/SeatInputProcessor.cpp | 2 +- xbmc/windowing/wayland/SeatInputProcessor.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xbmc/windowing/wayland/SeatInputProcessor.cpp b/xbmc/windowing/wayland/SeatInputProcessor.cpp index 07b504ecdfcb6..9d9bb0969dc0a 100644 --- a/xbmc/windowing/wayland/SeatInputProcessor.cpp +++ b/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); diff --git a/xbmc/windowing/wayland/SeatInputProcessor.h b/xbmc/windowing/wayland/SeatInputProcessor.h index 5c8e90628de5a..3019953a17529 100644 --- a/xbmc/windowing/wayland/SeatInputProcessor.h +++ b/xbmc/windowing/wayland/SeatInputProcessor.h @@ -152,7 +152,6 @@ class CSeatInputProcessor // Save complete XBMC_Event so no keymap lookups which might not be thread-safe // are needed in the repeat callback XBMC_Event m_keyToRepeat; - CTimer m_keyRepeatTimer; class CKeyRepeatCallback : public ITimerCallback { @@ -162,6 +161,7 @@ class CSeatInputProcessor void OnTimeout() override; }; CKeyRepeatCallback m_keyRepeatCallback; + CTimer m_keyRepeatTimer; }; } From 652f33acc8656f7468b521f8a744c8e3d3b10bb5 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Mon, 19 Jun 2017 11:22:52 +0200 Subject: [PATCH 05/22] Rework Wayland resolution handling Closes #23 --- xbmc/utils/MathUtils.h | 14 + xbmc/windowing/wayland/WinSystemWayland.cpp | 242 +++++++++++------- xbmc/windowing/wayland/WinSystemWayland.h | 5 +- .../wayland/WinSystemWaylandGLContext.cpp | 37 ++- 4 files changed, 191 insertions(+), 107 deletions(-) diff --git a/xbmc/utils/MathUtils.h b/xbmc/utils/MathUtils.h index 556787ef8263f..519e56cbf6bd3 100644 --- a/xbmc/utils/MathUtils.h +++ b/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 + 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. diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index dc2d92aabe79c..6dbd1c2ef2078 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -42,10 +42,33 @@ #include "utils/log.h" #include "utils/StringUtils.h" #include "WinEventsWayland.h" +#include "utils/MathUtils.h" using namespace KODI::WINDOWING::WAYLAND; using namespace std::placeholders; +namespace +{ + +// Caller should hold g_graphicsContext lock +RESOLUTION FindMatchingCustomResolution(int width, int height, float refreshRate) +{ + for (size_t res = RES_DESKTOP; res < CDisplaySettings::GetInstance().ResolutionInfoSize(); ++res) + { + auto const& resInfo = CDisplaySettings::GetInstance().GetResolutionInfo(res); + if (resInfo.iWidth == width && resInfo.iHeight == height && MathUtils::FloatEquals(resInfo.fRefreshRate, refreshRate, 0.0005f)) + { + return static_cast (res); + } + } + return RES_INVALID; +} + +const std::string CONFIGURE_RES_ID = "configure"; + +} + + CWinSystemWayland::CWinSystemWayland() : CWinSystemBase() { @@ -78,7 +101,7 @@ bool CWinSystemWayland::InitWindowSystem() { throw std::runtime_error("No outputs received from compositor"); } - + // Event loop is started in CreateWindow // pointer is by default not on this window, will be immediately rectified @@ -113,11 +136,11 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, RESOLUTION_INFO& res) { m_surface = m_connection->GetCompositor().create_surface(); - + // Try to get this resolution if compositor does not say otherwise m_nWidth = res.iWidth; m_nHeight = res.iHeight; - + auto xdgShell = m_connection->GetXdgShellUnstableV6(); if (xdgShell) { @@ -126,9 +149,9 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, else { CLog::Log(LOGWARNING, "Compositor does not support xdg_shell unstable v6 protocol - falling back to wl_shell, not all features might work"); - m_shellSurface.reset(new CShellSurfaceWlShell(m_connection->GetShell(), m_surface, name, "kodi")); + m_shellSurface.reset(new CShellSurfaceWlShell(m_connection->GetShell(), m_surface, name, "kodi")); } - + // Just remember initial width/height for context creation // This is used for sizing the EGLSurface m_shellSurface->OnConfigure() = [this](std::int32_t width, std::int32_t height) @@ -137,7 +160,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_nWidth = width; m_nHeight = height; }; - + if (fullScreen) { // Try to start on correct monitor @@ -147,29 +170,24 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_shellSurface->SetFullScreen(output->GetWaylandOutput(), res.fRefreshRate); } } - + m_shellSurface->Initialize(); - + // Set real handler during runtime m_shellSurface->OnConfigure() = std::bind(&CWinSystemWayland::HandleSurfaceConfigure, this, _1, _2); - + if (m_nWidth == 0 || m_nHeight == 0) { - // Adopt size from resolution if compositor did not specify anything + // Adopt size from resolution if compositor did not specify anything m_nWidth = res.iWidth; m_nHeight = res.iHeight; } else { // Update resolution with real size - res.iWidth = m_nWidth; - res.iHeight = m_nHeight; - res.iScreenWidth = m_nWidth; - res.iScreenHeight = m_nHeight; - res.iSubtitles = (int) (0.965 * m_nHeight); - g_graphicsContext.ResetOverscan(res); - } - + UpdateDesktopResolution(res, 0, m_nWidth, m_nHeight, res.fRefreshRate); + } + // Now start processing events // // There are two stages to the event handling: @@ -191,7 +209,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, // wl_output and wl_seat and thus to most if not all runtime object creation // cases we have to support. CWinEventsWayland::SetDisplay(&m_connection->GetDisplay()); - + return true; } @@ -246,13 +264,13 @@ void CWinSystemWayland::UpdateResolutions() std::string userOutput = CServiceBroker::GetSettings().GetString(CSettings::SETTING_VIDEOSCREEN_MONITOR); CSingleLock lock(m_outputsMutex); - + if (m_outputs.empty()) { // *Usually* this should not happen - just give up return; } - + COutput* output = FindOutputByUserFriendlyName(userOutput); if (!output) { @@ -264,7 +282,7 @@ void CWinSystemWayland::UpdateResolutions() // Well just use the first one output = m_outputs.begin()->second.get(); } - + std::string outputName = UserFriendlyOutputName(*output); auto const& modes = output->GetModes(); @@ -278,13 +296,10 @@ void CWinSystemWayland::UpdateResolutions() float pixelRatio = output->GetPixelRatioForMode(mode); CLog::Log(LOGINFO, "- %dx%d @%.3f Hz pixel ratio %.3f%s", mode.width, mode.height, mode.refreshMilliHz / 1000.0f, pixelRatio, isCurrent ? " current" : ""); - RESOLUTION_INFO res(mode.width, mode.height); - res.bFullScreen = true; - res.iScreen = 0; // not used + RESOLUTION_INFO res; + UpdateDesktopResolution(res, 0, mode.width, mode.height, mode.refreshMilliHz / 1000.0f); res.strOutput = outputName; res.fPixelRatio = pixelRatio; - res.fRefreshRate = mode.refreshMilliHz / 1000.0f; - g_graphicsContext.ResetOverscan(res); if (isCurrent) { @@ -324,57 +339,80 @@ COutput* CWinSystemWayland::FindOutputByUserFriendlyName(const std::string& name bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, bool blankOtherDisplays) { - CSingleLock lock(m_configurationMutex); - - if (m_currentOutput == res.strOutput && m_nWidth == res.iWidth && m_nHeight == res.iHeight && m_fRefreshRate == res.fRefreshRate && m_bFullScreen == fullScreen) - { - // Nothing to do - return true; - } + // FIXME Our configuration is protected by graphicsContext lock + // If we'd use a mutex private to this class, we would have to lock both + // that one and graphicsContext (because the resolutions get updated), + // leading to a possible deadlock. + CSingleLock lock(g_graphicsContext); + + CLog::LogF(LOGINFO, "Wayland asked to switch mode to %dx%d @%.3f Hz on output \"%s\"", res.iWidth, res.iHeight, res.fRefreshRate, res.strOutput.c_str()); - CLog::Log(LOGINFO, "Wayland trying to switch mode to %dx%d @%.3f Hz on output \"%s\"", res.iWidth, res.iHeight, res.fRefreshRate, res.strOutput.c_str()); + // In fullscreen modes, we never change the surface size on Kodi's request, + // but only when the compositor tells us to. At least xdg_shell specifies + // that with state fullscreen the dimensions given in configure() must + // always be observed. + // This does mean that the compositor has no way of knowing which resolution + // we would (in theory) want. Since no compositor implements dynamic resolution + // switching at the moment, this is not a problem. If it is some day implemented + // in compositors, this code must be changed to match the behavior that is + // expected then anyway. + + m_bFullScreen = fullScreen; - // Try to match output - wayland::output_t output; + bool wasConfigure = (res.strId == CONFIGURE_RES_ID); + res.strId = ""; + + if (fullScreen) { - CSingleLock lock(m_outputsMutex); - - COutput* outputHandler = FindOutputByUserFriendlyName(res.strOutput); - if (outputHandler) + if (!wasConfigure || m_currentOutput != res.strOutput) { - output = outputHandler->GetWaylandOutput(); - CLog::Log(LOGDEBUG, "Resolved output \"%s\" to bound Wayland global %u", res.strOutput.c_str(), outputHandler->GetGlobalName()); + // There is -no- guarantee that the compositor will put the surface on this + // screen, but pretend that it does so we have any information at all + m_currentOutput = res.strOutput; + + // Try to match output + wayland::output_t output; + { + CSingleLock lock(m_outputsMutex); + + COutput* outputHandler = FindOutputByUserFriendlyName(res.strOutput); + if (outputHandler) + { + output = outputHandler->GetWaylandOutput(); + CLog::LogF(LOGDEBUG, "Resolved output \"%s\" to bound Wayland global %u", res.strOutput.c_str(), outputHandler->GetGlobalName()); + } + else + { + CLog::LogF(LOGINFO, "Could not match output \"%s\" to a currently available Wayland output, falling back to default output", res.strOutput.c_str()); + } + // Release lock only when output_t has been assigned to local variable so it + // cannot go away + } + CLog::LogF(LOGDEBUG, "Setting full-screen with refresh rate %.3f", res.fRefreshRate); + m_shellSurface->SetFullScreen(output, res.fRefreshRate); } else { - CLog::Log(LOGINFO, "Could not match output \"%s\" to a currently available Wayland output, falling back to default output", res.strOutput.c_str()); + // Switch done, do not SetFullScreen() again - otherwise we would + // get an endless repetition of setting full screen and configure events + CLog::LogF(LOGDEBUG, "Called in response to surface configure, not calling set_fullscreen on surface"); } - // Release lock only when output has been assigned to local variable so it - // cannot go away - } - - m_nWidth = res.iWidth; - m_nHeight = res.iHeight; - m_bFullScreen = fullScreen; - // This is just a guess since the compositor is free to ignore our frame rate - // request - m_fRefreshRate = res.fRefreshRate; - // There is -no- guarantee that the compositor will put the surface on this - // screen, but pretend that it does so we have any information at all - m_currentOutput = res.strOutput; - - if (fullScreen) - { - m_shellSurface->SetFullScreen(output, m_fRefreshRate); } else { // Shouldn't happen since we claim not to support windowed modes - CLog::Log(LOGWARNING, "Wayland windowing system asked to switch to windowed mode which is not really supported"); + CLog::LogF(LOGWARNING, "Wayland windowing system asked to switch to windowed mode which is not really supported"); m_shellSurface->SetWindowed(); } - return true; + bool wasInitialSetFullScreen = m_isInitialSetFullScreen; + m_isInitialSetFullScreen = false; + + // Need to return true: + // * when this SetFullScreen() call was initiated by a configure() event + // * on first SetFullScreen so GraphicsContext gets resolution + // Otherwise, Kodi must keep the old resolution. + return wasConfigure || wasInitialSetFullScreen; } void CWinSystemWayland::HandleSurfaceConfigure(std::int32_t width, std::int32_t height) @@ -385,47 +423,69 @@ void CWinSystemWayland::HandleSurfaceConfigure(std::int32_t width, std::int32_t // output for example // It is very important that the EGL native module and the rendering system use the // Wayland-announced size for rendering or corrupted graphics output will result. - - CLog::Log(LOGINFO, "Got Wayland surface size %dx%d", width, height); - { - CSingleLock lock(m_configurationMutex); - // Mark everything opaque so the compositor can render it faster - wayland::region_t opaqueRegion = m_connection->GetCompositor().create_region(); - opaqueRegion.add(0, 0, width, height); - m_surface.set_opaque_region(opaqueRegion); - // No surface commit, EGL context will do that when it changes the buffer + RESOLUTION switchToRes = RES_INVALID; + + // Mark everything opaque so the compositor can render it faster + wayland::region_t opaqueRegion = m_connection->GetCompositor().create_region(); + opaqueRegion.add(0, 0, width, height); + m_surface.set_opaque_region(opaqueRegion); + // No surface commit, EGL context will do that when it changes the buffer + + // FIXME See comment in SetFullScreen + CSingleLock lock(g_graphicsContext); - if (m_nWidth == width && m_nHeight == height) + CLog::Log(LOGINFO, "Got new Wayland surface size %dx%d", m_nWidth, m_nHeight); + + // Now update actual resolution with configured one + m_nWidth = width; + m_nHeight = height; + + // Get actual frame rate from monitor + // TODO Track wl_surface.enter() events and get frame rate of the output + // we are actually on + { + CSingleLock lockOut(m_outputsMutex); + COutput* output = FindOutputByUserFriendlyName(m_currentOutput); + if (output) { - // Nothing to do - return; + m_fRefreshRate = output->GetCurrentMode().refreshMilliHz / 1000.0f; } - - m_nWidth = width; - m_nHeight = height; } + // Find matching Kodi resolution member + switchToRes = FindMatchingCustomResolution(width, height, m_fRefreshRate); + + if (switchToRes == RES_INVALID) { - CSingleLock lock(g_graphicsContext); - // Update desktop resolution - auto& res = CDisplaySettings::GetInstance().GetCurrentResolutionInfo(); - res.iWidth = width; - res.iHeight = height; - res.iScreenWidth = width; - res.iScreenHeight = height; - res.iSubtitles = (int) (0.965 * height); - g_graphicsContext.ResetOverscan(res); + // Add new resolution if none found + RESOLUTION_INFO newResInfo; + UpdateDesktopResolution(newResInfo, 0, width, height, m_fRefreshRate); + newResInfo.strOutput = m_currentOutput; // we just assume the compositor put us on the right output + CDisplaySettings::GetInstance().AddResolutionInfo(newResInfo); CDisplaySettings::GetInstance().ApplyCalibrations(); + switchToRes = static_cast (CDisplaySettings::GetInstance().ResolutionInfoSize() - 1); } - + + // RES_DESKTOP does not change usually, it is still the current resolution + // of the selected output + + assert(switchToRes != RES_INVALID); + + // Mark resolution so that we know it came from configure + CDisplaySettings::GetInstance().GetResolutionInfo(switchToRes).strId = CONFIGURE_RES_ID; + + CSingleExit exit(g_graphicsContext); + // Force resolution update // SetVideoResolution() automatically delegates to main thread via internal // message if called from other threads // This will call SetFullScreen() with the new resolution, which also updates - // the size of the egl_window etc. + // the size of the egl_window etc. from m_nWidth/m_nHeight. // The call always blocks, so the configuration lock must be released beforehand. - g_graphicsContext.SetVideoResolution(g_graphicsContext.GetVideoResolution(), true); + // FIXME Ideally this class would be completely decoupled from g_graphicsContext, + // but this is not possible at the moment before the refactoring is done. + g_graphicsContext.SetVideoResolution(switchToRes, true); } std::string CWinSystemWayland::UserFriendlyOutputName(const COutput& output) @@ -545,7 +605,7 @@ void CWinSystemWayland::OnOutputDone(std::uint32_t name) { return; } - + { CSingleLock lock(m_outputsMutex); // Move from m_outputsInPreparation to m_outputs diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 18dfd438cb49b..8cd7aaa4628dc 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -101,9 +101,6 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC // information like modes is available void OnOutputDone(std::uint32_t name); - // Mutex for protecting modifications of m_nWidth, m_nHeight etc. - CCriticalSection m_configurationMutex; - std::unique_ptr m_connection; wayland::surface_t m_surface; std::unique_ptr m_shellSurface; @@ -124,6 +121,8 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC CCriticalSection m_dispResourcesMutex; std::string m_currentOutput; + // Whether this is the first call to SetFullScreen + bool m_isInitialSetFullScreen = true; }; diff --git a/xbmc/windowing/wayland/WinSystemWaylandGLContext.cpp b/xbmc/windowing/wayland/WinSystemWaylandGLContext.cpp index d69c7efd801fe..1e141dc0a9913 100644 --- a/xbmc/windowing/wayland/WinSystemWaylandGLContext.cpp +++ b/xbmc/windowing/wayland/WinSystemWaylandGLContext.cpp @@ -22,6 +22,7 @@ #include "Connection.h" #include "utils/log.h" +#include "guilib/GraphicContext.h" using namespace KODI::WINDOWING::WAYLAND; @@ -56,7 +57,7 @@ bool CWinSystemWaylandGLContext::CreateNewWindow(const std::string& name, return false; } - return SetFullScreen(fullScreen, res, false); + return true; } bool CWinSystemWaylandGLContext::DestroyWindow() @@ -75,25 +76,35 @@ bool CWinSystemWaylandGLContext::DestroyWindowSystem() bool CWinSystemWaylandGLContext::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, bool blankOtherDisplays) { - auto width = res.iWidth; - auto height = res.iHeight; - - int currWidth, currHeight; - m_glContext.GetAttachedSize(currWidth, currHeight); + // FIXME See CWinSystemWayland::SetFullScreen() + CSingleLock lock(g_graphicsContext); - if (width != currWidth || height != currHeight) - { - m_glContext.Resize(width, height); - } - if (!CWinSystemWayland::SetFullScreen(fullScreen, res, blankOtherDisplays)) { return false; } + + // Look only at m_nWidth and m_nHeight which represent the actual wl_surface + // size instead of res.iWidth and res.iHeight, which are only a "wish" - if (!CRenderSystemGL::ResetRenderSystem(width, height, fullScreen, res.fRefreshRate)) + int currWidth, currHeight; + m_glContext.GetAttachedSize(currWidth, currHeight); + + // Change EGL surface size if necessary + if (currWidth != m_nWidth || currHeight != m_nHeight) { - return false; + CLog::LogF(LOGDEBUG, "Updating egl_window size to %dx%d", m_nWidth, m_nHeight); + m_glContext.Resize(m_nWidth, m_nHeight); + } + + // Propagate changed dimensions to render system if necessary + if (m_nWidth != CRenderSystemGL::m_width || m_nHeight != CRenderSystemGL::m_height) + { + CLog::LogF(LOGDEBUG, "Resetting render system to %dx%d", m_nWidth, m_nHeight); + if (!CRenderSystemGL::ResetRenderSystem(m_nWidth, m_nHeight, fullScreen, res.fRefreshRate)) + { + return false; + } } return true; From 628d90bdb0f08537249be8fb9b139260d1e3d16e Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Mon, 19 Jun 2017 11:24:31 +0200 Subject: [PATCH 06/22] Make GraphicContext interpret the SetFullScreen() return value It is near impossible to implement the Wayland windowing system reliably otherwise since the resolution is not changed immediately, leading to a size mismatch between Kodi state and the windowing surface. --- xbmc/guilib/GraphicContext.cpp | 40 ++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/xbmc/guilib/GraphicContext.cpp b/xbmc/guilib/GraphicContext.cpp index 6065bb4bc31ba..f213b8835b751 100644 --- a/xbmc/guilib/GraphicContext.cpp +++ b/xbmc/guilib/GraphicContext.cpp @@ -415,20 +415,22 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat RESOLUTION_INFO info_mod = GetResInfo(res); - 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 ; - + 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 ) @@ -436,12 +438,22 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat 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); - - // 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); + if (switched) + { + 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 ; + + // 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); + } Unlock(); } From 5406e0e1ae9bd2a6e2937ded3100c3ab655673e7 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:29:51 +0200 Subject: [PATCH 07/22] Make access to COutput geometry and mode data thread-safe so it can safely be accessed from the main thread even when modes change --- xbmc/windowing/wayland/Output.cpp | 22 +++++++++++++++++++ xbmc/windowing/wayland/Output.h | 35 ++++++++++++++----------------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/xbmc/windowing/wayland/Output.cpp b/xbmc/windowing/wayland/Output.cpp index c300284f2f754..f0d58c55a9a26 100644 --- a/xbmc/windowing/wayland/Output.cpp +++ b/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) @@ -77,9 +79,29 @@ COutput::~COutput() 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; diff --git a/xbmc/windowing/wayland/Output.h b/xbmc/windowing/wayland/Output.h index 66b326a17643f..aaaba3f17a741 100644 --- a/xbmc/windowing/wayland/Output.h +++ b/xbmc/windowing/wayland/Output.h @@ -19,6 +19,7 @@ */ #pragma once +#include #include #include #include @@ -26,6 +27,9 @@ #include +#include "threads/CriticalSection.h" +#include "threads/SingleLock.h" + namespace KODI { namespace WINDOWING @@ -57,6 +61,7 @@ class COutput */ std::tuple GetPosition() const { + CSingleLock lock(m_geometryCriticalSection); return std::make_tuple(m_x, m_y); } /** @@ -65,14 +70,17 @@ class COutput */ std::tuple 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 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 m_scale = {1}; // default scale of 1 if no wl_output::scale is sent + std::set m_modes; // For std::set, insertion never invalidates existing iterators, and modes are // never removed, so the usage of iterators is safe From 0c196250d1f579adb2b9857a3afcf9165aaf475e Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:30:51 +0200 Subject: [PATCH 08/22] Implement pointer coordinate scaling for SeatInputProcessor Needed when surface and buffer sizes are different on HiDPI displays --- xbmc/windowing/wayland/SeatInputProcessor.cpp | 8 ++++---- xbmc/windowing/wayland/SeatInputProcessor.h | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/xbmc/windowing/wayland/SeatInputProcessor.cpp b/xbmc/windowing/wayland/SeatInputProcessor.cpp index 9d9bb0969dc0a..09b43624b8ae4 100644 --- a/xbmc/windowing/wayland/SeatInputProcessor.cpp +++ b/xbmc/windowing/wayland/SeatInputProcessor.cpp @@ -204,8 +204,8 @@ void CSeatInputProcessor::SendMouseMotion() event.type = XBMC_MOUSEMOTION; event.motion = { - .x = m_pointerX, - .y = m_pointerY + .x = static_cast (m_pointerX * m_coordinateScale), + .y = static_cast (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 (m_pointerX * m_coordinateScale), + .y = static_cast (m_pointerY * m_coordinateScale) }; m_handler->OnEvent(m_globalName, InputType::POINTER, event); } diff --git a/xbmc/windowing/wayland/SeatInputProcessor.h b/xbmc/windowing/wayland/SeatInputProcessor.h index 3019953a17529..ad13c242df93a 100644 --- a/xbmc/windowing/wayland/SeatInputProcessor.h +++ b/xbmc/windowing/wayland/SeatInputProcessor.h @@ -115,6 +115,10 @@ class CSeatInputProcessor { return m_touch; } + void SetCoordinateScale(std::int32_t scale) + { + m_coordinateScale = scale; + } private: CSeatInputProcessor(CSeatInputProcessor const& other) = delete; @@ -141,6 +145,7 @@ class CSeatInputProcessor wayland::keyboard_t m_keyboard; wayland::touch_t m_touch; + std::int32_t m_coordinateScale = 1; std::uint16_t m_pointerX = 0; std::uint16_t m_pointerY = 0; @@ -166,4 +171,4 @@ class CSeatInputProcessor } } -} \ No newline at end of file +} From b1dcdce7f35057c37b4eff3da0187dc88dfeec8b Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:37:21 +0200 Subject: [PATCH 09/22] Ack compositor configure() only when next buffer matches configuration After calling ack_configure(), the compositor will assume that our next attached buffer adheres to the size it requested (at least for fullscreen surfaces). So we must make sure that this assumption holds, or bad things will happen. Fix by delaying ack_configure() until the egl context is reconfigured. --- xbmc/windowing/wayland/ShellSurface.cpp | 4 +- xbmc/windowing/wayland/ShellSurface.h | 5 +- .../windowing/wayland/ShellSurfaceWlShell.cpp | 7 ++- xbmc/windowing/wayland/ShellSurfaceWlShell.h | 1 + .../ShellSurfaceXdgShellUnstableV6.cpp | 14 ++++- .../wayland/ShellSurfaceXdgShellUnstableV6.h | 1 + xbmc/windowing/wayland/WinSystemWayland.cpp | 52 +++++++++++++++++-- xbmc/windowing/wayland/WinSystemWayland.h | 7 ++- 8 files changed, 78 insertions(+), 13 deletions(-) diff --git a/xbmc/windowing/wayland/ShellSurface.cpp b/xbmc/windowing/wayland/ShellSurface.cpp index 97e98b24ad933..b1f7ddf20e93a 100644 --- a/xbmc/windowing/wayland/ShellSurface.cpp +++ b/xbmc/windowing/wayland/ShellSurface.cpp @@ -27,10 +27,10 @@ IShellSurface::ConfigureHandler& IShellSurface::OnConfigure() return m_onConfigure; } -void IShellSurface::InvokeOnConfigure(std::int32_t width, std::int32_t height) +void IShellSurface::InvokeOnConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height) { if (m_onConfigure) { - m_onConfigure(width, height); + m_onConfigure(serial, width, height); } } diff --git a/xbmc/windowing/wayland/ShellSurface.h b/xbmc/windowing/wayland/ShellSurface.h index 58c70fa78122d..56521191ae197 100644 --- a/xbmc/windowing/wayland/ShellSurface.h +++ b/xbmc/windowing/wayland/ShellSurface.h @@ -38,7 +38,7 @@ namespace WAYLAND class IShellSurface { protected: - void InvokeOnConfigure(std::int32_t width, std::int32_t height); + void InvokeOnConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height); public: /** @@ -63,12 +63,13 @@ class IShellSurface */ virtual void Initialize() = 0; - using ConfigureHandler = std::function; + using ConfigureHandler = std::function; virtual void SetFullScreen(wayland::output_t const& output, float refreshRate) = 0; virtual void SetWindowed() = 0; ConfigureHandler& OnConfigure(); + virtual void AckConfigure(std::uint32_t serial) = 0; private: ConfigureHandler m_onConfigure; diff --git a/xbmc/windowing/wayland/ShellSurfaceWlShell.cpp b/xbmc/windowing/wayland/ShellSurfaceWlShell.cpp index 71d7bfba0f720..d5bd483bc6b01 100644 --- a/xbmc/windowing/wayland/ShellSurfaceWlShell.cpp +++ b/xbmc/windowing/wayland/ShellSurfaceWlShell.cpp @@ -36,10 +36,15 @@ CShellSurfaceWlShell::CShellSurfaceWlShell(const wayland::shell_t& shell, const }; m_shellSurface.on_configure() = [this](wayland::shell_surface_resize, std::int32_t width, std::int32_t height) { - InvokeOnConfigure(width, height); + // wl_shell does not have serials + InvokeOnConfigure(0, width, height); }; } +void CShellSurfaceWlShell::AckConfigure(std::uint32_t) +{ +} + void CShellSurfaceWlShell::Initialize() { // Nothing to do here - constructor already handles it diff --git a/xbmc/windowing/wayland/ShellSurfaceWlShell.h b/xbmc/windowing/wayland/ShellSurfaceWlShell.h index 96fdd268eb5fe..cb5e7d013d642 100644 --- a/xbmc/windowing/wayland/ShellSurfaceWlShell.h +++ b/xbmc/windowing/wayland/ShellSurfaceWlShell.h @@ -53,6 +53,7 @@ class CShellSurfaceWlShell : public IShellSurface void SetFullScreen(wayland::output_t const& output, float refreshRate) override; void SetWindowed() override; + void AckConfigure(std::uint32_t serial) override; }; } diff --git a/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.cpp b/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.cpp index d216a28d745cd..b73aca6ca1fd0 100644 --- a/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.cpp +++ b/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.cpp @@ -36,9 +36,14 @@ CShellSurfaceXdgShellUnstableV6::CShellSurfaceXdgShellUnstableV6(wayland::displa // surface to be if (m_configuredWidth != 0 && m_configuredHeight != 0) { - InvokeOnConfigure(m_configuredWidth, m_configuredHeight); + InvokeOnConfigure(serial, m_configuredWidth, m_configuredHeight); + } + else + { + // WinSystem does not get the configure notification, so ack must be sent + // here + AckConfigure(serial); } - m_xdgSurface.ack_configure(serial); }; m_xdgToplevel.on_close() = [this]() { @@ -62,6 +67,11 @@ void CShellSurfaceXdgShellUnstableV6::Initialize() m_display->roundtrip(); } +void CShellSurfaceXdgShellUnstableV6::AckConfigure(std::uint32_t serial) +{ + m_xdgSurface.ack_configure(serial); +} + CShellSurfaceXdgShellUnstableV6::~CShellSurfaceXdgShellUnstableV6() { // xdg_shell is picky: must destroy toplevel role before surface diff --git a/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.h b/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.h index 4cd0108913b6a..42b54b4ccefac 100644 --- a/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.h +++ b/xbmc/windowing/wayland/ShellSurfaceXdgShellUnstableV6.h @@ -64,6 +64,7 @@ class CShellSurfaceXdgShellUnstableV6 : public IShellSurface void SetFullScreen(wayland::output_t const& output, float refreshRate) override; void SetWindowed() override; + void AckConfigure(std::uint32_t serial) override; }; } diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 6dbd1c2ef2078..039e0de675b99 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -154,11 +154,12 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, // Just remember initial width/height for context creation // This is used for sizing the EGLSurface - m_shellSurface->OnConfigure() = [this](std::int32_t width, std::int32_t height) + m_shellSurface->OnConfigure() = [this](std::uint32_t serial, std::int32_t width, std::int32_t height) { CLog::Log(LOGINFO, "Got initial Wayland surface size %dx%d", width, height); m_nWidth = width; m_nHeight = height; + AckConfigure(serial); }; if (fullScreen) @@ -173,8 +174,6 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_shellSurface->Initialize(); - // Set real handler during runtime - m_shellSurface->OnConfigure() = std::bind(&CWinSystemWayland::HandleSurfaceConfigure, this, _1, _2); if (m_nWidth == 0 || m_nHeight == 0) { @@ -187,6 +186,8 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, // Update resolution with real size UpdateDesktopResolution(res, 0, m_nWidth, m_nHeight, res.fRefreshRate); } + // Set real handler during runtime + m_shellSurface->OnConfigure() = std::bind(&CWinSystemWayland::HandleSurfaceConfigure, this, _1, _2, _3); // Now start processing events // @@ -405,17 +406,56 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo m_shellSurface->SetWindowed(); } + if (wasConfigure) + { + AckConfigure(m_currentConfigureSerial); + } + bool wasInitialSetFullScreen = m_isInitialSetFullScreen; m_isInitialSetFullScreen = false; - // Need to return true: + // Need to return true // * when this SetFullScreen() call was initiated by a configure() event // * on first SetFullScreen so GraphicsContext gets resolution // Otherwise, Kodi must keep the old resolution. return wasConfigure || wasInitialSetFullScreen; } -void CWinSystemWayland::HandleSurfaceConfigure(std::int32_t width, std::int32_t height) +void CWinSystemWayland::HandleSurfaceConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height) +{ + CSingleLock lock(g_graphicsContext); + CLog::LogF(LOGDEBUG, "Configure serial %u: size %dx%d", serial, width, height); + m_currentConfigureSerial = serial; + if (!ResetSurfaceSize(width, height, m_scale)) + { + // nothing changed, ack immediately + AckConfigure(serial); + } + // configure is acked when the Kodi surface has actually been reconfigured +} + +void CWinSystemWayland::AckConfigure(std::uint32_t serial) +{ + // Send ack if we have a new serial number or this is the first time + // this function is called + if (serial != m_lastAckedSerial || !m_firstSerialAcked) + { + CLog::LogF(LOGDEBUG, "Acking serial %u", serial); + m_shellSurface->AckConfigure(serial); + m_lastAckedSerial = serial; + m_firstSerialAcked = true; + } +} + +/** + * Set the internal surface size variables and perform resolution change + * + * Call only from Wayland event processing thread! + * + * \return Whether surface parameters changed and video resolution change was + * performed + */ +bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height, std::int32_t scale) { // Wayland will tell us here the size of the surface that was actually created, // which might be different from what we expected e.g. when fullscreening @@ -486,6 +526,8 @@ void CWinSystemWayland::HandleSurfaceConfigure(std::int32_t width, std::int32_t // FIXME Ideally this class would be completely decoupled from g_graphicsContext, // but this is not possible at the moment before the refactoring is done. g_graphicsContext.SetVideoResolution(switchToRes, true); + + return true; } std::string CWinSystemWayland::UserFriendlyOutputName(const COutput& output) diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 8cd7aaa4628dc..990ca4f1ad881 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -92,7 +92,8 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC protected: void LoadDefaultCursor(); void SendFocusChange(bool focus); - virtual void HandleSurfaceConfigure(std::int32_t width, std::int32_t height); + void HandleSurfaceConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height); + bool ResetSurfaceSize(std::int32_t width, std::int32_t height, std::int32_t scale); std::string UserFriendlyOutputName(COutput const& output); COutput* FindOutputByUserFriendlyName(std::string const& name); @@ -100,6 +101,7 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC // Called when wl_output::done is received for an output, i.e. associated // information like modes is available void OnOutputDone(std::uint32_t name); + void AckConfigure(std::uint32_t serial); std::unique_ptr m_connection; wayland::surface_t m_surface; @@ -121,6 +123,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC CCriticalSection m_dispResourcesMutex; std::string m_currentOutput; + std::uint32_t m_currentConfigureSerial = 0; + bool m_firstSerialAcked = false; + std::uint32_t m_lastAckedSerial = 0; // Whether this is the first call to SetFullScreen bool m_isInitialSetFullScreen = true; }; From 8954ef50aae182c8030fbf2e851a741ff44f547b Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:55:20 +0200 Subject: [PATCH 10/22] Make COutput references in CWinSystemWayland shared --- xbmc/windowing/wayland/WinSystemWayland.cpp | 84 +++++++++++---------- xbmc/windowing/wayland/WinSystemWayland.h | 7 +- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 039e0de675b99..42c8d66755da5 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -64,6 +64,14 @@ RESOLUTION FindMatchingCustomResolution(int width, int height, float refreshRate return RES_INVALID; } +struct OutputCurrentRefreshRateComparer +{ + bool operator()(std::shared_ptr const& output1, std::shared_ptr const& output2) + { + return output1->GetCurrentMode().refreshMilliHz < output2->GetCurrentMode().refreshMilliHz; + } +}; + const std::string CONFIGURE_RES_ID = "configure"; } @@ -165,7 +173,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, if (fullScreen) { // Try to start on correct monitor - COutput* output = FindOutputByUserFriendlyName(CServiceBroker::GetSettings().GetString(CSettings::SETTING_VIDEOSCREEN_MONITOR)); + auto output = FindOutputByUserFriendlyName(CServiceBroker::GetSettings().GetString(CSettings::SETTING_VIDEOSCREEN_MONITOR)); if (output) { m_shellSurface->SetFullScreen(output->GetWaylandOutput(), res.fRefreshRate); @@ -251,7 +259,7 @@ void CWinSystemWayland::GetConnectedOutputs(std::vector* outputs) std::transform(m_outputs.cbegin(), m_outputs.cend(), std::back_inserter(*outputs), [this](decltype(m_outputs)::value_type const& pair) { - return UserFriendlyOutputName(*pair.second); }); + return UserFriendlyOutputName(pair.second); }); } void CWinSystemWayland::UpdateResolutions() @@ -272,7 +280,7 @@ void CWinSystemWayland::UpdateResolutions() return; } - COutput* output = FindOutputByUserFriendlyName(userOutput); + auto output = FindOutputByUserFriendlyName(userOutput); if (!output) { // Fallback to current output @@ -281,10 +289,10 @@ void CWinSystemWayland::UpdateResolutions() if (!output) { // Well just use the first one - output = m_outputs.begin()->second.get(); + output = m_outputs.begin()->second; } - std::string outputName = UserFriendlyOutputName(*output); + std::string outputName = UserFriendlyOutputName(output); auto const& modes = output->GetModes(); auto const& currentMode = output->GetCurrentMode(); @@ -321,21 +329,28 @@ bool CWinSystemWayland::ResizeWindow(int newWidth, int newHeight, int newLeft, i return false; } -/** - * Find output by name - * - * Every caller of this function must hold a lock \ref m_outputsMutex - * for the duration of the call and as long as it uses the returned COutput. - */ -COutput* CWinSystemWayland::FindOutputByUserFriendlyName(const std::string& name) +std::shared_ptr CWinSystemWayland::FindOutputByUserFriendlyName(const std::string& name) { + CSingleLock lock(m_outputsMutex); auto outputIt = std::find_if(m_outputs.begin(), m_outputs.end(), [this, &name](decltype(m_outputs)::value_type const& entry) { - return (name == UserFriendlyOutputName(*entry.second)); + return (name == UserFriendlyOutputName(entry.second)); }); - return (outputIt == m_outputs.end() ? nullptr : outputIt->second.get()); + return (outputIt == m_outputs.end() ? nullptr : outputIt->second); +} + +std::shared_ptr CWinSystemWayland::FindOutputByWaylandOutput(wayland::output_t const& output) +{ + CSingleLock lock(m_outputsMutex); + auto outputIt = std::find_if(m_outputs.begin(), m_outputs.end(), + [this, &output](decltype(m_outputs)::value_type const& entry) + { + return (output == entry.second->GetWaylandOutput()); + }); + + return (outputIt == m_outputs.end() ? nullptr : outputIt->second); } bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, bool blankOtherDisplays) @@ -372,25 +387,18 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo m_currentOutput = res.strOutput; // Try to match output - wayland::output_t output; + auto output = FindOutputByUserFriendlyName(res.strOutput); + if (output) + { + CLog::LogF(LOGDEBUG, "Resolved output \"%s\" to bound Wayland global %u", res.strOutput.c_str(), output->GetGlobalName()); + } + else { - CSingleLock lock(m_outputsMutex); - - COutput* outputHandler = FindOutputByUserFriendlyName(res.strOutput); - if (outputHandler) - { - output = outputHandler->GetWaylandOutput(); - CLog::LogF(LOGDEBUG, "Resolved output \"%s\" to bound Wayland global %u", res.strOutput.c_str(), outputHandler->GetGlobalName()); - } - else - { - CLog::LogF(LOGINFO, "Could not match output \"%s\" to a currently available Wayland output, falling back to default output", res.strOutput.c_str()); - } - // Release lock only when output_t has been assigned to local variable so it - // cannot go away + CLog::LogF(LOGINFO, "Could not match output \"%s\" to a currently available Wayland output, falling back to default output", res.strOutput.c_str()); } + CLog::LogF(LOGDEBUG, "Setting full-screen with refresh rate %.3f", res.fRefreshRate); - m_shellSurface->SetFullScreen(output, res.fRefreshRate); + m_shellSurface->SetFullScreen(output ? output->GetWaylandOutput() : wayland::output_t(), res.fRefreshRate); } else { @@ -484,9 +492,9 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height // Get actual frame rate from monitor // TODO Track wl_surface.enter() events and get frame rate of the output // we are actually on - { + { CSingleLock lockOut(m_outputsMutex); - COutput* output = FindOutputByUserFriendlyName(m_currentOutput); + auto output = FindOutputByUserFriendlyName(m_currentOutput); if (output) { m_fRefreshRate = output->GetCurrentMode().refreshMilliHz / 1000.0f; @@ -530,16 +538,16 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height return true; } -std::string CWinSystemWayland::UserFriendlyOutputName(const COutput& output) +std::string CWinSystemWayland::UserFriendlyOutputName(std::shared_ptr const& output) { std::vector parts; - if (!output.GetMake().empty()) + if (!output->GetMake().empty()) { - parts.emplace_back(output.GetMake()); + parts.emplace_back(output->GetMake()); } - if (!output.GetModel().empty()) + if (!output->GetModel().empty()) { - parts.emplace_back(output.GetModel()); + parts.emplace_back(output->GetModel()); } if (parts.empty()) { @@ -549,7 +557,7 @@ std::string CWinSystemWayland::UserFriendlyOutputName(const COutput& output) // Add position std::int32_t x, y; - std::tie(x, y) = output.GetPosition(); + std::tie(x, y) = output->GetPosition(); if (x != 0 || y != 0) { parts.emplace_back(StringUtils::Format("@{}x{}", x, y)); diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 990ca4f1ad881..214b06e1f0642 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -95,8 +95,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC void HandleSurfaceConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height); bool ResetSurfaceSize(std::int32_t width, std::int32_t height, std::int32_t scale); - std::string UserFriendlyOutputName(COutput const& output); - COutput* FindOutputByUserFriendlyName(std::string const& name); + std::string UserFriendlyOutputName(std::shared_ptr const& output); + std::shared_ptr FindOutputByUserFriendlyName(std::string const& name); + std::shared_ptr FindOutputByWaylandOutput(wayland::output_t const& output); // Called when wl_output::done is received for an output, i.e. associated // information like modes is available @@ -110,7 +111,7 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC std::map m_seatProcessors; CCriticalSection m_seatProcessorsMutex; // m_outputsInPreparation did not receive their done event yet - std::map> m_outputs, m_outputsInPreparation; + std::map> m_outputs, m_outputsInPreparation; CCriticalSection m_outputsMutex; bool m_osCursorVisible = true; From fa566e3d3d63758f3ab9f82a9e6ea2639663860e Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:58:01 +0200 Subject: [PATCH 11/22] Introduce common function for updating surface size that honors scale --- xbmc/windowing/wayland/WinSystemWayland.cpp | 51 +++++++++++++-------- xbmc/windowing/wayland/WinSystemWayland.h | 4 ++ 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 42c8d66755da5..2b48bf628e223 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -146,8 +146,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_surface = m_connection->GetCompositor().create_surface(); // Try to get this resolution if compositor does not say otherwise - m_nWidth = res.iWidth; - m_nHeight = res.iHeight; + SetSizeFromSurfaceSize(res.iWidth, res.iHeight); auto xdgShell = m_connection->GetXdgShellUnstableV6(); if (xdgShell) @@ -165,8 +164,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_shellSurface->OnConfigure() = [this](std::uint32_t serial, std::int32_t width, std::int32_t height) { CLog::Log(LOGINFO, "Got initial Wayland surface size %dx%d", width, height); - m_nWidth = width; - m_nHeight = height; + SetSizeFromSurfaceSize(width, height); AckConfigure(serial); }; @@ -182,18 +180,9 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, m_shellSurface->Initialize(); + // Update resolution with real size as it could have changed due to configure() + UpdateDesktopResolution(res, 0, m_nWidth, m_nHeight, res.fRefreshRate); - if (m_nWidth == 0 || m_nHeight == 0) - { - // Adopt size from resolution if compositor did not specify anything - m_nWidth = res.iWidth; - m_nHeight = res.iHeight; - } - else - { - // Update resolution with real size - UpdateDesktopResolution(res, 0, m_nWidth, m_nHeight, res.fRefreshRate); - } // Set real handler during runtime m_shellSurface->OnConfigure() = std::bind(&CWinSystemWayland::HandleSurfaceConfigure, this, _1, _2, _3); @@ -486,8 +475,7 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height CLog::Log(LOGINFO, "Got new Wayland surface size %dx%d", m_nWidth, m_nHeight); // Now update actual resolution with configured one - m_nWidth = width; - m_nHeight = height; + bool sizeChanged = SetSizeFromSurfaceSize(width, height); // Get actual frame rate from monitor // TODO Track wl_surface.enter() events and get frame rate of the output @@ -502,13 +490,13 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height } // Find matching Kodi resolution member - switchToRes = FindMatchingCustomResolution(width, height, m_fRefreshRate); + switchToRes = FindMatchingCustomResolution(m_nWidth, m_nHeight, m_fRefreshRate); if (switchToRes == RES_INVALID) { // Add new resolution if none found RESOLUTION_INFO newResInfo; - UpdateDesktopResolution(newResInfo, 0, width, height, m_fRefreshRate); + UpdateDesktopResolution(newResInfo, 0, m_nWidth, m_nHeight, m_fRefreshRate); newResInfo.strOutput = m_currentOutput; // we just assume the compositor put us on the right output CDisplaySettings::GetInstance().AddResolutionInfo(newResInfo); CDisplaySettings::GetInstance().ApplyCalibrations(); @@ -538,6 +526,31 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height return true; } +/** + * Calculate internal resolution from surface size and set variables + * + * \return whether any size variable changed + */ +bool CWinSystemWayland::SetSizeFromSurfaceSize(std::int32_t surfaceWidth, std::int32_t surfaceHeight) +{ + std::int32_t newWidth = surfaceWidth * m_scale; + std::int32_t newHeight = surfaceHeight * m_scale; + + if (surfaceWidth != m_surfaceWidth || surfaceHeight != m_surfaceHeight || newWidth != m_nWidth || newHeight != m_nHeight) + { + m_surfaceWidth = surfaceWidth; + m_surfaceHeight = surfaceHeight; + m_nWidth = newWidth; + m_nHeight = newHeight; + CLog::LogF(LOGINFO, "Set surface size %dx%d at scale %d -> resolution %dx%d", m_surfaceWidth, m_surfaceHeight, m_scale, m_nWidth, m_nHeight); + return true; + } + else + { + return false; + } +} + std::string CWinSystemWayland::UserFriendlyOutputName(std::shared_ptr const& output) { std::vector parts; diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 214b06e1f0642..337feaac0c94e 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -94,6 +94,7 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC void SendFocusChange(bool focus); void HandleSurfaceConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height); bool ResetSurfaceSize(std::int32_t width, std::int32_t height, std::int32_t scale); + bool SetSizeFromSurfaceSize(std::int32_t surfaceWidth, std::int32_t surfaceHeight); std::string UserFriendlyOutputName(std::shared_ptr const& output); std::shared_ptr FindOutputByUserFriendlyName(std::string const& name); @@ -124,6 +125,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC CCriticalSection m_dispResourcesMutex; std::string m_currentOutput; + // Size of our surface in "surface coordinates", i.e. without scaling applied + std::int32_t m_surfaceWidth, m_surfaceHeight; + std::int32_t m_scale = 1; std::uint32_t m_currentConfigureSerial = 0; bool m_firstSerialAcked = false; std::uint32_t m_lastAckedSerial = 0; From daf39a1f13090118e63b89cda775f6bb03102e94 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 13:59:49 +0200 Subject: [PATCH 12/22] Keep track of active outputs and set scale accordingly Closes #2 --- xbmc/windowing/wayland/WinSystemWayland.cpp | 114 ++++++++++++++++---- xbmc/windowing/wayland/WinSystemWayland.h | 6 ++ 2 files changed, 97 insertions(+), 23 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 2b48bf628e223..0f17ad459b9b9 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -64,6 +64,14 @@ RESOLUTION FindMatchingCustomResolution(int width, int height, float refreshRate return RES_INVALID; } +struct OutputScaleComparer +{ + bool operator()(std::shared_ptr const& output1, std::shared_ptr const& output2) + { + return output1->GetScale() < output2->GetScale(); + } +}; + struct OutputCurrentRefreshRateComparer { bool operator()(std::shared_ptr const& output1, std::shared_ptr const& output2) @@ -76,7 +84,6 @@ const std::string CONFIGURE_RES_ID = "configure"; } - CWinSystemWayland::CWinSystemWayland() : CWinSystemBase() { @@ -134,6 +141,7 @@ bool CWinSystemWayland::DestroyWindowSystem() m_seatProcessors.clear(); m_outputsInPreparation.clear(); m_outputs.clear(); + m_surfaceOutputs.clear(); m_connection.reset(); return CWinSystemBase::DestroyWindowSystem(); @@ -144,6 +152,32 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, RESOLUTION_INFO& res) { m_surface = m_connection->GetCompositor().create_surface(); + m_surface.on_enter() = [this](wayland::output_t wloutput) + { + if (auto output = FindOutputByWaylandOutput(wloutput)) + { + CLog::Log(LOGDEBUG, "Entering output \"%s\" with scale %d", UserFriendlyOutputName(output).c_str(), output->GetScale()); + m_surfaceOutputs.emplace(output); + UpdateBufferScale(); + } + else + { + CLog::Log(LOGWARNING, "Entering output that was not configured yet, ignoring"); + } + }; + m_surface.on_leave() = [this](wayland::output_t wloutput) + { + if (auto output = FindOutputByWaylandOutput(wloutput)) + { + CLog::Log(LOGDEBUG, "Leaving output \"%s\" with scale %d", UserFriendlyOutputName(output).c_str(), output->GetScale()); + m_surfaceOutputs.erase(output); + UpdateBufferScale(); + } + else + { + CLog::Log(LOGWARNING, "Leaving output that was not configured yet, ignoring"); + } + }; // Try to get this resolution if compositor does not say otherwise SetSizeFromSurfaceSize(res.iWidth, res.iHeight); @@ -170,11 +204,13 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, if (fullScreen) { - // Try to start on correct monitor + // Try to start on correct monitor and with correct buffer scale auto output = FindOutputByUserFriendlyName(CServiceBroker::GetSettings().GetString(CSettings::SETTING_VIDEOSCREEN_MONITOR)); if (output) { m_shellSurface->SetFullScreen(output->GetWaylandOutput(), res.fRefreshRate); + m_scale = output->GetScale(); + ApplyBufferScale(m_scale); } } @@ -405,6 +441,11 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo if (wasConfigure) { + // Buffer scale must also match egl size configuration + ApplyBufferScale(m_scale); + + // Next buffer that the graphic context attaches will have the size corresponding + // to this configure, so go and ack it AckConfigure(m_currentConfigureSerial); } @@ -472,9 +513,8 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height // FIXME See comment in SetFullScreen CSingleLock lock(g_graphicsContext); - CLog::Log(LOGINFO, "Got new Wayland surface size %dx%d", m_nWidth, m_nHeight); - // Now update actual resolution with configured one + m_scale = scale; bool sizeChanged = SetSizeFromSurfaceSize(width, height); // Get actual frame rate from monitor @@ -652,7 +692,8 @@ void CWinSystemWayland::Unregister(IDispResource* resource) void CWinSystemWayland::OnSeatAdded(std::uint32_t name, wayland::seat_t& seat) { CSingleLock lock(m_seatProcessorsMutex); - m_seatProcessors.emplace(std::piecewise_construct, std::forward_as_tuple(name), std::forward_as_tuple(name, seat, this)); + auto newSeatEmplace = m_seatProcessors.emplace(std::piecewise_construct, std::forward_as_tuple(name), std::forward_as_tuple(name, seat, this)); + newSeatEmplace.first->second.SetCoordinateScale(m_scale); } void CWinSystemWayland::OnOutputAdded(std::uint32_t name, wayland::output_t& output) @@ -664,28 +705,31 @@ void CWinSystemWayland::OnOutputAdded(std::uint32_t name, wayland::output_t& out void CWinSystemWayland::OnOutputDone(std::uint32_t name) { auto it = m_outputsInPreparation.find(name); - if (it == m_outputsInPreparation.end()) + if (it != m_outputsInPreparation.end()) { - return; - } + // This output was added for the first time - done is also sent when + // output parameters change later - { - CSingleLock lock(m_outputsMutex); - // Move from m_outputsInPreparation to m_outputs - m_outputs.emplace(std::move(*it)); - m_outputsInPreparation.erase(it); - } + { + CSingleLock lock(m_outputsMutex); + // Move from m_outputsInPreparation to m_outputs + m_outputs.emplace(std::move(*it)); + m_outputsInPreparation.erase(it); + } - // Maybe the output that was added was the one we should be on? - if (m_bFullScreen) - { - CSingleLock lock(g_graphicsContext); - UpdateResolutions(); - // This will call SetFullScreen(), which will match the output against - // the information from the resolution and call set_fullscreen on the - // surface if it changed. - g_graphicsContext.SetVideoResolution(g_graphicsContext.GetVideoResolution(), true); + // Maybe the output that was added was the one we should be on? + if (m_bFullScreen) + { + CSingleLock lock(g_graphicsContext); + UpdateResolutions(); + // This will call SetFullScreen(), which will match the output against + // the information from the resolution and call set_fullscreen on the + // surface if it changed. + g_graphicsContext.SetVideoResolution(g_graphicsContext.GetVideoResolution(), true); + } } + + UpdateBufferScale(); } void CWinSystemWayland::OnGlobalRemoved(std::uint32_t name) @@ -763,6 +807,30 @@ void CWinSystemWayland::OnSetCursor(wayland::pointer_t& pointer, std::uint32_t s } } +void CWinSystemWayland::UpdateBufferScale() +{ + // Adjust our surface size to the output with the biggest scale in order + // to get the best quality + auto const maxBufferScaleIt = std::max_element(m_surfaceOutputs.cbegin(), m_surfaceOutputs.cend(), OutputScaleComparer()); + if (maxBufferScaleIt != m_surfaceOutputs.cend()) + { + auto const newScale = (*maxBufferScaleIt)->GetScale(); + // Recalculate resolution with new scale if it changed + ResetSurfaceSize(m_surfaceWidth, m_surfaceHeight, newScale); + } +} + +void CWinSystemWayland::ApplyBufferScale(std::int32_t scale) +{ + CLog::LogF(LOGINFO, "Setting Wayland buffer scale to %d", scale); + m_surface.set_buffer_scale(scale); + CSingleLock lock(m_seatProcessorsMutex); + for (auto& seatProcessor : m_seatProcessors) + { + seatProcessor.second.SetCoordinateScale(scale); + } +} + #if defined(HAVE_LIBVA) void* CWinSystemWayland::GetVaDisplay() { diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 337feaac0c94e..832461a9bfc00 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -103,6 +103,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC // Called when wl_output::done is received for an output, i.e. associated // information like modes is available void OnOutputDone(std::uint32_t name); + void UpdateBufferScale(); + void ApplyBufferScale(std::int32_t scale); + void AckConfigure(std::uint32_t serial); std::unique_ptr m_connection; @@ -125,6 +128,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC CCriticalSection m_dispResourcesMutex; std::string m_currentOutput; + // Set of outputs that show some part of our main surface as indicated by + // compositor + std::set> m_surfaceOutputs; // Size of our surface in "surface coordinates", i.e. without scaling applied std::int32_t m_surfaceWidth, m_surfaceHeight; std::int32_t m_scale = 1; From e869fb6fc88df8a49b76fc039aa4d794633f0938 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 14:00:26 +0200 Subject: [PATCH 13/22] Move opaque region setting to SetFullScreen so that the opaque region matches the surface size that gets attached --- xbmc/windowing/wayland/WinSystemWayland.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 0f17ad459b9b9..ac1bcf5f94c93 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -441,6 +441,12 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo if (wasConfigure) { + // Mark everything opaque so the compositor can render it faster + // Do it here so size always matches the configured egl surface + CLog::LogF(LOGDEBUG, "Setting opaque region size %dx%d", m_surfaceWidth, m_surfaceHeight); + wayland::region_t opaqueRegion = m_connection->GetCompositor().create_region(); + opaqueRegion.add(0, 0, m_surfaceWidth, m_surfaceHeight); + m_surface.set_opaque_region(opaqueRegion); // Buffer scale must also match egl size configuration ApplyBufferScale(m_scale); @@ -504,12 +510,6 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height RESOLUTION switchToRes = RES_INVALID; - // Mark everything opaque so the compositor can render it faster - wayland::region_t opaqueRegion = m_connection->GetCompositor().create_region(); - opaqueRegion.add(0, 0, width, height); - m_surface.set_opaque_region(opaqueRegion); - // No surface commit, EGL context will do that when it changes the buffer - // FIXME See comment in SetFullScreen CSingleLock lock(g_graphicsContext); From f864935e5de41b4b68bb81f097400a24c301f620 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 14:01:06 +0200 Subject: [PATCH 14/22] Update log messages --- xbmc/windowing/wayland/WinSystemWayland.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index ac1bcf5f94c93..2163c92a3dd97 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -179,7 +179,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, } }; - // Try to get this resolution if compositor does not say otherwise + // Try with this resolution if compositor does not say otherwise SetSizeFromSurfaceSize(res.iWidth, res.iHeight); auto xdgShell = m_connection->GetXdgShellUnstableV6(); @@ -189,7 +189,7 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, } else { - CLog::Log(LOGWARNING, "Compositor does not support xdg_shell unstable v6 protocol - falling back to wl_shell, not all features might work"); + CLog::LogF(LOGWARNING, "Compositor does not support xdg_shell unstable v6 protocol - falling back to wl_shell, not all features might work"); m_shellSurface.reset(new CShellSurfaceWlShell(m_connection->GetShell(), m_surface, name, "kodi")); } @@ -322,13 +322,13 @@ void CWinSystemWayland::UpdateResolutions() auto const& modes = output->GetModes(); auto const& currentMode = output->GetCurrentMode(); auto physicalSize = output->GetPhysicalSize(); - CLog::Log(LOGINFO, "User wanted output \"%s\", we now have \"%s\" size %dx%d mm with %zu mode(s):", userOutput.c_str(), outputName.c_str(), std::get<0>(physicalSize), std::get<1>(physicalSize), modes.size()); + CLog::LogF(LOGINFO, "User wanted output \"%s\", we now have \"%s\" size %dx%d mm with %zu mode(s):", userOutput.c_str(), outputName.c_str(), std::get<0>(physicalSize), std::get<1>(physicalSize), modes.size()); for (auto const& mode : modes) { bool isCurrent = (mode == currentMode); float pixelRatio = output->GetPixelRatioForMode(mode); - CLog::Log(LOGINFO, "- %dx%d @%.3f Hz pixel ratio %.3f%s", mode.width, mode.height, mode.refreshMilliHz / 1000.0f, pixelRatio, isCurrent ? " current" : ""); + CLog::LogF(LOGINFO, "- %dx%d @%.3f Hz pixel ratio %.3f%s", mode.width, mode.height, mode.refreshMilliHz / 1000.0f, pixelRatio, isCurrent ? " current" : ""); RESOLUTION_INFO res; UpdateDesktopResolution(res, 0, mode.width, mode.height, mode.refreshMilliHz / 1000.0f); From f6e132d585fb5cb27b525877895f18b830ea29d6 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 14:01:17 +0200 Subject: [PATCH 15/22] Correctly reset resolution configure flag --- xbmc/windowing/wayland/WinSystemWayland.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 2163c92a3dd97..ae5eef4e25e84 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -401,7 +401,13 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo m_bFullScreen = fullScreen; bool wasConfigure = (res.strId == CONFIGURE_RES_ID); - res.strId = ""; + // Reset configure flag + // Setting it in res will not modify the global information in CDisplaySettings + // and we don't know which resolution index this is, so just reset all + for (size_t resIdx = RES_DESKTOP; resIdx < CDisplaySettings::GetInstance().ResolutionInfoSize(); resIdx++) + { + CDisplaySettings::GetInstance().GetResolutionInfo(resIdx).strId = ""; + } if (fullScreen) { From 639932b8b1b039a4a6eea120c80dca919b5e0af8 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 14:01:31 +0200 Subject: [PATCH 16/22] Do not reconfigure Kodi context when size/scale/refreshrate stay the same --- xbmc/windowing/wayland/WinSystemWayland.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index ae5eef4e25e84..80db2b42d61df 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -520,9 +520,11 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height CSingleLock lock(g_graphicsContext); // Now update actual resolution with configured one + bool scaleChanged = (scale != m_scale); m_scale = scale; bool sizeChanged = SetSizeFromSurfaceSize(width, height); - + + float refreshRate = m_fRefreshRate; // Get actual frame rate from monitor // TODO Track wl_surface.enter() events and get frame rate of the output // we are actually on @@ -531,10 +533,16 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height auto output = FindOutputByUserFriendlyName(m_currentOutput); if (output) { - m_fRefreshRate = output->GetCurrentMode().refreshMilliHz / 1000.0f; + refreshRate = output->GetCurrentMode().refreshMilliHz / 1000.0f; } } + if (refreshRate == m_fRefreshRate && !scaleChanged && !sizeChanged) + { + CLog::LogF(LOGDEBUG, "No change in size, refresh rate, and scale, returning"); + return false; + } + // Find matching Kodi resolution member switchToRes = FindMatchingCustomResolution(m_nWidth, m_nHeight, m_fRefreshRate); From 027216474e126deb7638f94530b93bab006a2ae9 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 14:18:58 +0200 Subject: [PATCH 17/22] Get frame rate from actual output --- xbmc/windowing/wayland/WinSystemWayland.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 80db2b42d61df..ab5ce43f0a625 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -524,17 +524,14 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height m_scale = scale; bool sizeChanged = SetSizeFromSurfaceSize(width, height); + // Get actual frame rate from monitor, take highest frame rate if multiple + // m_surfaceOutputs is only updated from event handling thread, so no lock + auto maxRefreshIt = std::max_element(m_surfaceOutputs.cbegin(), m_surfaceOutputs.cend(), OutputCurrentRefreshRateComparer()); float refreshRate = m_fRefreshRate; - // Get actual frame rate from monitor - // TODO Track wl_surface.enter() events and get frame rate of the output - // we are actually on - { - CSingleLock lockOut(m_outputsMutex); - auto output = FindOutputByUserFriendlyName(m_currentOutput); - if (output) - { - refreshRate = output->GetCurrentMode().refreshMilliHz / 1000.0f; - } + if (maxRefreshIt != m_surfaceOutputs.cend()) + { + refreshRate = (*maxRefreshIt)->GetCurrentMode().refreshMilliHz / 1000.0f; + CLog::LogF(LOGDEBUG, "Resolved actual (maximum) refresh rate to %.3f Hz on output \"%s\"", refreshRate, UserFriendlyOutputName(*maxRefreshIt).c_str()); } if (refreshRate == m_fRefreshRate && !scaleChanged && !sizeChanged) @@ -543,6 +540,8 @@ bool CWinSystemWayland::ResetSurfaceSize(std::int32_t width, std::int32_t height return false; } + m_fRefreshRate = refreshRate; + // Find matching Kodi resolution member switchToRes = FindMatchingCustomResolution(m_nWidth, m_nHeight, m_fRefreshRate); From 9c09f70a70e652c41d9f8567df77ce41eb943aed Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 15:41:30 +0200 Subject: [PATCH 18/22] Set m_Resolution etc. for SetFullScreen() and reset if change fails --- xbmc/guilib/GraphicContext.cpp | 41 +++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/xbmc/guilib/GraphicContext.cpp b/xbmc/guilib/GraphicContext.cpp index f213b8835b751..09b4d2322b2e1 100644 --- a/xbmc/guilib/GraphicContext.cpp +++ b/xbmc/guilib/GraphicContext.cpp @@ -415,6 +415,21 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat RESOLUTION_INFO info_mod = GetResInfo(res); + // 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_Resolution = res; + m_fFPSOverride = 0; + bool switched = true; if (g_advancedSettings.m_fullScreen) { @@ -440,12 +455,7 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat if (switched) { - 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 ; // make sure all stereo stuff are correctly setup SetStereoView(RENDER_STEREO_VIEW_OFF); @@ -454,6 +464,27 @@ void CGraphicContext::SetVideoResolutionInternal(RESOLUTION res, bool forceUpdat 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(); } From af6abf2fa6a6b4d2918e8d13296632abf7537d87 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Wed, 21 Jun 2017 18:09:53 +0200 Subject: [PATCH 19/22] Reload skin on resolution change, but not while dialog is showing --- xbmc/settings/DisplaySettings.cpp | 40 ++++++++++++++++++--- xbmc/settings/DisplaySettings.h | 1 + xbmc/windowing/wayland/WinSystemWayland.cpp | 16 +++++++++ xbmc/windowing/wayland/WinSystemWayland.h | 6 +++- 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/xbmc/settings/DisplaySettings.cpp b/xbmc/settings/DisplaySettings.cpp index 2e5b92f0289ac..05df9853fd82c 100644 --- a/xbmc/settings/DisplaySettings.cpp +++ b/xbmc/settings/DisplaySettings.cpp @@ -288,14 +288,30 @@ bool CDisplaySettings::OnSettingChanging(std::shared_ptr 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 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; diff --git a/xbmc/settings/DisplaySettings.h b/xbmc/settings/DisplaySettings.h index 186dd70bdf2f4..8d3c0810f9eb0 100644 --- a/xbmc/settings/DisplaySettings.h +++ b/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; }; diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index ab5ce43f0a625..55e6b867f206d 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -456,6 +456,13 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo // Buffer scale must also match egl size configuration ApplyBufferScale(m_scale); + // FIXME This assumes that the resolution has already been set. Should + // be moved to some post-change callback when resolution setting is refactored. + if (!m_inhibitSkinReload) + { + g_application.ReloadSkin(); + } + // Next buffer that the graphic context attaches will have the size corresponding // to this configure, so go and ack it AckConfigure(m_currentConfigureSerial); @@ -471,6 +478,15 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo return wasConfigure || wasInitialSetFullScreen; } + +void CWinSystemWayland::SetInhibitSkinReload(bool inhibit) +{ + m_inhibitSkinReload = inhibit; + if (!inhibit) + { + g_application.ReloadSkin(); + } +} void CWinSystemWayland::HandleSurfaceConfigure(std::uint32_t serial, std::int32_t width, std::int32_t height) { CSingleLock lock(g_graphicsContext); diff --git a/xbmc/windowing/wayland/WinSystemWayland.h b/xbmc/windowing/wayland/WinSystemWayland.h index 832461a9bfc00..2803f567b826a 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.h +++ b/xbmc/windowing/wayland/WinSystemWayland.h @@ -69,6 +69,8 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC bool HasCursor() override; void ShowOSMouse(bool show) override; + + void SetInhibitSkinReload(bool inhibit); void* GetVaDisplay(); @@ -126,7 +128,9 @@ class CWinSystemWayland : public CWinSystemBase, public IInputHandler, public IC std::set m_dispResources; CCriticalSection m_dispResourcesMutex; - + + bool m_inhibitSkinReload = false; + std::string m_currentOutput; // Set of outputs that show some part of our main surface as indicated by // compositor From 9753b4ad0a20a16229ce506b4f75ea525dc7edea Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Thu, 22 Jun 2017 11:17:06 +0200 Subject: [PATCH 20/22] Remove display_t error checking which should be done in waylandpp --- xbmc/windowing/wayland/Connection.cpp | 5 +---- xbmc/windowing/wayland/WinEventsWayland.cpp | 5 +---- xbmc/windowing/wayland/WinSystemWayland.cpp | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/xbmc/windowing/wayland/Connection.cpp b/xbmc/windowing/wayland/Connection.cpp index db536cd61ffe2..5da0dfe389ece 100644 --- a/xbmc/windowing/wayland/Connection.cpp +++ b/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(); diff --git a/xbmc/windowing/wayland/WinEventsWayland.cpp b/xbmc/windowing/wayland/WinEventsWayland.cpp index f9e6fc3816aa2..88e5da8144396 100644 --- a/xbmc/windowing/wayland/WinEventsWayland.cpp +++ b/xbmc/windowing/wayland/WinEventsWayland.cpp @@ -127,10 +127,7 @@ class CWinEventsWaylandThread : CThread // Read events and release intent; this does not block readIntent.read(); // Dispatch default event queue - if (m_display->dispatch_pending() < 0) - { - throw std::system_error(errno, std::generic_category(), "Error dispatching Wayland events"); - } + m_display->dispatch_pending(); } CLog::Log(LOGDEBUG, "Wayland message pump stopped"); diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 55e6b867f206d..c384ff91b3a20 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -108,10 +108,7 @@ bool CWinSystemWayland::InitWindowSystem() CLog::Log(LOGWARNING, "Wayland compositor did not announce a wl_seat - you will not have any input devices for the time being"); } // Do another roundtrip to get initial wl_output information - if (m_connection->GetDisplay().roundtrip() < 0) - { - throw std::runtime_error("Wayland roundtrip failed"); - } + m_connection->GetDisplay().roundtrip(); if (m_outputs.empty()) { throw std::runtime_error("No outputs received from compositor"); From 09e7d21128e20168e3b417d6322f83f2f5e98dd1 Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Thu, 22 Jun 2017 11:17:24 +0200 Subject: [PATCH 21/22] Only modify buffer scale if compositor supports it --- xbmc/windowing/wayland/WinSystemWayland.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index c384ff91b3a20..3aed6a29439a2 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -206,8 +206,11 @@ bool CWinSystemWayland::CreateNewWindow(const std::string& name, if (output) { m_shellSurface->SetFullScreen(output->GetWaylandOutput(), res.fRefreshRate); - m_scale = output->GetScale(); - ApplyBufferScale(m_scale); + if (m_surface.can_set_buffer_scale()) + { + m_scale = output->GetScale(); + ApplyBufferScale(m_scale); + } } } @@ -450,8 +453,11 @@ bool CWinSystemWayland::SetFullScreen(bool fullScreen, RESOLUTION_INFO& res, boo wayland::region_t opaqueRegion = m_connection->GetCompositor().create_region(); opaqueRegion.add(0, 0, m_surfaceWidth, m_surfaceHeight); m_surface.set_opaque_region(opaqueRegion); - // Buffer scale must also match egl size configuration - ApplyBufferScale(m_scale); + if (m_surface.can_set_buffer_scale()) + { + // Buffer scale must also match egl size configuration + ApplyBufferScale(m_scale); + } // FIXME This assumes that the resolution has already been set. Should // be moved to some post-change callback when resolution setting is refactored. @@ -835,6 +841,12 @@ void CWinSystemWayland::OnSetCursor(wayland::pointer_t& pointer, std::uint32_t s void CWinSystemWayland::UpdateBufferScale() { + if (!m_surface || !m_surface.can_set_buffer_scale()) + { + // Never modify scale when we cannot set it + return; + } + // Adjust our surface size to the output with the biggest scale in order // to get the best quality auto const maxBufferScaleIt = std::max_element(m_surfaceOutputs.cbegin(), m_surfaceOutputs.cend(), OutputScaleComparer()); From 8fdbc62d237b76ef91de177246e66f87f07d0b2f Mon Sep 17 00:00:00 2001 From: Philipp Kerling Date: Fri, 23 Jun 2017 09:59:03 +0200 Subject: [PATCH 22/22] Add lock to FindMatchingCustomResolution --- xbmc/windowing/wayland/WinSystemWayland.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xbmc/windowing/wayland/WinSystemWayland.cpp b/xbmc/windowing/wayland/WinSystemWayland.cpp index 3aed6a29439a2..fd0b7194a7feb 100644 --- a/xbmc/windowing/wayland/WinSystemWayland.cpp +++ b/xbmc/windowing/wayland/WinSystemWayland.cpp @@ -50,9 +50,9 @@ using namespace std::placeholders; namespace { -// Caller should hold g_graphicsContext lock RESOLUTION FindMatchingCustomResolution(int width, int height, float refreshRate) { + CSingleLock lock(g_graphicsContext); for (size_t res = RES_DESKTOP; res < CDisplaySettings::GetInstance().ResolutionInfoSize(); ++res) { auto const& resInfo = CDisplaySettings::GetInstance().GetResolutionInfo(res);