From b4dddd330a5afe9c6895e6b0686a76f025f00105 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Wed, 17 Jul 2019 23:03:14 +0900 Subject: [PATCH] restorable: Refactoring: Use width/height instead of length at Pixels --- internal/restorable/image.go | 52 ++++++------ internal/restorable/images_test.go | 124 ++++++++++++++++------------- 2 files changed, 96 insertions(+), 80 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 3a0aa3b82..0dfcfc00e 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -28,7 +28,8 @@ import ( type Pixels struct { pixels []byte - length int + width int + height int // color is used only when pixels == nil color color.RGBA @@ -36,30 +37,23 @@ type Pixels struct { func (p *Pixels) CopyFrom(pix []byte, from int) { if p.pixels == nil { - p.pixels = make([]byte, p.length) + p.pixels = make([]byte, 4*p.width*p.height) } copy(p.pixels[from:from+len(pix)], pix) } -func (p *Pixels) At(i int) byte { - if i < 0 || p.length <= i { - panic(fmt.Sprintf("restorable: index out of range: %d for length: %d", i, p.length)) +func (p *Pixels) At(i, j int) (byte, byte, byte, byte) { + if i < 0 || p.width <= i { + panic(fmt.Sprintf("restorable: index out of range: %d for the width: %d", i, p.width)) + } + if j < 0 || p.height <= j { + panic(fmt.Sprintf("restorable: index out of range: %d for the height: %d", i, p.height)) } if p.pixels != nil { - return p.pixels[i] - } - switch i % 4 { - case 0: - return p.color.R - case 1: - return p.color.G - case 2: - return p.color.B - case 3: - return p.color.A - default: - panic("not reached") + 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 } // drawTrianglesHistoryItem is an item for history of draw-image commands. @@ -165,7 +159,8 @@ func (i *Image) Extend(width, height int) *Image { // Copy basePixels. newImg.basePixels = &Pixels{ pixels: make([]byte, 4*width*height), - length: 4 * width * height, + width: width, + height: height, } pix := i.basePixels.pixels idx := 0 @@ -258,7 +253,8 @@ func (i *Image) fill(r, g, b, a byte) { w, h := i.Size() i.basePixels = &Pixels{ color: color.RGBA{r, g, b, a}, - length: 4 * w * h, + width: w, + height: h, } i.drawTrianglesHistory = nil i.stale = false @@ -349,7 +345,8 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { if x == 0 && y == 0 && width == w && height == h { if i.basePixels == nil { i.basePixels = &Pixels{ - length: 4 * w * h, + width: w, + height: h, } } i.basePixels.CopyFrom(pixels, 0) @@ -378,7 +375,8 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { idx := 4 * (y*w + x) if i.basePixels == nil { i.basePixels = &Pixels{ - length: 4 * w * h, + width: w, + height: h, } } for j := 0; j < height; j++ { @@ -455,8 +453,7 @@ func (i *Image) At(x, y int) (byte, byte, byte, byte) { return 0, 0, 0, 0 } - idx := 4*x + 4*y*w - return i.basePixels.At(idx), i.basePixels.At(idx + 1), i.basePixels.At(idx + 2), i.basePixels.At(idx + 3) + return i.basePixels.At(x, y) } // makeStaleIfDependingOn makes the image stale if the image depends on target. @@ -471,10 +468,12 @@ 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{ pixels: pix, - length: len(pix), + width: w, + height: h, } i.drawTrianglesHistory = nil i.stale = false @@ -578,7 +577,8 @@ func (i *Image) restore() error { pix := gimg.Pixels() i.basePixels = &Pixels{ pixels: pix, - length: len(pix), + width: w, + height: h, } } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index 6ec0dbea2..1db343bcf 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -45,9 +45,9 @@ func TestMain(m *testing.M) { os.Exit(code) } -func pixelsToColor(p *Pixels, index int) color.RGBA { - i := index * 4 - return color.RGBA{p.At(i), p.At(i + 1), p.At(i + 2), p.At(i + 3)} +func pixelsToColor(p *Pixels, i, j int) color.RGBA { + r, g, b, a := p.At(i, j) + return color.RGBA{r, g, b, a} } func abs(x int) int { @@ -80,7 +80,7 @@ func TestRestore(t *testing.T) { t.Fatal(err) } want := clr0 - got := pixelsToColor(img0.BasePixelsForTesting(), 0) + got := pixelsToColor(img0.BasePixelsForTesting(), 0, 0) if !sameColors(got, want, 1) { t.Errorf("got %v, want %v", got, want) } @@ -97,11 +97,13 @@ func TestRestoreWithoutDraw(t *testing.T) { t.Fatal(err) } - for i := 0; i < 1024*1024; i++ { - want := color.RGBA{0x00, 0x00, 0x00, 0x00} - got := pixelsToColor(img0.BasePixelsForTesting(), i) - if !sameColors(got, want, 0) { - t.Errorf("got %v, want %v", got, want) + for j := 0; j < 1024; j++ { + for i := 0; i < 1024; i++ { + want := color.RGBA{0x00, 0x00, 0x00, 0x00} + got := pixelsToColor(img0.BasePixelsForTesting(), i, j) + if !sameColors(got, want, 0) { + t.Errorf("got %v, want %v", got, want) + } } } } @@ -139,7 +141,7 @@ func TestRestoreChain(t *testing.T) { } want := clr for i, img := range imgs { - got := pixelsToColor(img.BasePixelsForTesting(), 0) + got := pixelsToColor(img.BasePixelsForTesting(), 0, 0) if !sameColors(got, want, 1) { t.Errorf("%d: got %v, want %v", i, got, want) } @@ -186,7 +188,7 @@ func TestRestoreChain2(t *testing.T) { if i == 8 || i == 9 { want = clr7 } - got := pixelsToColor(img.BasePixelsForTesting(), 0) + got := pixelsToColor(img.BasePixelsForTesting(), 0, 0) if !sameColors(got, want, 1) { t.Errorf("%d: got %v, want %v", i, got, want) } @@ -228,22 +230,22 @@ func TestRestoreOverrideSource(t *testing.T) { { "0", clr1, - pixelsToColor(img0.BasePixelsForTesting(), 0), + pixelsToColor(img0.BasePixelsForTesting(), 0, 0), }, { "1", clr1, - pixelsToColor(img1.BasePixelsForTesting(), 0), + pixelsToColor(img1.BasePixelsForTesting(), 0, 0), }, { "2", clr0, - pixelsToColor(img2.BasePixelsForTesting(), 0), + pixelsToColor(img2.BasePixelsForTesting(), 0, 0), }, { "3", clr0, - pixelsToColor(img3.BasePixelsForTesting(), 0), + pixelsToColor(img3.BasePixelsForTesting(), 0, 0), }, } for _, c := range testCases { @@ -365,7 +367,7 @@ func TestRestoreComplexGraph(t *testing.T) { if c.out[i] == '*' { want = color.RGBA{0xff, 0xff, 0xff, 0xff} } - got := pixelsToColor(c.image.BasePixelsForTesting(), i) + got := pixelsToColor(c.image.BasePixelsForTesting(), i, 0) if !sameColors(got, want, 1) { t.Errorf("%s[%d]: got %v, want %v", c.name, i, got, want) } @@ -426,7 +428,7 @@ func TestRestoreRecursive(t *testing.T) { if c.out[i] == '*' { want = color.RGBA{0xff, 0xff, 0xff, 0xff} } - got := pixelsToColor(c.image.BasePixelsForTesting(), i) + got := pixelsToColor(c.image.BasePixelsForTesting(), i, 0) if !sameColors(got, want, 1) { t.Errorf("%s[%d]: got %v, want %v", c.name, i, got, want) } @@ -543,55 +545,66 @@ func TestClear(t *testing.T) { img.ReplacePixels(make([]byte, 4*4*4), 1, 1, 2, 2) cases := []struct { - Index int - Want color.RGBA + i int + j int + want color.RGBA }{ { - Index: 0, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 0, + j: 0, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 3, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 3, + j: 0, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 4, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 0, + j: 1, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 5, - Want: color.RGBA{0, 0, 0, 0}, + i: 1, + j: 1, + want: color.RGBA{0, 0, 0, 0}, }, { - Index: 7, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 3, + j: 1, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 8, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 0, + j: 2, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 10, - Want: color.RGBA{0, 0, 0, 0}, + i: 2, + j: 2, + want: color.RGBA{0, 0, 0, 0}, }, { - Index: 11, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 3, + j: 2, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 12, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 0, + j: 3, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, { - Index: 15, - Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + i: 3, + j: 3, + want: color.RGBA{0xff, 0xff, 0xff, 0xff}, }, } for _, c := range cases { - got := pixelsToColor(img.BasePixelsForTesting(), c.Index) - want := c.Want + got := pixelsToColor(img.BasePixelsForTesting(), c.i, c.j) + want := c.want if got != want { - t.Errorf("base pixel [%d]: got %v, want %v", c.Index, got, want) + t.Errorf("base pixel (%d, %d): got %v, want %v", c.i, c.j, got, want) } } } @@ -613,17 +626,20 @@ func TestReplacePixelsOnly(t *testing.T) { img0.ReplacePixels([]byte{5, 6, 7, 8}, 0, 0, 1, 1) // BasePixelsForTesting is available without GPU accessing. - for i := 0; i < w*h; i++ { - var want color.RGBA - switch { - case i == 0: - want = color.RGBA{5, 6, 7, 8} - case i%5 == 0: - want = color.RGBA{1, 2, 3, 4} - } - got := pixelsToColor(img0.BasePixelsForTesting(), i) - if !sameColors(got, want, 0) { - t.Errorf("got %v, want %v", got, want) + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + idx := j*w + i + var want color.RGBA + switch { + case idx == 0: + want = color.RGBA{5, 6, 7, 8} + case idx%5 == 0: + want = color.RGBA{1, 2, 3, 4} + } + got := pixelsToColor(img0.BasePixelsForTesting(), i, j) + if !sameColors(got, want, 0) { + t.Errorf("got %v, want %v", got, want) + } } } @@ -632,7 +648,7 @@ func TestReplacePixelsOnly(t *testing.T) { t.Fatal(err) } want := color.RGBA{1, 2, 3, 4} - got := pixelsToColor(img1.BasePixelsForTesting(), 0) + got := pixelsToColor(img1.BasePixelsForTesting(), 0, 0) if !sameColors(got, want, 0) { t.Errorf("got %v, want %v", got, want) }