Start refactoring the windowing system to use an event loop.

Userspace programs can now open /dev/gui_events and read a stream of GUI_Event structs one at a time. I was stuck on a stupid problem where we'd reenter Scheduler::yield() due to having one of the has_data_available_for_reading() implementations using locks.
luteresa · Jan 14, 2019 · b0e3f73 · b0e3f73
1 parent b4da4e8
commit b0e3f73
Show file tree

Hide file tree

Showing 46 changed files with 283 additions and 292 deletions.
diff --git a/Kernel/Console.cpp b/Kernel/Console.cpp
@@ -23,7 +23,7 @@ Console::~Console()
 {
 }
 
-bool Console::has_data_available_for_reading() const
+bool Console::has_data_available_for_reading(Process&) const
 {
  return false;
 }

diff --git a/Kernel/Console.h b/Kernel/Console.h
@@ -18,7 +18,7 @@ class Console final : public CharacterDevice {
  Console();
  virtual ~Console() override;
 
- virtual bool has_data_available_for_reading() const override;
+ virtual bool has_data_available_for_reading(Process&) const override;
  virtual ssize_t read(byte* buffer, size_t size) override;
  virtual ssize_t write(const byte* data, size_t size) override;
 

diff --git a/Kernel/GUITypes.h b/Kernel/GUITypes.h
@@ -11,23 +11,60 @@ struct GUI_WindowFlags { enum {
 
 typedef unsigned GUI_Color;
 
+struct GUI_Point {
+ int x;
+ int y;
+};
+
+struct GUI_Size {
+ int width;
+ int height;
+};
+
+struct GUI_Rect {
+ GUI_Point location;
+ GUI_Size size;
+};
+
 struct GUI_CreateWindowParameters {
- Rect rect;
+ GUI_Rect rect;
  Color background_color;
  unsigned flags { 0 };
  char title[128];
 };
 
-enum class GUI_WidgetType : unsigned {
- Label,
- Button,
+enum class GUI_MouseButton : unsigned char {
+ NoButton = 0,
+ Left = 1,
+ Right = 2,
+ Middle = 4,
 };
 
-struct GUI_CreateWidgetParameters {
- GUI_WidgetType type;
- Rect rect;
- Color background_color;
- bool opaque { true };
- unsigned flags { 0 };
- char text[256];
+struct GUI_Event {
+ enum Type : unsigned {
+ Invalid,
+ Paint,
+ MouseMove,
+ MouseDown,
+ MouseUp,
+ };
+ Type type { Invalid };
+ int window_id { -1 };
+
+ union {
+ struct {
+ GUI_Rect rect;
+ } paint;
+ struct {
+ GUI_Point position;
+ GUI_MouseButton button;
+ } mouse;
+ };
 };
+
+inline Rect::Rect(const GUI_Rect& r) : Rect(r.location, r.size) { }
+inline Point::Point(const GUI_Point& p) : Point(p.x, p.y) { }
+inline Size::Size(const GUI_Size& s) : Size(s.width, s.height) { }
+inline Rect::operator GUI_Rect() const { return { m_location, m_size }; }
+inline Point::operator GUI_Point() const { return { m_x, m_y }; }
+inline Size::operator GUI_Size() const { return { m_width, m_height }; }
diff --git a/Kernel/Keyboard.cpp b/Kernel/Keyboard.cpp
@@ -114,7 +114,7 @@ Keyboard::~Keyboard()
 {
 }
 
-bool Keyboard::has_data_available_for_reading() const
+bool Keyboard::has_data_available_for_reading(Process&) const
 {
  return !m_queue.is_empty();
 }

diff --git a/Kernel/Keyboard.h b/Kernel/Keyboard.h
@@ -39,7 +39,7 @@ class Keyboard final : public IRQHandler, public CharacterDevice {
  // ^CharacterDevice
  virtual ssize_t read(byte* buffer, size_t) override;
  virtual ssize_t write(const byte* buffer, size_t) override;
- virtual bool has_data_available_for_reading() const override;
+ virtual bool has_data_available_for_reading(Process&) const override;
 
  void emit(byte);
 

diff --git a/Kernel/Makefile b/Kernel/Makefile
@@ -62,7 +62,8 @@ WIDGETS_OBJS = \
  ../Widgets/ListBox.o \
  ../Widgets/CheckBox.o \
  ../Widgets/TextBox.o \
- ../Widgets/AbstractScreen.o
+ ../Widgets/AbstractScreen.o \
+ ../Widgets/GUIEventDevice.o \
 
 AK_OBJS = \
  ../AK/String.o \

diff --git a/Kernel/PS2MouseDevice.cpp b/Kernel/PS2MouseDevice.cpp
@@ -116,7 +116,7 @@ byte PS2MouseDevice::mouse_read()
  return IO::in8(0x60);
 }
 
-bool PS2MouseDevice::has_data_available_for_reading() const
+bool PS2MouseDevice::has_data_available_for_reading(Process&) const
 {
  return !m_buffer.is_empty();
 }

diff --git a/Kernel/PS2MouseDevice.h b/Kernel/PS2MouseDevice.h
@@ -12,7 +12,7 @@ class PS2MouseDevice final : public IRQHandler, public CharacterDevice {
  static PS2MouseDevice& the();
 
  // ^CharacterDevice
- virtual bool has_data_available_for_reading() const override;
+ virtual bool has_data_available_for_reading(Process&) const override;
  virtual ssize_t read(byte* buffer, size_t) override;
  virtual ssize_t write(const byte* buffer, size_t) override;
 

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
@@ -18,6 +18,7 @@
 #include "Scheduler.h"
 #include "FIFO.h"
 #include "KSyms.h"
+#include <Widgets/Window.h>
 
 //#define DEBUG_IO
 //#define TASK_DEBUG
@@ -1057,7 +1058,7 @@ ssize_t Process::sys$read(int fd, void* outbuf, size_t nread)
  if (!descriptor)
  return -EBADF;
  if (descriptor->is_blocking()) {
- if (!descriptor->has_data_available_for_reading()) {
+ if (!descriptor->has_data_available_for_reading(*this)) {
  m_blocked_fd = fd;
  block(BlockedRead);
  sched_yield();

diff --git a/Kernel/Process.h b/Kernel/Process.h
@@ -12,6 +12,7 @@
 #include <AK/AKString.h>
 #include <AK/Vector.h>
 #include <AK/WeakPtr.h>
+#include <AK/Lock.h>
 
 class FileDescriptor;
 class PageDirectory;
@@ -191,13 +192,12 @@ class Process : public InlineLinkedListNode<Process> {
 
  int gui$create_window(const GUI_CreateWindowParameters*);
  int gui$destroy_window(int window_id);
- int gui$create_widget(int window_id, const GUI_CreateWidgetParameters*);
- int gui$destroy_widget(int widget_id);
 
  DisplayInfo get_display_info();
 
  static void initialize();
  static void initialize_gui_statics();
+ int make_window_id();
 
  void crash() NORETURN;
  static int reap(Process&) WARN_UNUSED_RESULT;
@@ -248,6 +248,9 @@ class Process : public InlineLinkedListNode<Process> {
 
  bool is_root() const { return m_euid == 0; }
 
+ Vector<GUI_Event>& gui_events() { return m_gui_events; }
+ SpinLock& gui_events_lock() { return m_gui_events_lock; }
+
 private:
  friend class MemoryManager;
  friend class Scheduler;
@@ -342,8 +345,11 @@ class Process : public InlineLinkedListNode<Process> {
 
  RetainPtr<Region> m_display_framebuffer_region;
 
- Vector<WeakPtr<Window>> m_windows;
- Vector<WeakPtr<Widget>> m_widgets;
+ HashMap<int, OwnPtr<Window>> m_windows;
+
+ Vector<GUI_Event> m_gui_events;
+ SpinLock m_gui_events_lock;
+ int m_next_window_id { 1 };
 };
 
 extern Process* current;

diff --git a/Kernel/ProcessGUI.cpp b/Kernel/ProcessGUI.cpp
@@ -22,6 +22,14 @@ void Process::initialize_gui_statics()
  new EventLoop;
 }
 
+int Process::make_window_id()
+{
+ int new_id = m_next_window_id++;
+ while (!new_id || m_windows.contains(new_id))
+ new_id = m_next_window_id++;
+ return new_id;
+}
+
 static void wait_for_gui_server()
 {
  // FIXME: Time out after a while and return an error.
@@ -37,28 +45,26 @@ int Process::gui$create_window(const GUI_CreateWindowParameters* user_params)
  return -EFAULT;
 
  auto params = *user_params;
+ Rect rect = params.rect;
 
- if (params.rect.is_empty())
+ if (rect.is_empty())
  return -EINVAL;
 
  ProcessPagingScope scope(EventLoop::main().server_process());
 
- auto* window = new Window;
- if (!window)
+ int window_id = make_window_id();
+ if (!window_id)
  return -ENOMEM;
 
- int window_id = m_windows.size();
- m_windows.append(window->makeWeakPtr());
+ auto window = make<Window>(*this, window_id);
+ if (!window)
+ return -ENOMEM;
 
  window->setTitle(params.title);
- window->setRect(params.rect);
+ window->setRect(rect);
 
- auto* main_widget = new Widget;
- window->setMainWidget(main_widget);
- main_widget->setWindowRelativeRect({ 0, 0, params.rect.width(), params.rect.height() });
- main_widget->setBackgroundColor(params.background_color);
- main_widget->setFillWithBackgroundColor(true);
- dbgprintf("%s<%u> gui$create_window: %d with rect {%d,%d %dx%d}\n", name().characters(), pid(), window_id, params.rect.x(), params.rect.y(), params.rect.width(), params.rect.height());
+ m_windows.set(window_id, move(window));
+ dbgprintf("%s<%u> gui$create_window: %d with rect {%d,%d %dx%d}\n", name().characters(), pid(), window_id, rect.x(), rect.y(), rect.width(), rect.height());
 
  return window_id;
 }
@@ -70,65 +76,9 @@ int Process::gui$destroy_window(int window_id)
  return -EINVAL;
  if (window_id >= static_cast<int>(m_windows.size()))
  return -EBADWINDOW;
- auto* window = m_windows[window_id].ptr();
- if (!window)
- return -EBADWINDOW;
- window->deleteLater();
- return 0;
-}
-
-int Process::gui$create_widget(int window_id, const GUI_CreateWidgetParameters* user_params)
-{
- if (!validate_read_typed(user_params))
- return -EFAULT;
-
- if (window_id < 0)
- return -EINVAL;
- if (window_id >= static_cast<int>(m_windows.size()))
- return -EINVAL;
- if (!m_windows[window_id])
- return -EINVAL;
- auto& window = *m_windows[window_id];
-
- auto params = *user_params;
-
- if (params.rect.is_empty())
- return -EINVAL;
-
- Widget* widget = nullptr;
- switch (params.type) {
- case GUI_WidgetType::Label:
- widget = new Label(window.mainWidget());
- static_cast<Label*>(widget)->setText(params.text);
- widget->setFillWithBackgroundColor(params.opaque);
- break;
- case GUI_WidgetType::Button:
- widget = new Button(window.mainWidget());
- static_cast<Button*>(widget)->setCaption(params.text);
- break;
- }
-
- int widget_id = m_widgets.size();
- m_widgets.append(widget->makeWeakPtr());
-
- widget->setWindowRelativeRect(params.rect);
- widget->setBackgroundColor(params.background_color);
- dbgprintf("%s<%u> gui$create_widget: %d with rect {%d,%d %dx%d}\n", name().characters(), pid(), widget_id, params.rect.x(), params.rect.y(), params.rect.width(), params.rect.height());
-
- return window_id;
-}
-
-int Process::gui$destroy_widget(int widget_id)
-{
- dbgprintf("%s<%u> gui$destroy_widget (widget_id=%d)\n", name().characters(), pid(), widget_id);
- if (widget_id < 0)
- return -EINVAL;
- if (widget_id >= static_cast<int>(m_widgets.size()))
+ auto it = m_windows.find(window_id);
+ if (it == m_windows.end())
  return -EBADWINDOW;
- auto* widget = m_widgets[widget_id].ptr();
- if (!widget)
- return -EBADWIDGET;
- widget->deleteLater();
+ m_windows.remove(window_id);
  return 0;
 }
-
diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp
@@ -9,6 +9,7 @@ static const dword time_slice = 5; // *10 = 50ms
 
 Process* current;
 static Process* s_colonel_process;
+static bool s_in_yield;
 
 struct TaskRedirectionData {
  word selector;
@@ -51,7 +52,7 @@ bool Scheduler::pick_next()
  if (process.state() == Process::BlockedRead) {
  ASSERT(process.m_blocked_fd != -1);
  // FIXME: Block until the amount of data wanted is available.
- if (process.m_fds[process.m_blocked_fd].descriptor->has_data_available_for_reading())
+ if (process.m_fds[process.m_blocked_fd].descriptor->has_data_available_for_reading(process))
  process.unblock();
  return true;
  }
@@ -142,6 +143,9 @@ bool Scheduler::pick_next()
 
 bool Scheduler::yield()
 {
+ ASSERT(!s_in_yield);
+ s_in_yield = true;
+
  if (!current) {
  kprintf("PANIC: sched_yield() with !current");
  HANG;
@@ -150,9 +154,12 @@ bool Scheduler::yield()
  //dbgprintf("%s<%u> yield()\n", current->name().characters(), current->pid());
 
  InterruptDisabler disabler;
- if (!pick_next())
+ if (!pick_next()) {
+ s_in_yield = false;
  return 1;
+ }
 
+ s_in_yield = false;
  //dbgprintf("yield() jumping to new process: %x (%s)\n", current->farPtr().selector, current->name().characters());
  switch_now();
  return 0;
@@ -271,6 +278,7 @@ void Scheduler::initialize()
  initialize_redirection();
  s_colonel_process = Process::create_kernel_process("colonel", nullptr);
  current = nullptr;
+ s_in_yield = false;
  load_task_register(s_redirection.selector);
 }
 

diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp
@@ -191,10 +191,6 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2,
  return current->gui$create_window((const GUI_CreateWindowParameters*)arg1);
  case Syscall::SC_gui_destroy_window:
  return current->gui$destroy_window((int)arg1);
- case Syscall::SC_gui_create_widget:
- return current->gui$create_widget((int)arg1, (const GUI_CreateWidgetParameters*)arg2);
- case Syscall::SC_gui_destroy_widget:
- return current->gui$destroy_widget((int)arg1);
  default:
  kprintf("<%u> int0x80: Unknown function %u requested {%x, %x, %x}\n", current->pid(), function, arg1, arg2, arg3);
  break;

diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h
@@ -68,8 +68,6 @@
  __ENUMERATE_SYSCALL(sync) \
  __ENUMERATE_SYSCALL(gui_create_window) \
  __ENUMERATE_SYSCALL(gui_destroy_window) \
- __ENUMERATE_SYSCALL(gui_create_widget) \
- __ENUMERATE_SYSCALL(gui_destroy_widget) \
 
 namespace Syscall {