From 6ca71c69315fe953fb6384c163cf848d01b0c046 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Sat, 4 Mar 2017 01:22:51 +0900 Subject: [PATCH] graphics: Errors of NewImage* are always nil (#331) --- image.go | 66 ++++++++++++++++++++------------ image_test.go | 77 -------------------------------------- imageimpl.go | 37 +++--------------- internal/graphics/image.go | 2 +- 4 files changed, 50 insertions(+), 132 deletions(-) diff --git a/image.go b/image.go index 921fe673f..30d2e61aa 100644 --- a/image.go +++ b/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) - if err != nil { - return nil, err - } + checkSize(width, height) + img := newImageImpl(width, height, filter, false) 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) - if err != nil { - return nil, err - } + checkSize(width, height) + img := newImageImpl(width, height, filter, true) 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) - if err != nil { - return nil, err - } + size := source.Bounds().Size() + checkSize(size.X, size.Y) + img := newImageImplFromImage(source, filter) return theImagesForRestoring.add(img), nil } func newImageWithScreenFramebuffer(width, height int) (*Image, error) { - img, err := newScreenImageImpl(width, height) - if err != nil { - return nil, err - } + checkSize(width, height) + img := newScreenImageImpl(width, height) 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)) + } +} diff --git a/image_test.go b/image_test.go index e95b729d1..13a2bdcaf 100644 --- a/image_test.go +++ b/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 } diff --git a/imageimpl.go b/imageimpl.go index 84ff21416..ebc9640a4 100644 --- a/imageimpl.go +++ b/imageimpl.go @@ -32,39 +32,17 @@ type imageImpl struct { m sync.Mutex } -func checkSize(width, height int) error { - 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 - } +func newImageImpl(width, height int, filter Filter, volatile bool) *imageImpl { 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) { - if err := checkSize(width, height); err != nil { - return nil, err - } +func newScreenImageImpl(width, height int) *imageImpl { i := &imageImpl{ restorable: restorable.NewScreenFramebufferImage(width, height), } runtime.SetFinalizer(i, (*imageImpl).Dispose) - return i, nil + return i } func (i *imageImpl) Fill(clr color.Color) { diff --git a/internal/graphics/image.go b/internal/graphics/image.go index ef629896d..6494aafc6 100644 --- a/internal/graphics/image.go +++ b/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{