play nice with window managers to close window cleanly

panda/src/glxdisplay/glxGraphicsPipe.I

@@ -48,3 +48,14 @@ INLINE Window glxGraphicsPipe::
 get_root() const {
   return _root;
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: glxGraphicsPipe::get_wm_delete_window
+//       Access: Public
+//  Description: Returns the X atom that represents WM_DELETE_WINDOW
+//               to the current display.
+////////////////////////////////////////////////////////////////////
+INLINE Atom glxGraphicsPipe::
+get_wm_delete_window() const {
+  return _wm_delete_window;
+}

panda/src/glxdisplay/glxGraphicsPipe.cxx

@@ -25,6 +25,10 @@
 
 TypeHandle glxGraphicsPipe::_type_handle;
 
+bool glxGraphicsPipe::_error_handlers_installed = false;
+glxGraphicsPipe::ErrorHandlerFunc *glxGraphicsPipe::_prev_error_handler;
+glxGraphicsPipe::IOErrorHandlerFunc *glxGraphicsPipe::_prev_io_error_handler;
+
 ////////////////////////////////////////////////////////////////////
 //     Function: glxGraphicsPipe::Constructor
 //       Access: Public
@@ -46,6 +50,8 @@ glxGraphicsPipe(const string &display) {
   _screen = 0;
   _root = (Window)NULL;
 
+  install_error_handlers();
+
   _display = XOpenDisplay(display_spec.c_str());
   if (!_display) {
     glxdisplay_cat.error()
@@ -66,6 +72,9 @@ glxGraphicsPipe(const string &display) {
   _display_width = DisplayWidth(_display, _screen);
   _display_height = DisplayHeight(_display, _screen);
   _is_valid = true;
+
+  // Get the X atom number.
+  _wm_delete_window = XInternAtom(_display, "WM_DELETE_WINDOW", false);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -120,3 +129,62 @@ make_window() {
 
   return new glxGraphicsWindow(this);
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: glxGraphicsPipe::install_error_handlers
+//       Access: Private, Static
+//  Description: Installs new Xlib error handler functions if this is
+//               the first time this function has been called.  These
+//               error handler functions will attempt to reduce Xlib's
+//               annoying tendency to shut down the client at the
+//               first error.  Unfortunately, it is difficult to play
+//               nice with the client if it has already installed its
+//               own error handlers.
+////////////////////////////////////////////////////////////////////
+void glxGraphicsPipe::
+install_error_handlers() {
+  if (_error_handlers_installed) {
+    return;
+  }
+
+  _prev_error_handler = (ErrorHandlerFunc *)XSetErrorHandler(error_handler);
+  _prev_io_error_handler = (IOErrorHandlerFunc *)XSetIOErrorHandler(io_error_handler);
+  _error_handlers_installed = true;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: glxGraphicsPipe::error_handler
+//       Access: Private, Static
+//  Description: This function is installed as the error handler for a
+//               non-fatal Xlib error.
+////////////////////////////////////////////////////////////////////
+int glxGraphicsPipe::
+error_handler(Display *display, XErrorEvent *error) {
+  static const int msg_len = 80;
+  char msg[msg_len];
+  XGetErrorText(display, error->error_code, msg, msg_len);
+  glxdisplay_cat.error()
+    << msg << "\n";
+
+  // We return to allow the application to continue running, unlike
+  // the default X error handler which exits.
+  return 0;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: glxGraphicsPipe::io_error_handler
+//       Access: Private, Static
+//  Description: This function is installed as the error handler for a
+//               fatal Xlib error.
+////////////////////////////////////////////////////////////////////
+int glxGraphicsPipe::
+io_error_handler(Display *display) {
+  glxdisplay_cat.fatal()
+    << "X fatal error on display " << (void *)display << "\n";
+
+  // Unfortunately, we can't continue from this function, even if we
+  // promise never to use X again.  We're supposed to terminate
+  // without returning, and if we do return, the caller will exit
+  // anyway.  Sigh.  Very poor design on X's part.
+  return 0;
+}

panda/src/glxdisplay/glxGraphicsPipe.h

@@ -30,6 +30,7 @@ class glxGraphicsWindow;
 // A simple hack so interrogate can parse this file.
 typedef int Display;
 typedef int Window;
+typedef int XErrorEvent;
 #endif
 
 ////////////////////////////////////////////////////////////////////
@@ -50,14 +51,28 @@ public:
   INLINE int get_screen() const;
   INLINE Window get_root() const;
 
+  INLINE Atom get_wm_delete_window() const;
+
 protected:
   virtual PT(GraphicsWindow) make_window();
 
 private:
+  static void install_error_handlers();
+  static int error_handler(Display *display, XErrorEvent *error);
+  static int io_error_handler(Display *display);
+
   Display *_display;
   int _screen;
   Window _root;
 
+  Atom _wm_protocols;
+  Atom _wm_delete_window;
+
+  typedef int ErrorHandlerFunc(Display *, XErrorEvent *);
+  typedef int IOErrorHandlerFunc(Display *);
+  static bool _error_handlers_installed;
+  static ErrorHandlerFunc *_prev_error_handler;
+  static IOErrorHandlerFunc *_prev_io_error_handler;
 
 public:
   static TypeHandle get_class_type() {

panda/src/glxdisplay/glxGraphicsWindow.cxx

@@ -50,6 +50,7 @@ glxGraphicsWindow(GraphicsPipe *pipe) :
   _context = (GLXContext)0;
   _visual = (XVisualInfo *)NULL;
   _awaiting_configure = false;
+  _wm_delete_window = glx_pipe->get_wm_delete_window();
 
   GraphicsWindowInputDevice device =
     GraphicsWindowInputDevice::pointer_and_keyboard("keyboard/mouse");
@@ -182,7 +183,7 @@ process_events() {
   }
 
   XEvent event;
-  while (XCheckWindowEvent(_display, _xwindow, _event_mask, &event)) {
+  while (XCheckIfEvent(_display, &event, check_event, (char *)this)) {
     WindowProperties properties;
     ButtonHandle button;
 
@@ -268,12 +269,30 @@ process_events() {
       system_changed_properties(properties);
       break;
 
+    case ClientMessage:
+      if (event.xclient.data.l[0] == _wm_delete_window) {
+        // This is a message from the window manager indicating that
+        // the user has requested to close the window.  Honor the
+        // request.
+        // TODO: don't call release_gsg() in the window thread.
+        release_gsg();
+        close_window();
+        properties.set_open(false);
+        system_changed_properties(properties);
+      }
+      break;
+
     case DestroyNotify:
-      cerr << "destroy\n";
+      // Apparently, we never get a DestroyNotify on a toplevel
+      // window.  Instead, we rely on hints from the window manager
+      // (see above).
+      glxdisplay_cat.info()
+        << "DestroyNotify\n";
       break;
 
     default:
-      cerr << "unhandled X event type " << event.type << "\n";
+      glxdisplay_cat.error()
+        << "unhandled X event type " << event.type << "\n";
     }
   }
 }
@@ -392,7 +411,7 @@ open_window() {
   }
   setup_colormap();
 
-  _event_mask = 
+  _event_mask =
     ButtonPressMask | ButtonReleaseMask |
     KeyPressMask | KeyReleaseMask |
     EnterWindowMask | LeaveWindowMask |
@@ -406,7 +425,6 @@ open_window() {
   wa.border_pixel = 0;
   wa.colormap = _colormap;
   wa.event_mask = _event_mask;
-  wa.do_not_propagate_mask = 0;
 
   unsigned long attrib_mask = 
     CWBackPixel | CWBorderPixel | CWColormap | CWEventMask;
@@ -452,7 +470,8 @@ set_wm_properties(const WindowProperties &properties) {
     }
   }
 
-  // Setup size hints
+  // The size hints request a window of a particular size and/or a
+  // particular placement onscreen.
   XSizeHints *size_hints_p = NULL;
   if (properties.has_origin() || properties.has_size()) {
     size_hints_p = XAllocSizeHints();
@@ -470,7 +489,8 @@ set_wm_properties(const WindowProperties &properties) {
     }
   }
 
-  // Setup window manager hints
+  // The window manager hints include requests to the window manager
+  // other than those specific to window geometry.
   XWMHints *wm_hints_p = NULL;
   wm_hints_p = XAllocWMHints();
   if (wm_hints_p != (XWMHints *)NULL) {
@@ -505,6 +525,17 @@ set_wm_properties(const WindowProperties &properties) {
   if (class_hints_p != (XClassHint *)NULL) {
     XFree(class_hints_p);
   }
+
+  // Also, indicate to the window manager that we'd like to get a
+  // chance to close our windows cleanly, rather than being rudely
+  // disconnected from the X server if the user requests a window
+  // close.
+  Atom protocols[] = {
+    _wm_delete_window,
+  };
+
+  XSetWMProtocols(_display, _xwindow, protocols, 
+                  sizeof(protocols) / sizeof(Atom));
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -1147,3 +1178,19 @@ get_button(XKeyEvent *key_event) {
 
   return ButtonHandle::none();
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: glxGraphicsWindow::check_event
+//       Access: Private, Static
+//  Description: This function is used as a predicate to
+//               XCheckIfEvent() to determine if the indicated queued
+//               X event is relevant and should be returned to this
+//               window.
+////////////////////////////////////////////////////////////////////
+Bool glxGraphicsWindow::
+check_event(Display *display, XEvent *event, char *arg) {
+  const glxGraphicsWindow *self = (glxGraphicsWindow *)arg;
+
+  // We accept any event that is sent to our window.
+  return (event->xany.window == self->_xwindow);
+}

panda/src/glxdisplay/glxGraphicsWindow.h

@@ -62,6 +62,8 @@ private:
   void setup_colormap();
   ButtonHandle get_button(XKeyEvent *key_event);
 
+  static Bool check_event(Display *display, XEvent *event, char *arg);
+
 private:
   Display *_display;
   int _screen;
@@ -71,6 +73,7 @@ private:
   Colormap _colormap;
   long _event_mask;
   bool _awaiting_configure;
+  Atom _wm_delete_window;
 
 
 public: