graphicsdriver/metal: Add Sync to sync CPU and GPU asynchronously

Fixes #1414

internal/driver/graphics.go

@@ -68,6 +68,18 @@ type Image interface {
 	ID() ImageID
 	Dispose()
 	IsInvalidated() bool
+
+	// Sync syncs the texture data in CPU and GPU so that Pixels can return the texture data immediately.
+	// In most cases, Sync doesn't have to be called explicitly.
+	// Even without Sync, Pixels should does Sync automatically, but this might take long.
+	//
+	// Sync returns a channel that is closed when syncing finishes.
+	//
+	// Whatever the syncing state is, the other function should work correctly.
+	//
+	// TODO: Should the other functions be blocked during syncing?
+	Sync() <-chan struct{}
+
 	Pixels() ([]byte, error)
 	ReplacePixels(args []*ReplacePixelsArgs)
 }

internal/graphicscommand/command.go

@@ -508,6 +508,38 @@ func (c *replacePixelsCommand) CanMergeWithDrawTrianglesCommand(dst *Image, src
 	return false
 }
 
+type syncCommand struct {
+	result <-chan struct{}
+	img    *Image
+}
+
+func (c *syncCommand) Exec(indexOffset int) error {
+	c.result = c.img.image.Sync()
+	return nil
+}
+
+func (c *syncCommand) NumVertices() int {
+	return 0
+}
+
+func (c *syncCommand) NumIndices() int {
+	return 0
+}
+
+func (c *syncCommand) AddNumVertices(n int) {
+}
+
+func (c *syncCommand) AddNumIndices(n int) {
+}
+
+func (c *syncCommand) CanMergeWithDrawTrianglesCommand(dst *Image, src [graphics.ShaderImageNum]*Image, color *affine.ColorM, mode driver.CompositeMode, filter driver.Filter, address driver.Address, sourceRegion driver.Region, shader *Shader) bool {
+	return false
+}
+
+func (c *syncCommand) String() string {
+	return fmt.Sprintf("sync: image: %d", c.img.id)
+}
+
 type pixelsCommand struct {
 	result []byte
 	img    *Image

internal/graphicscommand/image.go

@@ -186,13 +186,29 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [gra
 	}
 }
 
+// Sync syncs the texture data in CPU and GPU so that Pixels can return the texture data immediately.
+// In most cases, Sync doesn't have to be called explicitly.
+// Even without Sync, Pixels should does Sync automatically, but this might take long.
+//
+// Sync returns a channel that is closed when syncing finishes.
+func (i *Image) Sync() (<-chan struct{}, error) {
+	i.resolveBufferedReplacePixels()
+	c := &syncCommand{
+		img: i,
+	}
+	theCommandQueue.Enqueue(c)
+	if err := theCommandQueue.Flush(); err != nil {
+		return nil, err
+	}
+	return c.result, nil
+}
+
 // Pixels returns the image's pixels.
 // Pixels might return nil when OpenGL error happens.
 func (i *Image) Pixels() ([]byte, error) {
 	i.resolveBufferedReplacePixels()
 	c := &pixelsCommand{
-		result: nil,
+		img: i,
-		img:    i,
 	}
 	theCommandQueue.Enqueue(c)
 	if err := theCommandQueue.Flush(); err != nil {

internal/graphicsdriver/metal/graphics.go

@@ -682,6 +682,8 @@ func (g *Graphics) draw(rps mtl.RenderPipelineState, dst *Image, srcs [graphics.
 
 func (g *Graphics) Draw(dstID, srcID driver.ImageID, indexLen int, indexOffset int, mode driver.CompositeMode, colorM *affine.ColorM, filter driver.Filter, address driver.Address, sourceRegion driver.Region) error {
 	dst := g.images[dstID]
+	dst.waitUntilSyncFinishes()
+
 	srcs := [graphics.ShaderImageNum]*Image{g.images[srcID]}
 
 	var rps mtl.RenderPipelineState
@@ -810,6 +812,15 @@ type Image struct {
 	height   int
 	screen   bool
 	texture  mtl.Texture
+	sync     <-chan struct{}
+}
+
+func (i *Image) waitUntilSyncFinishes() {
+	if i.sync == nil {
+		return
+	}
+	<-i.sync
+	i.sync = nil
 }
 
 func (i *Image) ID() driver.ImageID {
@@ -824,6 +835,9 @@ func (i *Image) internalSize() (int, int) {
 }
 
 func (i *Image) Dispose() {
+	// TODO: Is it necessary to wait for syncing?
+	i.waitUntilSyncFinishes()
+
 	if i.texture != (mtl.Texture{}) {
 		i.texture.Release()
 		i.texture = mtl.Texture{}
@@ -838,7 +852,13 @@ func (i *Image) IsInvalidated() bool {
 	return false
 }
 
-func (i *Image) syncTexture() {
+func (i *Image) Sync() <-chan struct{} {
+	if i.sync != nil {
+		return i.sync
+	}
+
+	i.graphics.flushIfNeeded(true, false)
+
 	// Calling SynchronizeTexture is ignored on iOS (see mtl.m), but it looks like committing BlitCommandEncoder
 	// is necessary (#1337).
 	if i.graphics.cb != (mtl.CommandBuffer{}) {
@@ -849,13 +869,22 @@ func (i *Image) syncTexture() {
 	bce := cb.MakeBlitCommandEncoder()
 	bce.SynchronizeTexture(i.texture, 0, 0)
 	bce.EndEncoding()
+
+	ch := make(chan struct{})
+	cb.AddCompletedHandler(func() {
+		close(ch)
+	})
 	cb.Commit()
-	cb.WaitUntilCompleted()
+
+	i.sync = ch
+	return i.sync
 }
 
 func (i *Image) Pixels() ([]byte, error) {
-	i.graphics.flushIfNeeded(true, false)
+	// Call Sync just in case when the user doesn't call Sync.
-	i.syncTexture()
+	// If the image is already synced, the channel should be closed immediately.
+	<-i.Sync()
+	i.sync = nil
 
 	b := make([]byte, 4*i.width*i.height)
 	i.texture.GetBytes(&b[0], uintptr(4*i.width), mtl.Region{
@@ -865,6 +894,8 @@ func (i *Image) Pixels() ([]byte, error) {
 }
 
 func (i *Image) ReplacePixels(args []*driver.ReplacePixelsArgs) {
+	i.waitUntilSyncFinishes()
+
 	g := i.graphics
 	g.flushIfNeeded(true, false)
 
@@ -918,6 +949,8 @@ func (i *Image) ReplacePixels(args []*driver.ReplacePixelsArgs) {
 
 func (g *Graphics) DrawShader(dstID driver.ImageID, srcIDs [graphics.ShaderImageNum]driver.ImageID, offsets [graphics.ShaderImageNum - 1][2]float32, shader driver.ShaderID, indexLen int, indexOffset int, sourceRegion driver.Region, mode driver.CompositeMode, uniforms []interface{}) error {
 	dst := g.images[dstID]
+	dst.waitUntilSyncFinishes()
+
 	var srcs [graphics.ShaderImageNum]*Image
 	for i, srcID := range srcIDs {
 		srcs[i] = g.images[srcID]

internal/graphicsdriver/metal/mtl/mtl.go

@@ -568,6 +568,23 @@ func (cb CommandBuffer) WaitUntilCompleted() {
 	C.CommandBuffer_WaitUntilCompleted(cb.commandBuffer)
 }
 
+var commandBufferCompletedHandlers = map[unsafe.Pointer]func(){}
+
+// AddCompletedHandler registers a block of code that Metal calls immediately after the GPU finishes executing the commands in the command buffer.
+//
+// Reference: https://developer.apple.com/documentation/metal/mtlcommandbuffer/1442997-addcompletedhandler
+func (cb CommandBuffer) AddCompletedHandler(f func()) {
+	commandBufferCompletedHandlers[cb.commandBuffer] = f
+	C.CommandBuffer_AddCompletedHandler(cb.commandBuffer)
+}
+
+//export commandBufferCompletedCallback
+func commandBufferCompletedCallback(commandBuffer unsafe.Pointer) {
+	f := commandBufferCompletedHandlers[commandBuffer]
+	delete(commandBufferCompletedHandlers, commandBuffer)
+	f()
+}
+
 // MakeRenderCommandEncoder creates an encoder object that can
 // encode graphics rendering commands into this command buffer.
 //

internal/graphicsdriver/metal/mtl/mtl.h

@@ -126,6 +126,7 @@ void CommandBuffer_Release(void *commandBuffer);
 void CommandBuffer_PresentDrawable(void *commandBuffer, void *drawable);
 void CommandBuffer_Commit(void *commandBuffer);
 void CommandBuffer_WaitUntilCompleted(void *commandBuffer);
+void CommandBuffer_AddCompletedHandler(void *commandBuffer);
 void *
 CommandBuffer_MakeRenderCommandEncoder(void *commandBuffer,
                                        struct RenderPassDescriptor descriptor);

internal/graphicsdriver/metal/mtl/mtl.m

@@ -18,6 +18,9 @@
 #import <Metal/Metal.h>
 #include <stdlib.h>
 
+// commandBufferCompletedCallback is an exported function from Go.
+void commandBufferCompletedCallback(void *commandBuffer);
+
 struct Device CreateSystemDefaultDevice() {
   id<MTLDevice> device = MTLCreateSystemDefaultDevice();
   if (!device) {
@@ -146,6 +149,13 @@ void CommandBuffer_WaitUntilCompleted(void *commandBuffer) {
   [(id<MTLCommandBuffer>)commandBuffer waitUntilCompleted];
 }
 
+void CommandBuffer_AddCompletedHandler(void *commandBuffer) {
+  [(id<MTLCommandBuffer>)commandBuffer
+      addCompletedHandler:^(id<MTLCommandBuffer> cb) {
+        commandBufferCompletedCallback(cb);
+      }];
+}
+
 void *
 CommandBuffer_MakeRenderCommandEncoder(void *commandBuffer,
                                        struct RenderPassDescriptor descriptor) {

internal/graphicsdriver/opengl/image.go

@@ -60,6 +60,13 @@ func (i *Image) setViewport() error {
 	return nil
 }
 
+func (i *Image) Sync() <-chan struct{} {
+	// There is no way to sync the textures data on the system memory and GPU.
+	ch := make(chan struct{})
+	close(ch)
+	return ch
+}
+
 func (i *Image) Pixels() ([]byte, error) {
 	if err := i.ensureFramebuffer(); err != nil {
 		return nil, err

internal/restorable/image.go

@@ -452,6 +452,17 @@ func (i *Image) readPixelsFromGPUIfNeeded() error {
 	return nil
 }
 
+func (i *Image) Sync() (<-chan struct{}, error) {
+	if err := graphicscommand.FlushCommands(); err != nil {
+		return nil, err
+	}
+	ch, err := i.image.Sync()
+	if err != nil {
+		return nil, err
+	}
+	return ch, nil
+}
+
 // At returns a color value at (x, y).
 //
 // Note that this must not be called until context is available.

internal/shareable/image.go

@@ -75,11 +75,25 @@ const MaxCountForShare = 10
 func makeImagesShared() error {
 	for i := range imagesToMakeShared {
 		i.nonUpdatedCount++
+		if i.nonUpdatedCount >= MaxCountForShare/2 && i.syncing == nil {
+			// Sync the pixel data on CPU and GPU sides explicitly in order not to block this process.
+			ch, err := i.backend.restorable.Sync()
+			if err != nil {
+				return err
+			}
+			i.syncing = ch
+		}
 		if i.nonUpdatedCount >= MaxCountForShare {
+			// 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.
+			<-i.syncing
+
 			if err := i.makeShared(); err != nil {
 				return err
 			}
+			i.nonUpdatedCount = 0
 			delete(imagesToMakeShared, i)
+			i.syncing = nil
 		}
 	}
 	return nil
@@ -172,6 +186,8 @@ type Image struct {
 	//
 	// ReplacePixels doesn't affect this value since ReplacePixels can be done on shared images.
 	nonUpdatedCount int
+
+	syncing <-chan struct{}
 }
 
 func (i *Image) moveTo(dst *Image) {
@@ -190,6 +206,7 @@ func (i *Image) isShared() bool {
 func (i *Image) resetNonUpdatedCount() {
 	i.nonUpdatedCount = 0
 	delete(imagesToMakeShared, i)
+	i.syncing = nil
 }
 
 func (i *Image) ensureNotShared() {
@@ -264,6 +281,7 @@ func (i *Image) makeShared() error {
 	newI.replacePixels(pixels)
 	newI.moveTo(i)
 	i.nonUpdatedCount = 0
+	i.syncing = nil
 	return nil
 }