From c60a32a4797db3bd694dca547305ffb6248b6cb5 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 16 Nov 2019 01:51:32 +0900 Subject: [PATCH] graphics: Avoid all copying pixels This is a breaking change: ReplacePixels now takes the ownership of the given pixels. Fixes #983 --- image.go | 3 +++ image_test.go | 15 ++------------- internal/graphicscommand/image.go | 4 +--- internal/restorable/rect.go | 11 ++++------- 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/image.go b/image.go index 42a2f9558..f4e3428e9 100644 --- a/image.go +++ b/image.go @@ -432,6 +432,9 @@ func (i *Image) Dispose() error { // // The given p must represent RGBA pre-multiplied alpha values. len(p) must equal to 4 * (image width) * (image height). // +// ReplacePixels takes the ownership of the given p. This means that p must not be modified after ReplacePixels is +// called. +// // ReplacePixels may be slow (as for implementation, this calls glTexSubImage2D). // // When len(p) is not appropriate, ReplacePixels panics. diff --git a/image_test.go b/image_test.go index c57cb3c42..a9cefbc33 100644 --- a/image_test.go +++ b/image_test.go @@ -354,19 +354,8 @@ func TestImageReplacePixels(t *testing.T) { t.Fatal(err) return } - // Even if p is changed after calling ReplacePixel, img0 uses the original values. - for i := range p { - p[i] = 0 - } - for j := 0; j < img0.Bounds().Size().Y; j++ { - for i := 0; i < img0.Bounds().Size().X; i++ { - got := img0.At(i, j) - want := color.RGBA{0x80, 0x80, 0x80, 0x80} - if got != want { - t.Errorf("img0 At(%d, %d): got %#v; want %#v", i, j, got, want) - } - } - } + + // p cannot be modified as of 1.11.0-alpha. } func TestImageReplacePixelsNil(t *testing.T) { diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 94f29e3c1..ef6f89f55 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -155,15 +155,13 @@ func (i *Image) Pixels() []byte { return c.result } -func (i *Image) ReplacePixels(p []byte, x, y, width, height int) { +func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { // ReplacePixels for a part might invalidate the current image that are drawn by DrawTriangles (#593, #738). if i.lastCommand == lastCommandDrawTriangles { if x != 0 || y != 0 || i.width != width || i.height != height { panic("graphicscommand: ReplacePixels for a part after DrawTriangles is forbidden") } } - pixels := make([]byte, len(p)) - copy(pixels, p) c := &replacePixelsCommand{ dst: i, pixels: pixels, diff --git a/internal/restorable/rect.go b/internal/restorable/rect.go index 842d43e8c..54088d7c4 100644 --- a/internal/restorable/rect.go +++ b/internal/restorable/rect.go @@ -37,16 +37,13 @@ func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) { rtp.m = map[image.Rectangle][]byte{} } - copied := make([]byte, len(pixels)) - copy(copied, pixels) - newr := image.Rect(x, y, x+width, y+height) for r := range rtp.m { if r == newr { // Replace the region. - rtp.m[r] = copied + rtp.m[r] = pixels if r == rtp.lastR { - rtp.lastPix = copied + rtp.lastPix = pixels } return } @@ -56,9 +53,9 @@ func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) { } // Add the region. - rtp.m[newr] = copied + rtp.m[newr] = pixels if newr == rtp.lastR { - rtp.lastPix = copied + rtp.lastPix = pixels } }