diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 94f29e3c1..8661e8c81 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -155,15 +155,17 @@ func (i *Image) Pixels() []byte { return c.result } -func (i *Image) ReplacePixels(p []byte, x, y, width, height int) { +// 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) { // 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 bebd978ff..bb5fc3d3a 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -328,6 +328,13 @@ 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 { @@ -337,14 +344,20 @@ 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 { - i.basePixels.AddOrReplace(pixels, 0, 0, w, h) + if needsRestoring() { + i.basePixels.AddOrReplace(copied, 0, 0, w, h) + } else { + i.makeStale() + } } else { i.basePixels.Remove(0, 0, w, h) } - i.drawTrianglesHistory = nil - i.stale = false return } @@ -361,7 +374,11 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } if pixels != nil { - i.basePixels.AddOrReplace(pixels, x, y, width, height) + if needsRestoring() { + i.basePixels.AddOrReplace(copied, x, y, width, height) + } else { + i.makeStale() + } } else { i.basePixels.Remove(x, y, width, height) } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index e9d09a059..e042c7af0 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -829,3 +829,70 @@ 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 842d43e8c..23603f493 100644 --- a/internal/restorable/rect.go +++ b/internal/restorable/rect.go @@ -28,6 +28,7 @@ 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))) @@ -37,16 +38,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 +54,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 } }