From b59dd45239140336516bce8899d9198f431efe96 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sun, 20 Mar 2022 03:51:57 +0900 Subject: [PATCH] internal/buffered: separate ReplacePixels with the large-area and small-area versions For the large-area version, this doesn't require a graphics driver. This is necessary to ensure that ReplacePixels never needs a graphics driver. --- image.go | 4 ++-- image_test.go | 12 +++++++++++ internal/atlas/image.go | 40 +++++++++++++++++++++++------------ internal/atlas/image_test.go | 38 ++++++++++++++++----------------- internal/atlas/shader_test.go | 4 ++-- internal/buffered/image.go | 30 ++++++++++++++++++++++---- internal/mipmap/mipmap.go | 12 +++++++++-- internal/ui/image.go | 8 +++++-- 8 files changed, 103 insertions(+), 45 deletions(-) diff --git a/image.go b/image.go index 445ef64a5..93a1ec85d 100644 --- a/image.go +++ b/image.go @@ -764,7 +764,7 @@ func (i *Image) Set(x, y int, clr color.Color) { r, g, b, a := clr.RGBA() pix := []byte{byte(r >> 8), byte(g >> 8), byte(b >> 8), byte(a >> 8)} - if err := i.image.ReplacePixels(pix, x, y, 1, 1); err != nil { + if err := i.image.ReplaceSmallAreaPixels(pix, x, y, 1, 1); err != nil { ui.SetError(err) } } @@ -816,7 +816,7 @@ func (i *Image) ReplacePixels(pixels []byte) { // Do not need to copy pixels here. // * In internal/mipmap, pixels are copied when necessary. // * In internal/shareable, pixels are copied to make its paddings. - if err := i.image.ReplacePixels(pixels, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil { + if err := i.image.ReplaceLargeAreaPixels(pixels, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil { ui.SetError(err) } } diff --git a/image_test.go b/image_test.go index 5d56d6c12..fad58081a 100644 --- a/image_test.go +++ b/image_test.go @@ -1882,6 +1882,18 @@ func TestImageReplacePixelsOnSubImage(t *testing.T) { r1 := image.Rect(11, 10, 16, 13) dst.SubImage(r1).(*ebiten.Image).ReplacePixels(pix1) + // Clear the pixels. This should not affect the result. + idx = 0 + for j := 0; j < 3; j++ { + for i := 0; i < 5; i++ { + pix1[4*idx] = 0 + pix1[4*idx+1] = 0 + pix1[4*idx+2] = 0 + pix1[4*idx+3] = 0 + idx++ + } + } + for j := 0; j < 31; j++ { for i := 0; i < 17; i++ { got := dst.At(i, j).(color.RGBA) diff --git a/internal/atlas/image.go b/internal/atlas/image.go index 0028d4ce2..eac31b314 100644 --- a/internal/atlas/image.go +++ b/internal/atlas/image.go @@ -337,7 +337,7 @@ func (i *Image) putOnAtlas(graphicsDriver graphicsdriver.Graphics) error { pixels[4*(i.width*y+x)+3] = a } } - newI.replacePixels(pixels) + newI.replacePixels(pixels, 0, 0, i.width, i.height) } else { // If the underlying graphics driver doesn't require restoring from the context lost, just a regular // rendering works. @@ -538,13 +538,13 @@ func (i *Image) drawTriangles(srcs [graphics.ShaderImageNum]*Image, vertices []f } } -func (i *Image) ReplacePixels(pix []byte) { +func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) { backendsM.Lock() defer backendsM.Unlock() - i.replacePixels(pix) + i.replacePixels(pix, x, y, width, height) } -func (i *Image) replacePixels(pix []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") } @@ -558,25 +558,37 @@ func (i *Image) replacePixels(pix []byte) { i.allocate(true) } - x, y, w, h := i.regionWithPadding() - if pix == nil { - i.backend.restorable.ReplacePixels(nil, x, y, w, h) + // If the replacing area is small, replace the pixels without the padding. + if x != 0 || y != 0 || width != i.width || height != i.height { + ox, oy, _, _ := i.regionWithPadding() + x += ox + paddingSize + y += oy + paddingSize + copied := make([]byte, len(pix)) + copy(copied, pix) + i.backend.restorable.ReplacePixels(copied, x, y, width, height) return } - ow, oh := w-2*paddingSize, h-2*paddingSize + // If the whole area is being replaced, add the padding. + px, py, pw, ph := i.regionWithPadding() + if pix == nil { + i.backend.restorable.ReplacePixels(nil, px, py, pw, ph) + return + } + + ow, oh := pw-2*paddingSize, ph-2*paddingSize if l := 4 * ow * oh; len(pix) != l { panic(fmt.Sprintf("atlas: len(p) must be %d but %d", l, len(pix))) } - pixb := theTemporaryPixels.alloc(4 * w * h) + pixb := theTemporaryPixels.alloc(4 * pw * ph) // Clear the edges. pixb might not be zero-cleared. - rowPixels := 4 * w + rowPixels := 4 * pw for i := 0; i < rowPixels; i++ { pixb[i] = 0 } - for j := 1; j < h-1; j++ { + for j := 1; j < ph-1; j++ { pixb[rowPixels*j] = 0 pixb[rowPixels*j+1] = 0 pixb[rowPixels*j+2] = 0 @@ -587,15 +599,15 @@ func (i *Image) replacePixels(pix []byte) { pixb[rowPixels*(j+1)-1] = 0 } for i := 0; i < rowPixels; i++ { - pixb[rowPixels*(h-1)+i] = 0 + pixb[rowPixels*(ph-1)+i] = 0 } // Copy the content. for j := 0; j < oh; j++ { - copy(pixb[4*((j+paddingSize)*w+paddingSize):], pix[4*j*ow:4*(j+1)*ow]) + copy(pixb[4*((j+paddingSize)*pw+paddingSize):], pix[4*j*ow:4*(j+1)*ow]) } - i.backend.restorable.ReplacePixels(pixb, x, y, w, h) + i.backend.restorable.ReplacePixels(pixb, px, py, pw, ph) } func (img *Image) Pixels(graphicsDriver graphicsdriver.Graphics, x, y, width, height int) ([]byte, error) { diff --git a/internal/atlas/image_test.go b/internal/atlas/image_test.go index 440d19d37..f6cc9a6d3 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) defer img1.MarkDisposed() // Ensure img1's region is allocated. - img1.ReplacePixels(make([]byte, 4*bigSize*100)) + img1.ReplacePixels(make([]byte, 4*bigSize*100), 0, 0, bigSize, 100) img2 := atlas.NewImage(100, bigSize) defer img2.MarkDisposed() - img2.ReplacePixels(make([]byte, 4*100*bigSize)) + img2.ReplacePixels(make([]byte, 4*100*bigSize), 0, 0, 100, bigSize) const size = 32 img3 := atlas.NewImage(size/2, size/2) defer img3.MarkDisposed() - img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4)) + img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4), 0, 0, size/2, size/2) img4 := atlas.NewImage(size, size) defer img4.MarkDisposed() @@ -87,7 +87,7 @@ func TestEnsureIsolated(t *testing.T) { pix[4*(i+j*size)+3] = byte(i + j) } } - img4.ReplacePixels(pix) + img4.ReplacePixels(pix, 0, 0, size, size) const ( dx0 = size / 4 @@ -142,11 +142,11 @@ func TestReputOnAtlas(t *testing.T) { img0 := atlas.NewImage(size, size) defer img0.MarkDisposed() - img0.ReplacePixels(make([]byte, 4*size*size)) + img0.ReplacePixels(make([]byte, 4*size*size), 0, 0, size, size) img1 := atlas.NewImage(size, size) defer img1.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*size*size)) + 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) } @@ -162,12 +162,12 @@ func TestReputOnAtlas(t *testing.T) { pix[4*(i+j*size)+3] = byte(i + j) } } - img2.ReplacePixels(pix) + img2.ReplacePixels(pix, 0, 0, size, size) img3 := atlas.NewImage(size, size) img3.SetVolatile(true) defer img3.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*size*size)) + 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) } @@ -256,7 +256,7 @@ func TestReputOnAtlas(t *testing.T) { if err := atlas.PutImagesOnAtlasForTesting(ui.GraphicsDriverForTesting()); err != nil { t.Fatal(err) } - img1.ReplacePixels(make([]byte, 4*size*size)) + 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) @@ -298,7 +298,7 @@ func TestExtend(t *testing.T) { p0[4*i+2] = byte(i) p0[4*i+3] = byte(i) } - img0.ReplacePixels(p0) + img0.ReplacePixels(p0, 0, 0, w0, h0) const w1, h1 = minImageSizeForTesting + 1, 100 img1 := atlas.NewImage(w1, h1) @@ -312,7 +312,7 @@ func TestExtend(t *testing.T) { p1[4*i+3] = byte(i) } // Ensure to allocate - img1.ReplacePixels(p1) + img1.ReplacePixels(p1, 0, 0, w1, h1) pix0, err := img0.Pixels(ui.GraphicsDriverForTesting(), 0, 0, w0, h0) if err != nil { @@ -367,7 +367,7 @@ func TestReplacePixelsAfterDrawTriangles(t *testing.T) { pix[4*i+2] = byte(i) pix[4*i+3] = byte(i) } - src.ReplacePixels(pix) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(w, h, 0, 0, 1) is := graphics.QuadIndices() @@ -378,7 +378,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) + dst.ReplacePixels(pix, 0, 0, w, h) pix, err := dst.Pixels(ui.GraphicsDriverForTesting(), 0, 0, w, h) if err != nil { @@ -415,7 +415,7 @@ func TestSmallImages(t *testing.T) { pix[4*i+2] = 0xff pix[4*i+3] = 0xff } - src.ReplacePixels(pix) + src.ReplacePixels(pix, 0, 0, w, h) vs := quadVertices(w, h, 0, 0, 1) is := graphics.QuadIndices() @@ -462,7 +462,7 @@ func TestLongImages(t *testing.T) { pix[4*i+2] = 0xff pix[4*i+3] = 0xff } - src.ReplacePixels(pix) + src.ReplacePixels(pix, 0, 0, w, h) const scale = 120 vs := quadVertices(w, h, 0, 0, scale) @@ -512,12 +512,12 @@ func TestExtendWithBigImage(t *testing.T) { img0 := atlas.NewImage(1, 1) defer img0.MarkDisposed() - img0.ReplacePixels(make([]byte, 4*1*1)) + img0.ReplacePixels(make([]byte, 4*1*1), 0, 0, 1, 1) img1 := atlas.NewImage(minImageSizeForTesting+1, minImageSizeForTesting+1) defer img1.MarkDisposed() - img1.ReplacePixels(make([]byte, 4*(minImageSizeForTesting+1)*(minImageSizeForTesting+1))) + img1.ReplacePixels(make([]byte, 4*(minImageSizeForTesting+1)*(minImageSizeForTesting+1)), 0, 0, minImageSizeForTesting+1, minImageSizeForTesting+1) } // Issue #1217 @@ -526,7 +526,7 @@ func TestMaxImageSize(t *testing.T) { s := maxImageSizeForTesting - 2*atlas.PaddingSize img := atlas.NewImage(s, s) defer img.MarkDisposed() - img.ReplacePixels(make([]byte, 4*s*s)) + img.ReplacePixels(make([]byte, 4*s*s), 0, 0, s, s) } // Issue #1217 (disabled) @@ -539,7 +539,7 @@ func Disable_TestMinImageSize(t *testing.T) { s := minImageSizeForTesting img := atlas.NewImage(s, s) defer img.MarkDisposed() - img.ReplacePixels(make([]byte, 4*s*s)) + img.ReplacePixels(make([]byte, 4*s*s), 0, 0, s, s) } // Issue #1421 diff --git a/internal/atlas/shader_test.go b/internal/atlas/shader_test.go index 5272e0285..dc3f5e406 100644 --- a/internal/atlas/shader_test.go +++ b/internal/atlas/shader_test.go @@ -65,9 +65,9 @@ func TestImageDrawTwice(t *testing.T) { dst := atlas.NewImage(w, h) src0 := atlas.NewImage(w, h) - src0.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff}) + src0.ReplacePixels([]byte{0xff, 0xff, 0xff, 0xff}, 0, 0, w, h) src1 := atlas.NewImage(w, h) - src1.ReplacePixels([]byte{0x80, 0x80, 0x80, 0xff}) + 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 19cce5b3a..dcea25c18 100644 --- a/internal/buffered/image.go +++ b/internal/buffered/image.go @@ -117,7 +117,7 @@ func (i *Image) invalidatePendingPixels() { func (i *Image) resolvePendingPixels(keepPendingPixels bool) { if i.needsToResolvePixels { - i.img.ReplacePixels(i.pixels) + i.img.ReplacePixels(i.pixels, 0, 0, i.width, i.height) if !keepPendingPixels { i.pixels = nil } @@ -166,7 +166,7 @@ func (i *Image) DumpScreenshot(graphicsDriver graphicsdriver.Graphics, name stri return i.img.DumpScreenshot(graphicsDriver, name, blackbg) } -func (i *Image) ReplacePixels(graphicsDriver graphicsdriver.Graphics, pix []byte, x, y, width, height int) error { +func (i *Image) ReplaceLargeAreaPixels(pix []byte, x, y, width, height int) error { if l := 4 * width * height; len(pix) != l { panic(fmt.Sprintf("buffered: len(pix) was %d but must be %d", len(pix), l)) } @@ -175,7 +175,29 @@ func (i *Image) ReplacePixels(graphicsDriver graphicsdriver.Graphics, pix []byte copied := make([]byte, len(pix)) copy(copied, pix) if tryAddDelayedCommand(func() error { - i.ReplacePixels(graphicsDriver, copied, x, y, width, height) + i.ReplaceLargeAreaPixels(copied, x, y, width, height) + return nil + }) { + return nil + } + } + + i.invalidatePendingPixels() + + i.img.ReplacePixels(pix, x, y, width, height) + return nil +} + +func (i *Image) ReplaceSmallAreaPixels(graphicsDriver graphicsdriver.Graphics, pix []byte, x, y, width, height int) error { + if l := 4 * width * height; len(pix) != l { + panic(fmt.Sprintf("buffered: len(pix) was %d but must be %d", len(pix), l)) + } + + if maybeCanAddDelayedCommand() { + copied := make([]byte, len(pix)) + copy(copied, pix) + if tryAddDelayedCommand(func() error { + i.ReplaceSmallAreaPixels(graphicsDriver, copied, x, y, width, height) return nil }) { return nil @@ -188,7 +210,7 @@ func (i *Image) ReplacePixels(graphicsDriver graphicsdriver.Graphics, pix []byte // Call ReplacePixels immediately. Do not buffer the command. // If a lot of new images are created but they are used at different timings, // pixels are sent to GPU at different timings, which is very inefficient. - i.img.ReplacePixels(pix) + i.img.ReplacePixels(pix, x, y, width, height) return nil } diff --git a/internal/mipmap/mipmap.go b/internal/mipmap/mipmap.go index d86bdbe62..899bab926 100644 --- a/internal/mipmap/mipmap.go +++ b/internal/mipmap/mipmap.go @@ -71,8 +71,16 @@ func (m *Mipmap) DumpScreenshot(graphicsDriver graphicsdriver.Graphics, name str return m.orig.DumpScreenshot(graphicsDriver, name, blackbg) } -func (m *Mipmap) ReplacePixels(graphicsDriver graphicsdriver.Graphics, pix []byte, x, y, width, height int) error { - if err := m.orig.ReplacePixels(graphicsDriver, pix, x, y, width, height); err != nil { +func (m *Mipmap) ReplaceLargeAreaPixels(pix []byte, x, y, width, height int) error { + if err := m.orig.ReplaceLargeAreaPixels(pix, x, y, width, height); err != nil { + return err + } + m.disposeMipmaps() + return nil +} + +func (m *Mipmap) ReplaceSmallAreaPixels(graphicsDriver graphicsdriver.Graphics, pix []byte, x, y, width, height int) error { + if err := m.orig.ReplaceSmallAreaPixels(graphicsDriver, pix, x, y, width, height); err != nil { return err } m.disposeMipmaps() diff --git a/internal/ui/image.go b/internal/ui/image.go index f808dd442..40f059603 100644 --- a/internal/ui/image.go +++ b/internal/ui/image.go @@ -60,8 +60,12 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, vertices []f i.mipmap.DrawTriangles(srcMipmaps, vertices, indices, colorm, mode, filter, address, dstRegion, srcRegion, subimageOffsets, s, uniforms, evenOdd, canSkipMipmap) } -func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error { - return i.mipmap.ReplacePixels(graphicsDriver(), pix, x, y, width, height) +func (i *Image) ReplaceLargeAreaPixels(pix []byte, x, y, width, height int) error { + return i.mipmap.ReplaceLargeAreaPixels(pix, x, y, width, height) +} + +func (i *Image) ReplaceSmallAreaPixels(pix []byte, x, y, width, height int) error { + return i.mipmap.ReplaceSmallAreaPixels(graphicsDriver(), pix, x, y, width, height) } func (i *Image) Pixels(x, y, width, height int) ([]byte, error) {