From 831f5ae1eb053616df08b0668f391cb039bedda6 Mon Sep 17 00:00:00 2001 From: Nigel Tao Date: Fri, 13 May 2016 14:36:22 +1000 Subject: [PATCH] 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 Reviewed-by: Andrew Gerrand --- shiny/driver/gldriver/cocoa.go | 2 + shiny/driver/gldriver/screen.go | 4 + shiny/driver/gldriver/win32.go | 2 + shiny/driver/gldriver/window.go | 9 ++- shiny/driver/gldriver/x11.c | 42 +++++++++- shiny/driver/gldriver/x11.go | 52 +++++++++++-- .../driver/internal/lifecycler/lifecycler.go | 77 +++++++++++++++++++ shiny/driver/internal/win32/win32.go | 3 + shiny/driver/x11driver/screen.go | 20 ++--- shiny/driver/x11driver/window.go | 45 ++--------- 10 files changed, 193 insertions(+), 63 deletions(-) create mode 100644 shiny/driver/internal/lifecycler/lifecycler.go diff --git a/shiny/driver/gldriver/cocoa.go b/shiny/driver/gldriver/cocoa.go index fe739de..a9ac19c 100644 --- a/shiny/driver/gldriver/cocoa.go +++ b/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() { diff --git a/shiny/driver/gldriver/screen.go b/shiny/driver/gldriver/screen.go index e9638cd..6f5e0f0 100644 --- a/shiny/driver/gldriver/screen.go +++ b/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 diff --git a/shiny/driver/gldriver/win32.go b/shiny/driver/gldriver/win32.go index 7df11f0..e7fdf82 100755 --- a/shiny/driver/gldriver/win32.go +++ b/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) }) } diff --git a/shiny/driver/gldriver/window.go b/shiny/driver/gldriver/window.go index dc39845..94e6050 100644 --- a/shiny/driver/gldriver/window.go +++ b/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 - // events?). + // - X11: the X11 server sends a WM_DELETE_WINDOW message. // - 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 diff --git a/shiny/driver/gldriver/x11.c b/shiny/driver/gldriver/x11.c index c367ed9..fc244c3 100644 --- a/shiny/driver/gldriver/x11.c +++ b/shiny/driver/gldriver/x11.c @@ -9,6 +9,9 @@ #include #include +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; } diff --git a/shiny/driver/gldriver/x11.go b/shiny/driver/gldriver/x11.go index e4fb610..3f25a02 100644 --- a/shiny/driver/gldriver/x11.go +++ b/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 -func onResize(id uintptr, width, height int32) { +//export onFocus +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) } diff --git a/shiny/driver/internal/lifecycler/lifecycler.go b/shiny/driver/internal/lifecycler/lifecycler.go new file mode 100644 index 0000000..596f5e3 --- /dev/null +++ b/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{}) +} diff --git a/shiny/driver/internal/win32/win32.go b/shiny/driver/internal/win32/win32.go index d374751..62d32f1 100644 --- a/shiny/driver/internal/win32/win32.go +++ b/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) { diff --git a/shiny/driver/x11driver/screen.go b/shiny/driver/x11driver/screen.go index 93773f4..7d51433 100644 --- a/shiny/driver/x11driver/screen.go +++ b/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.dead = true - w.mu.Unlock() - w.sendLifecycle() + w.lifecycler.SetDead(true) + w.lifecycler.SendEvent(w) } 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.focused = true - w.mu.Unlock() - w.sendLifecycle() + w.lifecycler.SetFocused(true) + w.lifecycler.SendEvent(w) } else { noWindowFound = true } case xproto.FocusOutEvent: if w := s.findWindow(ev.Event); w != nil { - w.mu.Lock() - w.focused = false - w.mu.Unlock() - w.sendLifecycle() + w.lifecycler.SetFocused(false) + w.lifecycler.SendEvent(w) } 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, diff --git a/shiny/driver/x11driver/window.go b/shiny/driver/x11driver/window.go index 77412bd..56627c5 100644 --- a/shiny/driver/x11driver/window.go +++ b/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.visible = (int(ev.X)+int(ev.Width)) >= 0 && (int(ev.Y)+int(ev.Height)) >= 0 - w.mu.Unlock() - - w.sendLifecycle() + w.lifecycler.SetVisible((int(ev.X)+int(ev.Width)) > 0 && (int(ev.Y)+int(ev.Height)) > 0) + w.lifecycler.SendEvent(w) newWidth, newHeight := int(ev.Width), int(ev.Height) if w.width == newWidth && w.height == newHeight {