From 8374725f988a6e46d30dda83f92ad67b8a55e75b Mon Sep 17 00:00:00 2001 From: reuk Date: Tue, 6 Dec 2022 17:03:21 +0000 Subject: [PATCH] XWindowSystem: Use ScopedWindowAssociation for improved DRYness --- modules/juce_gui_basics/juce_gui_basics.cpp | 1 + .../native/juce_linux_Windowing.cpp | 10 ++ .../x11/juce_linux_ScopedWindowAssociation.h | 118 ++++++++++++++++++ .../native/x11/juce_linux_XWindowSystem.cpp | 25 +--- modules/juce_gui_extra/juce_gui_extra.cpp | 1 + .../native/juce_linux_XEmbedComponent.cpp | 5 +- modules/juce_opengl/juce_opengl.cpp | 1 + .../native/juce_OpenGL_linux_X11.h | 81 +----------- 8 files changed, 141 insertions(+), 101 deletions(-) create mode 100644 modules/juce_gui_basics/native/x11/juce_linux_ScopedWindowAssociation.h diff --git a/modules/juce_gui_basics/juce_gui_basics.cpp b/modules/juce_gui_basics/juce_gui_basics.cpp index fa1761a68c..daa711279f 100644 --- a/modules/juce_gui_basics/juce_gui_basics.cpp +++ b/modules/juce_gui_basics/juce_gui_basics.cpp @@ -328,6 +328,7 @@ namespace juce JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wzero-as-null-pointer-constant") + #include "native/x11/juce_linux_ScopedWindowAssociation.h" #include "native/juce_linux_Windowing.cpp" #include "native/x11/juce_linux_XWindowSystem.cpp" diff --git a/modules/juce_gui_basics/native/juce_linux_Windowing.cpp b/modules/juce_gui_basics/native/juce_linux_Windowing.cpp index 29d702f4f6..76dc650d3d 100644 --- a/modules/juce_gui_basics/native/juce_linux_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_linux_Windowing.cpp @@ -392,6 +392,15 @@ public: } } + bool setWindowAssociation (::Window windowIn) + { + clearWindowAssociation(); + association = { this, windowIn }; + return association.isValid(); + } + + void clearWindowAssociation() { association = {}; } + //============================================================================== static bool isActiveApplication; bool focused = false; @@ -591,6 +600,7 @@ private: bool fullScreen = false, isAlwaysOnTop = false; double currentScaleFactor = 1.0; Array glRepaintListeners; + ScopedWindowAssociation association; //============================================================================== JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (LinuxComponentPeer) diff --git a/modules/juce_gui_basics/native/x11/juce_linux_ScopedWindowAssociation.h b/modules/juce_gui_basics/native/x11/juce_linux_ScopedWindowAssociation.h new file mode 100644 index 0000000000..82b9038853 --- /dev/null +++ b/modules/juce_gui_basics/native/x11/juce_linux_ScopedWindowAssociation.h @@ -0,0 +1,118 @@ +/* + ============================================================================== + + This file is part of the JUCE library. + Copyright (c) 2022 - Raw Material Software Limited + + JUCE is an open source library subject to commercial or open-source + licensing. + + By using JUCE, you agree to the terms of both the JUCE 7 End-User License + Agreement and JUCE Privacy Policy. + + End User License Agreement: www.juce.com/juce-7-licence + Privacy Policy: www.juce.com/juce-privacy-policy + + Or: You may also use this code under the terms of the GPL v3 (see + www.gnu.org/licenses). + + JUCE IS PROVIDED "AS IS" WITHOUT ANY WARRANTY, AND ALL WARRANTIES, WHETHER + EXPRESSED OR IMPLIED, INCLUDING MERCHANTABILITY AND FITNESS FOR PURPOSE, ARE + DISCLAIMED. + + ============================================================================== +*/ + +namespace juce +{ + +extern XContext windowHandleXContext; + +/* Attaches a pointer to a given window, so that it can be retrieved with XFindContext on + the windowHandleXContext. +*/ +class ScopedWindowAssociation +{ +public: + ScopedWindowAssociation() = default; + + ScopedWindowAssociation (void* associatedIn, Window windowIn) + : associatedPointer ([&]() -> void* + { + if (associatedIn == nullptr) + return nullptr; + + // If you hit this, there's already a pointer associated with this window. + const auto display = XWindowSystem::getInstance()->getDisplay(); + jassert (! getAssociatedPointer (display, windowIn).has_value()); + + if (X11Symbols::getInstance()->xSaveContext (display, + static_cast (windowIn), + windowHandleXContext, + unalignedPointerCast (associatedIn)) != 0) + { + jassertfalse; + return nullptr; + } + + return associatedIn; + }()), + window (static_cast (windowIn)) {} + + ScopedWindowAssociation (const ScopedWindowAssociation&) = delete; + ScopedWindowAssociation& operator= (const ScopedWindowAssociation&) = delete; + + ScopedWindowAssociation (ScopedWindowAssociation&& other) noexcept + : associatedPointer (std::exchange (other.associatedPointer, nullptr)), window (other.window) {} + + ScopedWindowAssociation& operator= (ScopedWindowAssociation&& other) noexcept + { + ScopedWindowAssociation { std::move (other) }.swap (*this); + return *this; + } + + ~ScopedWindowAssociation() noexcept + { + if (associatedPointer == nullptr) + return; + + const auto display = XWindowSystem::getInstance()->getDisplay(); + const auto ptr = getAssociatedPointer (display, window); + + if (! ptr.has_value()) + { + // If you hit this, something else has cleared this association before we were able to. + jassertfalse; + return; + } + + jassert (unalignedPointerCast (associatedPointer) == *ptr); + + if (X11Symbols::getInstance()->xDeleteContext (display, window, windowHandleXContext) != 0) + jassertfalse; + } + + bool isValid() const { return associatedPointer != nullptr; } + +private: + static std::optional getAssociatedPointer (Display* display, Window window) + { + XPointer ptr{}; + + if (X11Symbols::getInstance()->xFindContext (display, window, windowHandleXContext, &ptr) != 0) + return std::nullopt; + + return ptr; + } + + void swap (ScopedWindowAssociation& other) noexcept + { + std::swap (other.associatedPointer, associatedPointer); + std::swap (other.window, window); + } + + void* associatedPointer = nullptr; + XID window{}; +}; + +} // namespace juce diff --git a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp index 034812dfe3..6103b74497 100644 --- a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp +++ b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp @@ -1549,10 +1549,7 @@ static int getAllEventsMask (bool ignoresMouseClicks) &swa); // Set the window context to identify the window handle object - if (X11Symbols::getInstance()->xSaveContext (display, - static_cast (windowH), - windowHandleXContext, - unalignedPointerCast (peer)) != 0) + if (! peer->setWindowAssociation (windowH)) { // Failed jassertfalse; @@ -1634,13 +1631,7 @@ void XWindowSystem::destroyWindow (::Window windowH) XWindowSystemUtilities::ScopedXLock xLock; - XPointer handlePointer{}; - - if (X11Symbols::getInstance()->xFindContext (display, static_cast (windowH), windowHandleXContext, &handlePointer) == 0) - { - const auto result = X11Symbols::getInstance()->xDeleteContext (display, static_cast (windowH), windowHandleXContext); - jassert (result == 0); - } + peer->clearWindowAssociation(); X11Symbols::getInstance()->xDestroyWindow (display, windowH); @@ -2706,10 +2697,6 @@ Array XWindowSystem::findDisplays (float masterScale) const &swa); X11Symbols::getInstance()->xMapWindow (display, keyProxy); - X11Symbols::getInstance()->xSaveContext (display, - static_cast (keyProxy), - windowHandleXContext, - unalignedPointerCast (this)); return keyProxy; } @@ -2718,14 +2705,6 @@ void XWindowSystem::deleteKeyProxy (::Window keyProxy) const { jassert (keyProxy != 0); - XPointer handlePointer{}; - - if (X11Symbols::getInstance()->xFindContext (display, static_cast (keyProxy), windowHandleXContext, &handlePointer) == 0) - { - const auto result = X11Symbols::getInstance()->xDeleteContext (display, static_cast (keyProxy), windowHandleXContext); - jassert (result == 0); - } - X11Symbols::getInstance()->xDestroyWindow (display, keyProxy); X11Symbols::getInstance()->xSync (display, false); diff --git a/modules/juce_gui_extra/juce_gui_extra.cpp b/modules/juce_gui_extra/juce_gui_extra.cpp index bcffb999c8..9a50b88b42 100644 --- a/modules/juce_gui_extra/juce_gui_extra.cpp +++ b/modules/juce_gui_extra/juce_gui_extra.cpp @@ -168,6 +168,7 @@ #elif JUCE_LINUX || JUCE_BSD JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wzero-as-null-pointer-constant") + #include #include "native/juce_linux_XEmbedComponent.cpp" #if JUCE_WEB_BROWSER diff --git a/modules/juce_gui_extra/native/juce_linux_XEmbedComponent.cpp b/modules/juce_gui_extra/native/juce_linux_XEmbedComponent.cpp index a56538de15..768015277e 100644 --- a/modules/juce_gui_extra/native/juce_linux_XEmbedComponent.cpp +++ b/modules/juce_gui_extra/native/juce_linux_XEmbedComponent.cpp @@ -73,11 +73,13 @@ public: { SharedKeyWindow (ComponentPeer* peerToUse) : keyPeer (peerToUse), - keyProxy (juce_createKeyProxyWindow (keyPeer)) + keyProxy (juce_createKeyProxyWindow (keyPeer)), + association (peerToUse, keyProxy) {} ~SharedKeyWindow() { + association = {}; juce_deleteKeyProxyWindow (keyProxy); auto& keyWindows = getKeyWindows(); @@ -120,6 +122,7 @@ public: //============================================================================== ComponentPeer* keyPeer; Window keyProxy; + ScopedWindowAssociation association; static HashMap& getKeyWindows() { diff --git a/modules/juce_opengl/juce_opengl.cpp b/modules/juce_opengl/juce_opengl.cpp index 8a600470b5..66d7be3f8b 100644 --- a/modules/juce_opengl/juce_opengl.cpp +++ b/modules/juce_opengl/juce_opengl.cpp @@ -291,6 +291,7 @@ JUCE_IMPL_WGL_EXTENSION_FUNCTION (wglCreateContextAttribsARB) #undef JUCE_IMPL_WGL_EXTENSION_FUNCTION #elif JUCE_LINUX || JUCE_BSD + #include #include "native/juce_OpenGL_linux_X11.h" #elif JUCE_ANDROID diff --git a/modules/juce_opengl/native/juce_OpenGL_linux_X11.h b/modules/juce_opengl/native/juce_OpenGL_linux_X11.h index 5cc168f8cc..cd12b414f5 100644 --- a/modules/juce_opengl/native/juce_OpenGL_linux_X11.h +++ b/modules/juce_opengl/native/juce_OpenGL_linux_X11.h @@ -26,8 +26,6 @@ namespace juce { -extern XContext windowHandleXContext; - struct XFreeDeleter { void operator() (void* ptr) const @@ -45,84 +43,13 @@ std::unique_ptr makeXFreePtr (Data* raw) { return std::uniqu void juce_LinuxAddRepaintListener (ComponentPeer*, Component* dummy); void juce_LinuxRemoveRepaintListener (ComponentPeer*, Component* dummy); -/* Attaches a peer to a given window, so that it can be retrieved with XFindContext on - the windowHandleXContext. -*/ -class ScopedPeerAssociation -{ -public: - static ScopedPeerAssociation make (ComponentPeer* peerIn, Window windowIn) - { - if (peerIn == nullptr) - return { nullptr, windowIn }; - - const auto display = XWindowSystem::getInstance()->getDisplay(); - - if (X11Symbols::getInstance()->xSaveContext (display, - static_cast (windowIn), - windowHandleXContext, - unalignedPointerCast (peerIn)) != 0) - { - jassertfalse; - return { nullptr, windowIn }; - } - - return { peerIn, windowIn }; - } - - ScopedPeerAssociation (const ScopedPeerAssociation&) = delete; - ScopedPeerAssociation& operator= (const ScopedPeerAssociation&) = delete; - - ScopedPeerAssociation (ScopedPeerAssociation&& other) noexcept - : peer (std::exchange (other.peer, nullptr)), window (other.window) {} - - ScopedPeerAssociation& operator= (ScopedPeerAssociation&& other) noexcept - { - ScopedPeerAssociation { std::move (other) }.swap (*this); - return *this; - } - - ~ScopedPeerAssociation() noexcept - { - if (peer == nullptr) - return; - - const auto display = XWindowSystem::getInstance()->getDisplay(); - XPointer ptr{}; - - if (X11Symbols::getInstance()->xFindContext (display, window, windowHandleXContext, &ptr) != 0) - { - jassertfalse; - return; - } - - jassert (unalignedPointerCast (peer) == ptr); - - if (X11Symbols::getInstance()->xDeleteContext (display, window, windowHandleXContext) != 0) - jassertfalse; - } - -private: - ScopedPeerAssociation (ComponentPeer* componentPeer, Window embeddedWindow) - : peer (componentPeer), window (static_cast (embeddedWindow)) {} - - void swap (ScopedPeerAssociation& other) noexcept - { - std::swap (other.peer, peer); - std::swap (other.window, window); - } - - ComponentPeer* peer = nullptr; - XID window{}; -}; - class PeerListener : private ComponentMovementWatcher { public: PeerListener (Component& comp, Window embeddedWindow) : ComponentMovementWatcher (&comp), window (embeddedWindow), - association (ScopedPeerAssociation::make (comp.getPeer(), window)) {} + association (comp.getPeer(), window) {} private: using ComponentMovementWatcher::componentMovedOrResized, @@ -135,14 +62,14 @@ private: { // This should not be rewritten as a ternary expression or similar. // The old association must be destroyed before the new one is created. - association = ScopedPeerAssociation::make (nullptr, window); + association = {}; if (auto* comp = getComponent()) - association = ScopedPeerAssociation::make (comp->getPeer(), window); + association = ScopedWindowAssociation (comp->getPeer(), window); } Window window{}; - ScopedPeerAssociation association; + ScopedWindowAssociation association; }; //==============================================================================