restorable: Split pixel records into regions

Fixes #897

internal/restorable/image.go

@@ -26,32 +26,44 @@ import (
 )
 
 type Pixels struct {
-	pixels []byte
+	rectToPixels *rectToPixels
 
-	width  int
+	// color is used only when rectToPixels is nil.
-	height int
-
-	// color is used only when pixels == nil
 	color color.RGBA
 }
 
-func (p *Pixels) CopyFrom(pix []byte, from int) {
+func (p *Pixels) Apply(img *graphicscommand.Image) {
-	if p.pixels == nil {
+	if p.rectToPixels == nil {
-		p.pixels = make([]byte, 4*p.width*p.height)
+		fillImage(img, p.color.R, p.color.G, p.color.B, p.color.A)
+		return
 	}
-	copy(p.pixels[from:from+len(pix)], pix)
+
+	p.rectToPixels.apply(img)
+}
+
+func (p *Pixels) AddOrReplace(pix []byte, x, y, width, height int) {
+	if p.rectToPixels == nil {
+		p.rectToPixels = &rectToPixels{}
+	}
+	p.rectToPixels.addOrReplace(pix, x, y, width, height)
+	p.color = color.RGBA{}
+}
+
+func (p *Pixels) Remove(x, y, width, height int) {
+	// Note that we don't care whether the region is actually removed or not here. There is an actual case that
+	// the region is allocated but nothing is rendered. See TestDisposeImmediately at shareable package.
+	if p.rectToPixels == nil {
+		return
+	}
+	p.rectToPixels.remove(x, y, width, height)
 }
 
 func (p *Pixels) At(i, j int) (byte, byte, byte, byte) {
-	if i < 0 || p.width <= i {
+	if p.rectToPixels != nil {
-		panic(fmt.Sprintf("restorable: index out of range: %d for the width: %d", i, p.width))
+		if r, g, b, a, ok := p.rectToPixels.at(i, j); ok {
-	}
+			return r, g, b, a
-	if j < 0 || p.height <= j {
+		}
-		panic(fmt.Sprintf("restorable: index out of range: %d for the height: %d", i, p.height))
+		return 0, 0, 0, 0
-	}
-	if p.pixels != nil {
-		idx := 4 * (j*p.width + i)
-		return p.pixels[idx], p.pixels[idx+1], p.pixels[idx+2], p.pixels[idx+3]
 	}
 	return p.color.R, p.color.G, p.color.B, p.color.A
 }
@@ -152,22 +164,9 @@ func (i *Image) Extend(width, height int) *Image {
 
 	newImg := NewImage(width, height)
 
-	if i.basePixels != nil && i.basePixels.pixels != nil {
+	i.basePixels.Apply(newImg.image)
-		newImg.image.ReplacePixels(i.basePixels.pixels, 0, 0, w, h)
-	}
 
-	// Copy basePixels.
+	newImg.basePixels = i.basePixels
-	newImg.basePixels = &Pixels{
-		pixels: make([]byte, 4*width*height),
-		width:  width,
-		height: height,
-	}
-	pix := i.basePixels.pixels
-	idx := 0
-	for j := 0; j < h; j++ {
-		newImg.basePixels.CopyFrom(pix[4*j*w:4*(j+1)*w], idx)
-		idx += 4 * width
-	}
 
 	i.Dispose()
 
@@ -250,11 +249,8 @@ func (i *Image) fill(r, g, b, a byte) {
 
 	fillImage(i.image, r, g, b, a)
 
-	w, h := i.Size()
 	i.basePixels = &Pixels{
-		color:  color.RGBA{r, g, b, a},
+		color: color.RGBA{r, g, b, a},
-		width:  w,
-		height: h,
 	}
 	i.drawTrianglesHistory = nil
 	i.stale = false
@@ -316,18 +312,13 @@ func (i *Image) makeStale() {
 
 // ClearPixels clears the specified region by ReplacePixels.
 func (i *Image) ClearPixels(x, y, width, height int) {
-	// TODO: Allocating bytes for all pixels are wasteful. Allocate memory only for required regions (#897).
+	i.ReplacePixels(nil, x, y, width, height)
-	i.ReplacePixels(make([]byte, 4*width*height), x, y, width, height)
 }
 
 // ReplacePixels replaces the image pixels with the given pixels slice.
 //
 // ReplacePixels for a part is forbidden if the image is rendered with DrawTriangles or Fill.
 func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
-	if pixels == nil {
-		panic("restorable: pixels must not be nil")
-	}
-
 	w, h := i.image.Size()
 	if width <= 0 || height <= 0 {
 		panic("restorable: width/height must be positive")
@@ -340,17 +331,24 @@ 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)
 
-	i.image.ReplacePixels(pixels, x, y, width, height)
+	if pixels != nil {
+		i.image.ReplacePixels(pixels, x, y, width, height)
+	} else {
+		// TODO: When pixels == nil, we don't have to care the pixel state there. In such cases, the image
+		// accepts only ReplacePixels and not Fill or DrawTriangles.
+		// TODO: Separate Image struct into two: images for only-ReplacePixels, and the others.
+		i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height)
+	}
 
 	if x == 0 && y == 0 && width == w && height == h {
 		if i.basePixels == nil {
-			i.basePixels = &Pixels{
+			i.basePixels = &Pixels{}
-				width:  w,
+		}
-				height: h,
+		if pixels != nil {
-			}
+			i.basePixels.AddOrReplace(pixels, 0, 0, w, h)
+		} else {
+			i.basePixels.Remove(0, 0, w, h)
 		}
-		i.basePixels.CopyFrom(pixels, 0)
-		i.basePixels.color = color.RGBA{}
 		i.drawTrianglesHistory = nil
 		i.stale = false
 		return
@@ -372,16 +370,13 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) {
 		return
 	}
 
-	idx := 4 * (y*w + x)
 	if i.basePixels == nil {
-		i.basePixels = &Pixels{
+		i.basePixels = &Pixels{}
-			width:  w,
-			height: h,
-		}
 	}
-	for j := 0; j < height; j++ {
+	if pixels != nil {
-		i.basePixels.CopyFrom(pixels[4*j*width:4*(j+1)*width], idx)
+		i.basePixels.AddOrReplace(pixels, x, y, width, height)
-		idx += 4 * w
+	} else {
+		i.basePixels.Remove(x, y, width, height)
 	}
 }
 
@@ -469,12 +464,8 @@ func (i *Image) makeStaleIfDependingOn(target *Image) {
 // readPixelsFromGPU reads the pixels from GPU and resolves the image's 'stale' state.
 func (i *Image) readPixelsFromGPU() {
 	w, h := i.Size()
-	pix := i.image.Pixels()
+	i.basePixels = &Pixels{}
-	i.basePixels = &Pixels{
+	i.basePixels.AddOrReplace(i.image.Pixels(), 0, 0, w, h)
-		pixels: pix,
-		width:  w,
-		height: h,
-	}
 	i.drawTrianglesHistory = nil
 	i.stale = false
 }
@@ -547,25 +538,12 @@ func (i *Image) restore() error {
 	}
 
 	gimg := graphicscommand.NewImage(w, h)
+	// Clear the image explicitly.
+	fillImage(gimg, 0, 0, 0, 0)
 	if i.basePixels != nil {
-		if i.basePixels.pixels != nil {
+		i.basePixels.Apply(gimg)
-			// If ReplacePixels is the first command, the image doesn't have be cleared.
-			gimg.ReplacePixels(i.basePixels.pixels, 0, 0, w, h)
-		} else {
-			// Clear the image explicitly.
-			fillImage(gimg, 0, 0, 0, 0)
-			r := i.basePixels.color.R
-			g := i.basePixels.color.G
-			b := i.basePixels.color.B
-			a := i.basePixels.color.A
-			if a > 0 {
-				fillImage(gimg, r, g, b, a)
-			}
-		}
-	} else {
-		// Clear the image explicitly.
-		fillImage(gimg, 0, 0, 0, 0)
 	}
+
 	for _, c := range i.drawTrianglesHistory {
 		if c.image.hasDependency() {
 			panic("restorable: all dependencies must be already resolved but not")
@@ -574,12 +552,8 @@ func (i *Image) restore() error {
 	}
 
 	if len(i.drawTrianglesHistory) > 0 {
-		pix := gimg.Pixels()
+		i.basePixels = &Pixels{}
-		i.basePixels = &Pixels{
+		i.basePixels.AddOrReplace(gimg.Pixels(), 0, 0, w, h)
-			pixels: pix,
-			width:  w,
-			height: h,
-		}
 	}
 
 	i.image = gimg

internal/restorable/images_test.go

@@ -533,16 +533,15 @@ func TestDispose(t *testing.T) {
 	}
 }
 
-func TestClear(t *testing.T) {
+func TestReplacePixelsPart(t *testing.T) {
-	pix := make([]uint8, 4*4*4)
+	pix := make([]uint8, 4*2*2)
 	for i := range pix {
 		pix[i] = 0xff
 	}
 
 	img := NewImage(4, 4)
-	img.ReplacePixels(pix, 0, 0, 4, 4)
 	// This doesn't make the image stale. Its base pixels are available.
-	img.ReplacePixels(make([]byte, 4*4*4), 1, 1, 2, 2)
+	img.ReplacePixels(pix, 1, 1, 2, 2)
 
 	cases := []struct {
 		i    int
@@ -552,52 +551,52 @@ func TestClear(t *testing.T) {
 		{
 			i:    0,
 			j:    0,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
 			i:    3,
 			j:    0,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
 			i:    0,
 			j:    1,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
 			i:    1,
 			j:    1,
-			want: color.RGBA{0, 0, 0, 0},
+			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
 		},
 		{
 			i:    3,
 			j:    1,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
 			i:    0,
 			j:    2,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
-		},
-		{
-			i:    2,
-			j:    2,
 			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
-			i:    3,
+			i:    2,
 			j:    2,
 			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
 		},
+		{
+			i:    3,
+			j:    2,
+			want: color.RGBA{0, 0, 0, 0},
+		},
 		{
 			i:    0,
 			j:    3,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 		{
 			i:    3,
 			j:    3,
-			want: color.RGBA{0xff, 0xff, 0xff, 0xff},
+			want: color.RGBA{0, 0, 0, 0},
 		},
 	}
 	for _, c := range cases {
@@ -778,3 +777,15 @@ func TestFillAndExtend(t *testing.T) {
 	orig.Fill(0x01, 0x02, 0x03, 0x04)
 	orig.Extend(w*2, h*2)
 }
+
+func TestClearPixels(t *testing.T) {
+	const w, h = 16, 16
+	img := NewImage(w, h)
+	img.ReplacePixels(make([]byte, 4*4*4), 0, 0, 4, 4)
+	img.ReplacePixels(make([]byte, 4*4*4), 4, 0, 4, 4)
+	img.ClearPixels(0, 0, 4, 4)
+	img.ClearPixels(4, 0, 4, 4)
+
+	// After clearing, the regions will be available again.
+	img.ReplacePixels(make([]byte, 4*8*4), 0, 0, 8, 4)
+}

internal/restorable/rect.go

@@ -0,0 +1,105 @@
+// Copyright 2019 The Ebiten Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package restorable
+
+import (
+	"fmt"
+	"image"
+
+	"github.com/hajimehoshi/ebiten/internal/graphicscommand"
+)
+
+type rectToPixels struct {
+	m map[image.Rectangle][]byte
+
+	last image.Rectangle
+}
+
+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)))
+	}
+
+	if rtp.m == nil {
+		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
+			return
+		}
+		if r.Overlaps(newr) {
+			panic(fmt.Sprintf("restorable: region (%#v) conflicted with the other region (%#v)", newr, r))
+		}
+	}
+
+	// Add the region.
+	rtp.m[newr] = copied
+}
+
+func (rtp *rectToPixels) remove(x, y, width, height int) {
+	if rtp.m == nil {
+		return
+	}
+
+	newr := image.Rect(x, y, x+width, y+height)
+	for r := range rtp.m {
+		if r == newr {
+			delete(rtp.m, r)
+			return
+		}
+	}
+}
+
+func (rtp *rectToPixels) at(i, j int) (byte, byte, byte, byte, bool) {
+	if rtp.m == nil {
+		return 0, 0, 0, 0, false
+	}
+
+	var r *image.Rectangle
+	pt := image.Pt(i, j)
+	if pt.In(rtp.last) {
+		r = &rtp.last
+	} else {
+		for rr := range rtp.m {
+			if pt.In(rr) {
+				r = &rr
+				rtp.last = rr
+				break
+			}
+		}
+	}
+
+	if r == nil {
+		return 0, 0, 0, 0, false
+	}
+
+	pix := rtp.m[*r]
+	idx := 4 * ((j-r.Min.Y)*r.Dx() + (i - r.Min.X))
+	return pix[idx], pix[idx+1], pix[idx+2], pix[idx+3], true
+}
+
+func (rtp *rectToPixels) apply(img *graphicscommand.Image) {
+	// TODO: Isn't this too heavy? Can we merge the operations?
+	for r, pix := range rtp.m {
+		img.ReplacePixels(pix, r.Min.X, r.Min.Y, r.Dx(), r.Dy())
+	}
+}

internal/shareable/shareable_test.go

@@ -367,4 +367,20 @@ func TestLongImages(t *testing.T) {
 	}
 }
 
+func TestDisposeImmediately(t *testing.T) {
+	// This tests restorable.Image.ClearPixels is called but ReplacePixels is not called.
+
+	img0 := NewImage(16, 16)
+	vs := make([]float32, graphics.VertexFloatNum)
+	img0.PutVertex(vs, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1)
+
+	img1 := NewImage(16, 16)
+	img1.PutVertex(vs, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1)
+
+	// img0 and img1 should share the same backend in 99.9999% possibility.
+
+	img0.Dispose()
+	img1.Dispose()
+}
+
 // TODO: Add tests to extend shareable image out of the main loop