From 1cf4f30541245584ece4833577fd385d37cabede Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Wed, 25 Apr 2018 22:31:48 +0900 Subject: [PATCH] Revert "restorable: Merge Clear to ReplacePixels" This reverts commit fb641d88cd559917712ab7cebc71e38a0decfa19. Reason: When restoring the image where ReplacePixels is called, dummyImage might not be restored since there is no record of relationships between this image and dummyImage. Now pixels is not nil when reverting by chance, but this would cause problems in the future. --- internal/restorable/image.go | 39 +++++++----------- internal/restorable/images_test.go | 65 ------------------------------ internal/shareable/shareable.go | 3 +- 3 files changed, 15 insertions(+), 92 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index e5e0ae764..d91b6569f 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -89,10 +89,18 @@ func newImageWithoutInit(width, height int, volatile bool) *Image { // The returned image is cleared. func NewImage(width, height int, volatile bool) *Image { i := newImageWithoutInit(width, height, volatile) - i.ReplacePixels(nil, 0, 0, width, height) + i.Clear(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. @@ -104,7 +112,7 @@ func NewScreenFramebufferImage(width, height int) *Image { } theImages.add(i) runtime.SetFinalizer(i, (*Image).Dispose) - i.ReplacePixels(nil, 0, 0, width, height) + i.Clear(0, 0, width, height) return i } @@ -131,8 +139,6 @@ 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 { @@ -146,16 +152,7 @@ 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) - if pixels != nil { - i.image.ReplacePixels(pixels, x, y, width, height) - } else { - 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) - } + i.image.ReplacePixels(pixels, x, y, width, height) if x == 0 && y == 0 && width == w && height == h { if i.basePixels == nil { @@ -175,17 +172,9 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { return } idx := 4 * (y*w + x) - 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 - } + for j := 0; j < height; j++ { + copy(i.basePixels[idx:idx+4*width], pixels[4*j*width:4*(j+1)*width]) + idx += 4 * w } i.stale = false } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index d74eaf3a8..60bf4e1d6 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -569,69 +569,4 @@ 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 864b3c787..2392600be 100644 --- a/internal/shareable/shareable.go +++ b/internal/shareable/shareable.go @@ -189,8 +189,7 @@ 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. - x0, y0, x1, y1 := i.region() - i.backend.restorable.ReplacePixels(nil, x0, y0, x1, y1) + i.backend.restorable.Clear(i.region()) return }