internal/driver: Remove Image.Sync

Syncing is no longer needed for Metal, and additionally, OpenGL's sync
implementation was mock.

Updates #1508

internal/driver/graphics.go

@@ -68,18 +68,6 @@ 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

@@ -512,38 +512,6 @@ 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, dstRegion, srcRegion 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

@@ -163,23 +163,6 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [gra
 	theCommandQueue.EnqueueDrawTrianglesCommand(i, srcs, offsets, vertices, indices, clr, mode, filter, address, dstRegion, srcRegion, shader, uniforms)
 }
 
-// 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) {

internal/graphicsdriver/metal/graphics.go

@@ -690,7 +690,6 @@ func (g *Graphics) draw(rps mtl.RenderPipelineState, dst *Image, dstRegion drive
 
 func (g *Graphics) Draw(dstID, srcID driver.ImageID, indexLen int, indexOffset int, mode driver.CompositeMode, colorM *affine.ColorM, filter driver.Filter, address driver.Address, dstRegion, srcRegion driver.Region) error {
 	dst := g.images[dstID]
-	dst.waitUntilSyncFinishes()
 
 	srcs := [graphics.ShaderImageNum]*Image{g.images[srcID]}
 
@@ -820,15 +819,6 @@ 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 {
@@ -843,9 +833,6 @@ 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{}
@@ -860,13 +847,7 @@ func (i *Image) IsInvalidated() bool {
 	return false
 }
 
-func (i *Image) Sync() <-chan struct{} {
+func (i *Image) syncTexture() {
-	if i.sync != nil {
-		return i.sync
-	}
-
-	i.graphics.flushIfNeeded(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{}) {
@@ -878,21 +859,13 @@ func (i *Image) Sync() <-chan struct{} {
 	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) {
-	// Call Sync just in case when the user doesn't call Sync.
+	i.graphics.flushIfNeeded(false)
-	// If the image is already synced, the channel should be closed immediately.
+	i.syncTexture()
-	<-i.Sync()
-	i.sync = nil
 
 	b := make([]byte, 4*i.width*i.height)
 	i.texture.GetBytes(&b[0], uintptr(4*i.width), mtl.Region{
@@ -902,8 +875,6 @@ func (i *Image) Pixels() ([]byte, error) {
 }
 
 func (i *Image) ReplacePixels(args []*driver.ReplacePixelsArgs) {
-	i.waitUntilSyncFinishes()
-
 	g := i.graphics
 
 	// Use a temporary texture to send pixels asynchrounsly, whichever the memory is shared (e.g., iOS) or
@@ -941,7 +912,6 @@ 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, dstRegion, srcRegion driver.Region, mode driver.CompositeMode, uniforms []interface{}) error {
 	dst := g.images[dstID]
-	dst.waitUntilSyncFinishes()
 
 	var srcs [graphics.ShaderImageNum]*Image
 	for i, srcID := range srcIDs {

internal/graphicsdriver/metal/mtl/mtl.go

@@ -27,7 +27,6 @@ package mtl
 import (
 	"errors"
 	"fmt"
-	"sync"
 	"unsafe"
 )
 
@@ -569,32 +568,6 @@ func (cb CommandBuffer) WaitUntilCompleted() {
 	C.CommandBuffer_WaitUntilCompleted(cb.commandBuffer)
 }
 
-var (
-	commandBufferCompletedHandlers  = map[unsafe.Pointer]func(){}
-	commandBufferCompletedHandlersM sync.Mutex
-)
-
-// 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()) {
-	commandBufferCompletedHandlersM.Lock()
-	commandBufferCompletedHandlers[cb.commandBuffer] = f
-	commandBufferCompletedHandlersM.Unlock()
-
-	C.CommandBuffer_AddCompletedHandler(cb.commandBuffer)
-}
-
-//export commandBufferCompletedCallback
-func commandBufferCompletedCallback(commandBuffer unsafe.Pointer) {
-	commandBufferCompletedHandlersM.Lock()
-	f := commandBufferCompletedHandlers[commandBuffer]
-	delete(commandBufferCompletedHandlers, commandBuffer)
-	commandBufferCompletedHandlersM.Unlock()
-
-	f()
-}
-
 // MakeRenderCommandEncoder creates an encoder object that can
 // encode graphics rendering commands into this command buffer.
 //

internal/graphicsdriver/metal/mtl/mtl.h

@@ -133,7 +133,6 @@ 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,9 +18,6 @@
 #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) {
@@ -149,13 +146,6 @@ 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,13 +60,6 @@ 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

@@ -434,17 +434,6 @@ 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/export_test.go

@@ -15,8 +15,7 @@
 package shareable
 
 const (
-	MaxCountForShare     = maxCountForShare
+	MaxCountForShare = maxCountForShare
-	CountForStartSyncing = countForStartSyncing
 )
 
 func MakeImagesSharedForTesting() error {

internal/shareable/image.go

@@ -76,33 +76,15 @@ func resolveDeferred() {
 // maxCountForShare represents the time duration when the image can become shared.
 const maxCountForShare = 10
 
-// countForStartSyncing represents the count that the image starts to sync pixels between GPU and CPU.
-const countForStartSyncing = maxCountForShare / 2
-
 func makeImagesShared() error {
 	for i := range imagesToMakeShared {
 		i.usedAsSourceCount++
-		if restorable.NeedsRestoring() && i.usedAsSourceCount >= countForStartSyncing && 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.usedAsSourceCount >= maxCountForShare {
-			if restorable.NeedsRestoring() {
-				// 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.usedAsSourceCount = 0
 			delete(imagesToMakeShared, i)
-			i.syncing = nil
 		}
 	}
 
@@ -201,8 +183,6 @@ type Image struct {
 	//
 	// ReplacePixels doesn't affect this value since ReplacePixels can be done on shared images.
 	usedAsSourceCount int
-
-	syncing <-chan struct{}
 }
 
 func (i *Image) moveTo(dst *Image) {
@@ -221,7 +201,6 @@ func (i *Image) isShared() bool {
 func (i *Image) resetUsedAsSourceCount() {
 	i.usedAsSourceCount = 0
 	delete(imagesToMakeShared, i)
-	i.syncing = nil
 }
 
 func (i *Image) ensureNotShared() {
@@ -321,7 +300,6 @@ func (i *Image) makeShared() error {
 
 	newI.moveTo(i)
 	i.usedAsSourceCount = 0
-	i.syncing = nil
 	return nil
 }
 

internal/shareable/image_test.go

@@ -562,7 +562,7 @@ func TestDisposedAndReshared(t *testing.T) {
 	}
 
 	// Use src as a render source.
-	for i := 0; i < CountForStartSyncing; i++ {
+	for i := 0; i < MaxCountForShare/2; i++ {
 		if err := MakeImagesSharedForTesting(); err != nil {
 			t.Fatal(err)
 		}