Skip to content

Commit

Permalink
#5735: Fix crashes due to key events sent to the popup window in the …
Browse files Browse the repository at this point in the history
…middle of destruction. Destroy the modifier hint popup in a separate idle event.

Fix dependencies of FilterUserInterface.
  • Loading branch information
codereader committed Oct 1, 2021
1 parent 1f9ece6 commit bc27008
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 12 deletions.
9 changes: 6 additions & 3 deletions radiant/eventmanager/GlobalKeyEventFilter.cpp
Expand Up @@ -3,6 +3,7 @@
#include "imousetoolmanager.h"

#include <wx/event.h>
#include <wx/app.h>
#include <wx/window.h>
#include <wx/textctrl.h>
#include <wx/combobox.h>
Expand All @@ -12,7 +13,6 @@
#include "wxutil/Modifier.h"
#include "wxutil/dialog/DialogBase.h"

#include "itextstream.h"
#include "EventManager.h"

namespace ui
Expand Down Expand Up @@ -84,8 +84,11 @@ int GlobalKeyEventFilter::FilterEvent(wxEvent& event)
// Attempt to find an accelerator
bool acceleratorFound = handleAccelerator(keyEvent);

// Update the status bar in any case
GlobalMouseToolManager().updateStatusbar(wxutil::Modifier::GetStateForKeyEvent(keyEvent));
// Update the status bar if the app is active
if (wxTheApp->IsActive())
{
GlobalMouseToolManager().updateStatusbar(wxutil::Modifier::GetStateForKeyEvent(keyEvent));
}

return acceleratorFound ? Event_Processed : Event_Skip;
}
Expand Down
30 changes: 27 additions & 3 deletions radiant/eventmanager/ModifierHintPopup.h
@@ -1,6 +1,7 @@
#pragma once

#include <wx/popupwin.h>
#include "MouseToolManager.h"

namespace ui
{
Expand All @@ -10,18 +11,28 @@ class ModifierHintPopup :
{
private:
wxStaticText* _text;
MouseToolManager& _owner;

public:
ModifierHintPopup(wxWindow* parent) :
wxPopupWindow(parent, wxBORDER_SIMPLE)
ModifierHintPopup(wxWindow* parent, MouseToolManager& owner) :
wxPopupWindow(parent, wxBORDER_SIMPLE),
_owner(owner)
{
SetSizer(new wxBoxSizer(wxHORIZONTAL));

_text = new wxStaticText(this, wxID_ANY, "");
GetSizer()->Add(_text, 1, wxALL, 3);
SetText("");
}

// Show the window right now
Show();

// Subscribe to the parent events after showing, to prevent immediate self-destruction
parent->Bind(wxEVT_ICONIZE, &ModifierHintPopup::_onParentMinimized, this);
// Detect when the parent window is losing focus (e.g. by alt-tabbing)
parent->Bind(wxEVT_ACTIVATE, &ModifierHintPopup::_onParentActivate, this);
}

void SetText(const wxString& text)
{
_text->SetLabelText(text);
Expand All @@ -39,6 +50,19 @@ class ModifierHintPopup :
wxPoint popupPos = GetParent()->GetScreenPosition() + wxSize(20, GetParent()->GetSize().y - GetSize().y - 20);
Position(popupPos, wxSize(0, 0));
}

void _onParentActivate(wxActivateEvent& ev)
{
if (!ev.GetActive())
{
_owner.closeHintPopup();
}
}

void _onParentMinimized(wxIconizeEvent&)
{
_owner.closeHintPopup();
}
};

}
18 changes: 15 additions & 3 deletions radiant/eventmanager/MouseToolManager.cpp
Expand Up @@ -10,6 +10,7 @@
#include "wxutil/MouseButton.h"
#include "wxutil/Modifier.h"
#include "module/StaticModule.h"
#include "ModifierHintPopup.h"

namespace ui
{
Expand All @@ -22,7 +23,8 @@ namespace
MouseToolManager::MouseToolManager() :
_activeModifierState(0),
_hintCloseTimer(this),
_hintPopup(nullptr)
_hintPopup(nullptr),
_shouldClosePopup(false)
{
Bind(wxEVT_TIMER, &MouseToolManager::onCloseTimerIntervalReached, this);
}
Expand Down Expand Up @@ -229,19 +231,29 @@ void MouseToolManager::updateStatusbar(unsigned int newState)

// (Re-)start the timer
_hintCloseTimer.StartOnce(HINT_POPUP_CLOSE_TIMEOUT_MSECS);
_shouldClosePopup = false;

// Ensure the popup exists
if (!_hintPopup)
{
_hintPopup = new ModifierHintPopup(GlobalMainFrame().getWxTopLevelWindow());
_hintPopup->Show();
_hintPopup = new ModifierHintPopup(GlobalMainFrame().getWxTopLevelWindow(), *this);
}

_hintPopup->SetText(statusText);
}

void MouseToolManager::closeHintPopup()
{
_shouldClosePopup = true;
requestIdleCallback();
}

void MouseToolManager::onIdle()
{
if (!_shouldClosePopup) return;

_shouldClosePopup = false;

if (_hintPopup)
{
_hintPopup->Hide();
Expand Down
12 changes: 9 additions & 3 deletions radiant/eventmanager/MouseToolManager.h
Expand Up @@ -5,19 +5,21 @@
#include "imousetoolmanager.h"
#include "xmlutil/Node.h"
#include "MouseToolGroup.h"
#include "ModifierHintPopup.h"
#include <wx/event.h>
#include <wx/timer.h>
#include "wxutil/event/SingleIdleCallback.h"

namespace ui
{

class ModifierHintPopup;

/**
* Implementation of the IMouseToolManager interface.
* This is used by the GlobalXYWnd and GlobalCamera instances.
*/
class MouseToolManager :
public wxEvtHandler,
public wxutil::SingleIdleCallback,
public IMouseToolManager
{
protected:
Expand All @@ -28,6 +30,7 @@ class MouseToolManager :

wxTimer _hintCloseTimer;
ModifierHintPopup* _hintPopup;
bool _shouldClosePopup;

public:
MouseToolManager();
Expand All @@ -50,6 +53,10 @@ class MouseToolManager :
void updateStatusbar(unsigned int newState);

void resetBindingsToDefault();
void closeHintPopup();

protected:
void onIdle() override;

private:
void onMainFrameConstructed();
Expand All @@ -60,7 +67,6 @@ class MouseToolManager :
void saveToolMappings();

void onCloseTimerIntervalReached(wxTimerEvent& ev);
void closeHintPopup();
};

} // namespace
2 changes: 2 additions & 0 deletions radiant/ui/filters/FilterUserInterface.cpp
Expand Up @@ -3,6 +3,7 @@
#include "ifilter.h"
#include "ieventmanager.h"
#include "imainframe.h"
#include "imenumanager.h"
#include "icommandsystem.h"
#include <sigc++/functors/mem_fun.h>

Expand All @@ -28,6 +29,7 @@ const StringSet& FilterUserInterface::getDependencies() const
_dependencies.insert(MODULE_FILTERSYSTEM);
_dependencies.insert(MODULE_COMMANDSYSTEM);
_dependencies.insert(MODULE_EVENTMANAGER);
_dependencies.insert(MODULE_MENUMANAGER);
}

return _dependencies;
Expand Down

0 comments on commit bc27008

Please sign in to comment.