From daf349ab72c92cef9bc7bc8f779220754488e0c6 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 15 Oct 2022 01:54:43 +0900 Subject: [PATCH] internal/graphicscommand: bug fix: present at the end of the frame explicitly Before this change, presenting happened when the rendering destination was the final screen. Now this assumption is wrong as the final screen might be used in the middle of the commands due to DrawFinalScreen. Instead, this change adds a new argument `present` to FlushCommands to present the screen explicitly at the end of the frame. Closes #2386 --- internal/atlas/image.go | 2 +- internal/graphicscommand/command.go | 17 +++++++---------- internal/graphicscommand/image.go | 4 ++-- internal/restorable/images.go | 8 ++++---- internal/restorable/images_test.go | 28 ++++++++++++++-------------- internal/restorable/shader_test.go | 10 +++++----- 6 files changed, 33 insertions(+), 36 deletions(-) diff --git a/internal/atlas/image.go b/internal/atlas/image.go index 398ad2d88..fc7f78c35 100644 --- a/internal/atlas/image.go +++ b/internal/atlas/image.go @@ -740,7 +740,7 @@ func EndFrame(graphicsDriver graphicsdriver.Graphics) error { theTemporaryBytes.resetAtFrameEnd() - return restorable.ResolveStaleImages(graphicsDriver) + return restorable.ResolveStaleImages(graphicsDriver, true) } func BeginFrame(graphicsDriver graphicsdriver.Graphics) error { diff --git a/internal/graphicscommand/command.go b/internal/graphicscommand/command.go index cd3aa8f2e..67655087e 100644 --- a/internal/graphicscommand/command.go +++ b/internal/graphicscommand/command.go @@ -159,15 +159,15 @@ func (q *commandQueue) Enqueue(command command) { } // Flush flushes the command queue. -func (q *commandQueue) Flush(graphicsDriver graphicsdriver.Graphics) (err error) { +func (q *commandQueue) Flush(graphicsDriver graphicsdriver.Graphics, present bool) (err error) { runOnRenderingThread(func() { - err = q.flush(graphicsDriver) + err = q.flush(graphicsDriver, present) }) return } // flush must be called the main thread. -func (q *commandQueue) flush(graphicsDriver graphicsdriver.Graphics) error { +func (q *commandQueue) flush(graphicsDriver graphicsdriver.Graphics, present bool) error { if len(q.commands) == 0 { return nil } @@ -179,7 +179,6 @@ func (q *commandQueue) flush(graphicsDriver graphicsdriver.Graphics) error { if err := graphicsDriver.Begin(); err != nil { return err } - var present bool cs := q.commands for len(cs) > 0 { nv := 0 @@ -195,9 +194,6 @@ func (q *commandQueue) flush(graphicsDriver graphicsdriver.Graphics) error { } nv += dtc.numVertices() ne += dtc.numIndices() - if dtc.dst.screen { - present = true - } } nc++ } @@ -244,9 +240,10 @@ func (q *commandQueue) flush(graphicsDriver graphicsdriver.Graphics) error { return nil } -// FlushCommands flushes the command queue. -func FlushCommands(graphicsDriver graphicsdriver.Graphics) error { - return theCommandQueue.Flush(graphicsDriver) +// FlushCommands flushes the command queue and present the screen. +// If present is true, the current screen is used to present. +func FlushCommands(graphicsDriver graphicsdriver.Graphics, present bool) error { + return theCommandQueue.Flush(graphicsDriver, present) } // drawTrianglesCommand represents a drawing command to draw an image on another image. diff --git a/internal/graphicscommand/image.go b/internal/graphicscommand/image.go index 17fc2dcd7..a900597b6 100644 --- a/internal/graphicscommand/image.go +++ b/internal/graphicscommand/image.go @@ -155,7 +155,7 @@ func (i *Image) ReadPixels(graphicsDriver graphicsdriver.Graphics, buf []byte, x result: buf, } theCommandQueue.Enqueue(c) - if err := theCommandQueue.Flush(graphicsDriver); err != nil { + if err := theCommandQueue.Flush(graphicsDriver, false); err != nil { return err } return nil @@ -187,7 +187,7 @@ func (i *Image) IsInvalidated(graphicsDriver graphicsdriver.Graphics) (bool, err image: i, } theCommandQueue.Enqueue(c) - if err := theCommandQueue.Flush(graphicsDriver); err != nil { + if err := theCommandQueue.Flush(graphicsDriver, false); err != nil { return false, err } return c.result, nil diff --git a/internal/restorable/images.go b/internal/restorable/images.go index 03f89a778..3d07e0526 100644 --- a/internal/restorable/images.go +++ b/internal/restorable/images.go @@ -51,11 +51,11 @@ var theImages = &images{ shaders: map[*Shader]struct{}{}, } -// ResolveStaleImages flushes the queued draw commands and resolves -// all stale images. +// ResolveStaleImages flushes the queued draw commands and resolves all stale images. +// If present is true, the current screen is used to present when flushing the commands. // // ResolveStaleImages is intended to be called at the end of a frame. -func ResolveStaleImages(graphicsDriver graphicsdriver.Graphics) error { +func ResolveStaleImages(graphicsDriver graphicsdriver.Graphics, present bool) error { if debug.IsDebug { debug.Logf("Internal image sizes:\n") imgs := make([]*graphicscommand.Image, 0, len(theImages.images)) @@ -65,7 +65,7 @@ func ResolveStaleImages(graphicsDriver graphicsdriver.Graphics) error { graphicscommand.LogImagesInfo(imgs) } - if err := graphicscommand.FlushCommands(graphicsDriver); err != nil { + if err := graphicscommand.FlushCommands(graphicsDriver, present); err != nil { return err } if !needsRestoring() { diff --git a/internal/restorable/images_test.go b/internal/restorable/images_test.go index c31bec87a..40475b3e8 100644 --- a/internal/restorable/images_test.go +++ b/internal/restorable/images_test.go @@ -62,7 +62,7 @@ func TestRestore(t *testing.T) { clr0 := color.RGBA{0x00, 0x00, 0x00, 0xff} img0.WritePixels([]byte{clr0.R, clr0.G, clr0.B, clr0.A}, 0, 0, 1, 1) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -81,7 +81,7 @@ func TestRestoreWithoutDraw(t *testing.T) { // If there is no drawing command on img0, img0 is cleared when restored. - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -147,7 +147,7 @@ func TestRestoreChain(t *testing.T) { } imgs[i+1].DrawTriangles([graphics.ShaderImageCount]*restorable.Image{imgs[i]}, [graphics.ShaderImageCount - 1][2]float32{}, vs, is, graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -199,7 +199,7 @@ func TestRestoreChain2(t *testing.T) { imgs[i+1].DrawTriangles([graphics.ShaderImageCount]*restorable.Image{imgs[i]}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(imgs[i], w, h, 0, 0), is, graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -246,7 +246,7 @@ func TestRestoreOverrideSource(t *testing.T) { img3.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img2}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(img2, w, h, 0, 0), is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) img0.WritePixels([]byte{clr1.R, clr1.G, clr1.B, clr1.A}, 0, 0, w, h) img1.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img0}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(img0, w, h, 0, 0), is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -348,7 +348,7 @@ func TestRestoreComplexGraph(t *testing.T) { img7.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img2}, offsets, vs, is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) vs = quadVertices(img3, w, h, 2, 0) img7.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img3}, offsets, vs, is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -447,7 +447,7 @@ func TestRestoreRecursive(t *testing.T) { } img1.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img0}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(img0, w, h, 1, 0), is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) img0.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img1}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(img1, w, h, 1, 0), is, graphicsdriver.CompositeModeSourceOver, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -509,7 +509,7 @@ func TestWritePixels(t *testing.T) { } } } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -552,7 +552,7 @@ func TestDrawTrianglesAndWritePixels(t *testing.T) { img1.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img0}, [graphics.ShaderImageCount - 1][2]float32{}, vs, is, graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) img1.WritePixels([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, 0, 0, 2, 1) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -596,7 +596,7 @@ func TestDispose(t *testing.T) { img0.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{img1}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(img1, 1, 1, 0, 0), is, graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, restorable.NearestFilterShader, nil, false) img1.Dispose() - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -728,7 +728,7 @@ func TestWritePixelsOnly(t *testing.T) { } } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -823,7 +823,7 @@ func TestAllowWritePixelsForPartAfterDrawTriangles(t *testing.T) { dst.WritePixels(make([]byte, 4*2*2), 0, 0, 2, 2) // WritePixels for a part of image doesn't panic. - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -929,7 +929,7 @@ func TestMutateSlices(t *testing.T) { for i := range is { is[i] = 0 } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -1081,7 +1081,7 @@ func TestOverlappedPixels(t *testing.T) { } } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { diff --git a/internal/restorable/shader_test.go b/internal/restorable/shader_test.go index e5afc7785..a7dd8ff21 100644 --- a/internal/restorable/shader_test.go +++ b/internal/restorable/shader_test.go @@ -66,7 +66,7 @@ func TestShader(t *testing.T) { } img.DrawTriangles([graphics.ShaderImageCount]*restorable.Image{}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(nil, 1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, s, nil, false) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -102,7 +102,7 @@ func TestShaderChain(t *testing.T) { imgs[i+1].DrawTriangles([graphics.ShaderImageCount]*restorable.Image{imgs[i]}, [graphics.ShaderImageCount - 1][2]float32{}, quadVertices(imgs[i], 1, 1, 0, 0), graphics.QuadIndices(), graphicsdriver.CompositeModeCopy, dr, graphicsdriver.Region{}, s, nil, false) } - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -142,7 +142,7 @@ func TestShaderMultipleSources(t *testing.T) { // Clear one of the sources after DrawTriangles. dst should not be affected. clearImage(srcs[0], 1, 1) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -183,7 +183,7 @@ func TestShaderMultipleSourcesOnOneTexture(t *testing.T) { // Clear one of the sources after DrawTriangles. dst should not be affected. clearImage(srcs[0], 3, 1) - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil { @@ -214,7 +214,7 @@ func TestShaderDispose(t *testing.T) { // stale. s.Dispose() - if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting()); err != nil { + if err := restorable.ResolveStaleImages(ui.GraphicsDriverForTesting(), false); err != nil { t.Fatal(err) } if err := restorable.RestoreIfNeeded(ui.GraphicsDriverForTesting()); err != nil {