diff --git a/shiny/driver/gldriver/buffer.go b/shiny/driver/gldriver/buffer.go index acbf210..94b4b9a 100644 --- a/shiny/driver/gldriver/buffer.go +++ b/shiny/driver/gldriver/buffer.go @@ -7,11 +7,26 @@ package gldriver import "image" type bufferImpl struct { - rgba *image.RGBA + // buf should always be equal to (i.e. the same ptr, len, cap as) rgba.Pix. + // It is a separate, redundant field in order to detect modifications to + // the rgba field that are invalid as per the screen.Buffer documentation. + buf []byte + rgba image.RGBA size image.Point } func (b *bufferImpl) Release() {} func (b *bufferImpl) Size() image.Point { return b.size } func (b *bufferImpl) Bounds() image.Rectangle { return image.Rectangle{Max: b.size} } -func (b *bufferImpl) RGBA() *image.RGBA { return b.rgba } +func (b *bufferImpl) RGBA() *image.RGBA { return &b.rgba } + +func (b *bufferImpl) preUpload() { + // Check that the program hasn't tried to modify the rgba field via the + // pointer returned by the bufferImpl.RGBA method. This check doesn't catch + // 100% of all cases; it simply tries to detect some invalid uses of a + // screen.Buffer such as: + // *buffer.RGBA() = anotherImageRGBA + if len(b.buf) != 0 && len(b.rgba.Pix) != 0 && &b.buf[0] != &b.rgba.Pix[0] { + panic("gldriver: invalid Buffer.RGBA modification") + } +} diff --git a/shiny/driver/gldriver/screen.go b/shiny/driver/gldriver/screen.go index b9caa71..e9638cd 100644 --- a/shiny/driver/gldriver/screen.go +++ b/shiny/driver/gldriver/screen.go @@ -40,8 +40,10 @@ type screenImpl struct { } func (s *screenImpl) NewBuffer(size image.Point) (retBuf screen.Buffer, retErr error) { + m := image.NewRGBA(image.Rectangle{Max: size}) return &bufferImpl{ - rgba: image.NewRGBA(image.Rectangle{Max: size}), + buf: m.Pix, + rgba: *m, size: size, }, nil } diff --git a/shiny/driver/gldriver/texture.go b/shiny/driver/gldriver/texture.go index 9db7191..ca5a598 100644 --- a/shiny/driver/gldriver/texture.go +++ b/shiny/driver/gldriver/texture.go @@ -32,12 +32,15 @@ func (t *textureImpl) Release() { } func (t *textureImpl) Upload(dp image.Point, src screen.Buffer, sr image.Rectangle) { + buf := src.(*bufferImpl) + buf.preUpload() + t.w.glctxMu.Lock() defer t.w.glctxMu.Unlock() // TODO: adjust if dp is outside dst bounds, or r is outside src bounds. t.w.glctx.BindTexture(gl.TEXTURE_2D, t.id) - m := src.(*bufferImpl).rgba.SubImage(sr).(*image.RGBA) + m := buf.rgba.SubImage(sr).(*image.RGBA) b := m.Bounds() // TODO check m bounds smaller than t.size t.w.glctx.TexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, b.Dx(), b.Dy(), gl.RGBA, gl.UNSIGNED_BYTE, m.Pix) diff --git a/shiny/driver/windriver/buffer.go b/shiny/driver/windriver/buffer.go index 5223873..3914f7a 100644 --- a/shiny/driver/windriver/buffer.go +++ b/shiny/driver/windriver/buffer.go @@ -32,6 +32,15 @@ func (b *bufferImpl) Bounds() image.Rectangle { return image.Rectangle{Max: b.si func (b *bufferImpl) RGBA() *image.RGBA { return &b.rgba } func (b *bufferImpl) preUpload(reusable bool) { + // Check that the program hasn't tried to modify the rgba field via the + // pointer returned by the bufferImpl.RGBA method. This check doesn't catch + // 100% of all cases; it simply tries to detect some invalid uses of a + // screen.Buffer such as: + // *buffer.RGBA() = anotherImageRGBA + if len(b.buf) != 0 && len(b.rgba.Pix) != 0 && &b.buf[0] != &b.rgba.Pix[0] { + panic("windriver: invalid Buffer.RGBA modification") + } + b.mu.Lock() defer b.mu.Unlock() diff --git a/shiny/driver/x11driver/buffer.go b/shiny/driver/x11driver/buffer.go index 4de5081..698540a 100644 --- a/shiny/driver/x11driver/buffer.go +++ b/shiny/driver/x11driver/buffer.go @@ -41,6 +41,15 @@ func (b *bufferImpl) Bounds() image.Rectangle { return image.Rectangle{Max: b.si func (b *bufferImpl) RGBA() *image.RGBA { return &b.rgba } func (b *bufferImpl) preUpload() { + // Check that the program hasn't tried to modify the rgba field via the + // pointer returned by the bufferImpl.RGBA method. This check doesn't catch + // 100% of all cases; it simply tries to detect some invalid uses of a + // screen.Buffer such as: + // *buffer.RGBA() = anotherImageRGBA + if len(b.buf) != 0 && len(b.rgba.Pix) != 0 && &b.buf[0] != &b.rgba.Pix[0] { + panic("x11driver: invalid Buffer.RGBA modification") + } + b.mu.Lock() defer b.mu.Unlock() diff --git a/shiny/screen/screen.go b/shiny/screen/screen.go index aab85f5..68beb78 100644 --- a/shiny/screen/screen.go +++ b/shiny/screen/screen.go @@ -80,7 +80,10 @@ type Screen interface { // Buffer is an in-memory pixel buffer. Its pixels can be modified by any Go // code that takes an *image.RGBA, such as the standard library's image/draw -// package. +// package. A Buffer is essentially an *image.RGBA, but not all *image.RGBA +// values (including those returned by image.NewRGBA) are valid Buffers, as a +// driver may assume that the memory backing a Buffer's pixels are specially +// allocated. // // To see a Buffer's contents on a screen, upload it to a Texture (and then // draw the Texture on a Window) or upload it directly to a Window. @@ -105,6 +108,31 @@ type Buffer interface { // RGBA returns the pixel buffer as an *image.RGBA. // // Its contents should not be accessed while the Buffer is uploading. + // + // The contents of the returned *image.RGBA's Pix field (of type []byte) + // can be modified at other times, but that Pix slice itself (i.e. its + // underlying pointer, length and capacity) should not be modified at any + // time. + // + // The following is valid: + // m := buffer.RGBA() + // if len(m.Pix) >= 4 { + // m.Pix[0] = 0xff + // m.Pix[1] = 0x00 + // m.Pix[2] = 0x00 + // m.Pix[3] = 0xff + // } + // or, equivalently: + // m := buffer.RGBA() + // m.SetRGBA(m.Rect.Min.X, m.Rect.Min.Y, color.RGBA{0xff, 0x00, 0x00, 0xff}) + // and using the standard library's image/draw package is also valid: + // dst := buffer.RGBA() + // draw.Draw(dst, dst.Bounds(), etc) + // but the following is invalid: + // m := buffer.RGBA() + // m.Pix = anotherByteSlice + // and so is this: + // *buffer.RGBA() = anotherImageRGBA RGBA() *image.RGBA }