From 1c2c932c6c121eeed219bfe5d538cf45747772c7 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 21 Sep 2019 14:53:52 +0900 Subject: [PATCH] graphics: Remove MakeVolatile/IsVolatile --- image.go | 36 ++++-------------- internal/restorable/image.go | 24 ++++++------ internal/restorable/images_test.go | 59 +++++++++++++++--------------- internal/shareable/image.go | 49 ++++++++----------------- internal/shareable/image_test.go | 37 +++++++++---------- mipmap.go | 31 +++++++--------- uicontext.go | 4 +- 7 files changed, 97 insertions(+), 143 deletions(-) diff --git a/image.go b/image.go index 9071ff712..b6031a10b 100644 --- a/image.go +++ b/image.go @@ -663,38 +663,16 @@ type DrawImageOptions struct { // // Error returned by NewImage is always nil as of 1.5.0-alpha. func NewImage(width, height int, filter Filter) (*Image, error) { + return newImage(width, height, filter, false), nil +} + +func newImage(width, height int, filter Filter, volatile bool) *Image { i := &Image{ - mipmap: newMipmap(width, height), + mipmap: newMipmap(width, height, volatile), filter: filter, } i.addr = i - return i, nil -} - -// makeVolatile makes the image 'volatile'. -// A volatile image is always cleared at the start of a frame. -// -// This is suitable for offscreen images that pixels are changed often. -// -// Regular non-volatile images need to record drawing history or read its pixels from GPU if necessary so that all -// the images can be restored automatically from the context lost. However, such recording the drawing history or -// reading pixels from GPU are expensive operations. Volatile images can skip such oprations, but the image content -// is cleared every frame instead. -// -// When the image is disposed, makeVolatile does nothing. -func (i *Image) makeVolatile() { - if enqueueImageOpIfNeeded(func() func() { - return func() { - i.makeVolatile() - } - }) { - return - } - - if i.isDisposed() { - return - } - i.mipmap.makeVolatile() + return i } // NewImageFromImage creates a new image with the given image (source). @@ -711,7 +689,7 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) { width, height := size.X, size.Y i := &Image{ - mipmap: newMipmap(width, height), + mipmap: newMipmap(width, height, false), filter: filter, } i.addr = i diff --git a/internal/restorable/image.go b/internal/restorable/image.go index 28d365d27..b64a98985 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -129,14 +129,20 @@ func init() { // NewImage creates an empty image with the given size. // +// volatile indicates whether the image is volatile. Regular non-volatile images need to record drawing history or +// read its pixels from GPU if necessary so that all the images can be restored automatically from the context lost. +// However, such recording the drawing history or reading pixels from GPU are expensive operations. Volatile images +// can skip such oprations, but the image content is cleared every frame instead. +// // The returned image is cleared. // // Note that Dispose is not called automatically. -func NewImage(width, height int) *Image { +func NewImage(width, height int, volatile bool) *Image { i := &Image{ - image: graphicscommand.NewImage(width, height), - width: width, - height: height, + image: graphicscommand.NewImage(width, height), + width: width, + height: height, + volatile: volatile, } i.clear() theImages.add(i) @@ -165,7 +171,7 @@ func (i *Image) Extend(width, height int) *Image { panic("restorable: Extend after DrawTriangles is forbidden") } - newImg := NewImage(width, height) + newImg := NewImage(width, height, i.volatile) i.basePixels.Apply(newImg.image) if i.basePixels.baseColor != (color.RGBA{}) { @@ -178,10 +184,6 @@ func (i *Image) Extend(width, height int) *Image { return newImg } -func (i *Image) MakeVolatile() { - i.volatile = true -} - // NewScreenFramebufferImage creates a special image that framebuffer is one for the screen. // // The returned image is cleared. @@ -288,10 +290,6 @@ func fillImage(i *graphicscommand.Image, clr color.RGBA) { i.DrawTriangles(emptyImage.image, vs, is, nil, compositemode, driver.FilterNearest, driver.AddressClampToZero) } -func (i *Image) IsVolatile() bool { - return i.volatile -} - // BasePixelsForTesting returns the image's basePixels for testing. func (i *Image) BasePixelsForTesting() *Pixels { return &i.basePixels diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index 1fe4d3cc4..556aede63 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -70,7 +70,7 @@ func sameColors(c1, c2 color.RGBA, delta int) bool { } func TestRestore(t *testing.T) { - img0 := NewImage(1, 1) + img0 := NewImage(1, 1, false) defer img0.Dispose() clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff} @@ -87,7 +87,7 @@ func TestRestore(t *testing.T) { } func TestRestoreWithoutDraw(t *testing.T) { - img0 := NewImage(1024, 1024) + img0 := NewImage(1024, 1024, false) defer img0.Dispose() // If there is no drawing command on img0, img0 is cleared when restored. @@ -129,7 +129,7 @@ func TestRestoreChain(t *testing.T) { const num = 10 imgs := []*Image{} for i := 0; i < num; i++ { - img := NewImage(1, 1) + img := NewImage(1, 1, false) imgs = append(imgs, img) } defer func() { @@ -165,7 +165,7 @@ func TestRestoreChain2(t *testing.T) { ) imgs := []*Image{} for i := 0; i < num; i++ { - img := NewImage(w, h) + img := NewImage(w, h, false) imgs = append(imgs, img) } defer func() { @@ -209,10 +209,10 @@ func TestRestoreOverrideSource(t *testing.T) { w = 1 h = 1 ) - img0 := NewImage(w, h) - img1 := NewImage(w, h) - img2 := NewImage(w, h) - img3 := NewImage(w, h) + img0 := NewImage(w, h, false) + img1 := NewImage(w, h, false) + img2 := NewImage(w, h, false) + img3 := NewImage(w, h, false) defer func() { img3.Dispose() img2.Dispose() @@ -286,11 +286,11 @@ func TestRestoreComplexGraph(t *testing.T) { img0 := newImageFromImage(base) img1 := newImageFromImage(base) img2 := newImageFromImage(base) - img3 := NewImage(w, h) - img4 := NewImage(w, h) - img5 := NewImage(w, h) - img6 := NewImage(w, h) - img7 := NewImage(w, h) + img3 := NewImage(w, h, false) + img4 := NewImage(w, h, false) + img5 := NewImage(w, h, false) + img6 := NewImage(w, h, false) + img7 := NewImage(w, h, false) defer func() { img7.Dispose() img6.Dispose() @@ -386,7 +386,7 @@ func TestRestoreComplexGraph(t *testing.T) { func newImageFromImage(rgba *image.RGBA) *Image { s := rgba.Bounds().Size() - img := NewImage(s.X, s.Y) + img := NewImage(s.X, s.Y, false) img.ReplacePixels(rgba.Pix, 0, 0, s.X, s.Y) return img } @@ -403,7 +403,7 @@ func TestRestoreRecursive(t *testing.T) { base.Pix[3] = 0xff img0 := newImageFromImage(base) - img1 := NewImage(w, h) + img1 := NewImage(w, h, false) defer func() { img1.Dispose() img0.Dispose() @@ -446,7 +446,7 @@ func TestRestoreRecursive(t *testing.T) { } func TestReplacePixels(t *testing.T) { - img := NewImage(17, 31) + img := NewImage(17, 31, false) defer img.Dispose() pix := make([]byte, 4*4*4) @@ -489,7 +489,7 @@ func TestDrawTrianglesAndReplacePixels(t *testing.T) { base.Pix[3] = 0xff img0 := newImageFromImage(base) defer img0.Dispose() - img1 := NewImage(2, 1) + img1 := NewImage(2, 1, false) defer img1.Dispose() vs := quadVertices(1, 1, 0, 0) @@ -548,7 +548,7 @@ func TestReplacePixelsPart(t *testing.T) { pix[i] = 0xff } - img := NewImage(4, 4) + img := NewImage(4, 4, false) // This doesn't make the image stale. Its base pixels are available. img.ReplacePixels(pix, 1, 1, 2, 2) @@ -619,9 +619,9 @@ func TestReplacePixelsPart(t *testing.T) { func TestReplacePixelsOnly(t *testing.T) { const w, h = 128, 128 - img0 := NewImage(w, h) + img0 := NewImage(w, h, false) defer img0.Dispose() - img1 := NewImage(1, 1) + img1 := NewImage(1, 1, false) defer img1.Dispose() for i := 0; i < w*h; i += 5 { @@ -667,9 +667,8 @@ func TestReplacePixelsOnly(t *testing.T) { // Issue #793 func TestReadPixelsFromVolatileImage(t *testing.T) { const w, h = 16, 16 - dst := NewImage(w, h) - dst.MakeVolatile() - src := NewImage(w, h) + dst := NewImage(w, h, true /* volatile */) + src := NewImage(w, h, false) // First, make sure that dst has pixels dst.ReplacePixels(make([]byte, 4*w*h), 0, 0, w, h) @@ -695,8 +694,8 @@ func TestReadPixelsFromVolatileImage(t *testing.T) { func TestAllowReplacePixelsAfterDrawTriangles(t *testing.T) { const w, h = 16, 16 - src := NewImage(w, h) - dst := NewImage(w, h) + src := NewImage(w, h, false) + dst := NewImage(w, h, false) vs := quadVertices(w, h, 0, 0) is := graphics.QuadIndices() @@ -713,8 +712,8 @@ func TestDisallowReplacePixelsForPartAfterDrawTriangles(t *testing.T) { }() const w, h = 16, 16 - src := NewImage(w, h) - dst := NewImage(w, h) + src := NewImage(w, h, false) + dst := NewImage(w, h, false) vs := quadVertices(w, h, 0, 0) is := graphics.QuadIndices() @@ -728,7 +727,7 @@ func TestExtend(t *testing.T) { } const w, h = 16, 16 - orig := NewImage(w, h) + orig := NewImage(w, h, false) pix := make([]byte, 4*w*h) for j := 0; j < h; j++ { for i := 0; i < w; i++ { @@ -760,7 +759,7 @@ func TestExtend(t *testing.T) { func TestClearPixels(t *testing.T) { const w, h = 16, 16 - img := NewImage(w, h) + img := NewImage(w, h, false) 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) @@ -772,7 +771,7 @@ func TestClearPixels(t *testing.T) { func TestFill(t *testing.T) { const w, h = 16, 16 - img := NewImage(w, h) + img := NewImage(w, h, false) img.Fill(color.RGBA{0xff, 0, 0, 0xff}) ResolveStaleImages() if err := RestoreIfNeeded(); err != nil { diff --git a/internal/shareable/image.go b/internal/shareable/image.go index 419719c04..d957ea8a4 100644 --- a/internal/shareable/image.go +++ b/internal/shareable/image.go @@ -150,6 +150,7 @@ type Image struct { width int height int disposed bool + volatile bool screen bool backend *backend @@ -165,8 +166,6 @@ type Image struct { // // ReplacePixels doesn't affect this value since ReplacePixels can be done on shared images. nonUpdatedCount int - - neverShared bool } func (i *Image) moveTo(dst *Image) { @@ -201,7 +200,7 @@ func (i *Image) ensureNotShared() { sy0 := float32(oy) sx1 := float32(ox + w) sy1 := float32(oy + h) - newImg := restorable.NewImage(w, h) + newImg := restorable.NewImage(w, h, i.volatile) vs := []float32{ dx0, dy0, sx0, sy0, sx0, sy0, sx1, sy1, 1, 1, 1, 1, dx1, dy0, sx1, sy0, sx0, sy0, sx1, sy1, 1, 1, 1, 1, @@ -231,7 +230,7 @@ func (i *Image) makeShared() { panic("shareable: makeShared cannot be called on a non-shareable image") } - newI := NewImage(i.width, i.height) + newI := NewImage(i.width, i.height, i.volatile) pixels := make([]byte, 4*i.width*i.height) for y := 0; y < i.height; y++ { for x := 0; x < i.width; x++ { @@ -462,21 +461,12 @@ func (i *Image) dispose(markDisposed bool) { theBackends = append(theBackends[:index], theBackends[index+1:]...) } -func (i *Image) IsVolatile() bool { - backendsM.Lock() - defer backendsM.Unlock() - if i.backend == nil { - // Not allocated yet. Only non-volatile images can do lazy allocation so far. - return false - } - return i.backend.restorable.IsVolatile() -} - -func NewImage(width, height int) *Image { +func NewImage(width, height int, volatile bool) *Image { // Actual allocation is done lazily, and the lock is not needed. return &Image{ - width: width, - height: height, + width: width, + height: height, + volatile: volatile, } } @@ -484,7 +474,10 @@ func (i *Image) shareable() bool { if minSize == 0 || maxSize == 0 { panic("shareable: minSize or maxSize must be initialized") } - if i.neverShared { + if i.volatile { + return false + } + if i.screen { return false } return i.width <= maxSize && i.height <= maxSize @@ -505,7 +498,7 @@ func (i *Image) allocate(shareable bool) { if !shareable || !i.shareable() { i.backend = &backend{ - restorable: restorable.NewImage(i.width, i.height), + restorable: restorable.NewImage(i.width, i.height, i.volatile), } runtime.SetFinalizer(i, (*Image).disposeFromFinalizer) return @@ -528,7 +521,7 @@ func (i *Image) allocate(shareable bool) { } b := &backend{ - restorable: restorable.NewImage(size, size), + restorable: restorable.NewImage(size, size, i.volatile), page: packing.NewPage(size, maxSize), } theBackends = append(theBackends, b) @@ -542,15 +535,6 @@ func (i *Image) allocate(shareable bool) { runtime.SetFinalizer(i, (*Image).disposeFromFinalizer) } -func (i *Image) MakeVolatile() { - backendsM.Lock() - defer backendsM.Unlock() - - i.ensureNotShared() - i.backend.restorable.MakeVolatile() - i.neverShared = true -} - func (i *Image) Dump(path string) error { backendsM.Lock() defer backendsM.Unlock() @@ -561,10 +545,9 @@ func (i *Image) Dump(path string) error { func NewScreenFramebufferImage(width, height int) *Image { // Actual allocation is done lazily. i := &Image{ - width: width, - height: height, - screen: true, - neverShared: true, + width: width, + height: height, + screen: true, } return i } diff --git a/internal/shareable/image_test.go b/internal/shareable/image_test.go index 0d9559edf..a2f90519c 100644 --- a/internal/shareable/image_test.go +++ b/internal/shareable/image_test.go @@ -66,22 +66,22 @@ const bigSize = 2049 func TestEnsureNotShared(t *testing.T) { // Create img1 and img2 with this size so that the next images are allocated // with non-upper-left location. - img1 := NewImage(bigSize, 100) + img1 := NewImage(bigSize, 100, false) defer img1.Dispose() // Ensure img1's region is allocated. img1.ReplacePixels(make([]byte, 4*bigSize*100)) - img2 := NewImage(100, bigSize) + img2 := NewImage(100, bigSize, false) defer img2.Dispose() img2.ReplacePixels(make([]byte, 4*100*bigSize)) const size = 32 - img3 := NewImage(size/2, size/2) + img3 := NewImage(size/2, size/2, false) defer img3.Dispose() img3.ReplacePixels(make([]byte, (size/2)*(size/2)*4)) - img4 := NewImage(size, size) + img4 := NewImage(size, size, false) defer img4.Dispose() pix := make([]byte, size*size*4) @@ -133,18 +133,18 @@ func TestEnsureNotShared(t *testing.T) { func TestReshared(t *testing.T) { const size = 16 - img0 := NewImage(size, size) + img0 := NewImage(size, size, false) defer img0.Dispose() img0.ReplacePixels(make([]byte, 4*size*size)) - img1 := NewImage(size, size) + img1 := NewImage(size, size, false) defer img1.Dispose() img1.ReplacePixels(make([]byte, 4*size*size)) if got, want := img1.IsSharedForTesting(), true; got != want { t.Errorf("got: %v, want: %v", got, want) } - img2 := NewImage(size, size) + img2 := NewImage(size, size, false) defer img2.Dispose() pix := make([]byte, 4*size*size) for j := 0; j < size; j++ { @@ -157,8 +157,7 @@ func TestReshared(t *testing.T) { } img2.ReplacePixels(pix) - img3 := NewImage(size, size) - img3.MakeVolatile() + img3 := NewImage(size, size, true /* volatile */) defer img3.Dispose() img1.ReplacePixels(make([]byte, 4*size*size)) if got, want := img3.IsSharedForTesting(), false; got != want { @@ -224,7 +223,7 @@ func TestReshared(t *testing.T) { func TestExtend(t *testing.T) { const w0, h0 = 100, 100 - img0 := NewImage(w0, h0) + img0 := NewImage(w0, h0, false) defer img0.Dispose() p0 := make([]byte, 4*w0*h0) for i := 0; i < w0*h0; i++ { @@ -236,7 +235,7 @@ func TestExtend(t *testing.T) { img0.ReplacePixels(p0) const w1, h1 = 1025, 100 - img1 := NewImage(w1, h1) + img1 := NewImage(w1, h1, false) defer img1.Dispose() p1 := make([]byte, 4*w1*h1) for i := 0; i < w1*h1; i++ { @@ -278,9 +277,9 @@ func TestExtend(t *testing.T) { func TestReplacePixelsAfterDrawTriangles(t *testing.T) { const w, h = 256, 256 - src := NewImage(w, h) + src := NewImage(w, h, false) defer src.Dispose() - dst := NewImage(w, h) + dst := NewImage(w, h, false) defer dst.Dispose() pix := make([]byte, 4*w*h) @@ -313,9 +312,9 @@ func TestReplacePixelsAfterDrawTriangles(t *testing.T) { // Issue #887 func TestSmallImages(t *testing.T) { const w, h = 4, 8 - src := NewImage(w, h) + src := NewImage(w, h, false) defer src.Dispose() - dst := NewImage(w, h) + dst := NewImage(w, h, false) defer dst.Dispose() pix := make([]byte, 4*w*h) @@ -347,9 +346,9 @@ func TestSmallImages(t *testing.T) { // Issue #887 func TestLongImages(t *testing.T) { const w, h = 1, 6 - src := NewImage(w, h) + src := NewImage(w, h, false) defer src.Dispose() - dst := NewImage(256, 256) + dst := NewImage(256, 256, false) defer dst.Dispose() pix := make([]byte, 4*w*h) @@ -382,10 +381,10 @@ func TestLongImages(t *testing.T) { func TestDisposeImmediately(t *testing.T) { // This tests restorable.Image.ClearPixels is called but ReplacePixels is not called. - img0 := NewImage(16, 16) + img0 := NewImage(16, 16, false) img0.EnsureNotSharedForTesting() - img1 := NewImage(16, 16) + img1 := NewImage(16, 16, false) img1.EnsureNotSharedForTesting() // img0 and img1 should share the same backend in 99.9999% possibility. diff --git a/mipmap.go b/mipmap.go index 6e39938c2..c3812b4dd 100644 --- a/mipmap.go +++ b/mipmap.go @@ -29,18 +29,20 @@ import ( type levelToImage map[int]*shareable.Image type mipmap struct { - width int - height int - orig *shareable.Image - imgs map[image.Rectangle]levelToImage + width int + height int + volatile bool + orig *shareable.Image + imgs map[image.Rectangle]levelToImage } -func newMipmap(width, height int) *mipmap { +func newMipmap(width, height int, volatile bool) *mipmap { return &mipmap{ - width: width, - height: height, - orig: shareable.NewImage(width, height), - imgs: map[image.Rectangle]levelToImage{}, + width: width, + height: height, + volatile: volatile, + orig: shareable.NewImage(width, height, volatile), + imgs: map[image.Rectangle]levelToImage{}, } } @@ -53,11 +55,6 @@ func newScreenFramebufferMipmap(width, height int) *mipmap { } } -func (m *mipmap) makeVolatile() { - m.orig.MakeVolatile() - m.disposeMipmaps() -} - func (m *mipmap) dump(name string) error { return m.orig.Dump(name) } @@ -176,7 +173,7 @@ func (m *mipmap) level(r image.Rectangle, level int) *shareable.Image { panic("ebiten: level must be non-zero at level") } - if m.orig.IsVolatile() { + if m.volatile { panic("ebiten: mipmap images for a volatile image is not implemented yet") } @@ -229,7 +226,7 @@ func (m *mipmap) level(r image.Rectangle, level int) *shareable.Image { imgs[level] = nil return nil } - s := shareable.NewImage(w2, h2) + s := shareable.NewImage(w2, h2, m.volatile) s.DrawTriangles(src, vs, is, nil, driver.CompositeModeCopy, filter, driver.AddressClampToZero) imgs[level] = s @@ -325,7 +322,7 @@ func (m *mipmap) mipmapLevel(geom *GeoM, width, height int, filter driver.Filter if filter != driver.FilterLinear { return 0 } - if m.orig.IsVolatile() { + if m.volatile { return 0 } diff --git a/uicontext.go b/uicontext.go index 063ad874b..1cdad28aa 100644 --- a/uicontext.go +++ b/uicontext.go @@ -56,8 +56,8 @@ func (c *uiContext) SetSize(screenWidth, screenHeight int, screenScale float64) if c.offscreen != nil { _ = c.offscreen.Dispose() } - c.offscreen, _ = NewImage(screenWidth, screenHeight, FilterDefault) - c.offscreen.makeVolatile() + + c.offscreen = newImage(screenWidth, screenHeight, FilterDefault, true) // Round up the screensize not to cause glitches e.g. on Xperia (#622) w := int(math.Ceil(float64(screenWidth) * screenScale))