From 6ba6cdc721c31fcd05898ae05ada5d4f076d17ad Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Fri, 16 Aug 2019 00:36:54 +0900 Subject: [PATCH] restorable: Bug fix: Dispose all the image before start restoring A current texture ID and a new texture ID can be duplicated. Disposing part of textures and creating textures at the same time can make contradicted situation. --- internal/restorable/image.go | 16 ++++++---------- internal/restorable/images.go | 14 +++++++++++++- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/internal/restorable/image.go b/internal/restorable/image.go index d9aaa2908..c20f6addb 100644 --- a/internal/restorable/image.go +++ b/internal/restorable/image.go @@ -472,24 +472,20 @@ func (i *Image) hasDependency() bool { } // Restore restores *graphicscommand.Image from the pixels using its state. -func (i *Image) restore() { - w, h := i.Size() - - // Dispose the internal image after getting its size for safety. - i.image.Dispose() - i.image = nil +func (i *Image) restore(width, height int) { + // Do not dispose the image here. The image should be already disposed. if i.screen { // The screen image should also be recreated because framebuffer might // be changed. - i.image = graphicscommand.NewScreenFramebufferImage(w, h) + i.image = graphicscommand.NewScreenFramebufferImage(width, height) i.basePixels = Pixels{} i.drawTrianglesHistory = nil i.stale = false return } if i.volatile { - i.image = graphicscommand.NewImage(w, h) + i.image = graphicscommand.NewImage(width, height) i.clear() return } @@ -497,7 +493,7 @@ func (i *Image) restore() { panic("restorable: pixels must not be stale when restoring") } - gimg := graphicscommand.NewImage(w, h) + gimg := graphicscommand.NewImage(width, height) // Clear the image explicitly. if i != emptyImage { // As clearImage uses emptyImage, clearImage cannot be called on emptyImage. @@ -515,7 +511,7 @@ func (i *Image) restore() { if len(i.drawTrianglesHistory) > 0 { i.basePixels = Pixels{} - i.basePixels.AddOrReplace(gimg.Pixels(), 0, 0, w, h) + i.basePixels.AddOrReplace(gimg.Pixels(), 0, 0, width, height) } i.image = gimg diff --git a/internal/restorable/images.go b/internal/restorable/images.go index 8a23de1ad..f29408572 100644 --- a/internal/restorable/images.go +++ b/internal/restorable/images.go @@ -152,6 +152,17 @@ func (i *images) restore() { panic("restorable: restore cannot be called when restoring is disabled") } + // Dispose all the images ahead of restoring. A current texture ID and a new texture ID can be duplicated. + // TODO: Write a test to confirm that ID duplication never happens. + sizes := map[*Image]struct{ w, h int }{} + for i := range i.images { + // Keep the size before disposing i.image. + w, h := i.Size() + sizes[i] = struct{ w, h int }{w, h} + i.image.Dispose() + i.image = nil + } + // Let's do topological sort based on dependencies of drawing history. // It is assured that there are not loops since cyclic drawing makes images stale. type edge struct { @@ -204,7 +215,8 @@ func (i *images) restore() { } for _, img := range sorted { - img.restore() + s := sizes[img] + img.restore(s.w, s.h) } }