From f1091910bd6fdb1f54fe8bc5d11a43eab73855a6 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 16 Nov 2019 00:43:22 +0900 Subject: [PATCH] Revert "graphicscommand: Remove copying pixels" This reverts commit 339a96b7e6e186b5c217fc3e44cae003747701bc. Reason: this causes panic on extending textures --- internal/graphicscommand/image.go | 8 ++-- internal/restorable/image.go | 25 ++--------- internal/restorable/images_test.go | 67 ------------------------------ internal/restorable/rect.go | 12 +++--- 4 files changed, 14 insertions(+), 98 deletions(-) diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 8661e8c81..94f29e3c1 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -155,17 +155,15 @@ func (i *Image) Pixels() []byte { return c.result } -// ReplacePixels replaces the pixels on the given region. -// -// ReplacePixels takes the ownership of pixels. This means that pixels must not be modified after calling -// ReplacePixels. -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/image.go b/internal/restorable/image.go index bb5fc3d3a..bebd978ff 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -328,13 +328,6 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { // For this purpuse, images should remember which part of that is used for DrawTriangles. theImages.makeStaleIfDependingOn(i) - // pixels are taken by graphicscommand.Image. Copy it for the later usage. - var copied []byte - if needsRestoring() && pixels != nil { - copied = make([]byte, len(pixels)) - copy(copied, pixels) - } - if pixels != nil { i.image.ReplacePixels(pixels, x, y, width, height) } else { @@ -344,20 +337,14 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height) } - // pixels are no longer available. Use copied when necessary. - if x == 0 && y == 0 && width == w && height == h { - i.drawTrianglesHistory = nil - i.stale = false if pixels != nil { - if needsRestoring() { - i.basePixels.AddOrReplace(copied, 0, 0, w, h) - } else { - i.makeStale() - } + i.basePixels.AddOrReplace(pixels, 0, 0, w, h) } else { i.basePixels.Remove(0, 0, w, h) } + i.drawTrianglesHistory = nil + i.stale = false return } @@ -374,11 +361,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } if pixels != nil { - if needsRestoring() { - i.basePixels.AddOrReplace(copied, x, y, width, height) - } else { - i.makeStale() - } + i.basePixels.AddOrReplace(pixels, x, y, width, height) } else { i.basePixels.Remove(x, y, width, height) } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index e042c7af0..e9d09a059 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -829,70 +829,3 @@ func TestMutateSlices(t *testing.T) { } } } - -func TestReplacePixelsAndModify(t *testing.T) { - const w, h = 16, 16 - img := NewImage(w, h, false) - - pix := make([]byte, 4*w*h) - for i := 0; i < w*h; i++ { - pix[4*i] = byte(i) - pix[4*i+1] = byte(i) - pix[4*i+2] = byte(i) - pix[4*i+3] = byte(i) - } - img.ReplacePixels(pix, 0, 0, w, h) - - // Modify pix after ReplacePixels - for i := 0; i < w*h; i++ { - pix[4*i] = 0 - pix[4*i+1] = 0 - pix[4*i+2] = 0 - pix[4*i+3] = 0 - } - - for i := 0; i < w*h; i++ { - want := color.RGBA{byte(i), byte(i), byte(i), byte(i)} - r, g, b, a := img.At(i%w, i/w) - got := color.RGBA{r, g, b, a} - if got != want { - t.Errorf("At(%d, %d): got: %v, want: %v", i%w, i/w, got, want) - } - } -} - -func TestReplacePixelsPartAndModify(t *testing.T) { - const w, h = 16, 16 - const w2, h2 = 8, 8 - img := NewImage(w, h, false) - - pix := make([]byte, 4*w2*h2) - for i := 0; i < w2*h2; i++ { - pix[4*i] = byte(i) - pix[4*i+1] = byte(i) - pix[4*i+2] = byte(i) - pix[4*i+3] = byte(i) - } - img.ReplacePixels(pix, 4, 4, w2, h2) - - // Modify pix after ReplacePixels - for i := 0; i < w2*h2; i++ { - pix[4*i] = 0 - pix[4*i+1] = 0 - pix[4*i+2] = 0 - pix[4*i+3] = 0 - } - - idx := 0 - for j := 4; j < 4+h2; j++ { - for i := 4; i < 4+w2; i++ { - want := color.RGBA{byte(idx), byte(idx), byte(idx), byte(idx)} - r, g, b, a := img.At(i, j) - got := color.RGBA{r, g, b, a} - if got != want { - t.Errorf("At(%d, %d): got: %v, want: %v", i, j, got, want) - } - idx++ - } - } -} diff --git a/internal/restorable/rect.go b/internal/restorable/rect.go index 23603f493..842d43e8c 100644 --- a/internal/restorable/rect.go +++ b/internal/restorable/rect.go @@ -28,7 +28,6 @@ type rectToPixels struct { lastPix []byte } -// addOrReplace takes the ownership of pixels. func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) { if len(pixels) != 4*width*height { panic(fmt.Sprintf("restorable: len(pixels) must be %d but %d", 4*width*height, len(pixels))) @@ -38,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 } @@ -54,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 } }