From aa6fc67736912f9e39f7d3f356feee9901ed7577 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 16 Nov 2019 02:04:39 +0900 Subject: [PATCH] Revert "graphics: Avoid all copying pixels" This reverts commit c60a32a4797db3bd694dca547305ffb6248b6cb5. Reason: This breaks backward comptibility and it was not obvious how to fix examples. --- image.go | 3 --- image_test.go | 15 +++++++++++++-- internal/graphicscommand/image.go | 4 +++- internal/restorable/rect.go | 11 +++++++---- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/image.go b/image.go index f4e3428e9..42a2f9558 100644 --- a/image.go +++ b/image.go @@ -432,9 +432,6 @@ 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 a9cefbc33..c57cb3c42 100644 --- a/image_test.go +++ b/image_test.go @@ -354,8 +354,19 @@ func TestImageReplacePixels(t *testing.T) { t.Fatal(err) return } - - // p cannot be modified as of 1.11.0-alpha. + // 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) + } + } + } } func TestImageReplacePixelsNil(t *testing.T) { diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index ef6f89f55..94f29e3c1 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -155,13 +155,15 @@ func (i *Image) Pixels() []byte { return c.result } -func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { +func (i *Image) ReplacePixels(p []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 54088d7c4..842d43e8c 100644 --- a/internal/restorable/rect.go +++ b/internal/restorable/rect.go @@ -37,13 +37,16 @@ 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] = pixels + rtp.m[r] = copied if r == rtp.lastR { - rtp.lastPix = pixels + rtp.lastPix = copied } return } @@ -53,9 +56,9 @@ func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) { } // Add the region. - rtp.m[newr] = pixels + rtp.m[newr] = copied if newr == rtp.lastR { - rtp.lastPix = pixels + rtp.lastPix = copied } }