Add a simple close button ("X") to windows.

Clicking the button generates a WindowCloseRequest event which the client app then has to deal with. The default behavior for GWindow is to close() itself. I also added a flag, GWindow::should_exit_event_loop_on_close() which does what it sounds like it does. This patch exposed some bugs in GWindow and GWidget teardown.
luteresa · Feb 5, 2019 · 11db8c1 · 11db8c1
1 parent d0078b6
commit 11db8c1
Show file tree

Hide file tree

Showing 14 changed files with 103 additions and 1 deletion.
diff --git a/Clock/main.cpp b/Clock/main.cpp
@@ -8,7 +8,8 @@ int main(int, char**)
 
  auto* window = new GWindow;
  window->set_title("Clock");
- window->set_rect({ 100, 100, 100, 40 });
+ window->set_rect({ 600, 100, 100, 40 });
+ window->set_should_exit_app_on_close(true);
 
  auto* clock_widget = new ClockWidget;
  clock_widget->set_relative_rect({ 0, 0, 100, 40 });

diff --git a/Kernel/GUITypes.h b/Kernel/GUITypes.h
@@ -66,6 +66,7 @@ struct GUI_Event {
  KeyUp,
  WindowActivated,
  WindowDeactivated,
+ WindowCloseRequest,
  };
  Type type { Invalid };
  int window_id { -1 };

diff --git a/LibGUI/GEvent.h b/LibGUI/GEvent.h
@@ -24,6 +24,7 @@ class GEvent {
  WindowBecameActive,
  FocusIn,
  FocusOut,
+ WindowCloseRequest,
  };
 
  GEvent() { }

diff --git a/LibGUI/GEventLoop.cpp b/LibGUI/GEventLoop.cpp
@@ -35,6 +35,12 @@ GEventLoop& GEventLoop::main()
  return *s_mainGEventLoop;
 }
 
+void GEventLoop::exit(int code)
+{
+ m_exit_requested = true;
+ m_exit_code = code;
+}
+
 int GEventLoop::exec()
 {
  m_event_fd = open("/dev/gui_events", O_RDONLY | O_NONBLOCK | O_CLOEXEC);
@@ -45,6 +51,8 @@ int GEventLoop::exec()
 
  m_running = true;
  for (;;) {
+ if (m_exit_requested)
+ return m_exit_code;
  if (m_queued_events.is_empty())
  wait_for_event();
  Vector<QueuedEvent> events = move(m_queued_events);
@@ -69,6 +77,7 @@ int GEventLoop::exec()
  }
  }
  }
+ ASSERT_NOT_REACHED();
 }
 
 void GEventLoop::post_event(GObject* receiver, OwnPtr<GEvent>&& event)
@@ -95,6 +104,11 @@ void GEventLoop::handle_window_activation_event(const GUI_Event& event, GWindow&
  post_event(&window, make<GEvent>(event.type == GUI_Event::Type::WindowActivated ? GEvent::WindowBecameActive : GEvent::WindowBecameInactive));
 }
 
+void GEventLoop::handle_window_close_request_event(const GUI_Event&, GWindow& window)
+{
+ post_event(&window, make<GEvent>(GEvent::WindowCloseRequest));
+}
+
 void GEventLoop::handle_key_event(const GUI_Event& event, GWindow& window)
 {
 #ifdef GEVENTLOOP_DEBUG
@@ -192,6 +206,9 @@ void GEventLoop::wait_for_event()
  case GUI_Event::Type::WindowDeactivated:
  handle_window_activation_event(event, *window);
  break;
+ case GUI_Event::Type::WindowCloseRequest:
+ handle_window_close_request_event(event, *window);
+ break;
  case GUI_Event::Type::KeyDown:
  case GUI_Event::Type::KeyUp:
  handle_key_event(event, *window);

diff --git a/LibGUI/GEventLoop.h b/LibGUI/GEventLoop.h
@@ -27,12 +27,15 @@ class GEventLoop {
  int register_timer(GObject&, int milliseconds, bool should_reload);
  bool unregister_timer(int timer_id);
 
+ void exit(int);
+
 private:
  void wait_for_event();
  void handle_paint_event(const GUI_Event&, GWindow&);
  void handle_mouse_event(const GUI_Event&, GWindow&);
  void handle_key_event(const GUI_Event&, GWindow&);
  void handle_window_activation_event(const GUI_Event&, GWindow&);
+ void handle_window_close_request_event(const GUI_Event&, GWindow&);
 
  void get_next_timer_expiration(timeval&);
 
@@ -44,6 +47,8 @@ class GEventLoop {
 
  int m_event_fd { -1 };
  bool m_running { false };
+ bool m_exit_requested { false };
+ int m_exit_code { 0 };
 
  int m_next_timer_id { 1 };
 

diff --git a/LibGUI/GObject.cpp b/LibGUI/GObject.cpp
@@ -12,6 +12,7 @@ GObject::GObject(GObject* parent)
 
 GObject::~GObject()
 {
+ stop_timer();
  if (m_parent)
  m_parent->remove_child(*this);
  auto children_to_delete = move(m_children);

diff --git a/LibGUI/GWindow.cpp b/LibGUI/GWindow.cpp
@@ -35,11 +35,17 @@ GWindow::GWindow(GObject* parent)
 
 GWindow::~GWindow()
 {
+ if (m_main_widget)
+ delete m_main_widget;
  hide();
 }
 
 void GWindow::close()
 {
+ // FIXME: If we exit the event loop, we're never gonna deal with the delete_later request!
+ // This will become relevant once we support nested event loops.
+ if (should_exit_app_on_close())
+ GEventLoop::main().exit(0);
  delete_later();
 }
 
@@ -160,6 +166,11 @@ void GWindow::event(GEvent& event)
  return;
  }
 
+ if (event.type() == GEvent::WindowCloseRequest) {
+ close();
+ return;
+ }
+
  GObject::event(event);
 }
 

diff --git a/LibGUI/GWindow.h b/LibGUI/GWindow.h
@@ -53,6 +53,9 @@ class GWindow final : public GObject {
  GWidget* global_cursor_tracking_widget() { return m_global_cursor_tracking_widget.ptr(); }
  const GWidget* global_cursor_tracking_widget() const { return m_global_cursor_tracking_widget.ptr(); }
 
+ bool should_exit_app_on_close() const { return m_should_exit_app_on_close; }
+ void set_should_exit_app_on_close(bool b) { m_should_exit_app_on_close = b; }
+
 private:
  RetainPtr<GraphicsBitmap> m_backing;
  int m_window_id { 0 };
@@ -62,5 +65,6 @@ class GWindow final : public GObject {
  WeakPtr<GWidget> m_global_cursor_tracking_widget;
  Rect m_rect_when_windowless;
  String m_title_when_windowless;
+ bool m_should_exit_app_on_close { false };
 };
 
diff --git a/Terminal/main.cpp b/Terminal/main.cpp
@@ -149,6 +149,8 @@ int main(int, char**)
  terminal.set_in_active_window(true);
  } else if (event.type == GUI_Event::Type::WindowDeactivated) {
  terminal.set_in_active_window(false);
+ } else if (event.type == GUI_Event::Type::WindowCloseRequest) {
+ return 0;
  }
  }
  }

diff --git a/Userland/guitest2.cpp b/Userland/guitest2.cpp
@@ -40,6 +40,7 @@ int main(int argc, char** argv)
 #endif
 
  auto* launcher_window = make_launcher_window();
+ launcher_window->set_should_exit_app_on_close(true);
  launcher_window->show();
 
  return loop.exec();

diff --git a/WindowServer/WSMessage.h b/WindowServer/WSMessage.h
@@ -22,6 +22,7 @@ class WSMessage {
  KeyUp,
  WindowActivated,
  WindowDeactivated,
+ WindowCloseRequest,
  };
 
  WSMessage() { }

diff --git a/WindowServer/WSWindow.cpp b/WindowServer/WSWindow.cpp
@@ -118,6 +118,9 @@ void WSWindow::on_message(WSMessage& message)
  case WSMessage::WindowDeactivated:
  gui_event.type = GUI_Event::Type::WindowDeactivated;
  break;
+ case WSMessage::WindowCloseRequest:
+ gui_event.type = GUI_Event::Type::WindowCloseRequest;
+ break;
  default:
  break;
  }

diff --git a/WindowServer/WSWindowManager.cpp b/WindowServer/WSWindowManager.cpp
@@ -36,6 +36,19 @@ static inline Rect title_bar_text_rect(const Rect& window)
  };
 }
 
+static inline Rect close_button_rect_for_window(const Rect& window_rect)
+{
+ auto titlebar_inner_rect = title_bar_text_rect(window_rect);
+ int close_button_margin = 1;
+ int close_button_size = titlebar_inner_rect.height() - close_button_margin * 2;
+ return Rect {
+ titlebar_inner_rect.right() - close_button_size,
+ titlebar_inner_rect.top() + close_button_margin,
+ close_button_size,
+ close_button_size
+ };
+}
+
 static inline Rect border_window_rect(const Rect& window)
 {
  auto titlebar_rect = title_bar_rect(window);
@@ -152,6 +165,22 @@ WSWindowManager::~WSWindowManager()
 {
 }
 
+static const char* s_close_button_bitmap_data = {
+ " ## ## "
+ " ## ## "
+ " ## ## "
+ " ### "
+ " # "
+ " ### "
+ " ## ## "
+ " ## ## "
+ " ## ## "
+};
+
+static CharacterBitmap* s_close_button_bitmap;
+static const int s_close_button_bitmap_width = 11;
+static const int s_close_button_bitmap_height = 11;
+
 void WSWindowManager::paint_window_frame(WSWindow& window)
 {
  LOCKER(m_lock);
@@ -204,6 +233,17 @@ void WSWindowManager::paint_window_frame(WSWindow& window)
  m_back_painter->draw_rect(inner_border_rect, border_color);
  m_back_painter->draw_text(titlebar_title_rect, window.title(), Painter::TextAlignment::CenterLeft, title_color);
 
+ Rect close_button_rect = close_button_rect_for_window(window.rect());
+ if (!s_close_button_bitmap)
+ s_close_button_bitmap = CharacterBitmap::create_from_ascii(s_close_button_bitmap_data, s_close_button_bitmap_width, s_close_button_bitmap_height).leak_ref();
+
+ m_back_painter->fill_rect_with_gradient(close_button_rect, Color::LightGray, Color::White);
+ m_back_painter->draw_rect(close_button_rect, Color::Black);
+ auto x_location = close_button_rect.location();
+ x_location.move_by(2, 2);
+ m_back_painter->draw_bitmap(x_location, *s_close_button_bitmap, Color::Black);
+
+
 #ifdef DEBUG_WID_IN_TITLE_BAR
  Color metadata_color(96, 96, 96);
  m_back_painter->draw_text(
@@ -276,6 +316,15 @@ void WSWindowManager::handle_titlebar_mouse_event(WSWindow& window, WSMouseEvent
  }
 }
 
+void WSWindowManager::handle_close_button_mouse_event(WSWindow& window, WSMouseEvent& event)
+{
+ if (event.type() == WSMessage::MouseDown && event.button() == MouseButton::Left) {
+ WSMessage message(WSMessage::WindowCloseRequest);
+ window.on_message(message);
+ return;
+ }
+}
+
 void WSWindowManager::process_mouse_event(WSMouseEvent& event)
 {
  if (event.type() == WSMessage::MouseUp && event.button() == MouseButton::Left) {
@@ -320,6 +369,10 @@ void WSWindowManager::process_mouse_event(WSMouseEvent& event)
  move_to_front(*window);
  set_active_window(window);
  }
+ if (close_button_rect_for_window(window->rect()).contains(event.position())) {
+ handle_close_button_mouse_event(*window, event);
+ return;
+ }
  handle_titlebar_mouse_event(*window, event);
  return;
  }

diff --git a/WindowServer/WSWindowManager.h b/WindowServer/WSWindowManager.h
@@ -48,6 +48,7 @@ class WSWindowManager : public WSMessageReceiver {
 
  void process_mouse_event(WSMouseEvent&);
  void handle_titlebar_mouse_event(WSWindow&, WSMouseEvent&);
+ void handle_close_button_mouse_event(WSWindow&, WSMouseEvent&);
 
  void set_active_window(WSWindow*);