From 736a840d537e41408469e44a5d9c3403465698e7 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Wed, 17 Jul 2019 11:13:57 +0900 Subject: [PATCH] restorable: More restricted Extend As a side effect, ReplacePixels always record pixels even when restoring is not needed. As CopyPixels reads pixels in any cases, this shortcut was originally useless. --- internal/restorable/image.go | 66 ++++++++++++++++++++---------- internal/restorable/images_test.go | 49 ++++++++++++++++++++++ 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 3f6184a1c..1ea55894c 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -45,19 +45,21 @@ 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)) } - if p.pixels == nil { - 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 - } + 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") } - return p.pixels[i] } // drawTrianglesHistoryItem is an item for history of draw-image commands. @@ -132,20 +134,45 @@ func NewImage(width, height int) *Image { // Extend disposes itself after its call. // // If the given size (width and height) is smaller than the source image, ExtendImage panics. +// +// The image must be ReplacePixels-only image. Extend panics when Fill or DrawTriangles are applied on the image. +// +// Extend panics when the image is stale. func (i *Image) Extend(width, height int) *Image { - if w, h := i.Size(); w > width || h > height { + w, h := i.Size() + if w > width || h > height { panic(fmt.Sprintf("restorable: the original size (%d, %d) cannot be extended to (%d, %d)", w, h, width, height)) } + if i.stale { + panic("restorable: Extend at a stale image is forbidden") + } + + if len(i.drawTrianglesHistory) > 0 { + panic("restorable: Extend after DrawTriangles is forbidden") + } + + if i.basePixels != nil && i.basePixels.color.A > 0 { + panic("restorable: Extend after Fill is forbidden") + } + newImg := NewImage(width, height) // Do not use DrawTriangles here. ReplacePixels will be called on a part of newImg later, and it looked like // ReplacePixels on a part of image deletes other region that are rendered by DrawTriangles (#593, #758). newImg.image.CopyPixels(i.image) - // As pixels should not be obtained here, making the image stale is inevitable. - // TODO: Copy pixel data from the source instead of making this stale (#897). - newImg.makeStale() + // Copy basePixels. + newImg.basePixels = &Pixels{ + pixels: make([]byte, 4*width*height), + length: 4 * width * 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() @@ -319,11 +346,6 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { i.image.ReplacePixels(pixels, x, y, width, height) - if !needsRestoring() { - i.makeStale() - return - } - if x == 0 && y == 0 && width == w && height == h { if i.basePixels == nil { i.basePixels = &Pixels{ @@ -331,6 +353,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } } i.basePixels.CopyFrom(pixels, 0) + i.basePixels.color = color.RGBA{} i.drawTrianglesHistory = nil i.stale = false return @@ -345,6 +368,7 @@ func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { } if i.stale { + // TODO: panic here? return } diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index ec734d868..6ec0dbea2 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -713,3 +713,52 @@ func TestDisallowReplacePixelsForPartAfterDrawTriangles(t *testing.T) { dst.DrawTriangles(src, vs, is, nil, driver.CompositeModeSourceOver, driver.FilterNearest, driver.AddressClampToZero) dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1) } + +func TestExtend(t *testing.T) { + pixAt := func(i, j int) byte { + return byte(17*i + 13*j + 0x40) + } + + const w, h = 16, 16 + orig := NewImage(w, h) + pix := make([]byte, 4*w*h) + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + idx := j*w + i + v := pixAt(i, j) + pix[4*idx] = v + pix[4*idx+1] = v + pix[4*idx+2] = v + pix[4*idx+3] = v + } + } + + orig.ReplacePixels(pix, 0, 0, w, h) + extended := orig.Extend(w*2, h*2) // After this, orig is already disposed. + + for j := 0; j < h*2; j++ { + for i := 0; i < w*2; i++ { + got, _, _, _ := extended.At(i, j) + want := byte(0) + if i < w && j < h { + want = pixAt(i, j) + } + if got != want { + t.Errorf("extended.At(%d, %d): got: %v, want: %v", i, j, got, want) + } + } + } +} + +func TestFillAndExtend(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Errorf("Extend after Fill must panic but not") + } + }() + + const w, h = 16, 16 + orig := NewImage(w, h) + orig.Fill(0x01, 0x02, 0x03, 0x04) + orig.Extend(w*2, h*2) +}