Revert "graphicscommand: Remove copying pixels"

This reverts commit 339a96b7e6.

Reason: this causes panic on extending textures
This commit is contained in:
Hajime Hoshi 2019-11-16 00:43:22 +09:00
parent 339a96b7e6
commit f1091910bd
4 changed files with 14 additions and 98 deletions

View File

@ -155,17 +155,15 @@ func (i *Image) Pixels() []byte {
return c.result return c.result
} }
// ReplacePixels replaces the pixels on the given region. func (i *Image) ReplacePixels(p []byte, x, y, width, height int) {
//
// 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). // ReplacePixels for a part might invalidate the current image that are drawn by DrawTriangles (#593, #738).
if i.lastCommand == lastCommandDrawTriangles { if i.lastCommand == lastCommandDrawTriangles {
if x != 0 || y != 0 || i.width != width || i.height != height { if x != 0 || y != 0 || i.width != width || i.height != height {
panic("graphicscommand: ReplacePixels for a part after DrawTriangles is forbidden") panic("graphicscommand: ReplacePixels for a part after DrawTriangles is forbidden")
} }
} }
pixels := make([]byte, len(p))
copy(pixels, p)
c := &replacePixelsCommand{ c := &replacePixelsCommand{
dst: i, dst: i,
pixels: pixels, pixels: pixels,

View File

@ -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. // For this purpuse, images should remember which part of that is used for DrawTriangles.
theImages.makeStaleIfDependingOn(i) 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 { if pixels != nil {
i.image.ReplacePixels(pixels, x, y, width, height) i.image.ReplacePixels(pixels, x, y, width, height)
} else { } 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) 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 { if x == 0 && y == 0 && width == w && height == h {
i.drawTrianglesHistory = nil
i.stale = false
if pixels != nil { if pixels != nil {
if needsRestoring() { i.basePixels.AddOrReplace(pixels, 0, 0, w, h)
i.basePixels.AddOrReplace(copied, 0, 0, w, h)
} else {
i.makeStale()
}
} else { } else {
i.basePixels.Remove(0, 0, w, h) i.basePixels.Remove(0, 0, w, h)
} }
i.drawTrianglesHistory = nil
i.stale = false
return return
} }
@ -374,11 +361,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
} }
if pixels != nil { if pixels != nil {
if needsRestoring() { i.basePixels.AddOrReplace(pixels, x, y, width, height)
i.basePixels.AddOrReplace(copied, x, y, width, height)
} else {
i.makeStale()
}
} else { } else {
i.basePixels.Remove(x, y, width, height) i.basePixels.Remove(x, y, width, height)
} }

View File

@ -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++
}
}
}

View File

@ -28,7 +28,6 @@ type rectToPixels struct {
lastPix []byte lastPix []byte
} }
// addOrReplace takes the ownership of pixels.
func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) { func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) {
if len(pixels) != 4*width*height { if len(pixels) != 4*width*height {
panic(fmt.Sprintf("restorable: len(pixels) must be %d but %d", 4*width*height, len(pixels))) 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{} rtp.m = map[image.Rectangle][]byte{}
} }
copied := make([]byte, len(pixels))
copy(copied, pixels)
newr := image.Rect(x, y, x+width, y+height) newr := image.Rect(x, y, x+width, y+height)
for r := range rtp.m { for r := range rtp.m {
if r == newr { if r == newr {
// Replace the region. // Replace the region.
rtp.m[r] = pixels rtp.m[r] = copied
if r == rtp.lastR { if r == rtp.lastR {
rtp.lastPix = pixels rtp.lastPix = copied
} }
return return
} }
@ -54,9 +56,9 @@ func (rtp *rectToPixels) addOrReplace(pixels []byte, x, y, width, height int) {
} }
// Add the region. // Add the region.
rtp.m[newr] = pixels rtp.m[newr] = copied
if newr == rtp.lastR { if newr == rtp.lastR {
rtp.lastPix = pixels rtp.lastPix = copied
} }
} }