shareable: Rename notUpdatedCount -> usedAsSourceCount and modify implementation

Before this change, the image automatically becomes on a shareable
texture again when the image is not modified for a while. This was
problematic when an offscreen is not updated for a while, but used
as a render target again, as an independent image for the image is
created every time when the image is used as a render target.

After this change, the count is updated only when the image is used
as a rendering source.

Closes #1465
This commit is contained in:
Hajime Hoshi 2021-01-17 18:22:45 +09:00
parent 2432530b51
commit 0c578f1670
2 changed files with 74 additions and 13 deletions

View File

@ -85,8 +85,8 @@ const CountForStartSyncing = MaxCountForShare / 2
func makeImagesShared() error { func makeImagesShared() error {
for i := range imagesToMakeShared { for i := range imagesToMakeShared {
i.nonUpdatedCount++ i.usedAsSourceCount++
if i.nonUpdatedCount >= CountForStartSyncing && i.syncing == nil { if i.usedAsSourceCount >= CountForStartSyncing && i.syncing == nil {
// Sync the pixel data on CPU and GPU sides explicitly in order not to block this process. // Sync the pixel data on CPU and GPU sides explicitly in order not to block this process.
ch, err := i.backend.restorable.Sync() ch, err := i.backend.restorable.Sync()
if err != nil { if err != nil {
@ -94,7 +94,7 @@ func makeImagesShared() error {
} }
i.syncing = ch i.syncing = ch
} }
if i.nonUpdatedCount >= MaxCountForShare { if i.usedAsSourceCount >= MaxCountForShare {
// TODO: Instead of waiting for the channel, use select-case and continue the loop if this // TODO: Instead of waiting for the channel, use select-case and continue the loop if this
// channel is blocking. However, this might make the tests difficult. // channel is blocking. However, this might make the tests difficult.
<-i.syncing <-i.syncing
@ -102,11 +102,14 @@ func makeImagesShared() error {
if err := i.makeShared(); err != nil { if err := i.makeShared(); err != nil {
return err return err
} }
i.nonUpdatedCount = 0 i.usedAsSourceCount = 0
delete(imagesToMakeShared, i) delete(imagesToMakeShared, i)
i.syncing = nil i.syncing = nil
} }
} }
// Reset the images. The images will be registered again when it is used as a rendering source.
imagesToMakeShared = map[*Image]struct{}{}
return nil return nil
} }
@ -188,15 +191,16 @@ type Image struct {
node *packing.Node node *packing.Node
// nonUpdatedCount represents how long the image is kept not modified with DrawTriangles. // usedAsSourceCount represents how long the image is used as a rendering source and kept not modified with
// DrawTriangles.
// In the current implementation, if an image is being modified by DrawTriangles, the image is separated from // In the current implementation, if an image is being modified by DrawTriangles, the image is separated from
// a shared (restorable) image by ensureNotShared. // a shared (restorable) image by ensureNotShared.
// //
// nonUpdatedCount is increased every frame if the image is not modified, or set to 0 if the image is // usedAsSourceCount is increased if the image is used as a rendering source, or set to 0 if the image is
// modified. // modified.
// //
// ReplacePixels doesn't affect this value since ReplacePixels can be done on shared images. // ReplacePixels doesn't affect this value since ReplacePixels can be done on shared images.
nonUpdatedCount int usedAsSourceCount int
syncing <-chan struct{} syncing <-chan struct{}
} }
@ -214,14 +218,14 @@ func (i *Image) isShared() bool {
return i.node != nil return i.node != nil
} }
func (i *Image) resetNonUpdatedCount() { func (i *Image) resetUsedAsSourceCount() {
i.nonUpdatedCount = 0 i.usedAsSourceCount = 0
delete(imagesToMakeShared, i) delete(imagesToMakeShared, i)
i.syncing = nil i.syncing = nil
} }
func (i *Image) ensureNotShared() { func (i *Image) ensureNotShared() {
i.resetNonUpdatedCount() i.resetUsedAsSourceCount()
if i.backend == nil { if i.backend == nil {
i.allocate(false) i.allocate(false)
@ -297,7 +301,7 @@ func (i *Image) makeShared() error {
} }
newI.replacePixels(pixels) newI.replacePixels(pixels)
newI.moveTo(i) newI.moveTo(i)
i.nonUpdatedCount = 0 i.usedAsSourceCount = 0
i.syncing = nil i.syncing = nil
return nil return nil
} }
@ -443,7 +447,7 @@ func (i *Image) replacePixels(pix []byte) {
panic("shareable: the image must not be disposed at replacePixels") panic("shareable: the image must not be disposed at replacePixels")
} }
i.resetNonUpdatedCount() i.resetUsedAsSourceCount()
if i.backend == nil { if i.backend == nil {
if pix == nil { if pix == nil {
@ -535,7 +539,7 @@ func (i *Image) dispose(markDisposed bool) {
} }
}() }()
i.resetNonUpdatedCount() i.resetUsedAsSourceCount()
if i.disposed { if i.disposed {
return return

View File

@ -584,4 +584,61 @@ func TestDisposedAndReshared(t *testing.T) {
} }
} }
// Issue #1456
func TestImageIsNotResharedWithoutUsingAsSource(t *testing.T) {
const size = 16
src := NewImage(size, size)
defer src.MarkDisposed()
src2 := NewImage(size, size)
defer src2.MarkDisposed()
dst := NewImage(size, size)
defer dst.MarkDisposed()
// Use src as a render target so that src is not on the shared image.
vs := quadVertices(size, size, 0, 0, 1)
is := graphics.QuadIndices()
dr := driver.Region{
X: 0,
Y: 0,
Width: size,
Height: size,
}
// Use src2 as a rendering target, and make src2 an independent image.
src2.DrawTriangles([graphics.ShaderImageNum]*Image{src}, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil)
if got, want := src2.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
// Update the count without using src2 as a rendering source.
// This should not affect whether src2 is on a shareable image or not.
for i := 0; i < MaxCountForShare; i++ {
if err := MakeImagesSharedForTesting(); err != nil {
t.Fatal(err)
}
if got, want := src2.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
}
// Update the count with using src2 as a rendering source.
for i := 0; i < MaxCountForShare; i++ {
if err := MakeImagesSharedForTesting(); err != nil {
t.Fatal(err)
}
dst.DrawTriangles([graphics.ShaderImageNum]*Image{src2}, vs, is, nil, driver.CompositeModeCopy, driver.FilterNearest, driver.AddressUnsafe, dr, driver.Region{}, [graphics.ShaderImageNum - 1][2]float32{}, nil, nil)
if got, want := src2.IsSharedForTesting(), false; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
}
if err := MakeImagesSharedForTesting(); err != nil {
t.Fatal(err)
}
if got, want := src2.IsSharedForTesting(), true; got != want {
t.Errorf("got: %v, want: %v", got, want)
}
}
// TODO: Add tests to extend shareable image out of the main loop // TODO: Add tests to extend shareable image out of the main loop