Cleanup Focus Handling (#152)

* Removed FocusGained/FocusLost/HasFocus from the BaseEditor. * Every QWidget has a Qt provided `hasFocus()` which the framework automatically handles for us including the focus traversal order. It's extremely rare that you should ever be doing your own custom focus handling in any framework really. https://doc.qt.io/qt-5/qwidget.html#focus-prop https://doc.qt.io/qt-5/focus.html * Removed the `MainWindow::MDIWindowChanged` slot which was handling the custom focus we no longer need. * Removed the custom `_parentHasFocus` member and its setter from the asset scroll area background. * Switched A and D to left/right respectively in the asset scroll area background because they were backwards before. * Allowed the asset scroll area background to handle events when it's not in focus. It's supposed to do this by design because widgets actually need to repaint as another window is moved around on top of them. * The asset scroll area background will not handle events like mouse movement when another MDI window occludes them. The Qt platform does this for us automatically using a mouse trap. Anything else would be _direct_ input, hence the name of DirectInput or raw input. The way GUI frameworks deliver input events after converting them to their own representation is typically by recursing down the tree of widgets starting at the root ancestor window and only delivering the events to the widget where the mouse was actually at. * I verified the above by printing, and it's also behavior I expect from UI frameworks based on my experience with Swing/Qt. * GM 8.1 & LGM have the same expected behavior, the mouse position is updated when not in focus but not if occluded.
enigma-dev · Aug 14, 2020 · bedf65e · bedf65e
1 parent 9251834
commit bedf65e
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 82 deletions.
diff --git a/Editors/BaseEditor.cpp b/Editors/BaseEditor.cpp
@@ -14,9 +14,6 @@ BaseEditor::BaseEditor(MessageModel* treeNodeModel, QWidget* parent)
   _resMapper->GetModel()->BackupModel(this);
 
   connect(_model, &QAbstractItemModel::modelReset, [this]() { this->RebindSubModels(); });
-
-  connect(this, &BaseEditor::FocusGained, [=]() { _hasFocus = true; });
-  connect(this, &BaseEditor::FocusLost, [=]() { _hasFocus = false; });
 }
 
 void BaseEditor::closeEvent(QCloseEvent* event) {
@@ -42,8 +39,6 @@ void BaseEditor::closeEvent(QCloseEvent* event) {
   event->accept();
 }
 
-bool BaseEditor::HasFocus() { return _hasFocus; }
-
 void BaseEditor::ReplaceBuffer(google::protobuf::Message* buffer) { _resMapper->ReplaceBuffer(buffer); }
 
 void BaseEditor::dataChanged(const QModelIndex& topLeft, const QModelIndex& /*bottomRight*/, const QVariant& oldValue,

diff --git a/Editors/BaseEditor.h b/Editors/BaseEditor.h
@@ -22,12 +22,9 @@ class BaseEditor : public QWidget {
   explicit BaseEditor(MessageModel *treeNodeModel, QWidget *parent);
 
   void ReplaceBuffer(google::protobuf::Message *buffer);
-  bool HasFocus();
 
  signals:
   void ResourceRenamed(TypeCase type, const QString &oldName, const QString &newName);
-  void FocusGained();
-  void FocusLost();
 
  public slots:
   virtual void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
@@ -41,7 +38,6 @@ class BaseEditor : public QWidget {
   ModelMapper *_nodeMapper;
   ModelMapper *_resMapper;
   MessageModel *_model;
-  bool _hasFocus = false;
 };
 
 #endif  // BASEEDTIOR_H
diff --git a/Editors/PathEditor.cpp b/Editors/PathEditor.cpp
@@ -84,9 +84,6 @@ PathEditor::PathEditor(MessageModel* model, QWidget* parent) : BaseEditor(model,
   connect(ySnap, QOverload<int>::of(&QSpinBox::valueChanged), _ui->pathPreviewBackground,
           &AssetScrollAreaBackground::SetGridVSnap);
 
-  connect(this, &BaseEditor::FocusGained, [this]() { _ui->pathPreviewBackground->SetParentHasFocus(true); });
-  connect(this, &BaseEditor::FocusLost, [this]() { _ui->pathPreviewBackground->SetParentHasFocus(false); });
-
   _nodeMapper->addMapping(_ui->nameEdit, TreeNode::kNameFieldNumber);
 
   _resMapper->addMapping(_ui->smoothCheckBox, Path::kSmoothFieldNumber);

diff --git a/MainWindow.cpp b/MainWindow.cpp
@@ -96,7 +96,7 @@ QFileInfo MainWindow::getEnigmaRoot() {
         break;
     }
   }
-  
+
   return EnigmaRoot;
 }
 
@@ -112,9 +112,9 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), _ui(new Ui::MainW
     ss << internal_events.readAll().toStdString();
     _event_data = std::make_unique<EventData>(ParseEventFile(ss));
   }
-  
+
   egm = egm::EGM(_event_data.get());
-  
+
   ArtManager::Init();
 
   _instance = this;
@@ -161,7 +161,6 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), _ui(new Ui::MainW
   this->_recentFiles = new RecentFiles(this, this->_ui->menuRecent, this->_ui->actionClearRecentMenu);
 
   _ui->mdiArea->setBackground(QImage(":/banner.png"));
-  connect(_ui->mdiArea, &QMdiArea::subWindowActivated, this, &MainWindow::MDIWindowChanged);
   connect(_ui->menuWindow, &QMenu::aboutToShow, this, &MainWindow::updateWindowMenu);
 
   auto settingsButton = static_cast<QToolButton *>(_ui->mainToolBar->widgetForAction(_ui->actionSettings));
@@ -278,18 +277,6 @@ void MainWindow::openSubWindow(buffers::TreeNode *item) {
   _ui->mdiArea->setActiveSubWindow(subWindow);
 }
 
-void MainWindow::MDIWindowChanged(QMdiSubWindow *window) {
-  for (QMdiSubWindow *subWindow : _subWindows) {
-    if (subWindow == nullptr) continue;
-    BaseEditor *editor = static_cast<BaseEditor *>(subWindow->widget());
-    if (window == subWindow) {
-      emit editor->FocusGained();
-    } else if (editor->HasFocus()) {
-      emit editor->FocusLost();
-    }
-  }
-}
-
 void MainWindow::updateWindowMenu() {
   static QList<QAction *> windowActions;
   foreach (auto action, windowActions) {

diff --git a/MainWindow.h b/MainWindow.h
@@ -36,7 +36,7 @@ class MainWindow : public QMainWindow {
   ~MainWindow();
   void openProject(std::unique_ptr<buffers::Project> openedProject);
   buffers::Game *Game() const { return this->_project->mutable_game(); }
-  
+
   static QList<QString> EnigmaSearchPaths;
   static QFileInfo EnigmaRoot;
 
@@ -50,8 +50,6 @@ class MainWindow : public QMainWindow {
   static void setCurrentConfig(const buffers::resources::Settings &settings);
 
  private slots:
-  void MDIWindowChanged(QMdiSubWindow *window);
-
   // file menu
   void on_actionNew_triggered();
   void on_actionOpen_triggered();
@@ -114,7 +112,7 @@ class MainWindow : public QMainWindow {
 
   std::unique_ptr<buffers::Project> _project;
   QPointer<RecentFiles> _recentFiles;
-  
+
   std::unique_ptr<EventData> _event_data;
   egm::EGM egm;
 

diff --git a/Widgets/AssetScrollAreaBackground.cpp b/Widgets/AssetScrollAreaBackground.cpp
@@ -17,7 +17,6 @@ AssetScrollAreaBackground::AssetScrollAreaBackground(AssetScrollArea* parent)
       _maxZoom(3200),
       _minZoom(0.0625),
       _backgroundColor(Qt::GlobalColor::gray),
-      _parentHasFocus(false),
       _viewMoveSpeed(4) {
   installEventFilter(this);
   setMouseTracking(true);
@@ -90,8 +89,6 @@ void AssetScrollAreaBackground::SetGridVSnap(int vSnap) {
   }
 }
 
-void AssetScrollAreaBackground::SetParentHasFocus(bool focus) { _parentHasFocus = focus; }
-
 void AssetScrollAreaBackground::PaintGrid(QPainter& painter, int gridHorSpacing, int gridVertSpacing, int gridHorOff,
                                           int gridVertOff) {
   // save the painter state so we can restore it before returning
@@ -206,52 +203,42 @@ void AssetScrollAreaBackground::paintEvent(QPaintEvent* /* event */) {
 }
 
 bool AssetScrollAreaBackground::eventFilter(QObject* obj, QEvent* event) {
-  if (_parentHasFocus) {
-    switch (event->type()) {
-      case QEvent::MouseMove: {
-        QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
-        QPoint roomPos = mouseEvent->pos() - _totalDrawOffset;
-        roomPos /= _currentZoom;
-        emit MouseMoved(roomPos.x(), roomPos.y());
-        break;
-      }
-      case QEvent::MouseButtonPress: {
-        QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
-        emit MousePressed(mouseEvent->button());
-        break;
-      }
-      case QEvent::MouseButtonRelease: {
-        QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
-        emit MouseReleased(mouseEvent->button());
-        break;
-      }
-      case QEvent::Enter: {
-        setFocus();
-        break;
-      }
-      case QEvent::Leave: {
-        clearFocus();
-        break;
-      }
-      case QEvent::KeyPress: {
-        QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
-        _pressedKeys += keyEvent->key();
-        _userDrawOffset.setX(_userDrawOffset.x() +
-                             (_pressedKeys.contains(Qt::Key::Key_D) - _pressedKeys.contains(Qt::Key::Key_A)) *
-                                 _viewMoveSpeed);
-        _userDrawOffset.setY(_userDrawOffset.y() +
-                             (_pressedKeys.contains(Qt::Key::Key_W) - _pressedKeys.contains(Qt::Key::Key_S)) *
-                                 _viewMoveSpeed);
-        update();
-        break;
-      }
-      case QEvent::KeyRelease: {
-        QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
-        _pressedKeys -= keyEvent->key();
-        break;
-      }
-      default: break;
+  switch (event->type()) {
+    case QEvent::MouseMove: {
+      QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
+      QPoint roomPos = mouseEvent->pos() - _totalDrawOffset;
+      roomPos /= _currentZoom;
+      emit MouseMoved(roomPos.x(), roomPos.y());
+      break;
+    }
+    case QEvent::MouseButtonPress: {
+      QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
+      emit MousePressed(mouseEvent->button());
+      break;
+    }
+    case QEvent::MouseButtonRelease: {
+      QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
+      emit MouseReleased(mouseEvent->button());
+      break;
+    }
+    case QEvent::KeyPress: {
+      QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
+      _pressedKeys += keyEvent->key();
+      _userDrawOffset.setX(_userDrawOffset.x() +
+                           (_pressedKeys.contains(Qt::Key::Key_A) - _pressedKeys.contains(Qt::Key::Key_D)) *
+                               _viewMoveSpeed);
+      _userDrawOffset.setY(_userDrawOffset.y() +
+                           (_pressedKeys.contains(Qt::Key::Key_W) - _pressedKeys.contains(Qt::Key::Key_S)) *
+                               _viewMoveSpeed);
+      update();
+      break;
+    }
+    case QEvent::KeyRelease: {
+      QKeyEvent* keyEvent = static_cast<QKeyEvent*>(event);
+      _pressedKeys -= keyEvent->key();
+      break;
     }
+    default: break;
   }
   return QWidget::eventFilter(obj, event);
 }
diff --git a/Widgets/AssetScrollAreaBackground.h b/Widgets/AssetScrollAreaBackground.h
@@ -40,7 +40,6 @@ class AssetScrollAreaBackground : public QWidget {
   void SetGridVisible(bool visible);
   void SetGridHSnap(int hSnap);
   void SetGridVSnap(int vSnap);
-  void SetParentHasFocus(bool focus);
 
  signals:
   void MouseMoved(int x, int y);
@@ -66,7 +65,6 @@ class AssetScrollAreaBackground : public QWidget {
   QPoint _userDrawOffset;
   QSet<int> _pressedKeys;
   QColor _backgroundColor;
-  bool _parentHasFocus;
   int _viewMoveSpeed;
 };