From 45afb6db67e58f6c8892647683fbf0d682340eec Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 28 Apr 2018 22:50:00 +0900 Subject: [PATCH] Reland: restorable: Merge Clear to ReplacePixel --- internal/restorable/image.go | 42 ++++++++++++------- internal/restorable/images_test.go | 65 ++++++++++++++++++++++++++++++ internal/shareable/shareable.go | 3 +- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 4d0cd3032..9b25cb390 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -92,18 +92,10 @@ func newImageWithoutInit(width, height int, volatile bool) *Image { // Note that Dispose is not called automatically. func NewImage(width, height int, volatile bool) *Image { i := newImageWithoutInit(width, height, volatile) - i.Clear(0, 0, width, height) + i.ReplacePixels(nil, 0, 0, width, height) return i } -func (i *Image) Clear(x, y, width, height int) { - w, h := dummyImage.Size() - geom := (*affine.GeoM)(nil).Scale(float64(width)/float64(w), float64(height)/float64(h)) - geom = geom.Translate(float64(x), float64(y)) - colorm := (*affine.ColorM)(nil).Scale(0, 0, 0, 0) - i.DrawImage(dummyImage, 0, 0, w, h, geom, colorm, opengl.CompositeModeCopy, graphics.FilterNearest) -} - // NewScreenFramebufferImage creates a special image that framebuffer is one for the screen. // // The returned image is cleared. @@ -116,7 +108,7 @@ func NewScreenFramebufferImage(width, height int) *Image { screen: true, } theImages.add(i) - i.Clear(0, 0, width, height) + i.ReplacePixels(nil, 0, 0, width, height) return i } @@ -143,6 +135,8 @@ func (i *Image) makeStale() { } // ReplacePixels replaces the image pixels with the given pixels slice. +// +// If pixels is nil, ReplacePixels clears the specified reagion. func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { w, h := i.image.Size() if width <= 0 || height <= 0 { @@ -156,7 +150,19 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { // For this purpuse, images should remember which part of that is used for DrawImage. theImages.makeStaleIfDependingOn(i) - i.image.ReplacePixels(pixels, x, y, width, height) + if pixels != nil { + i.image.ReplacePixels(pixels, x, y, width, height) + } else { + // There is not 'drawImageHistoryItem' for this image and dummyImage. + // This means dummyImage might not be restored yet when this image is restored. + // However, that's ok since this image will be stale or have updated pixel data. + w, h := dummyImage.Size() + geom := (*affine.GeoM)(nil).Scale(float64(width)/float64(w), float64(height)/float64(h)) + geom = geom.Translate(float64(x), float64(y)) + colorm := (*affine.ColorM)(nil).Scale(0, 0, 0, 0) + vs := vertices(w, h, 0, 0, w, h, geom) + i.image.DrawImage(dummyImage.image, vs, colorm, opengl.CompositeModeCopy, graphics.FilterNearest) + } if x == 0 && y == 0 && width == w && height == h { if i.basePixels == nil { @@ -176,9 +182,17 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { return } idx := 4 * (y*w + x) - for j := 0; j < height; j++ { - copy(i.basePixels[idx:idx+4*width], pixels[4*j*width:4*(j+1)*width]) - idx += 4 * w + if pixels != nil { + for j := 0; j < height; j++ { + copy(i.basePixels[idx:idx+4*width], pixels[4*j*width:4*(j+1)*width]) + idx += 4 * w + } + } else { + zeros := make([]byte, 4*width) + for j := 0; j < height; j++ { + copy(i.basePixels[idx:idx+4*width], zeros) + idx += 4 * w + } } i.stale = false } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index 60bf4e1d6..d74eaf3a8 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -569,4 +569,69 @@ func TestDoubleResolve(t *testing.T) { } } +func TestClear(t *testing.T) { + pix := make([]uint8, 4*4*4) + for i := range pix { + pix[i] = 0xff + } + + img := NewImage(4, 4, false) + img.ReplacePixels(pix, 0, 0, 4, 4) + // This doesn't make the image stale. Its base pixels are available. + img.ReplacePixels(nil, 1, 1, 2, 2) + + cases := []struct { + Index int + Want color.RGBA + }{ + { + Index: 0, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 3, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 4, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 5, + Want: color.RGBA{0, 0, 0, 0}, + }, + { + Index: 7, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 8, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 10, + Want: color.RGBA{0, 0, 0, 0}, + }, + { + Index: 11, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 12, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + { + Index: 15, + Want: color.RGBA{0xff, 0xff, 0xff, 0xff}, + }, + } + for _, c := range cases { + got := byteSliceToColor(img.BasePixelsForTesting(), c.Index) + want := c.Want + if got != want { + t.Errorf("base pixel [%d]: got %v, want %v", c.Index, got, want) + } + } +} + // TODO: How about volatile/screen images? diff --git a/internal/shareable/shareable.go b/internal/shareable/shareable.go index aea892802..f9b0938c7 100644 --- a/internal/shareable/shareable.go +++ b/internal/shareable/shareable.go @@ -190,7 +190,8 @@ func (i *Image) dispose() { i.backend.page.Free(i.node) if !i.backend.page.IsEmpty() { // As this part can be reused, this should be cleared explicitly. - i.backend.restorable.Clear(i.region()) + x0, y0, x1, y1 := i.region() + i.backend.restorable.ReplacePixels(nil, x0, y0, x1, y1) return }