From 53e1f9014629783450ff80edbff1b28c27cce369 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Tue, 30 Aug 2016 23:31:59 +0900 Subject: [PATCH] graphics: Bug fix: Don't store interface color.Color value inside (#261) --- image_test.go | 32 ++++++++++++++++++++++++++++++++ imageimpl.go | 7 ++++--- internal/graphics/command.go | 6 +++--- internal/graphics/image.go | 2 +- internal/pixels/pixels.go | 20 ++++++++++---------- 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/image_test.go b/image_test.go index dcb386459..e2ad53bcd 100644 --- a/image_test.go +++ b/image_test.go @@ -387,3 +387,35 @@ func TestNewImageFromSubImage(t *testing.T) { } } } + +type mutableRGBA struct { + r, g, b, a uint8 +} + +func (c *mutableRGBA) RGBA() (r, g, b, a uint32) { + return uint32(c.r) * 0x101, uint32(c.g) * 0x101, uint32(c.b) * 0x101, uint32(c.a) * 0x101 +} + +func TestImageFill(t *testing.T) { + w, h := 10, 10 + img, err := NewImage(w, h, FilterNearest) + if err != nil { + t.Fatal(err) + return + } + clr := &mutableRGBA{0x80, 0x80, 0x80, 0x80} + if err := img.Fill(clr); err != nil { + t.Fatal(err) + return + } + clr.r = 0 + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + got := img.At(i, j) + want := color.RGBA{0x80, 0x80, 0x80, 0x80} + if got != want { + t.Errorf("img At(%d, %d): got %#v; want %#v", i, j, got, want) + } + } + } +} diff --git a/imageimpl.go b/imageimpl.go index 95575e928..c1894819a 100644 --- a/imageimpl.go +++ b/imageimpl.go @@ -111,8 +111,9 @@ func (i *imageImpl) Fill(clr color.Color) error { if i.disposed { return errors.New("ebiten: image is already disposed") } - i.pixels.Fill(clr) - return i.image.Fill(clr) + rgba := color.RGBAModel.Convert(clr).(color.RGBA) + i.pixels.Fill(rgba) + return i.image.Fill(rgba) } func (i *imageImpl) clearIfVolatile() error { @@ -125,7 +126,7 @@ func (i *imageImpl) clearIfVolatile() error { return nil } i.pixels.Clear() - return i.image.Fill(color.Transparent) + return i.image.Fill(color.RGBA{}) } func (i *imageImpl) DrawImage(image *Image, options *DrawImageOptions) error { diff --git a/internal/graphics/command.go b/internal/graphics/command.go index f6040f43b..2562cdaf4 100644 --- a/internal/graphics/command.go +++ b/internal/graphics/command.go @@ -136,15 +136,15 @@ func FlushCommands(context *opengl.Context) error { type fillCommand struct { dst *Image - color color.Color + color color.RGBA } func (c *fillCommand) Exec(context *opengl.Context, indexOffsetInBytes int) error { if err := c.dst.framebuffer.setAsViewport(context); err != nil { return err } - cr, cg, cb, ca := c.color.RGBA() - const max = math.MaxUint16 + cr, cg, cb, ca := c.color.R, c.color.G, c.color.B, c.color.A + const max = math.MaxUint8 r := float64(cr) / max g := float64(cg) / max b := float64(cb) / max diff --git a/internal/graphics/image.go b/internal/graphics/image.go index 50c6e194e..a285e476b 100644 --- a/internal/graphics/image.go +++ b/internal/graphics/image.go @@ -84,7 +84,7 @@ func (i *Image) Size() (int, int) { return i.width, i.height } -func (i *Image) Fill(clr color.Color) error { +func (i *Image) Fill(clr color.RGBA) error { // TODO: Need to clone clr value c := &fillCommand{ dst: i, diff --git a/internal/pixels/pixels.go b/internal/pixels/pixels.go index 6a1c6df2f..cb2bc5bd7 100644 --- a/internal/pixels/pixels.go +++ b/internal/pixels/pixels.go @@ -35,7 +35,7 @@ type drawImageHistoryItem struct { type Pixels struct { // basePixels and baseColor are exclusive. basePixels []uint8 - baseColor color.Color + baseColor color.RGBA drawImageHistory []*drawImageHistoryItem stale bool } @@ -46,19 +46,19 @@ func (p *Pixels) IsStale() bool { func (p *Pixels) MakeStale() { p.basePixels = nil - p.baseColor = nil + p.baseColor = color.RGBA{} p.drawImageHistory = nil p.stale = true } func (p *Pixels) Clear() { p.basePixels = nil - p.baseColor = nil + p.baseColor = color.RGBA{} p.drawImageHistory = nil p.stale = false } -func (p *Pixels) Fill(clr color.Color) { +func (p *Pixels) Fill(clr color.RGBA) { p.basePixels = nil p.baseColor = clr p.drawImageHistory = nil @@ -70,7 +70,7 @@ func (p *Pixels) ReplacePixels(pixels []uint8) { p.basePixels = make([]uint8, len(pixels)) } copy(p.basePixels, pixels) - p.baseColor = nil + p.baseColor = color.RGBA{} p.drawImageHistory = nil p.stale = false } @@ -95,10 +95,10 @@ func (p *Pixels) AppendDrawImageHistory(image *graphics.Image, vertices []int16, // // Note that this must not be called until context is available. // This means Pixels members must match with acutal state in VRAM. -func (p *Pixels) At(idx int, image *graphics.Image, context *opengl.Context) (color.Color, error) { +func (p *Pixels) At(idx int, image *graphics.Image, context *opengl.Context) (color.RGBA, error) { if p.basePixels == nil || p.drawImageHistory != nil || p.stale { if err := p.readPixelsFromVRAM(image, context); err != nil { - return nil, err + return color.RGBA{}, err } } r, g, b, a := p.basePixels[idx], p.basePixels[idx+1], p.basePixels[idx+2], p.basePixels[idx+3] @@ -124,7 +124,7 @@ func (p *Pixels) readPixelsFromVRAM(image *graphics.Image, context *opengl.Conte if err != nil { return err } - p.baseColor = nil + p.baseColor = color.RGBA{} p.drawImageHistory = nil p.stale = false return nil @@ -159,7 +159,7 @@ func (p *Pixels) CreateImage(context *opengl.Context, width, height int, filter if err != nil { return nil, err } - if p.baseColor != nil { + if p.baseColor != (color.RGBA{}) { if p.basePixels != nil { panic("not reach") } @@ -180,7 +180,7 @@ func (p *Pixels) CreateImage(context *opengl.Context, width, height int, filter if err != nil { return nil, err } - p.baseColor = nil + p.baseColor = color.RGBA{} p.drawImageHistory = nil p.stale = false return gimg, nil