graphics: Errors of NewImage* are always nil (#331)

image.go

@@ -15,11 +15,13 @@
 package ebiten
 
 import (
+	"fmt"
 	"image"
 	"image/color"
 	"runtime"
 	"sync"
 
+	"github.com/hajimehoshi/ebiten/internal/graphics"
 	"github.com/hajimehoshi/ebiten/internal/opengl"
 )
 
@@ -144,9 +146,9 @@ func (i *Image) Size() (width, height int) {
 
 // Clear resets the pixels of the image into 0.
 //
-// When the image is disposed, this function does nothing.
+// When the image is disposed, Clear does nothing.
 //
-// This function always returns nil as of 1.5.0-alpha.
+// Clear always returns nil as of 1.5.0-alpha.
 //
 // This function is concurrent-safe.
 func (i *Image) Clear() error {
@@ -157,9 +159,9 @@ func (i *Image) Clear() error {
 
 // Fill fills the image with a solid color.
 //
-// When the image is disposed, this function does nothing.
+// When the image is disposed, Fill does nothing.
 //
-// This function always returns nil as of 1.5.0-alpha.
+// Fill always returns nil as of 1.5.0-alpha.
 //
 // This function is concurrent-safe.
 func (i *Image) Fill(clr color.Color) error {
@@ -235,7 +237,7 @@ func (i *Image) Dispose() error {
 //
 // The given p must represent RGBA pre-multiplied alpha values. len(p) must equal to 4 * (image width) * (image height).
 //
-// This function may be slow (as for implementation, this calls glTexSubImage2D).
+// ReplacePixels may be slow (as for implementation, this calls glTexSubImage2D).
 //
 // This function is concurrent-safe.
 func (i *Image) ReplacePixels(p []uint8) error {
@@ -256,14 +258,14 @@ type DrawImageOptions struct {
 
 // NewImage returns an empty image.
 //
-// NewImage generates a new texture and a new framebuffer.
+// If width or height is less than 1 or more than MaxImageSize, NewImage panics.
+//
+// Error returned by NewImage is always nil as of 1.5.0-alpha.
 //
 // This function is concurrent-safe.
 func NewImage(width, height int, filter Filter) (*Image, error) {
-	img, err := newImageImpl(width, height, filter, false)
+	checkSize(width, height)
-	if err != nil {
+	img := newImageImpl(width, height, filter, false)
-		return nil, err
-	}
 	img.Fill(color.Transparent)
 	return theImagesForRestoring.add(img), nil
 }
@@ -278,33 +280,51 @@ func NewImage(width, height int, filter Filter) (*Image, error) {
 // On the other hand, pixels in volatile images are not saved.
 // Saving pixels is an expensive operation, and it is desirable to avoid it if possible.
 //
+// If width or height is less than 1 or more than MaxImageSize, newVolatileImage panics.
+//
+// Error returned by newVolatileImage is always nil as of 1.5.0-alpha.
+//
 // This function is concurrent-safe.
 func newVolatileImage(width, height int, filter Filter) (*Image, error) {
-	img, err := newImageImpl(width, height, filter, true)
+	checkSize(width, height)
-	if err != nil {
+	img := newImageImpl(width, height, filter, true)
-		return nil, err
-	}
 	img.Fill(color.Transparent)
 	return theImagesForRestoring.add(img), nil
 }
 
 // NewImageFromImage creates a new image with the given image (source).
 //
-// NewImageFromImage generates a new texture and a new framebuffer.
+// If source's width or height is less than 1 or more than MaxImageSize, NewImageFromImage panics.
+//
+// Error returned by NewImageFromImage is always nil as of 1.5.0-alpha.
 //
 // This function is concurrent-safe.
 func NewImageFromImage(source image.Image, filter Filter) (*Image, error) {
-	img, err := newImageImplFromImage(source, filter)
+	size := source.Bounds().Size()
-	if err != nil {
+	checkSize(size.X, size.Y)
-		return nil, err
+	img := newImageImplFromImage(source, filter)
-	}
 	return theImagesForRestoring.add(img), nil
 }
 
 func newImageWithScreenFramebuffer(width, height int) (*Image, error) {
-	img, err := newScreenImageImpl(width, height)
+	checkSize(width, height)
-	if err != nil {
+	img := newScreenImageImpl(width, height)
-		return nil, err
-	}
 	return theImagesForRestoring.add(img), nil
 }
+
+const MaxImageSize = graphics.MaxImageSize
+
+func checkSize(width, height int) {
+	if width <= 0 {
+		panic("ebiten: width must be more than 0")
+	}
+	if height <= 0 {
+		panic("ebiten: height must be more than 0")
+	}
+	if width > MaxImageSize {
+		panic(fmt.Sprintf("ebiten: width must be less than or equal to %d", MaxImageSize))
+	}
+	if height > MaxImageSize {
+		panic(fmt.Sprintf("ebiten: height must be less than or equal to %d", MaxImageSize))
+	}
+}

image_test.go

@@ -487,83 +487,6 @@ func TestImageFill(t *testing.T) {
 	}
 }
 
-func TestImageSize(t *testing.T) {
-	sizes := []struct {
-		width  int
-		height int
-		error  bool
-	}{
-		{
-			width:  -1,
-			height: -1,
-			error:  true,
-		},
-		{
-			width:  -1,
-			height: 1,
-			error:  true,
-		},
-		{
-			width:  1,
-			height: -1,
-			error:  true,
-		},
-		{
-			width:  0,
-			height: 0,
-			error:  true,
-		},
-		{
-			width:  0,
-			height: 1,
-			error:  true,
-		},
-		{
-			width:  1,
-			height: 0,
-			error:  true,
-		},
-		{
-			width:  1,
-			height: 1,
-			error:  false,
-		},
-		{
-			width:  4096,
-			height: 4096,
-			error:  false,
-		},
-		{
-			width:  4096,
-			height: 4097,
-			error:  true,
-		},
-		{
-			width:  4097,
-			height: 4096,
-			error:  true,
-		},
-		{
-			width:  4097,
-			height: 4097,
-			error:  true,
-		},
-	}
-	for _, size := range sizes {
-		_, err := NewImage(size.width, size.height, FilterNearest)
-		if err == nil {
-			if size.error {
-				t.Errorf("NewImage(%d, %d, ...) must cause error but not", size.width, size.height)
-			}
-			return
-		} else {
-			if !size.error {
-				t.Errorf("NewImage(%d, %d, ...) must not cause error but did: %s", size.width, size.height, err)
-			}
-		}
-	}
-}
-
 type halfImagePart struct {
 	image *Image
 }

imageimpl.go

@@ -32,39 +32,17 @@ type imageImpl struct {
 	m          sync.Mutex
 }
 
-func checkSize(width, height int) error {
+func newImageImpl(width, height int, filter Filter, volatile bool) *imageImpl {
-	if width <= 0 {
-		return fmt.Errorf("ebiten: width must be more than 0")
-	}
-	if height <= 0 {
-		return fmt.Errorf("ebiten: height must be more than 0")
-	}
-	if width > graphics.ImageMaxSize {
-		return fmt.Errorf("ebiten: width must be less than or equal to %d", graphics.ImageMaxSize)
-	}
-	if height > graphics.ImageMaxSize {
-		return fmt.Errorf("ebiten: height must be less than or equal to %d", graphics.ImageMaxSize)
-	}
-	return nil
-}
-
-func newImageImpl(width, height int, filter Filter, volatile bool) (*imageImpl, error) {
-	if err := checkSize(width, height); err != nil {
-		return nil, err
-	}
 	i := &imageImpl{
 		restorable: restorable.NewImage(width, height, glFilter(filter), volatile),
 	}
 	runtime.SetFinalizer(i, (*imageImpl).Dispose)
-	return i, nil
+	return i
 }
 
-func newImageImplFromImage(source image.Image, filter Filter) (*imageImpl, error) {
+func newImageImplFromImage(source image.Image, filter Filter) *imageImpl {
 	size := source.Bounds().Size()
 	w, h := size.X, size.Y
-	if err := checkSize(w, h); err != nil {
-		return nil, err
-	}
 
 	// Don't lock while manipulating an image.Image interface.
 
@@ -76,18 +54,15 @@ func newImageImplFromImage(source image.Image, filter Filter) (*imageImpl, error
 		restorable: restorable.NewImageFromImage(rgbaImg, w, h, glFilter(filter)),
 	}
 	runtime.SetFinalizer(i, (*imageImpl).Dispose)
-	return i, nil
+	return i
 }
 
-func newScreenImageImpl(width, height int) (*imageImpl, error) {
+func newScreenImageImpl(width, height int) *imageImpl {
-	if err := checkSize(width, height); err != nil {
-		return nil, err
-	}
 	i := &imageImpl{
 		restorable: restorable.NewScreenFramebufferImage(width, height),
 	}
 	runtime.SetFinalizer(i, (*imageImpl).Dispose)
-	return i, nil
+	return i
 }
 
 func (i *imageImpl) Fill(clr color.Color) {

internal/graphics/image.go

@@ -76,7 +76,7 @@ type Image struct {
 	height      int
 }
 
-const ImageMaxSize = viewportSize
+const MaxImageSize = viewportSize
 
 func NewImage(width, height int, filter opengl.Filter) *Image {
 	i := &Image{