shiny/driver/gldriver: implement lifecycle events for X11.

This adds a new shiny/driver/internal/lifecycler package, to share code
across the various drivers.

Also remove the X11 gldriver's ButtonMotionMask event subscription, just
like the x11driver. It is redundant with PointerMotionMask.

Also fix an off-by-one error in the X11 "is a window visible"
calcuation. A 10 pixel wide window located at X == -10 will not be
visible, as the (x+width) upper bound is exclusive, not inclusive.

Change-Id: I0ded722160588e30d1e323a283a0d396ffd398a5
Reviewed-on: https://go-review.googlesource.com/23079
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

shiny/driver/gldriver/cocoa.go

@@ -43,6 +43,8 @@ import (
 	"golang.org/x/mobile/gl"
 )
 
+const useLifecycler = false
+
 var initThreadID C.uint64_t
 
 func init() {

shiny/driver/gldriver/screen.go

@@ -138,6 +138,10 @@ func (s *screenImpl) NewWindow(opts *screen.NewWindowOptions) (screen.Window, er
 	s.windows[id] = w
 	s.mu.Unlock()
 
+	if useLifecycler {
+		w.lifecycler.SendEvent(w)
+	}
+
 	showWindow(w)
 
 	return w, nil

shiny/driver/gldriver/win32.go

@@ -23,6 +23,8 @@ import (
 	"golang.org/x/mobile/gl"
 )
 
+const useLifecycler = false
+
 func main(f func(screen.Screen)) error {
 	return win32.Main(func() { f(theScreen) })
 }

shiny/driver/gldriver/window.go

@@ -12,6 +12,7 @@ import (
 
 	"golang.org/x/exp/shiny/driver/internal/drawer"
 	"golang.org/x/exp/shiny/driver/internal/event"
+	"golang.org/x/exp/shiny/driver/internal/lifecycler"
 	"golang.org/x/exp/shiny/screen"
 	"golang.org/x/image/math/f64"
 	"golang.org/x/mobile/event/lifecycle"
@@ -34,6 +35,9 @@ type windowImpl struct {
 	//	- Windows: ctxWin32
 	ctx interface{}
 
+	lifecycler lifecycler.State
+	// TODO: Delete the field below (and the useLifecycler constant), and use
+	// the field above for cocoa and win32.
 	lifecycleStage lifecycle.Stage // current stage
 
 	event.Queue
@@ -62,8 +66,7 @@ func (w *windowImpl) Release() {
 	//
 	// When the OS closes a window:
 	//	- Cocoa:   Obj-C's windowWillClose calls Go's windowClosing.
-	//	- X11:     TODO: catch WM_DELETE_WINDOW messages (and DestroyNotify
+	//	- X11:     the X11 server sends a WM_DELETE_WINDOW message.
-	//	           events?).
 	//	- Windows: TODO: implement and document this.
 	//
 	// This should send a lifecycle event (To: StageDead) to the Go app's event
@@ -74,7 +77,7 @@ func (w *windowImpl) Release() {
 	//	- Cocoa:   calls Obj-C's performClose, which emulates the red button
 	//	           being clicked. (TODO: document how this actually cleans up
 	//	           resources??)
-	//	- X11:     TODO: call DestroyWindow.
+	//	- X11:     calls C's XDestroyWindow.
 	//	- Windows: TODO: implement and document this.
 	//
 	// On Cocoa, if these two approaches race, experiments suggest that the

shiny/driver/gldriver/x11.c

@@ -9,6 +9,9 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+Atom wm_delete_window;
+Atom wm_protocols;
+Atom wm_take_focus;
 EGLConfig e_config;
 EGLContext e_ctx;
 EGLDisplay e_dpy;
@@ -123,6 +126,10 @@ startDriver() {
 		fprintf(stderr, "eglCreateContext failed: %s\n", eglGetErrorStr());
 		exit(1);
 	}
+
+	wm_delete_window = XInternAtom(x_dpy, "WM_DELETE_WINDOW", False);
+	wm_protocols = XInternAtom(x_dpy, "WM_PROTOCOLS", False);
+	wm_take_focus= XInternAtom(x_dpy, "WM_TAKE_FOCUS", False);
 }
 
 void
@@ -139,6 +146,10 @@ processEvents() {
 		case MotionNotify:
 			onMouse(ev.xmotion.window, ev.xmotion.x, ev.xmotion.y, ev.xmotion.state, 0, 0);
 			break;
+		case FocusIn:
+		case FocusOut:
+			onFocus(ev.xmotion.window, ev.type == FocusIn);
+			break;
 		case Expose:
 			// A non-zero Count means that there are more expose events coming. For
 			// example, a non-rectangular exposure (e.g. from a partially overlapped
@@ -150,7 +161,18 @@ processEvents() {
 			}
 			break;
 		case ConfigureNotify:
-			onResize(ev.xconfigure.window, ev.xconfigure.width, ev.xconfigure.height);
+			onConfigure(ev.xconfigure.window, ev.xconfigure.x, ev.xconfigure.y, ev.xconfigure.width, ev.xconfigure.height);
+			break;
+		case ClientMessage:
+			if ((ev.xclient.message_type != wm_protocols) || (ev.xclient.format != 32)) {
+				break;
+			}
+			Atom a = ev.xclient.data.l[0];
+			if (a == wm_delete_window) {
+				onDeleteWindow(ev.xclient.window);
+			} else if (a == wm_take_focus) {
+				XSetInputFocus(x_dpy, ev.xclient.window, RevertToParent, ev.xclient.data.l[1]);
+			}
 			break;
 		}
 	}
@@ -174,6 +196,12 @@ swapBuffers(uintptr_t surface) {
 	}
 }
 
+void
+doCloseWindow(uintptr_t id) {
+	Window win = (Window)(id);
+	XDestroyWindow(x_dpy, win);
+}
+
 uintptr_t
 doNewWindow(int width, int height) {
 	XSetWindowAttributes attr;
@@ -182,17 +210,25 @@ doNewWindow(int width, int height) {
 		ButtonPressMask |
 		ButtonReleaseMask |
 		PointerMotionMask |
-		ButtonMotionMask |
 		ExposureMask |
-		StructureNotifyMask;
+		StructureNotifyMask |
+		FocusChangeMask;
+
 	Window win = XCreateWindow(
 		x_dpy, x_root, 0, 0, width, height, 0, x_visual_info->depth, InputOutput,
 		x_visual_info->visual, CWColormap | CWEventMask, &attr);
+
 	XSizeHints sizehints;
 	sizehints.width = width;
 	sizehints.height = height;
 	sizehints.flags = USSize;
 	XSetNormalHints(x_dpy, win, &sizehints);
+
+	Atom atoms[2];
+	atoms[0] = wm_delete_window;
+	atoms[1] = wm_take_focus;
+	XSetWMProtocols(x_dpy, win, atoms, 2);
+
 	XSetStandardProperties(x_dpy, win, "App", "App", None, (char **)NULL, 0, &sizehints);
 	return win;
 }

shiny/driver/gldriver/x11.go

@@ -15,6 +15,7 @@ void startDriver();
 void processEvents();
 void makeCurrent(uintptr_t ctx);
 void swapBuffers(uintptr_t ctx);
+void doCloseWindow(uintptr_t id);
 uintptr_t doNewWindow(int width, int height);
 uintptr_t doShowWindow(uintptr_t id);
 */
@@ -32,6 +33,8 @@ import (
 	"golang.org/x/mobile/gl"
 )
 
+const useLifecycler = true
+
 func init() {
 	// It might not be necessary, but it probably doesn't hurt to try to make
 	// 'the main thread' be 'the X11 / OpenGL thread'.
@@ -66,7 +69,12 @@ func showWindow(w *windowImpl) {
 }
 
 func closeWindow(id uintptr) {
-	// TODO.
+	uic <- uiClosure{
+		f: func() uintptr {
+			C.doCloseWindow(C.uintptr_t(id))
+			return 0
+		},
+	}
 }
 
 func drawLoop(w *windowImpl) {
@@ -136,7 +144,10 @@ func main(f func(screen.Screen)) error {
 			C.swapBuffers(C.uintptr_t(w.ctx.(uintptr)))
 			w.publishDone <- screen.PublishResult{}
 		case req := <-uic:
-			req.retc <- req.f()
+			ret := req.f()
+			if req.retc != nil {
+				req.retc <- ret
+			}
 		case <-heartbeat.C:
 			C.processEvents()
 		case <-workAvailable:
@@ -179,8 +190,22 @@ func onMouse(id uintptr, x, y, state, button, dir int32) {
 	})
 }
 
-//export onResize
+//export onFocus
-func onResize(id uintptr, width, height int32) {
+func onFocus(id uintptr, focused bool) {
+	theScreen.mu.Lock()
+	w := theScreen.windows[id]
+	theScreen.mu.Unlock()
+
+	if w == nil {
+		return
+	}
+
+	w.lifecycler.SetFocused(focused)
+	w.lifecycler.SendEvent(w)
+}
+
+//export onConfigure
+func onConfigure(id uintptr, x, y, width, height int32) {
 	theScreen.mu.Lock()
 	w := theScreen.windows[id]
 	theScreen.mu.Unlock()
@@ -198,6 +223,9 @@ func onResize(id uintptr, width, height int32) {
 		w.glctxMu.Unlock()
 	}()
 
+	w.lifecycler.SetVisible(x+width > 0 && y+height > 0)
+	w.lifecycler.SendEvent(w)
+
 	sz := size.Event{
 		WidthPx:  int(width),
 		HeightPx: int(height),
@@ -213,6 +241,18 @@ func onResize(id uintptr, width, height int32) {
 	w.szMu.Unlock()
 
 	w.Send(sz)
-
+}
-	// TODO: lifecycle events?
+
+//export onDeleteWindow
+func onDeleteWindow(id uintptr) {
+	theScreen.mu.Lock()
+	w := theScreen.windows[id]
+	theScreen.mu.Unlock()
+
+	if w == nil {
+		return
+	}
+
+	w.lifecycler.SetDead(true)
+	w.lifecycler.SendEvent(w)
 }

shiny/driver/internal/lifecycler/lifecycler.go

@@ -0,0 +1,77 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package lifecycler tracks a window's lifecycle state.
+//
+// It eliminates sending redundant lifecycle events, ones where the From and To
+// stages are equal. For example, moving a window from one part of the screen
+// to another should not send multiple events from StageVisible to
+// StageVisible, even though the underlying window system's message might only
+// hold the new position, and not whether the window was previously visible.
+package lifecycler // import "golang.org/x/exp/shiny/driver/internal/lifecycler"
+
+import (
+	"sync"
+
+	"golang.org/x/mobile/event/lifecycle"
+)
+
+// State is a window's lifecycle state.
+type State struct {
+	mu      sync.Mutex
+	stage   lifecycle.Stage
+	dead    bool
+	focused bool
+	visible bool
+}
+
+func (s *State) SetDead(b bool) {
+	s.mu.Lock()
+	s.dead = b
+	s.mu.Unlock()
+}
+
+func (s *State) SetFocused(b bool) {
+	s.mu.Lock()
+	s.focused = b
+	s.mu.Unlock()
+}
+
+func (s *State) SetVisible(b bool) {
+	s.mu.Lock()
+	s.visible = b
+	s.mu.Unlock()
+}
+
+func (s *State) SendEvent(r Sender) {
+	s.mu.Lock()
+	from, to := s.stage, lifecycle.StageAlive
+	// The order of these if's is important. For example, once a window becomes
+	// StageDead, it should never change stage again.
+	//
+	// Similarly, focused trumps visible. It's hard to imagine a situation
+	// where a window is focused and not visible on screen, but in that
+	// unlikely case, StageFocused seems the most appropriate stage.
+	if s.dead {
+		to = lifecycle.StageDead
+	} else if s.focused {
+		to = lifecycle.StageFocused
+	} else if s.visible {
+		to = lifecycle.StageVisible
+	}
+	s.stage = to
+	s.mu.Unlock()
+
+	if from != to {
+		r.Send(lifecycle.Event{
+			From: from,
+			To:   to,
+		})
+	}
+}
+
+// Sender is who to send the lifecycle event to.
+type Sender interface {
+	Send(event interface{})
+}

shiny/driver/internal/win32/win32.go

@@ -252,6 +252,9 @@ var (
 	SizeEvent      func(hwnd syscall.Handle, e size.Event)
 	KeyEvent       func(hwnd syscall.Handle, e key.Event)
 	LifecycleEvent func(hwnd syscall.Handle, e lifecycle.Stage)
+
+	// TODO: use the golang.org/x/exp/shiny/driver/internal/lifecycler package
+	// instead of or together with the LifecycleEvent callback?
 )
 
 func sendPaint(hwnd syscall.Handle, uMsg uint32, wParam, lParam uintptr) (lResult uintptr) {

shiny/driver/x11driver/screen.go

@@ -109,10 +109,8 @@ func (s *screenImpl) run() {
 			switch xproto.Atom(ev.Data.Data32[0]) {
 			case s.atomWMDeleteWindow:
 				if w := s.findWindow(ev.Window); w != nil {
-					w.mu.Lock()
+					w.lifecycler.SetDead(true)
-					w.dead = true
+					w.lifecycler.SendEvent(w)
-					w.mu.Unlock()
-					w.sendLifecycle()
 				} else {
 					noWindowFound = true
 				}
@@ -144,20 +142,16 @@ func (s *screenImpl) run() {
 
 		case xproto.FocusInEvent:
 			if w := s.findWindow(ev.Event); w != nil {
-				w.mu.Lock()
+				w.lifecycler.SetFocused(true)
-				w.focused = true
+				w.lifecycler.SendEvent(w)
-				w.mu.Unlock()
-				w.sendLifecycle()
 			} else {
 				noWindowFound = true
 			}
 
 		case xproto.FocusOutEvent:
 			if w := s.findWindow(ev.Event); w != nil {
-				w.mu.Lock()
+				w.lifecycler.SetFocused(false)
-				w.focused = false
+				w.lifecycler.SendEvent(w)
-				w.mu.Unlock()
-				w.sendLifecycle()
 			} else {
 				noWindowFound = true
 			}
@@ -375,7 +369,7 @@ func (s *screenImpl) NewWindow(opts *screen.NewWindowOptions) (screen.Window, er
 	s.windows[xw] = w
 	s.mu.Unlock()
 
-	w.sendLifecycle()
+	w.lifecycler.SendEvent(w)
 
 	xproto.CreateWindow(s.xc, s.xsi.RootDepth, xw, s.xsi.Root,
 		0, 0, uint16(width), uint16(height), 0,

shiny/driver/x11driver/window.go

@@ -18,11 +18,11 @@ import (
 
 	"golang.org/x/exp/shiny/driver/internal/drawer"
 	"golang.org/x/exp/shiny/driver/internal/event"
+	"golang.org/x/exp/shiny/driver/internal/lifecycler"
 	"golang.org/x/exp/shiny/driver/internal/x11key"
 	"golang.org/x/exp/shiny/screen"
 	"golang.org/x/image/math/f64"
 	"golang.org/x/mobile/event/key"
-	"golang.org/x/mobile/event/lifecycle"
 	"golang.org/x/mobile/event/mouse"
 	"golang.org/x/mobile/event/paint"
 	"golang.org/x/mobile/event/size"
@@ -43,48 +43,20 @@ type windowImpl struct {
 	// screenImpl.run goroutine.
 	width, height int
 
+	lifecycler lifecycler.State
+
 	mu       sync.Mutex
-	stage    lifecycle.Stage
-	dead     bool
-	visible  bool
-	focused  bool
 	released bool
 }
 
-func (w *windowImpl) sendLifecycle() {
-	w.mu.Lock()
-	from, to := w.stage, lifecycle.StageAlive
-	// The order of these if's is important. For example, once a window becomes
-	// StageDead, it should never change stage again.
-	//
-	// Similarly, focused trumps visible. It's hard to imagine a situation
-	// where an X11 window is focused and not visible on screen, but in that
-	// unlikely case, StageFocused seems the most appropriate stage.
-	if w.dead {
-		to = lifecycle.StageDead
-	} else if w.focused {
-		to = lifecycle.StageFocused
-	} else if w.visible {
-		to = lifecycle.StageVisible
-	}
-	w.stage = to
-	w.mu.Unlock()
-
-	if from != to {
-		w.Send(lifecycle.Event{
-			From: from,
-			To:   to,
-		})
-	}
-}
-
 func (w *windowImpl) Release() {
 	w.mu.Lock()
 	released := w.released
 	w.released = true
 	w.mu.Unlock()
 
-	// TODO: set w.dead and call w.sendLifecycle, a la handling atomWMDeleteWindow?
+	// TODO: call w.lifecycler.SetDead and w.lifecycler.SendEvent, a la
+	// handling atomWMDeleteWindow?
 
 	if released {
 		return
@@ -126,11 +98,8 @@ func (w *windowImpl) Publish() screen.PublishResult {
 func (w *windowImpl) handleConfigureNotify(ev xproto.ConfigureNotifyEvent) {
 	// TODO: does the order of these lifecycle and size events matter? Should
 	// they really be a single, atomic event?
-	w.mu.Lock()
+	w.lifecycler.SetVisible((int(ev.X)+int(ev.Width)) > 0 && (int(ev.Y)+int(ev.Height)) > 0)
-	w.visible = (int(ev.X)+int(ev.Width)) >= 0 && (int(ev.Y)+int(ev.Height)) >= 0
+	w.lifecycler.SendEvent(w)
-	w.mu.Unlock()
-
-	w.sendLifecycle()
 
 	newWidth, newHeight := int(ev.Width), int(ev.Height)
 	if w.width == newWidth && w.height == newHeight {