From a7811661c5d8d0bfc3ee8c1fdef5c5078640ab9c Mon Sep 17 00:00:00 2001 From: attila Date: Fri, 26 Nov 2021 17:43:23 +0100 Subject: [PATCH] Linux: Fix restoreWindowFromStateString() when the peer already exists --- BREAKING-CHANGES.txt | 25 +++++++++++++ .../Unity/juce_Unity_Wrapper.cpp | 1 + .../native/juce_android_Windowing.cpp | 6 +++ .../native/juce_ios_UIViewComponentPeer.mm | 13 ++++--- .../native/juce_linux_Windowing.cpp | 19 ++++++++-- .../native/juce_mac_NSViewComponentPeer.mm | 18 +++++++-- .../native/juce_win32_Windowing.cpp | 5 +++ .../native/x11/juce_linux_XWindowSystem.cpp | 4 +- .../native/x11/juce_linux_XWindowSystem.h | 4 +- .../windows/juce_ComponentPeer.h | 37 +++++++++++++++++++ .../windows/juce_ResizableWindow.cpp | 10 +++-- 11 files changed, 121 insertions(+), 21 deletions(-) diff --git a/BREAKING-CHANGES.txt b/BREAKING-CHANGES.txt index cffdd352d2..e9a2b033e6 100644 --- a/BREAKING-CHANGES.txt +++ b/BREAKING-CHANGES.txt @@ -1,6 +1,31 @@ JUCE breaking changes ===================== +develop +======= + +Change +------ +The return type of XWindowSystem::getBorderSize() was changed to +ComponentPeer::OptionalBorderSize. + +Possible Issues +--------------- +User code that uses XWindowSystem::getBorderSize() will fail to build. + +Workaround +---------- +Use operator bool() to determine the validity of the new return value and +access the contained value using operator*(). + +Rationale +--------- +The XWindow system cannot immediately report the correct border size after +window creation. The underlying X11 calls will signal whether querying the +border size was successful, but there was no way to forward this information +through XWindowSystem::getBorderSize() until this change. + + Version 6.1.5 ============= diff --git a/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp b/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp index 08018247c7..df7d30a3da 100644 --- a/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp +++ b/modules/juce_audio_plugin_client/Unity/juce_Unity_Wrapper.cpp @@ -270,6 +270,7 @@ private: bool isFocused() const override { return true; } void grabFocus() override {} void* getNativeHandle() const override { return nullptr; } + OptionalBorderSize getFrameSizeIfPresent() const override { return {}; } BorderSize getFrameSize() const override { return {}; } void setVisible (bool) override {} void setTitle (const String&) override {} diff --git a/modules/juce_gui_basics/native/juce_android_Windowing.cpp b/modules/juce_gui_basics/native/juce_android_Windowing.cpp index a6ba22a53c..7796feaf92 100644 --- a/modules/juce_gui_basics/native/juce_android_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_android_Windowing.cpp @@ -631,6 +631,12 @@ public: (float) localPos.y * scale)); } + OptionalBorderSize getFrameSizeIfPresent() const override + { + // TODO + return {}; + } + BorderSize getFrameSize() const override { // TODO diff --git a/modules/juce_gui_basics/native/juce_ios_UIViewComponentPeer.mm b/modules/juce_gui_basics/native/juce_ios_UIViewComponentPeer.mm index 8ff1ddfc07..184ef9d3ce 100644 --- a/modules/juce_gui_basics/native/juce_ios_UIViewComponentPeer.mm +++ b/modules/juce_gui_basics/native/juce_ios_UIViewComponentPeer.mm @@ -211,24 +211,25 @@ public: controller = [newController retain]; } - Rectangle getBounds() const override { return getBounds (! isSharedWindow); } + Rectangle getBounds() const override { return getBounds (! isSharedWindow); } Rectangle getBounds (bool global) const; Point localToGlobal (Point relativePosition) override; Point globalToLocal (Point screenPosition) override; using ComponentPeer::localToGlobal; using ComponentPeer::globalToLocal; void setAlpha (float newAlpha) override; - void setMinimised (bool) override {} - bool isMinimised() const override { return false; } + void setMinimised (bool) override {} + bool isMinimised() const override { return false; } void setFullScreen (bool shouldBeFullScreen) override; - bool isFullScreen() const override { return fullScreen; } + bool isFullScreen() const override { return fullScreen; } bool contains (Point localPos, bool trueIfInAChildWindow) const override; - BorderSize getFrameSize() const override { return BorderSize(); } + OptionalBorderSize getFrameSizeIfPresent() const override { return {}; } + BorderSize getFrameSize() const override { return BorderSize(); } bool setAlwaysOnTop (bool alwaysOnTop) override; void toFront (bool makeActiveWindow) override; void toBehind (ComponentPeer* other) override; void setIcon (const Image& newIcon) override; - StringArray getAvailableRenderingEngines() override { return StringArray ("CoreGraphics Renderer"); } + StringArray getAvailableRenderingEngines() override { return StringArray ("CoreGraphics Renderer"); } void drawRect (CGRect); bool canBecomeKeyWindow(); diff --git a/modules/juce_gui_basics/native/juce_linux_Windowing.cpp b/modules/juce_gui_basics/native/juce_linux_Windowing.cpp index e81cc47ddc..58cd716379 100644 --- a/modules/juce_gui_basics/native/juce_linux_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_linux_Windowing.cpp @@ -141,11 +141,17 @@ public: return bounds; } - BorderSize getFrameSize() const override + OptionalBorderSize getFrameSizeIfPresent() const override { return windowBorder; } + BorderSize getFrameSize() const override + { + const auto optionalBorderSize = getFrameSizeIfPresent(); + return optionalBorderSize ? (*optionalBorderSize) : BorderSize(); + } + Point localToGlobal (Point relativePosition) override { return relativePosition + getScreenPosition (false).toFloat(); @@ -362,9 +368,14 @@ public: void updateBorderSize() { if ((styleFlags & windowHasTitleBar) == 0) - windowBorder = {}; - else if (windowBorder.getTopAndBottom() == 0 && windowBorder.getLeftAndRight() == 0) + { + windowBorder = ComponentPeer::OptionalBorderSize { BorderSize() }; + } + else if (! windowBorder + || ((*windowBorder).getTopAndBottom() == 0 && (*windowBorder).getLeftAndRight() == 0)) + { windowBorder = XWindowSystem::getInstance()->getBorderSize (windowH); + } } //============================================================================== @@ -504,7 +515,7 @@ private: ::Window windowH = {}, parentWindow = {}; Rectangle bounds; - BorderSize windowBorder; + ComponentPeer::OptionalBorderSize windowBorder; bool fullScreen = false, isAlwaysOnTop = false; double currentScaleFactor = 1.0; Array glRepaintListeners; diff --git a/modules/juce_gui_basics/native/juce_mac_NSViewComponentPeer.mm b/modules/juce_gui_basics/native/juce_mac_NSViewComponentPeer.mm index dda6d5e48b..960d53e7a2 100644 --- a/modules/juce_gui_basics/native/juce_mac_NSViewComponentPeer.mm +++ b/modules/juce_gui_basics/native/juce_mac_NSViewComponentPeer.mm @@ -493,12 +493,12 @@ public: : (v == view); } - BorderSize getFrameSize() const override + OptionalBorderSize getFrameSizeIfPresent() const override { - BorderSize b; - if (! isSharedWindow) { + BorderSize b; + NSRect v = [view convertRect: [view frame] toView: nil]; NSRect w = [window frame]; @@ -506,9 +506,19 @@ public: b.setBottom ((int) v.origin.y); b.setLeft ((int) v.origin.x); b.setRight ((int) (w.size.width - (v.origin.x + v.size.width))); + + return OptionalBorderSize { b }; } - return b; + return {}; + } + + BorderSize getFrameSize() const override + { + if (const auto frameSize = getFrameSizeIfPresent()) + return *frameSize; + + return {}; } bool hasNativeTitleBar() const diff --git a/modules/juce_gui_basics/native/juce_win32_Windowing.cpp b/modules/juce_gui_basics/native/juce_win32_Windowing.cpp index 089386a6ff..514f6b7ec5 100644 --- a/modules/juce_gui_basics/native/juce_win32_Windowing.cpp +++ b/modules/juce_gui_basics/native/juce_win32_Windowing.cpp @@ -1764,6 +1764,11 @@ public: return w == hwnd || (trueIfInAChildWindow && (IsChild (hwnd, w) != 0)); } + OptionalBorderSize getFrameSizeIfPresent() const override + { + return ComponentPeer::OptionalBorderSize { windowBorder }; + } + BorderSize getFrameSize() const override { return windowBorder; 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 8e9a87d571..85c581c234 100644 --- a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp +++ b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.cpp @@ -1802,7 +1802,7 @@ bool XWindowSystem::contains (::Window windowH, Point localPos) const && child == None; } -BorderSize XWindowSystem::getBorderSize (::Window windowH) const +ComponentPeer::OptionalBorderSize XWindowSystem::getBorderSize (::Window windowH) const { jassert (windowH != 0); @@ -1824,7 +1824,7 @@ BorderSize XWindowSystem::getBorderSize (::Window windowH) const data += sizeof (unsigned long); } - return { (int) sizes[2], (int) sizes[0], (int) sizes[3], (int) sizes[1] }; + return ComponentPeer::OptionalBorderSize ({ (int) sizes[2], (int) sizes[0], (int) sizes[3], (int) sizes[1] }); } } diff --git a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.h b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.h index 9369f8244a..587b81e150 100644 --- a/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.h +++ b/modules/juce_gui_basics/native/x11/juce_linux_XWindowSystem.h @@ -181,8 +181,8 @@ public: void setBounds (::Window, Rectangle, bool fullScreen) const; void updateConstraints (::Window) const; - BorderSize getBorderSize (::Window) const; - Rectangle getWindowBounds (::Window, ::Window parentWindow); + ComponentPeer::OptionalBorderSize getBorderSize (::Window) const; + Rectangle getWindowBounds (::Window, ::Window parentWindow); Point getPhysicalParentScreenPosition() const; bool contains (::Window, Point localPos) const; diff --git a/modules/juce_gui_basics/windows/juce_ComponentPeer.h b/modules/juce_gui_basics/windows/juce_ComponentPeer.h index 639e012e72..42d5718f2c 100644 --- a/modules/juce_gui_basics/windows/juce_ComponentPeer.h +++ b/modules/juce_gui_basics/windows/juce_ComponentPeer.h @@ -76,6 +76,31 @@ public: }; + class OptionalBorderSize final + { + public: + OptionalBorderSize() : valid (false) {} + explicit OptionalBorderSize (BorderSize size) : valid (true), borderSize (std::move (size)) {} + + explicit operator bool() const noexcept { return valid; } + + const auto& operator*() const noexcept + { + jassert (valid); + return borderSize; + } + + const auto* operator->() const noexcept + { + jassert (valid); + return &borderSize; + } + + private: + bool valid; + BorderSize borderSize; + }; + //============================================================================== /** Creates a peer. @@ -219,6 +244,18 @@ public: */ virtual bool contains (Point localPos, bool trueIfInAChildWindow) const = 0; + /** Returns the size of the window frame that's around this window. + + Depending on the platform the border size may be invalid for a short transient + after creating a new window. Hence the returned value must be checked using + operator bool() and the contained value can be accessed using operator*() only + if it is present. + + Whether or not the window has a normal window frame depends on the flags + that were set when the window was created by Component::addToDesktop() + */ + virtual OptionalBorderSize getFrameSizeIfPresent() const = 0; + /** Returns the size of the window frame that's around this window. Whether or not the window has a normal window frame depends on the flags that were set when the window was created by Component::addToDesktop() diff --git a/modules/juce_gui_basics/windows/juce_ResizableWindow.cpp b/modules/juce_gui_basics/windows/juce_ResizableWindow.cpp index d1d009a457..c766f54fcf 100644 --- a/modules/juce_gui_basics/windows/juce_ResizableWindow.cpp +++ b/modules/juce_gui_basics/windows/juce_ResizableWindow.cpp @@ -566,10 +566,12 @@ bool ResizableWindow::restoreWindowStateFromString (const String& s) if (peer != nullptr) { - peer->getFrameSize().addTo (newPos); + if (const auto frameSize = peer->getFrameSizeIfPresent()) + frameSize->addTo (newPos); } + #if JUCE_LINUX - else + if (peer == nullptr || ! peer->getFrameSizeIfPresent()) { // We need to adjust for the frame size before we create a peer, as X11 // doesn't provide this information at construction time. @@ -580,7 +582,9 @@ bool ResizableWindow::restoreWindowStateFromString (const String& s) tokens[firstCoord + 7].getIntValue(), tokens[firstCoord + 8].getIntValue() }; - frame.addTo (newPos); + newPos.setX (newPos.getX() - frame.getLeft()); + newPos.setY (newPos.getY() - frame.getTop()); + setBounds (newPos); } }