exp/draw: rename Context to Window, and add a Close method.

exp/draw/x11: allow clean shutdown when the user closes the window.

R=r
CC=golang-dev
https://golang.org/cl/2134045

src/pkg/exp/draw/event.go

@@ -6,19 +6,20 @@ package draw
 
 import (
 	"image"
+	"os"
 )
 
-// A Context represents a single graphics window.
-type Context interface {
-	// Screen returns an editable Image of window.
+// A Window represents a single graphics window.
+type Window interface {
+	// Screen returns an editable Image for the window.
 	Screen() Image
-
 	// FlushImage flushes changes made to Screen() back to screen.
 	FlushImage()
-
 	// EventChan returns a channel carrying UI events such as key presses,
 	// mouse movements and window resizes.
 	EventChan() <-chan interface{}
+	// Close closes the window.
+	Close() os.Error
 }
 
 // A KeyEvent is sent for a key press or release.
@@ -44,7 +45,12 @@ type MouseEvent struct {
 }
 
 // A ConfigEvent is sent each time the window's color model or size changes.
-// The client should respond by calling Context.Screen to obtain a new image.
+// The client should respond by calling Window.Screen to obtain a new image.
 type ConfigEvent struct {
 	Config image.Config
 }
+
+// An ErrEvent is sent when an error occurs.
+type ErrEvent struct {
+	Err os.Error
+}

src/pkg/exp/draw/x11/conn.go

@@ -15,6 +15,7 @@ import (
 	"exp/draw"
 	"image"
 	"io"
+	"log"
 	"net"
 	"os"
 	"strconv"
@@ -36,9 +37,6 @@ const (
 )
 
 type conn struct {
-	// TODO(nigeltao): Figure out which goroutine should be responsible for closing c,
-	// or if there is a race condition if one goroutine calls c.Close whilst another one
-	// is reading from r, or writing to w.
 	c io.Closer
 	r *bufio.Reader
 	w *bufio.Writer
@@ -56,15 +54,12 @@ type conn struct {
 	flushBuf1 [4 * 1024]byte
 }
 
-// flusher runs in its own goroutine, serving both FlushImage calls directly from the exp/draw client
-// and indirectly from X expose events. It paints c.img to the X server via PutImage requests.
-func (c *conn) flusher() {
-	for {
-		_ = <-c.flush
-		if closed(c.flush) {
-			return
-		}
-
+// writeSocket runs in its own goroutine, serving both FlushImage calls
+// directly from the exp/draw client and indirectly from X expose events.
+// It paints c.img to the X server via PutImage requests.
+func (c *conn) writeSocket() {
+	defer c.c.Close()
+	for _ = range c.flush {
 		b := c.img.Bounds()
 		if b.Empty() {
 			continue
@@ -76,8 +71,7 @@ func (c *conn) flusher() {
 		// TODO(nigeltao): See what XCB's xcb_put_image does in this situation.
 		units := 6 + b.Dx()
 		if units > 0xffff || b.Dy() > 0xffff {
-			// This window is too large for X.
-			close(c.flush)
+			log.Stderr("x11: window is too large for PutImage")
 			return
 		}
 
@@ -92,9 +86,10 @@ func (c *conn) flusher() {
 
 		for y := b.Min.Y; y < b.Max.Y; y++ {
 			setU32LE(c.flushBuf0[16:20], uint32(y<<16))
-			_, err := c.w.Write(c.flushBuf0[0:24])
-			if err != nil {
-				close(c.flush)
+			if _, err := c.w.Write(c.flushBuf0[0:24]); err != nil {
+				if err != os.EOF {
+					log.Stderr("x11: " + err.String())
+				}
 				return
 			}
 			p := c.img.Pix[y*c.img.Stride : (y+1)*c.img.Stride]
@@ -109,15 +104,18 @@ func (c *conn) flusher() {
 					c.flushBuf1[4*i+2] = rgba.R
 				}
 				x += nx
-				_, err := c.w.Write(c.flushBuf1[0 : 4*nx])
-				if err != nil {
-					close(c.flush)
+				if _, err := c.w.Write(c.flushBuf1[0 : 4*nx]); err != nil {
+					if err != os.EOF {
+						log.Stderr("x11: " + err.String())
+					}
 					return
 				}
 			}
 		}
-		if c.w.Flush() != nil {
-			close(c.flush)
+		if err := c.w.Flush(); err != nil {
+			if err != os.EOF {
+				log.Stderr("x11: " + err.String())
+			}
 			return
 		}
 	}
@@ -132,26 +130,32 @@ func (c *conn) FlushImage() {
 	_ = c.flush <- false
 }
 
+func (c *conn) Close() os.Error {
+	// Shut down the writeSocket goroutine. This will close the socket to the
+	// X11 server, which will cause c.eventc to close.
+	close(c.flush)
+	for _ = range c.eventc {
+		// Drain the channel to allow the readSocket goroutine to shut down.
+	}
+	return nil
+}
+
 func (c *conn) EventChan() <-chan interface{} { return c.eventc }
 
-// pumper runs in its own goroutine, reading X events and demuxing them over the kbd / mouse / resize / quit chans.
-func (c *conn) pumper() {
+// readSocket runs in its own goroutine, reading X events and sending draw
+// events on c's EventChan.
+func (c *conn) readSocket() {
 	var (
 		keymap            [256][]int
 		keysymsPerKeycode int
 	)
-	defer close(c.flush)
-	// TODO(nigeltao): Is this the right place for defer c.c.Close()?
-	// TODO(nigeltao): Should we explicitly defer close our kbd/mouse/resize/quit chans?
+	defer close(c.eventc)
 	for {
 		// X events are always 32 bytes long.
-		_, err := io.ReadFull(c.r, c.buf[0:32])
-		if err != nil {
-			// TODO(nigeltao): should draw.Context expose err?
-			// TODO(nigeltao): should we do c.quit<-true? Should c.quit be a buffered channel?
-			// Or is c.quit only for non-exceptional closing (e.g. when the window manager destroys
-			// our window), and not for e.g. an I/O error?
-			os.Stderr.Write([]byte(err.String()))
+		if _, err := io.ReadFull(c.r, c.buf[0:32]); err != nil {
+			if err != os.EOF {
+				c.eventc <- draw.ErrEvent{err}
+			}
 			return
 		}
 		switch c.buf[0] {
@@ -160,7 +164,7 @@ func (c *conn) pumper() {
 			if cookie != 1 {
 				// We issued only one request (GetKeyboardMapping) with a cookie of 1,
 				// so we shouldn't get any other reply from the X server.
-				os.Stderr.Write([]byte("exp/draw/x11: unexpected cookie\n"))
+				c.eventc <- draw.ErrEvent{os.NewError("x11: unexpected cookie")}
 				return
 			}
 			keysymsPerKeycode = int(c.buf[1])
@@ -173,7 +177,9 @@ func (c *conn) pumper() {
 				for j := range m {
 					u, err := readU32LE(c.r, c.buf[0:4])
 					if err != nil {
-						os.Stderr.Write([]byte(err.String()))
+						if err != os.EOF {
+							c.eventc <- draw.ErrEvent{err}
+						}
 						return
 					}
 					m[j] = int(u)
@@ -194,10 +200,10 @@ func (c *conn) pumper() {
 			if keysym == 0 {
 				keysym = keymap[keycode][0]
 			}
-			// TODO(nigeltao): Should we send KeyboardChan ints for Shift/Ctrl/Alt? Should Shift-A send
+			// TODO(nigeltao): Should we send KeyEvents for Shift/Ctrl/Alt? Should Shift-A send
 			// the same int down the channel as the sent on just the A key?
 			// TODO(nigeltao): How should IME events (e.g. key presses that should generate CJK text) work? Or
-			// is that outside the scope of the draw.Context interface?
+			// is that outside the scope of the draw.Window interface?
 			if c.buf[0] == 0x03 {
 				keysym = -keysym
 			}
@@ -545,7 +551,7 @@ func (c *conn) handshake() os.Error {
 }
 
 // NewWindow calls NewWindowDisplay with $DISPLAY.
-func NewWindow() (draw.Context, os.Error) {
+func NewWindow() (draw.Window, os.Error) {
 	display := os.Getenv("DISPLAY")
 	if len(display) == 0 {
 		return nil, os.NewError("$DISPLAY not set")
@@ -553,10 +559,10 @@ func NewWindow() (draw.Context, os.Error) {
 	return NewWindowDisplay(display)
 }
 
-// NewWindowDisplay returns a new draw.Context, backed by a newly created and
+// NewWindowDisplay returns a new draw.Window, backed by a newly created and
 // mapped X11 window. The X server to connect to is specified by the display
 // string, such as ":1".
-func NewWindowDisplay(display string) (draw.Context, os.Error) {
+func NewWindowDisplay(display string) (draw.Window, os.Error) {
 	socket, displayStr, err := connect(display)
 	if err != nil {
 		return nil, err
@@ -611,9 +617,9 @@ func NewWindowDisplay(display string) (draw.Context, os.Error) {
 	}
 
 	c.img = image.NewRGBA(windowWidth, windowHeight)
-	c.eventc = make(chan interface{})
+	c.eventc = make(chan interface{}, 16)
 	c.flush = make(chan bool, 1)
-	go c.flusher()
-	go c.pumper()
+	go c.readSocket()
+	go c.writeSocket()
 	return c, nil
 }