Revert "internal/graphicscommand: bug fix: using an image just after ReplacePixels might fail on Metal"

This reverts commit c31cc4ecff.

Reason: This didn't fix the issue.

Updates #2154

internal/graphicscommand/image.go

@@ -18,7 +18,6 @@ import (
 	"fmt"
 	"image"
 	"os"
-	"runtime"
 	"sort"
 	"strings"
 
@@ -55,11 +54,6 @@ func genNextID() int {
 	return id
 }
 
-var (
-	dummyDst *Image
-	dummySrc *Image
-)
-
 // NewImage returns a new image.
 //
 // Note that the image is not initialized yet.
@@ -80,9 +74,9 @@ func NewImage(width, height int, screenFramebuffer bool) *Image {
 	return i
 }
 
-func (i *Image) resolveBufferedReplacePixels() bool {
+func (i *Image) resolveBufferedReplacePixels() {
 	if len(i.bufferedRP) == 0 {
-		return false
+		return
 	}
 	c := &replacePixelsCommand{
 		dst:  i,
@@ -90,8 +84,6 @@ func (i *Image) resolveBufferedReplacePixels() bool {
 	}
 	theCommandQueue.Enqueue(c)
 	i.bufferedRP = nil
-
-	return true
 }
 
 func (i *Image) Dispose() {
@@ -136,16 +128,13 @@ func (i *Image) InternalSize() (int, int) {
 // If the source image is not specified, i.e., src is nil and there is no image in the uniform variables, the
 // elements for the source image are not used.
 func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [graphics.ShaderImageNum - 1][2]float32, vertices []float32, indices []uint16, clr affine.ColorM, mode graphicsdriver.CompositeMode, filter graphicsdriver.Filter, address graphicsdriver.Address, dstRegion, srcRegion graphicsdriver.Region, shader *Shader, uniforms [][]float32, evenOdd bool) {
-	var needDummyRendering bool
 	if shader == nil {
 		// Fast path for rendering without a shader (#1355).
 		img := srcs[0]
 		if img.screen {
 			panic("graphicscommand: the screen image cannot be the rendering source")
 		}
-		if img.resolveBufferedReplacePixels() {
+		img.resolveBufferedReplacePixels()
-			needDummyRendering = true
-		}
 	} else {
 		for _, src := range srcs {
 			if src == nil {
@@ -154,31 +143,10 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderImageNum]*Image, offsets [gra
 			if src.screen {
 				panic("graphicscommand: the screen image cannot be the rendering source")
 			}
-			if src.resolveBufferedReplacePixels() {
+			src.resolveBufferedReplacePixels()
-				needDummyRendering = true
-			}
 		}
 	}
-	if i.resolveBufferedReplacePixels() {
+	i.resolveBufferedReplacePixels()
-		needDummyRendering = true
-	}
-
-	// On Metal, using an image just after ReplacePixel might fail (#2154).
-	// This is a very dirty hack but there seemed no other way.
-	// In order to reproduce the issue, run:
-	//
-	//     go test ./internal/processtest/ -run=/issue2154.go -count=100
-	//
-	// TODO: Detect the graphics library and apply this logic only when the graphics library is Metal.
-	if runtime.GOOS == "darwin" && needDummyRendering {
-		if dummyDst == nil {
-			dummyDst = NewImage(1, 1, false)
-		}
-		if dummySrc == nil {
-			dummySrc = NewImage(1, 1, false)
-		}
-		dummyDst.DrawTriangles([graphics.ShaderImageNum]*Image{dummySrc}, [graphics.ShaderImageNum - 1][2]float32{}, make([]float32, 8*3), []uint16{0, 1, 2}, affine.ColorMIdentity{}, graphicsdriver.CompositeModeSourceOver, graphicsdriver.FilterNearest, graphicsdriver.AddressUnsafe, graphicsdriver.Region{}, graphicsdriver.Region{}, nil, nil, false)
-	}
 
 	theCommandQueue.EnqueueDrawTrianglesCommand(i, srcs, offsets, vertices, indices, clr, mode, filter, address, dstRegion, srcRegion, shader, uniforms, evenOdd)
 }

internal/graphicsdriver/metal/graphics_darwin.go

@@ -1185,6 +1185,7 @@ func (i *Image) syncTexture() {
 	bce.EndEncoding()
 
 	cb.Commit()
+	// TODO: Are fences available here?
 	cb.WaitUntilCompleted()
 }
 

internal/processtest/testdata/issue2154.go

@@ -1,121 +0,0 @@
-// Copyright 2022 The Ebitengine Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//go:build ignore
-// +build ignore
-
-package main
-
-import (
-	"errors"
-	"fmt"
-	"image/color"
-
-	"github.com/hajimehoshi/ebiten/v2"
-)
-
-var regularTermination = errors.New("regular termination")
-
-var srcInit *ebiten.Image
-
-func init() {
-	const (
-		w = 2
-		h = 2
-	)
-
-	//src2 := ebiten.NewImage(1, 1)
-
-	src0 := ebiten.NewImage(w, h)
-	src0.Fill(color.RGBA{0xff, 0xff, 0xff, 0xff})
-	src0.Set(0, 0, color.RGBA{0, 0, 0, 0xff})
-	src0.Set(0, 1, color.RGBA{0, 0, 0, 0xff})
-	src0.Set(1, 0, color.RGBA{0, 0, 0, 0xff})
-
-	src1 := ebiten.NewImage(w, h)
-	// Using the image as a source just after Set caused troubles on Metal.
-	// For example, inserting src1.Fill(color.RGBA{0, 0xff, 0, 0xff}) here hid the error.
-	src1.DrawImage(src0, nil)
-
-	srcInit = src1
-}
-
-type Game struct {
-	count int
-	dst   *ebiten.Image
-}
-
-func (g *Game) Update() error {
-	g.count++
-	if g.count == 16 {
-		return regularTermination
-	}
-	return nil
-}
-
-func (g *Game) Draw(screen *ebiten.Image) {
-	screen.Fill(color.RGBA{0xff, 0xff, 0xff, 0xff})
-	screen.DrawImage(srcInit, nil)
-
-	if g.dst == nil {
-		g.dst = ebiten.NewImage(screen.Size())
-		return
-	}
-
-	g.dst.DrawImage(screen, nil)
-	if got, want := g.dst.At(0, 0), (color.RGBA{0, 0, 0, 0xff}); got != want {
-		panic(fmt.Sprintf("count: %d, got: %v, want: %v", g.count, got, want))
-	}
-	if got, want := g.dst.At(1, 1), (color.RGBA{0xff, 0xff, 0xff, 0xff}); got != want {
-		panic(fmt.Sprintf("count: %d, got: %v, want: %v", g.count, got, want))
-	}
-	g.dst.Clear()
-
-	const (
-		w = 2
-		h = 2
-	)
-
-	src0 := ebiten.NewImage(w, h)
-	defer src0.Dispose()
-	src0.Fill(color.RGBA{0xff, 0xff, 0xff, 0xff})
-	src0.Set(0, 0, color.RGBA{0, 0, 0, 0xff})
-	src0.Set(0, 1, color.RGBA{0, 0, 0, 0xff})
-	src0.Set(1, 0, color.RGBA{0, 0, 0, 0xff})
-
-	src1 := ebiten.NewImage(w, h)
-	defer src1.Dispose()
-	src1.DrawImage(src0, nil)
-
-	screen.Fill(color.RGBA{0xff, 0xff, 0xff, 0xff})
-	screen.DrawImage(src1, nil)
-
-	g.dst.DrawImage(screen, nil)
-	if got, want := g.dst.At(0, 0), (color.RGBA{0, 0, 0, 0xff}); got != want {
-		panic(fmt.Sprintf("count: %d, got: %v, want: %v", g.count, got, want))
-	}
-	if got, want := g.dst.At(1, 1), (color.RGBA{0xff, 0xff, 0xff, 0xff}); got != want {
-		panic(fmt.Sprintf("count: %d, got: %v, want: %v", g.count, got, want))
-	}
-}
-
-func (g *Game) Layout(width, height int) (int, int) {
-	return width, height
-}
-
-func main() {
-	if err := ebiten.RunGame(&Game{}); err != nil && err != regularTermination {
-		panic(err)
-	}
-}