From: Nigel Tao Date: Thu, 9 Sep 2010 08:06:59 +0000 (+1000) Subject: exp/draw: rename Context to Window, and add a Close method. X-Git-Tag: weekly.2010-09-15~76 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6c8b85273c2a38cd72fa3f21e947c61503bb7113;p=gostls13.git 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 --- diff --git a/src/pkg/exp/draw/event.go b/src/pkg/exp/draw/event.go index c4ec43702a..b777d912e1 100644 --- a/src/pkg/exp/draw/event.go +++ b/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 +} diff --git a/src/pkg/exp/draw/x11/conn.go b/src/pkg/exp/draw/x11/conn.go index 7c95883263..8dfa9a48c9 100644 --- a/src/pkg/exp/draw/x11/conn.go +++ b/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 }