graphics: Bug fix: now ebiten.Image can be passed to NewImageFromImage (#213)

image.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"image"
 	"image/color"
+	"image/draw"
 	"runtime"
 	"sync"
 
@@ -26,24 +27,46 @@ import (
 	"github.com/hajimehoshi/ebiten/internal/graphics/opengl"
 )
 
-var imageM sync.Mutex
-
 var (
-	lazyImageProcesses = []func() error{}
+	imageM sync.Mutex
 )
 
-func execDelayedImageProcesses() error {
+type delayedImageTasks struct {
-	imageM.Lock()
+	tasks      []func() error
-	defer imageM.Unlock()
+	m          sync.Mutex
-	for _, f := range lazyImageProcesses {
+	execCalled bool
+}
+
+var theDelayedImageTasks = &delayedImageTasks{
+	tasks: []func() error{},
+}
+
+func (t *delayedImageTasks) add(f func() error) bool {
+	t.m.Lock()
+	defer t.m.Unlock()
+	if t.execCalled {
+		return false
+	}
+	t.tasks = append(t.tasks, f)
+	return true
+}
+
+func (t *delayedImageTasks) exec() error {
+	t.m.Lock()
+	defer t.m.Unlock()
+	t.execCalled = true
+	for _, f := range t.tasks {
 		if err := f(); err != nil {
 			return err
 		}
 	}
-	lazyImageProcesses = nil
 	return nil
 }
 
+func (t *delayedImageTasks) isExecCalled() bool {
+	t.execCalled
+}
+
 // Image represents an image.
 // The pixel format is alpha-premultiplied.
 // Image implements image.Image.
@@ -67,8 +90,6 @@ func (i *Image) Size() (width, height int) {
 //
 // This function is concurrent-safe.
 func (i *Image) Clear() (err error) {
-	imageM.Lock()
-	defer imageM.Unlock()
 	return i.clear()
 }
 
@@ -80,22 +101,21 @@ func (i *Image) clear() (err error) {
 //
 // This function is concurrent-safe.
 func (i *Image) Fill(clr color.Color) (err error) {
-	imageM.Lock()
-	defer imageM.Unlock()
 	return i.fill(clr)
 }
 
 func (i *Image) fill(clr color.Color) (err error) {
 	f := func() error {
+		imageM.Lock()
+		defer imageM.Unlock()
 		if i.isDisposed() {
 			return errors.New("ebiten: image is already disposed")
 		}
 		i.pixels = nil
 		return i.framebuffer.Fill(glContext, clr)
 	}
-	if lazyImageProcesses != nil {
+	if theDelayedImageTasks.add(f) {
-		lazyImageProcesses = append(lazyImageProcesses, f)
+		return nil
-		return
 	}
 	return f()
 
@@ -142,12 +162,12 @@ func (i *Image) DrawImage(image *Image, options *DrawImageOptions) (err error) {
 		return nil
 	}
 
-	imageM.Lock()
-	defer imageM.Unlock()
 	if i == image {
 		return errors.New("ebiten: Image.DrawImage: image should be different from the receiver")
 	}
 	f := func() error {
+		imageM.Lock()
+		defer imageM.Unlock()
 		if i.isDisposed() {
 			return errors.New("ebiten: image is already disposed")
 		}
@@ -155,8 +175,7 @@ func (i *Image) DrawImage(image *Image, options *DrawImageOptions) (err error) {
 		m := opengl.CompositeMode(options.CompositeMode)
 		return i.framebuffer.DrawTexture(glContext, image.texture, vertices[:16*n], &options.GeoM, &options.ColorM, m)
 	}
-	if lazyImageProcesses != nil {
+	if theDelayedImageTasks.add(f) {
-		lazyImageProcesses = append(lazyImageProcesses, f)
 		return nil
 	}
 	return f()
@@ -187,7 +206,7 @@ func (i *Image) At(x, y int) color.Color {
 	// TODO: What if At is called internaly (like from image parts?)
 	imageM.Lock()
 	defer imageM.Unlock()
-	if lazyImageProcesses != nil {
+	if !theDelayedImageTasks.isExecCalled() {
 		panic("ebiten: At can't be called when the GL context is not initialized (this panic happens as of version 1.4.0-alpha)")
 	}
 	if i.isDisposed() {
@@ -213,9 +232,9 @@ func (i *Image) At(x, y int) color.Color {
 //
 // This function is concurrent-safe.
 func (i *Image) Dispose() error {
+	f := func() error {
 		imageM.Lock()
 		defer imageM.Unlock()
-	f := func() error {
 		if i.isDisposed() {
 			return errors.New("ebiten: image is already disposed")
 		}
@@ -236,8 +255,8 @@ func (i *Image) Dispose() error {
 		runtime.SetFinalizer(i, nil)
 		return nil
 	}
-	if lazyImageProcesses != nil {
+
-		lazyImageProcesses = append(lazyImageProcesses, f)
+	if theDelayedImageTasks.add(f) {
 		return nil
 	}
 	return f()
@@ -255,12 +274,12 @@ func (i *Image) isDisposed() bool {
 //
 // This function is concurrent-safe.
 func (i *Image) ReplacePixels(p []uint8) error {
-	imageM.Lock()
-	defer imageM.Unlock()
 	if l := 4 * i.width * i.height; len(p) != l {
 		return fmt.Errorf("ebiten: p's length must be %d", l)
 	}
 	f := func() error {
+		imageM.Lock()
+		defer imageM.Unlock()
 		// Don't set i.pixels here because i.pixels is used not every time.
 		i.pixels = nil
 		if i.isDisposed() {
@@ -268,8 +287,7 @@ func (i *Image) ReplacePixels(p []uint8) error {
 		}
 		return i.framebuffer.ReplacePixels(glContext, i.texture, p)
 	}
-	if lazyImageProcesses != nil {
+	if theDelayedImageTasks.add(f) {
-		lazyImageProcesses = append(lazyImageProcesses, f)
 		return nil
 	}
 	return f()
@@ -295,13 +313,13 @@ type DrawImageOptions struct {
 //
 // This function is concurrent-safe.
 func NewImage(width, height int, filter Filter) (*Image, error) {
-	imageM.Lock()
-	defer imageM.Unlock()
 	image := &Image{
 		width:  width,
 		height: height,
 	}
 	f := func() error {
+		imageM.Lock()
+		defer imageM.Unlock()
 		texture, err := graphics.NewTexture(glContext, width, height, glFilter(glContext, filter))
 		if err != nil {
 			return err
@@ -319,8 +337,7 @@ func NewImage(width, height int, filter Filter) (*Image, error) {
 		}
 		return nil
 	}
-	if lazyImageProcesses != nil {
+	if theDelayedImageTasks.add(f) {
-		lazyImageProcesses = append(lazyImageProcesses, f)
 		return image, nil
 	}
 	if err := f(); err != nil {
@@ -338,21 +355,24 @@ func NewImage(width, height int, filter Filter) (*Image, error) {
 //
 // This function is concurrent-safe.
 func NewImageFromImage(img image.Image, filter Filter) (*Image, error) {
-	// Can't call (*ebiten.Image).At here because of the lock.
-	if _, ok := img.(*Image); ok {
-		return nil, errors.New("ebiten: NewImageFromImage can't take *ebiten.Image")
-	}
-
-	imageM.Lock()
-	defer imageM.Unlock()
 	size := img.Bounds().Size()
 	w, h := size.X, size.Y
-	image := &Image{
+	eimg := &Image{
 		width:  w,
 		height: h,
 	}
 	f := func() error {
-		texture, err := graphics.NewTextureFromImage(glContext, img, glFilter(glContext, filter))
+		// Don't lock while manipulating an image.Image interface.
+		rgbaImg, ok := img.(*image.RGBA)
+		if !ok {
+			origImg := img
+			newImg := image.NewRGBA(origImg.Bounds())
+			draw.Draw(newImg, newImg.Bounds(), origImg, origImg.Bounds().Min, draw.Src)
+			rgbaImg = newImg
+		}
+		imageM.Lock()
+		defer imageM.Unlock()
+		texture, err := graphics.NewTextureFromImage(glContext, rgbaImg, glFilter(glContext, filter))
 		if err != nil {
 			return err
 		}
@@ -361,17 +381,16 @@ func NewImageFromImage(img image.Image, filter Filter) (*Image, error) {
 			// TODO: texture should be removed here?
 			return err
 		}
-		image.framebuffer = framebuffer
+		eimg.framebuffer = framebuffer
-		image.texture = texture
+		eimg.texture = texture
-		runtime.SetFinalizer(image, (*Image).Dispose)
+		runtime.SetFinalizer(eimg, (*Image).Dispose)
 		return nil
 	}
-	if lazyImageProcesses != nil {
+	if theDelayedImageTasks.add(f) {
-		lazyImageProcesses = append(lazyImageProcesses, f)
+		return eimg, nil
-		return image, nil
 	}
 	if err := f(); err != nil {
 		return nil, err
 	}
-	return image, nil
+	return eimg, nil
 }

image_test.go

@@ -71,14 +71,3 @@ func TestImageDispose(t *testing.T) {
 		t.Errorf("img.Dipose() returns error: %v", err)
 	}
 }
-
-func TestNewImageFromEbitenImage(t *testing.T) {
-	img, _, err := openEbitenImage("testdata/ebiten.png")
-	if err != nil {
-		t.Fatal(err)
-		return
-	}
-	if _, err := NewImageFromImage(img, FilterNearest); err == nil {
-		t.Errorf("NewImageFromImage with an *ebiten.Image must return an error")
-	}
-}

internal/graphics/texture.go

@@ -16,12 +16,13 @@ package graphics
 
 import (
 	"errors"
-	"github.com/hajimehoshi/ebiten/internal/graphics/opengl"
 	"image"
 	"image/draw"
+
+	"github.com/hajimehoshi/ebiten/internal/graphics/opengl"
 )
 
-func adjustImageForTexture(img image.Image) *image.RGBA {
+func adjustImageForTexture(img *image.RGBA) *image.RGBA {
 	width, height := img.Bounds().Size().X, img.Bounds().Size().Y
 	adjustedImageBounds := image.Rectangle{
 		image.ZP,
@@ -30,8 +31,8 @@ func adjustImageForTexture(img image.Image) *image.RGBA {
 			int(NextPowerOf2Int32(int32(height))),
 		},
 	}
-	if adjustedImage, ok := img.(*image.RGBA); ok && img.Bounds() == adjustedImageBounds {
+	if img.Bounds() == adjustedImageBounds {
-		return adjustedImage
+		return img
 	}
 
 	adjustedImage := image.NewRGBA(adjustedImageBounds)
@@ -39,7 +40,7 @@ func adjustImageForTexture(img image.Image) *image.RGBA {
 		image.ZP,
 		img.Bounds().Size(),
 	}
-	draw.Draw(adjustedImage, dstBounds, img, image.ZP, draw.Src)
+	draw.Draw(adjustedImage, dstBounds, img, img.Bounds().Min, draw.Src)
 	return adjustedImage
 }
 
@@ -69,7 +70,7 @@ func NewTexture(c *opengl.Context, width, height int, filter opengl.Filter) (*Te
 	return &Texture{native, width, height}, nil
 }
 
-func NewTextureFromImage(c *opengl.Context, img image.Image, filter opengl.Filter) (*Texture, error) {
+func NewTextureFromImage(c *opengl.Context, img *image.RGBA, filter opengl.Filter) (*Texture, error) {
 	origSize := img.Bounds().Size()
 	if origSize.X < 4 {
 		return nil, errors.New("width must be equal or more than 4.")

run.go

@@ -189,7 +189,7 @@ func run(f func(*Image) error, width, height, scale int, title string) error {
 		return err
 	}
 
-	if err := execDelayedImageProcesses(); err != nil {
+	if err := theDelayedImageTasks.exec(); err != nil {
 		return err
 	}