From 8522bfd0bf30ca41031df3230ea67c14a1da1a96 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Tue, 5 Jul 2022 13:26:45 +0900 Subject: [PATCH] internal/graphicscommand: bug fix: replacePixelsCommand should not read pixels Reading pixels, writing pixels, and using the image as a source might cause a flaky behavior with Metal. Stop reading pixels if possible. Closes #2180 --- internal/atlas/image.go | 82 +++------ internal/atlas/image_test.go | 44 ++--- internal/atlas/shader_test.go | 4 +- internal/buffered/image.go | 71 +------- internal/graphicscommand/command.go | 34 +--- internal/graphicscommand/image.go | 3 +- internal/graphicscommand/image_test.go | 51 +----- internal/graphicsdriver/graphics.go | 1 - internal/processtest/testdata/issue2182.go | 93 ++++++++++ internal/restorable/image.go | 22 +-- internal/restorable/images_test.go | 200 +++------------------ internal/restorable/rect.go | 57 +----- internal/restorable/shader_test.go | 10 +- 13 files changed, 195 insertions(+), 477 deletions(-) create mode 100644 internal/processtest/testdata/issue2182.go diff --git a/internal/atlas/image.go b/internal/atlas/image.go index 18608ebed..9c1f8f6bd 100644 --- a/internal/atlas/image.go +++ b/internal/atlas/image.go @@ -495,17 +495,21 @@ func (i *Image) drawTriangles(srcs [graphics.ShaderImageNum]*Image, vertices []f } // ReplacePixels replaces the pixels on the image. -func (i *Image) ReplacePixels(pix []byte, mask []byte) { +func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) { backendsM.Lock() defer backendsM.Unlock() - i.replacePixels(pix, mask) + i.replacePixels(pix, x, y, width, height) } -func (i *Image) replacePixels(pix []byte, mask []byte) { +func (i *Image) replacePixels(pix []byte, x, y, width, height int) { if i.disposed { panic("atlas: the image must not be disposed at replacePixels") } + if l := 4 * width * height; len(pix) != l { + panic(fmt.Sprintf("atlas: len(p) must be %d but %d", l, len(pix))) + } + i.resetUsedAsSourceCount() if i.backend == nil { @@ -516,29 +520,20 @@ func (i *Image) replacePixels(pix []byte, mask []byte) { } px, py, pw, ph := i.regionWithPadding() - if pix == nil { - if mask != nil { - panic("atlas: mask must be nil when pix is nil") + + if x != 0 || y != 0 || width != i.width || height != i.height || i.paddingSize() == 0 { + x += px + i.paddingSize() + y += py + i.paddingSize() + + if pix == nil { + i.backend.restorable.ReplacePixels(nil, x, y, width, height) + return } - i.backend.restorable.ReplacePixels(nil, nil, px, py, pw, ph) - return - } - ow, oh := pw-2*i.paddingSize(), ph-2*i.paddingSize() - if l := 4 * ow * oh; len(pix) != l { - panic(fmt.Sprintf("atlas: len(p) must be %d but %d", l, len(pix))) - } - - if i.paddingSize() == 0 { // Copy pixels in the case when pix is modified before the graphics command is executed. - pix2 := make([]byte, len(pix)) + pix2 := theTemporaryBytes.alloc(len(pix)) copy(pix2, pix) - var mask2 []byte - if mask != nil { - mask2 = make([]byte, len(mask)) - copy(mask2, mask) - } - i.backend.restorable.ReplacePixels(pix2, mask2, px, py, pw, ph) + i.backend.restorable.ReplacePixels(pix2, x, y, width, height) return } @@ -546,6 +541,7 @@ func (i *Image) replacePixels(pix []byte, mask []byte) { // Clear the edges. pixb might not be zero-cleared. // TODO: These loops assume that paddingSize is 1. + // TODO: Is clearing edges explicitly really needed? const paddingSize = 1 if paddingSize != i.paddingSize() { panic(fmt.Sprintf("atlas: replacePixels assumes the padding is always 1 but the actual padding was %d", i.paddingSize())) @@ -567,45 +563,13 @@ func (i *Image) replacePixels(pix []byte, mask []byte) { } // Copy the content. - for j := 0; j < oh; j++ { - copy(pixb[4*((j+paddingSize)*pw+paddingSize):], pix[4*j*ow:4*(j+1)*ow]) + for j := 0; j < height; j++ { + copy(pixb[4*((j+paddingSize)*pw+paddingSize):], pix[4*j*width:4*(j+1)*width]) } - // Add the paddings to the mask if needed. - if mask != nil { - origMask := mask - mask = theTemporaryBytes.alloc((pw*ph-1)/8 + 1) - // Clear the allocated region explicitly (#2089). - for i := range mask { - mask[i] = 0 - } - for i := 0; i < pw; i++ { - // Top edge - idx := i - mask[idx/8] |= 1 << (idx % 8) - // Bottom edge - idx = (ph-1)*pw + i - mask[idx/8] |= 1 << (idx % 8) - } - for j := 1; j < ph-1; j++ { - // Left edge - idx := j * pw - mask[idx/8] |= 1 << (idx % 8) - // Right edge - idx = j*pw + pw - 1 - mask[idx/8] |= 1 << (idx % 8) - - // Content - for i := 1; i < pw-1; i++ { - idx := j*pw + i - origIdx := (j-paddingSize)*(pw-paddingSize*2) + i - paddingSize - origValue := (origMask[origIdx/8] >> (origIdx % 8)) & 1 - mask[idx/8] |= origValue << (idx % 8) - } - } - } - - i.backend.restorable.ReplacePixels(pixb, mask, px, py, pw, ph) + x += px + y += py + i.backend.restorable.ReplacePixels(pixb, x, y, pw, ph) } func (img *Image) Pixels(graphicsDriver graphicsdriver.Graphics) ([]byte, error) { diff --git a/internal/atlas/image_test.go b/internal/atlas/image_test.go index e9ceb2e43..2945013e7 100644 --- a/internal/atlas/image_test.go +++ b/internal/atlas/image_test.go @@ -63,17 +63,17 @@ func TestEnsureIsolated(t *testing.T) { img1 := atlas.NewImage(bigSize, 100, atlas.ImageTypeRegular) defer img1.MarkDisposed() // Ensure img1's region is allocated. - img1.ReplacePixels(make([]byte, 4*bigSize*100), nil) + img1.ReplacePixels(make([]byte, 4*bigSize*100), 0, 0, bigSize, 100) img2 := atlas.NewImage(100, bigSize, atlas.ImageTypeRegular) defer img2.MarkDisposed() - img2.ReplacePixels(make([]byte, 4*100*bigSize), nil) + img2.ReplacePixels(make([]byte, 4*100*bigSize), 0, 0, 100, bigSize) const size = 32 img3 := atlas.NewImage(size/2, size/2, atlas.ImageTypeRegular) defer img3.MarkDisposed() - img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4), nil) + img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4), 0, 0, size/2, size/2) img4 := atlas.NewImage(size, size, atlas.ImageTypeRegular) defer img4.MarkDisposed() @@ -90,7 +90,7 @@ func TestEnsureIsolated(t *testing.T) { pix[4*(i+j*size)+3] = byte(i + j) } } - img4.ReplacePixels(pix, nil) + img4.ReplacePixels(pix, 0, 0, size, size) const ( dx0 = size / 4 @@ -157,11 +157,11 @@ func TestReputOnAtlas(t *testing.T) { img0 := atlas.NewImage(size, size, atlas.ImageTypeRegular) defer img0.MarkDisposed() - img0.ReplacePixels(make([]byte, 4*size*size), nil) + img0.ReplacePixels(make([]byte, 4*size*size), 0, 0, size, size) img1 := atlas.NewImage(size, size, atlas.ImageTypeRegular) defer img1.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*size*size), nil) + img1.ReplacePixels(make([]byte, 4*size*size), 0, 0, size, size) if got, want := img1.IsOnAtlasForTesting(), true; got != want { t.Errorf("got: %v, want: %v", got, want) } @@ -177,12 +177,12 @@ func TestReputOnAtlas(t *testing.T) { pix[4*(i+j*size)+3] = byte(i + j) } } - img2.ReplacePixels(pix, nil) + img2.ReplacePixels(pix, 0, 0, size, size) // Create a volatile image. This should always be isolated. img3 := atlas.NewImage(size, size, atlas.ImageTypeVolatile) defer img3.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*size*size), nil) + img1.ReplacePixels(make([]byte, 4*size*size), 0, 0, size, size) if got, want := img3.IsOnAtlasForTesting(), false; got != want { t.Errorf("got: %v, want: %v", got, want) } @@ -271,7 +271,7 @@ func TestReputOnAtlas(t *testing.T) { if err := atlas.PutImagesOnAtlasForTesting(ui.GraphicsDriverForTesting()); err != nil { t.Fatal(err) } - img1.ReplacePixels(make([]byte, 4*size*size), nil) + img1.ReplacePixels(make([]byte, 4*size*size), 0, 0, size, size) img0.DrawTriangles([graphics.ShaderImageNum]*atlas.Image{img1}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeCopy, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil, false) if got, want := img1.IsOnAtlasForTesting(), false; got != want { t.Errorf("got: %v, want: %v", got, want) @@ -313,7 +313,7 @@ func TestExtend(t *testing.T) { p0[4*i+2] = byte(i) p0[4*i+3] = byte(i) } - img0.ReplacePixels(p0, nil) + img0.ReplacePixels(p0, 0, 0, w0, h0) const w1, h1 = minImageSizeForTesting + 1, 100 img1 := atlas.NewImage(w1, h1, atlas.ImageTypeRegular) @@ -327,7 +327,7 @@ func TestExtend(t *testing.T) { p1[4*i+3] = byte(i) } // Ensure to allocate - img1.ReplacePixels(p1, nil) + img1.ReplacePixels(p1, 0, 0, w1, h1) pix0, err := img0.Pixels(ui.GraphicsDriverForTesting()) if err != nil { @@ -382,7 +382,7 @@ func TestReplacePixelsAfterDrawTriangles(t *testing.T) { pix[4*i+2] = byte(i) pix[4*i+3] = byte(i) } - src.ReplacePixels(pix, nil) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(w, h, 0, 0, 1) is := graphics.QuadIndices() @@ -393,7 +393,7 @@ func TestReplacePixelsAfterDrawTriangles(t *testing.T) { Height: h, } dst.DrawTriangles([graphics.ShaderImageNum]*atlas.Image{src}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeCopy, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil, false) - dst.ReplacePixels(pix, nil) + dst.ReplacePixels(pix, 0, 0, w, h) pix, err := dst.Pixels(ui.GraphicsDriverForTesting()) if err != nil { @@ -430,7 +430,7 @@ func TestSmallImages(t *testing.T) { pix[4*i+2] = 0xff pix[4*i+3] = 0xff } - src.ReplacePixels(pix, nil) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(w, h, 0, 0, 1) is := graphics.QuadIndices() @@ -477,7 +477,7 @@ func TestLongImages(t *testing.T) { pix[4*i+2] = 0xff pix[4*i+3] = 0xff } - src.ReplacePixels(pix, nil) + src.ReplacePixels(pix, 0, 0, w, h) const scale = 120 vs := quadVertices(w, h, 0, 0, scale) @@ -527,12 +527,12 @@ func TestExtendWithBigImage(t *testing.T) { img0 := atlas.NewImage(1, 1, atlas.ImageTypeRegular) defer img0.MarkDisposed() - img0.ReplacePixels(make([]byte, 4*1*1), nil) + img0.ReplacePixels(make([]byte, 4*1*1), 0, 0, 1, 1) img1 := atlas.NewImage(minImageSizeForTesting+1, minImageSizeForTesting+1, atlas.ImageTypeRegular) defer img1.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*(minImageSizeForTesting+1)*(minImageSizeForTesting+1)), nil) + img1.ReplacePixels(make([]byte, 4*(minImageSizeForTesting+1)*(minImageSizeForTesting+1)), 0, 0, minImageSizeForTesting+1, minImageSizeForTesting+1) } // Issue #1217 @@ -545,7 +545,7 @@ func TestMaxImageSize(t *testing.T) { s := maxImageSizeForTesting - 2*paddingSize img1 := atlas.NewImage(s, s, atlas.ImageTypeRegular) defer img1.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*s*s), nil) + img1.ReplacePixels(make([]byte, 4*s*s), 0, 0, s, s) } // Issue #1217 (disabled) @@ -558,7 +558,7 @@ func Disable_TestMinImageSize(t *testing.T) { s := minImageSizeForTesting img := atlas.NewImage(s, s, atlas.ImageTypeRegular) defer img.MarkDisposed() - img.ReplacePixels(make([]byte, 4*s*s), nil) + img.ReplacePixels(make([]byte, 4*s*s), 0, 0, s, s) } func TestMaxImageSizeJust(t *testing.T) { @@ -567,7 +567,7 @@ func TestMaxImageSizeJust(t *testing.T) { // TODO: Should we allow such this size for ImageTypeRegular? img := atlas.NewImage(s, s, atlas.ImageTypeUnmanaged) defer img.MarkDisposed() - img.ReplacePixels(make([]byte, 4*s*s), nil) + img.ReplacePixels(make([]byte, 4*s*s), 0, 0, s, s) } func TestMaxImageSizeExceeded(t *testing.T) { @@ -582,7 +582,7 @@ func TestMaxImageSizeExceeded(t *testing.T) { } }() - img.ReplacePixels(make([]byte, 4*(s+1)*s), nil) + img.ReplacePixels(make([]byte, 4*(s+1)*s), 0, 0, s+1, s) } // Issue #1421 @@ -704,7 +704,7 @@ func TestImageReplacePixelsModify(t *testing.T) { pix[4*(i+j*size)+3] = byte(i + j) } } - img.ReplacePixels(pix, nil) + img.ReplacePixels(pix, 0, 0, size, size) // Modify pix after ReplacePixels. for j := 0; j < size; j++ { diff --git a/internal/atlas/shader_test.go b/internal/atlas/shader_test.go index 8c164d2e3..08ee6c967 100644 --- a/internal/atlas/shader_test.go +++ b/internal/atlas/shader_test.go @@ -62,9 +62,9 @@ func TestImageDrawTwice(t *testing.T) { dst := atlas.NewImage(w, h, atlas.ImageTypeRegular) src0 := atlas.NewImage(w, h, atlas.ImageTypeRegular) - src0.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff}, nil) + src0.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff}, 0, 0, w, h) src1 := atlas.NewImage(w, h, atlas.ImageTypeRegular) - src1.ReplacePixels([]byte{0x80, 0x80, 0x80, 0xff}, nil) + src1.ReplacePixels([]byte{0x80, 0x80, 0x80, 0xff}, 0, 0, w, h) vs := quadVertices(w, h, 0, 0, 1) is := graphics.QuadIndices() diff --git a/internal/buffered/image.go b/internal/buffered/image.go index 529a4104c..9e3a6529c 100644 --- a/internal/buffered/image.go +++ b/internal/buffered/image.go @@ -29,9 +29,7 @@ type Image struct { width int height int - pixels []byte - mask []byte - needsToResolvePixels bool + pixels []byte } func BeginFrame(graphicsDriver graphicsdriver.Graphics) error { @@ -68,21 +66,6 @@ func (i *Image) initialize(imageType atlas.ImageType) { func (i *Image) invalidatePixels() { i.pixels = nil - i.mask = nil - i.needsToResolvePixels = false -} - -func (i *Image) resolvePendingPixels(keepPendingPixels bool) { - if !i.needsToResolvePixels { - return - } - - i.img.ReplacePixels(i.pixels, i.mask) - if !keepPendingPixels { - i.pixels = nil - i.mask = nil - } - i.needsToResolvePixels = false } func (i *Image) MarkDisposed() { @@ -102,14 +85,7 @@ func (img *Image) At(graphicsDriver graphicsdriver.Graphics, x, y int) (r, g, b, idx := (y*img.width + x) if img.pixels != nil { - if img.mask == nil { - return img.pixels[4*idx], img.pixels[4*idx+1], img.pixels[4*idx+2], img.pixels[4*idx+3], nil - } - if img.mask[idx/8]<<(idx%8)&1 != 0 { - return img.pixels[4*idx], img.pixels[4*idx+1], img.pixels[4*idx+2], img.pixels[4*idx+3], nil - } - - img.resolvePendingPixels(false) + return img.pixels[4*idx], img.pixels[4*idx+1], img.pixels[4*idx+2], img.pixels[4*idx+3], nil } pix, err := img.img.Pixels(graphicsDriver) @@ -117,8 +93,6 @@ func (img *Image) At(graphicsDriver graphicsdriver.Graphics, x, y int) (r, g, b, return 0, 0, 0, 0, err } img.pixels = pix - // When pixels represents the whole pixels, the mask is not needed. - img.mask = nil return img.pixels[4*idx], img.pixels[4*idx+1], img.pixels[4*idx+2], img.pixels[4*idx+3], nil } @@ -143,40 +117,8 @@ func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) { } } - if x == 0 && y == 0 && width == i.width && height == i.height { - i.invalidatePixels() - i.img.ReplacePixels(pix, nil) - return - } - - // TODO: If width/height is big enough, ReplacePixels can be called instead of replacePendingPixels. - // Check if this is efficient. - - i.replacePendingPixels(pix, x, y, width, height) -} - -func (img *Image) replacePendingPixels(pix []byte, x, y, width, height int) { - if img.pixels == nil { - img.pixels = make([]byte, 4*img.width*img.height) - if img.mask == nil { - img.mask = make([]byte, (img.width*img.height-1)/8+1) - } - } - for j := 0; j < height; j++ { - copy(img.pixels[4*((j+y)*img.width+x):], pix[4*j*width:4*(j+1)*width]) - } - - // A mask is created only when partial regions are replaced by replacePendingPixels. - if img.mask != nil { - for j := 0; j < height; j++ { - for i := 0; i < width; i++ { - idx := (y+j)*img.width + x + i - img.mask[idx/8] |= 1 << (idx % 8) - } - } - } - - img.needsToResolvePixels = true + i.invalidatePixels() + i.img.ReplacePixels(pix, x, y, width, height) } // DrawTriangles draws the src image with the given vertices. @@ -203,22 +145,19 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, vertices []f if shader == nil { // Fast path for rendering without a shader (#1355). img := srcs[0] - img.resolvePendingPixels(true) imgs[0] = img.img } else { for i, img := range srcs { if img == nil { continue } - img.resolvePendingPixels(true) imgs[i] = img.img } s = shader.shader } - i.resolvePendingPixels(false) - i.img.DrawTriangles(imgs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, s, uniforms, evenOdd) i.invalidatePixels() + i.img.DrawTriangles(imgs, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, s, uniforms, evenOdd) } type Shader struct { diff --git a/internal/graphicscommand/command.go b/internal/graphicscommand/command.go index 40bcfb32a..b5ca48e2d 100644 --- a/internal/graphicscommand/command.go +++ b/internal/graphicscommand/command.go @@ -523,37 +523,11 @@ func (c *replacePixelsCommand) String() string { // Exec executes the replacePixelsCommand. func (c *replacePixelsCommand) Exec(graphicsDriver graphicsdriver.Graphics, indexOffset int) error { - var lastArgIdx int - for i, a := range c.args { - if a.Mask == nil { - continue - } - if len(c.args[lastArgIdx:i]) > 0 { - c.dst.image.ReplacePixels(c.args[lastArgIdx:i]) - lastArgIdx = i - } - - orig := make([]byte, 4*c.dst.width*c.dst.height) - if err := c.dst.image.ReadPixels(orig); err != nil { - return err - } - for j := 0; j < a.Height; j++ { - for i := 0; i < a.Width; i++ { - idx := j*a.Width + i - if a.Mask[idx/8]>>(idx%8)&1 == 0 { - srcIdx := (a.Y+j)*c.dst.width + a.X + i - copy(a.Pixels[4*idx:4*(idx+1)], orig[4*srcIdx:4*(srcIdx+1)]) - } - } - } - - a.Mask = nil + if len(c.args) == 0 { + return nil } - - if len(c.args[lastArgIdx:]) > 0 { - if err := c.dst.image.ReplacePixels(c.args[lastArgIdx:]); err != nil { - return err - } + if err := c.dst.image.ReplacePixels(c.args); err != nil { + return err } return nil } diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 67fdb780a..88aa792ea 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -166,10 +166,9 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, buf []byte) e return nil } -func (i *Image) ReplacePixels(pixels []byte, mask []byte, x, y, width, height int) { +func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { i.bufferedRP = append(i.bufferedRP, &graphicsdriver.ReplacePixelsArgs{ Pixels: pixels, - Mask: mask, X: x, Y: y, Width: width, diff --git a/internal/graphicscommand/image_test.go b/internal/graphicscommand/image_test.go index f205666b9..d8d219e21 100644 --- a/internal/graphicscommand/image_test.go +++ b/internal/graphicscommand/image_test.go @@ -87,60 +87,11 @@ func TestReplacePixelsPartAfterDrawTriangles(t *testing.T) { } dst.DrawTriangles([graphics.ShaderImageNum]*graphicscommand.Image{clr}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeClear, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) dst.DrawTriangles([graphics.ShaderImageNum]*graphicscommand.Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - dst.ReplacePixels(make([]byte, 4), nil, 0, 0, 1, 1) + dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1) // TODO: Check the result. } -func TestReplacePixelsWithMask(t *testing.T) { - const w, h = 4, 3 - src := graphicscommand.NewImage(w, h, false) - dst := graphicscommand.NewImage(w, h, false) - - vs := quadVertices(src, w, h) - is := graphics.QuadIndices() - dr := graphicsdriver.Region{ - X: 0, - Y: 0, - Width: w, - Height: h, - } - dst.DrawTriangles([graphics.ShaderImageNum]*graphicscommand.Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeClear, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - - pix0 := make([]byte, 4*w*h) - for i := range pix0 { - pix0[i] = 0x40 - } - dst.ReplacePixels(pix0, nil, 0, 0, w, h) - - pix1 := make([]byte, 4*w*h) - for i := range pix1 { - pix1[i] = 0x80 - } - mask1 := []byte{0b11110110, 0b00000110} - dst.ReplacePixels(pix1, mask1, 0, 0, w, h) - - readPix := make([]byte, 4*w*h) - if err := dst.ReadPixels(ui.GraphicsDriverForTesting(), readPix); err != nil { - t.Fatal(err) - } - for j := 0; j < h; j++ { - for i := 0; i < w; i++ { - idx := 4 * (i + w*j) - got := color.RGBA{readPix[idx], readPix[idx+1], readPix[idx+2], readPix[idx+3]} - var want color.RGBA - if (i != 0 && i != w-1) || (j != 0 && j != h-1) { - want = color.RGBA{0x80, 0x80, 0x80, 0x80} - } else { - want = color.RGBA{0x40, 0x40, 0x40, 0x40} - } - if got != want { - t.Errorf("dst.At(%d, %d) after ReplacePixels: got %v, want: %v", i, j, got, want) - } - } - } -} - func TestShader(t *testing.T) { const w, h = 16, 16 clr := graphicscommand.NewImage(w, h, false) diff --git a/internal/graphicsdriver/graphics.go b/internal/graphicsdriver/graphics.go index a1b096a4b..485cf27b6 100644 --- a/internal/graphicsdriver/graphics.go +++ b/internal/graphicsdriver/graphics.go @@ -77,7 +77,6 @@ type ImageID int type ReplacePixelsArgs struct { Pixels []byte - Mask []byte X int Y int Width int diff --git a/internal/processtest/testdata/issue2182.go b/internal/processtest/testdata/issue2182.go new file mode 100644 index 000000000..e0620d840 --- /dev/null +++ b/internal/processtest/testdata/issue2182.go @@ -0,0 +1,93 @@ +// Copyright 2022 The Ebitengine 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. + +//go:build ignore +// +build ignore + +package main + +import ( + "errors" + "fmt" + "image" + "image/color" + + "github.com/hajimehoshi/ebiten/v2" + "github.com/hajimehoshi/ebiten/v2/ebitenutil" +) + +var regularTermination = errors.New("regular termination") + +var ( + baseImage *ebiten.Image + derivedImage *ebiten.Image +) + +func init() { + const ( + w = 36 + h = 40 + ) + + baseImage = ebiten.NewImage(w, h) + derivedImage = ebiten.NewImage(w, h) + + baseImage.Fill(color.White) + for j := 0; j < h; j++ { + for i := 0; i < w; i++ { + baseImage.SubImage(image.Rect(i, j, i+1, j+1)).(*ebiten.Image).ReplacePixels([]byte{0, 0, 0, 0xff}) + } + } + derivedImage.DrawImage(baseImage, nil) +} + +type Game struct { + count int +} + +func (g *Game) Update() error { + g.count++ + if g.count == 16 { + return regularTermination + } + return nil +} + +func (g *Game) Draw(screen *ebiten.Image) { + screen.Fill(color.White) + + screen.DrawImage(derivedImage, nil) + if g.count >= 8 { + if got, want := screen.At(0, 0), (color.RGBA{0, 0, 0, 0xff}); got != want { + panic(fmt.Sprintf("got: %v, want: %v", got, want)) + } + } + + // The blow 3 line matters to reproduce #2154. + mx, my := ebiten.CursorPosition() + msg := fmt.Sprintf("TPS: %.01f; FPS: %.01f; cursor: (%d, %d)", ebiten.CurrentTPS(), ebiten.CurrentFPS(), mx, my) + ebitenutil.DebugPrint(screen, msg) +} + +func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) { + return 640, 480 +} + +func main() { + ebiten.SetWindowTitle("Test") + + if err := ebiten.RunGame(&Game{}); err != nil && err != regularTermination { + panic(err) + } +} diff --git a/internal/restorable/image.go b/internal/restorable/image.go index edfaec2a6..ce849e306 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -38,11 +38,11 @@ func (p *Pixels) Apply(img *graphicscommand.Image) { p.pixelsRecords.apply(img) } -func (p *Pixels) AddOrReplace(pix []byte, mask []byte, x, y, width, height int) { +func (p *Pixels) AddOrReplace(pix []byte, x, y, width, height int) { if p.pixelsRecords == nil { p.pixelsRecords = &pixelsRecords{} } - p.pixelsRecords.addOrReplace(pix, mask, x, y, width, height) + p.pixelsRecords.addOrReplace(pix, x, y, width, height) } func (p *Pixels) Clear(x, y, width, height int) { @@ -144,7 +144,7 @@ func ensureEmptyImage() *Image { // As emptyImage is the source at clearImage, initialize this with ReplacePixels, not clearImage. // This operation is also important when restoring emptyImage. - emptyImage.ReplacePixels(pix, nil, 0, 0, w, h) + emptyImage.ReplacePixels(pix, 0, 0, w, h) theImages.add(emptyImage) return emptyImage } @@ -267,7 +267,7 @@ func (i *Image) makeStale() { // ClearPixels clears the specified region by ReplacePixels. func (i *Image) ClearPixels(x, y, width, height int) { - i.ReplacePixels(nil, nil, x, y, width, height) + i.ReplacePixels(nil, x, y, width, height) } func (i *Image) needsRestoring() bool { @@ -277,7 +277,7 @@ func (i *Image) needsRestoring() bool { // ReplacePixels replaces the image pixels with the given pixels slice. // // The specified region must not be overlapped with other regions by ReplacePixels. -func (i *Image) ReplacePixels(pixels []byte, mask []byte, x, y, width, height int) { +func (i *Image) ReplacePixels(pixels []byte, x, y, width, height int) { if width <= 0 || height <= 0 { panic("restorable: width/height must be positive") } @@ -291,12 +291,12 @@ func (i *Image) ReplacePixels(pixels []byte, mask []byte, x, y, width, height in theImages.makeStaleIfDependingOn(i) if pixels != nil { - i.image.ReplacePixels(pixels, mask, x, y, width, height) + 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), nil, x, y, width, height) + i.image.ReplacePixels(make([]byte, 4*width*height), x, y, width, height) } if !needsRestoring() || !i.needsRestoring() { @@ -310,7 +310,7 @@ func (i *Image) ReplacePixels(pixels []byte, mask []byte, x, y, width, height in // This function is responsible to copy this. copiedPixels := make([]byte, len(pixels)) copy(copiedPixels, pixels) - i.basePixels.AddOrReplace(copiedPixels, mask, 0, 0, w, h) + i.basePixels.AddOrReplace(copiedPixels, 0, 0, w, h) } else { i.basePixels.Clear(0, 0, w, h) } @@ -334,7 +334,7 @@ func (i *Image) ReplacePixels(pixels []byte, mask []byte, x, y, width, height in // This function is responsible to copy this. copiedPixels := make([]byte, len(pixels)) copy(copiedPixels, pixels) - i.basePixels.AddOrReplace(copiedPixels, mask, x, y, width, height) + i.basePixels.AddOrReplace(copiedPixels, x, y, width, height) } else { i.basePixels.Clear(x, y, width, height) } @@ -490,7 +490,7 @@ func (i *Image) readPixelsFromGPU(graphicsDriver graphicsdriver.Graphics) error return err } i.basePixels = Pixels{} - i.basePixels.AddOrReplace(pix, nil, 0, 0, i.width, i.height) + i.basePixels.AddOrReplace(pix, 0, 0, i.width, i.height) i.clearDrawTrianglesHistory() i.stale = false return nil @@ -615,7 +615,7 @@ func (i *Image) restore(graphicsDriver graphicsdriver.Graphics) error { if err := gimg.ReadPixels(graphicsDriver, pix); err != nil { return err } - i.basePixels.AddOrReplace(pix, nil, 0, 0, w, h) + i.basePixels.AddOrReplace(pix, 0, 0, w, h) } i.image = gimg diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index 09f787968..ef92da260 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -61,7 +61,7 @@ func TestRestore(t *testing.T) { defer img0.Dispose() clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff} - img0.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, nil, 0, 0, 1, 1) + img0.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, 1, 1) if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { t.Fatal(err) } @@ -135,7 +135,7 @@ func TestRestoreChain(t *testing.T) { } }() clr := color.RGBA{0x00, 0x00, 0x00, 0xff} - imgs[0].ReplacePixels([]byte{clr.R, clr.G, clr.B, clr.A}, nil, 0, 0, 1, 1) + imgs[0].ReplacePixels([]byte{clr.R, clr.G, clr.B, clr.A}, 0, 0, 1, 1) for i := 0; i < num-1; i++ { vs := quadVertices(imgs[i], 1, 1, 0, 0) is := graphics.QuadIndices() @@ -180,11 +180,11 @@ func TestRestoreChain2(t *testing.T) { }() clr0 := color.RGBA{0xff, 0x00, 0x00, 0xff} - imgs[0].ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, nil, 0, 0, w, h) + imgs[0].ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, w, h) clr7 := color.RGBA{0x00, 0xff, 0x00, 0xff} - imgs[7].ReplacePixels([]byte{clr7.R, clr7.G, clr7.B, clr7.A}, nil, 0, 0, w, h) + imgs[7].ReplacePixels([]byte{clr7.R, clr7.G, clr7.B, clr7.A}, 0, 0, w, h) clr8 := color.RGBA{0x00, 0x00, 0xff, 0xff} - imgs[8].ReplacePixels([]byte{clr8.R, clr8.G, clr8.B, clr8.A}, nil, 0, 0, w, h) + imgs[8].ReplacePixels([]byte{clr8.R, clr8.G, clr8.B, clr8.A}, 0, 0, w, h) is := graphics.QuadIndices() dr := graphicsdriver.Region{ @@ -234,7 +234,7 @@ func TestRestoreOverrideSource(t *testing.T) { }() clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff} clr1 := color.RGBA{0x00, 0x00, 0x01, 0xff} - img1.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, nil, 0, 0, w, h) + img1.ReplacePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, w, h) is := graphics.QuadIndices() dr := graphicsdriver.Region{ X: 0, @@ -244,7 +244,7 @@ func TestRestoreOverrideSource(t *testing.T) { } img2.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{img1}, [graphics.ShaderImageNum - 1][2]float32{}, quadVertices(img1, w, h, 0, 0), is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) img3.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{img2}, [graphics.ShaderImageNum - 1][2]float32{}, quadVertices(img2, w, h, 0, 0), is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - img0.ReplacePixels([]byte{clr1.R, clr1.G, clr1.B, clr1.A}, nil, 0, 0, w, h) + img0.ReplacePixels([]byte{clr1.R, clr1.G, clr1.B, clr1.A}, 0, 0, w, h) img1.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{img0}, [graphics.ShaderImageNum - 1][2]float32{}, quadVertices(img0, w, h, 0, 0), is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { t.Fatal(err) @@ -417,7 +417,7 @@ func TestRestoreComplexGraph(t *testing.T) { func newImageFromImage(rgba *image.RGBA) *restorable.Image { s := rgba.Bounds().Size() img := restorable.NewImage(s.X, s.Y, restorable.ImageTypeRegular) - img.ReplacePixels(rgba.Pix, nil, 0, 0, s.X, s.Y) + img.ReplacePixels(rgba.Pix, 0, 0, s.X, s.Y) return img } @@ -491,7 +491,7 @@ func TestReplacePixels(t *testing.T) { for i := range pix { pix[i] = 0xff } - img.ReplacePixels(pix, nil, 5, 7, 4, 4) + img.ReplacePixels(pix, 5, 7, 4, 4) // Check the region (5, 7)-(9, 11). Outside state is indeterministic. for j := 7; j < 11; j++ { for i := 5; i < 9; i++ { @@ -547,7 +547,7 @@ func TestDrawTrianglesAndReplacePixels(t *testing.T) { Height: 1, } img1.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{img0}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeCopy, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - img1.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, nil, 0, 0, 2, 1) + img1.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 0, 0, 2, 1) if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { t.Fatal(err) @@ -618,7 +618,7 @@ func TestReplacePixelsPart(t *testing.T) { img := restorable.NewImage(4, 4, restorable.ImageTypeRegular) // This doesn't make the image stale. Its base pixels are available. - img.ReplacePixels(pix, nil, 1, 1, 2, 2) + img.ReplacePixels(pix, 1, 1, 2, 2) cases := []struct { i int @@ -693,7 +693,7 @@ func TestReplacePixelsOnly(t *testing.T) { defer img1.Dispose() for i := 0; i < w*h; i += 5 { - img0.ReplacePixels([]byte{1, 2, 3, 4}, nil, i%w, i/w, 1, 1) + img0.ReplacePixels([]byte{1, 2, 3, 4}, i%w, i/w, 1, 1) } vs := quadVertices(img0, 1, 1, 0, 0) @@ -705,7 +705,7 @@ func TestReplacePixelsOnly(t *testing.T) { Height: 1, } img1.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{img0}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeCopy, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - img0.ReplacePixels([]byte{5, 6, 7, 8}, nil, 0, 0, 1, 1) + img0.ReplacePixels([]byte{5, 6, 7, 8}, 0, 0, 1, 1) // BasePixelsForTesting is available without GPU accessing. for j := 0; j < h; j++ { @@ -747,14 +747,14 @@ func TestReadPixelsFromVolatileImage(t *testing.T) { src := restorable.NewImage(w, h, restorable.ImageTypeRegular) // First, make sure that dst has pixels - dst.ReplacePixels(make([]byte, 4*w*h), nil, 0, 0, w, h) + dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h) // Second, draw src to dst. If the implementation is correct, dst becomes stale. pix := make([]byte, 4*w*h) for i := range pix { pix[i] = 0xff } - src.ReplacePixels(pix, nil, 0, 0, w, h) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(src, 1, 1, 0, 0) is := graphics.QuadIndices() dr := graphicsdriver.Region{ @@ -791,7 +791,7 @@ func TestAllowReplacePixelsAfterDrawTriangles(t *testing.T) { Height: h, } dst.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - dst.ReplacePixels(make([]byte, 4*w*h), nil, 0, 0, w, h) + dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h) // ReplacePixels for a whole image doesn't panic. } @@ -815,7 +815,7 @@ func TestDisallowReplacePixelsForPartAfterDrawTriangles(t *testing.T) { Height: h, } dst.DrawTriangles([graphics.ShaderImageNum]*restorable.Image{src}, [graphics.ShaderImageNum - 1][2]float32{}, vs, is, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, dr, graphicsdriver.Region{}, nil, nil, false) - dst.ReplacePixels(make([]byte, 4), nil, 0, 0, 1, 1) + dst.ReplacePixels(make([]byte, 4), 0, 0, 1, 1) } func TestExtend(t *testing.T) { @@ -837,7 +837,7 @@ func TestExtend(t *testing.T) { } } - orig.ReplacePixels(pix, nil, 0, 0, w, h) + 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++ { @@ -860,13 +860,13 @@ func TestExtend(t *testing.T) { func TestClearPixels(t *testing.T) { const w, h = 16, 16 img := restorable.NewImage(w, h, restorable.ImageTypeRegular) - img.ReplacePixels(make([]byte, 4*4*4), nil, 0, 0, 4, 4) - img.ReplacePixels(make([]byte, 4*4*4), nil, 4, 0, 4, 4) + 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), nil, 0, 0, 8, 4) + img.ReplacePixels(make([]byte, 4*8*4), 0, 0, 8, 4) } func TestMutateSlices(t *testing.T) { @@ -880,7 +880,7 @@ func TestMutateSlices(t *testing.T) { pix[4*i+2] = byte(i) pix[4*i+3] = 0xff } - src.ReplacePixels(pix, nil, 0, 0, w, h) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(src, w, h, 0, 0) is := make([]uint16, len(graphics.QuadIndices())) @@ -937,7 +937,7 @@ func TestOverlappedPixels(t *testing.T) { pix0[idx+3] = 0xff } } - dst.ReplacePixels(pix0, nil, 0, 0, 2, 2) + dst.ReplacePixels(pix0, 0, 0, 2, 2) pix1 := make([]byte, 4*2*2) for j := 0; j < 2; j++ { @@ -949,7 +949,7 @@ func TestOverlappedPixels(t *testing.T) { pix1[idx+3] = 0xff } } - dst.ReplacePixels(pix1, nil, 1, 1, 2, 2) + dst.ReplacePixels(pix1, 1, 1, 2, 2) wantColors := []color.RGBA{ {0xff, 0, 0, 0xff}, @@ -978,7 +978,7 @@ func TestOverlappedPixels(t *testing.T) { } } - dst.ReplacePixels(nil, nil, 1, 0, 2, 2) + dst.ReplacePixels(nil, 1, 0, 2, 2) wantColors = []color.RGBA{ {0xff, 0, 0, 0xff}, @@ -1017,7 +1017,7 @@ func TestOverlappedPixels(t *testing.T) { pix2[idx+3] = 0xff } } - dst.ReplacePixels(pix2, nil, 1, 1, 2, 2) + dst.ReplacePixels(pix2, 1, 1, 2, 2) wantColors = []color.RGBA{ {0xff, 0, 0, 0xff}, @@ -1067,151 +1067,3 @@ func TestOverlappedPixels(t *testing.T) { } } } - -func TestReplacePixelsWithMask(t *testing.T) { - dst := restorable.NewImage(3, 3, restorable.ImageTypeRegular) - - pix0 := make([]byte, 4*2*2) - for j := 0; j < 2; j++ { - for i := 0; i < 2; i++ { - idx := 4 * (j*2 + i) - pix0[idx] = 0xff - pix0[idx+1] = 0 - pix0[idx+2] = 0 - pix0[idx+3] = 0xff - } - } - dst.ReplacePixels(pix0, nil, 0, 0, 2, 2) - - pix1 := make([]byte, 4*2*2) - for j := 0; j < 2; j++ { - for i := 0; i < 2; i++ { - idx := 4 * (j*2 + i) - pix1[idx] = 0 - pix1[idx+1] = 0xff - pix1[idx+2] = 0 - pix1[idx+3] = 0xff - } - } - - mask1 := []byte{0b00001110} - dst.ReplacePixels(pix1, mask1, 1, 1, 2, 2) - - wantColors := []color.RGBA{ - {0xff, 0, 0, 0xff}, - {0xff, 0, 0, 0xff}, - {0, 0, 0, 0}, - - {0xff, 0, 0, 0xff}, - {0xff, 0, 0, 0xff}, - {0, 0xff, 0, 0xff}, - - {0, 0, 0, 0}, - {0, 0xff, 0, 0xff}, - {0, 0xff, 0, 0xff}, - } - for j := 0; j < 3; j++ { - for i := 0; i < 3; i++ { - r, g, b, a, err := dst.At(ui.GraphicsDriverForTesting(), i, j) - if err != nil { - t.Fatal(err) - } - got := color.RGBA{r, g, b, a} - want := wantColors[3*j+i] - if got != want { - t.Errorf("color at (%d, %d): got %v, want: %v", i, j, got, want) - } - } - } - - dst.ReplacePixels(nil, nil, 1, 0, 2, 2) - - wantColors = []color.RGBA{ - {0xff, 0, 0, 0xff}, - {0, 0, 0, 0}, - {0, 0, 0, 0}, - - {0xff, 0, 0, 0xff}, - {0, 0, 0, 0}, - {0, 0, 0, 0}, - - {0, 0, 0, 0}, - {0, 0xff, 0, 0xff}, - {0, 0xff, 0, 0xff}, - } - for j := 0; j < 3; j++ { - for i := 0; i < 3; i++ { - r, g, b, a, err := dst.At(ui.GraphicsDriverForTesting(), i, j) - if err != nil { - t.Fatal(err) - } - got := color.RGBA{r, g, b, a} - want := wantColors[3*j+i] - if got != want { - t.Errorf("color at (%d, %d): got %v, want: %v", i, j, got, want) - } - } - } - - // Update the same region with pix1/mask1. - pix2 := make([]byte, 4*2*2) - for j := 0; j < 2; j++ { - for i := 0; i < 2; i++ { - idx := 4 * (j*2 + i) - pix2[idx] = 0 - pix2[idx+1] = 0 - pix2[idx+2] = 0xff - pix2[idx+3] = 0xff - } - } - mask2 := []byte{0b00000111} - dst.ReplacePixels(pix2, mask2, 1, 1, 2, 2) - - wantColors = []color.RGBA{ - {0xff, 0, 0, 0xff}, - {0, 0, 0, 0}, - {0, 0, 0, 0}, - - {0xff, 0, 0, 0xff}, - {0, 0, 0xff, 0xff}, - {0, 0, 0xff, 0xff}, - - {0, 0, 0, 0}, - {0, 0, 0xff, 0xff}, - {0, 0xff, 0, 0xff}, - } - for j := 0; j < 3; j++ { - for i := 0; i < 3; i++ { - r, g, b, a, err := dst.At(ui.GraphicsDriverForTesting(), i, j) - if err != nil { - t.Fatal(err) - } - got := color.RGBA{r, g, b, a} - want := wantColors[3*j+i] - if got != want { - t.Errorf("color at (%d, %d): got %v, want: %v", i, j, got, want) - } - } - } - - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { - t.Fatal(err) - } - if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { - t.Fatal(err) - } - - for j := 0; j < 3; j++ { - for i := 0; i < 3; i++ { - r, g, b, a, err := dst.At(ui.GraphicsDriverForTesting(), i, j) - if err != nil { - t.Fatal(err) - } - got := color.RGBA{r, g, b, a} - want := wantColors[3*j+i] - if got != want { - t.Errorf("color at (%d, %d): got %v, want: %v", i, j, got, want) - } - } - } -} diff --git a/internal/restorable/rect.go b/internal/restorable/rect.go index 9fee2c810..3528cb881 100644 --- a/internal/restorable/rect.go +++ b/internal/restorable/rect.go @@ -24,7 +24,6 @@ import ( type pixelsRecord struct { rect image.Rectangle pix []byte - mask []byte } func (p *pixelsRecord) clearIfOverlapped(rect image.Rectangle) { @@ -43,40 +42,11 @@ func (p *pixelsRecord) clearIfOverlapped(rect image.Rectangle) { } } -func (p *pixelsRecord) merge(pix []byte, mask []byte) { - if len(p.pix) != len(pix) { - panic(fmt.Sprintf("restorable: len(p.pix) (%d) and len(pix) (%d) must match but not", len(p.pix), len(pix))) - } - - if mask == nil { - p.pix = pix - return - } - - if p.mask == nil { - p.mask = mask - } - if len(p.mask) != len(mask) { - panic(fmt.Sprintf("restorable: len(p.mask) (%d) and len(mask) (%d) must match but not", len(p.mask), len(mask))) - } - - for i := 0; i < len(p.pix)/4; i++ { - if mask[i/8]>>(i%8)&1 == 0 { - continue - } - p.mask[i/8] |= 1 << (i % 8) - copy(p.pix[4*i:4*(i+1)], pix[4*i:4*(i+1)]) - } -} - func (p *pixelsRecord) at(x, y int) (r, g, b, a byte, ok bool) { if !image.Pt(x, y).In(p.rect) { return 0, 0, 0, 0, false } idx := ((y-p.rect.Min.Y)*p.rect.Dx() + (x - p.rect.Min.X)) - if p.mask != nil && p.mask[idx/8]>>(idx%8)&1 == 0 { - return 0, 0, 0, 0, false - } return p.pix[4*idx], p.pix[4*idx+1], p.pix[4*idx+2], p.pix[4*idx+3], true } @@ -84,7 +54,7 @@ type pixelsRecords struct { records []*pixelsRecord } -func (pr *pixelsRecords) addOrReplace(pixels []byte, mask []byte, x, y, width, height int) { +func (pr *pixelsRecords) addOrReplace(pixels []byte, x, y, width, height int) { if len(pixels) != 4*width*height { msg := fmt.Sprintf("restorable: len(pixels) must be 4*%d*%d = %d but %d", width, height, 4*width*height, len(pixels)) if pixels == nil { @@ -94,29 +64,6 @@ func (pr *pixelsRecords) addOrReplace(pixels []byte, mask []byte, x, y, width, h } rect := image.Rect(x, y, x+width, y+height) - if mask != nil { - // If a mask is specified, try merging the records. - var merged bool - for idx := len(pr.records) - 1; idx >= 0; idx-- { - if r := pr.records[idx]; r.rect.Overlaps(rect) { - if r.rect == rect { - r.merge(pixels, mask) - merged = true - } - // If there is an overlap with other regions in the existing records, - // give up modifying them and just add a record. - break - } - } - if !merged { - pr.records = append(pr.records, &pixelsRecord{ - rect: rect, - pix: pixels, - mask: mask, - }) - } - return - } // Remove or update the duplicated records first. var n int @@ -171,6 +118,6 @@ func (pr *pixelsRecords) at(i, j int) (r, g, b, a byte, ok bool) { func (pr *pixelsRecords) apply(img *graphicscommand.Image) { // TODO: Isn't this too heavy? Can we merge the operations? for _, r := range pr.records { - img.ReplacePixels(r.pix, r.mask, r.rect.Min.X, r.rect.Min.Y, r.rect.Dx(), r.rect.Dy()) + img.ReplacePixels(r.pix, r.rect.Min.X, r.rect.Min.Y, r.rect.Dx(), r.rect.Dy()) } } diff --git a/internal/restorable/shader_test.go b/internal/restorable/shader_test.go index e045823eb..148e4eef5 100644 --- a/internal/restorable/shader_test.go +++ b/internal/restorable/shader_test.go @@ -90,7 +90,7 @@ func TestShaderChain(t *testing.T) { imgs = append(imgs, img) } - imgs[0].ReplacePixels([]byte{0xff, 0, 0, 0xff}, nil, 0, 0, 1, 1) + imgs[0].ReplacePixels([]byte{0xff, 0, 0, 0xff}, 0, 0, 1, 1) s := restorable.NewShader(etesting.ShaderProgramImages(1)) for i := 0; i < num-1; i++ { @@ -124,9 +124,9 @@ func TestShaderMultipleSources(t *testing.T) { for i := range srcs { srcs[i] = restorable.NewImage(1, 1, restorable.ImageTypeRegular) } - srcs[0].ReplacePixels([]byte{0x40, 0, 0, 0xff}, nil, 0, 0, 1, 1) - srcs[1].ReplacePixels([]byte{0, 0x80, 0, 0xff}, nil, 0, 0, 1, 1) - srcs[2].ReplacePixels([]byte{0, 0, 0xc0, 0xff}, nil, 0, 0, 1, 1) + srcs[0].ReplacePixels([]byte{0x40, 0, 0, 0xff}, 0, 0, 1, 1) + srcs[1].ReplacePixels([]byte{0, 0x80, 0, 0xff}, 0, 0, 1, 1) + srcs[2].ReplacePixels([]byte{0, 0, 0xc0, 0xff}, 0, 0, 1, 1) dst := restorable.NewImage(1, 1, restorable.ImageTypeRegular) @@ -163,7 +163,7 @@ func TestShaderMultipleSourcesOnOneTexture(t *testing.T) { 0x40, 0, 0, 0xff, 0, 0x80, 0, 0xff, 0, 0, 0xc0, 0xff, - }, nil, 0, 0, 3, 1) + }, 0, 0, 3, 1) srcs := [graphics.ShaderImageNum]*restorable.Image{src, src, src} dst := restorable.NewImage(1, 1, restorable.ImageTypeRegular)