ebiten: Make ebiten.Image and buffered.Image 1:1

This change creates a new buffered.Image even for a sub-image. This
can increase a memory usage a little, but decrease the GPU memory
usage since only the necessary pixels are allocated on a texture
atlas.

Fixes #896
Updates #1194

image.go

@@ -37,8 +37,11 @@ type Image struct {
 
 	bounds   image.Rectangle
 	original *Image
+	subs     map[image.Rectangle]*Image
 
 	filter Filter
+
+	disposed bool
 }
 
 func (i *Image) copyCheck() {
@@ -54,7 +57,11 @@ func (i *Image) Size() (width, height int) {
 }
 
 func (i *Image) isDisposed() bool {
-	return i.buffered == nil
+	return i.disposed
+}
+
+func (i *Image) isEmpty() bool {
+	return i.bounds.Dx() == 0 || i.bounds.Dy() == 0
 }
 
 func (i *Image) isSubImage() bool {
@@ -82,13 +89,18 @@ func (i *Image) Fill(clr color.Color) error {
 	if i.isDisposed() {
 		return nil
 	}
+	if i.isEmpty() {
+		return nil
+	}
 
 	// TODO: Implement this.
 	if i.isSubImage() {
 		panic("ebiten: render to a subimage is not implemented (Fill)")
 	}
 
+	i.desyncSubImages()
 	i.buffered.Fill(color.RGBAModel.Convert(clr).(color.RGBA))
+
 	return nil
 }
 
@@ -135,15 +147,27 @@ func (i *Image) DrawImage(img *Image, options *DrawImageOptions) error {
 	if img.isDisposed() {
 		panic("ebiten: the given image to DrawImage must not be disposed")
 	}
+	if img.isEmpty() {
+		return nil
+	}
 	if i.isDisposed() {
 		return nil
 	}
+	if i.isEmpty() {
+		return nil
+	}
 
 	// TODO: Implement this.
 	if i.isSubImage() {
 		panic("ebiten: render to a subimage is not implemented (drawImage)")
 	}
 
+	if err := img.syncWithOriginalIfNeeded(); err != nil {
+		theUIContext.setError(err)
+		return nil
+	}
+	i.desyncSubImages()
+
 	// Calculate vertices before locking because the user can do anything in
 	// options.ImageParts interface without deadlock (e.g. Call Image functions).
 	if options == nil {
@@ -198,7 +222,9 @@ func (i *Image) DrawImage(img *Image, options *DrawImageOptions) error {
 	}
 
 	a, b, c, d, tx, ty := geom.elements32()
-	i.buffered.DrawImage(img.buffered, img.Bounds(), a, b, c, d, tx, ty, options.ColorM.impl, mode, filter)
+	w, h := img.Size()
+	i.buffered.DrawImage(img.buffered, image.Rect(0, 0, w, h), a, b, c, d, tx, ty, options.ColorM.impl, mode, filter)
+
 	return nil
 }
 
@@ -280,10 +306,12 @@ func (i *Image) DrawTriangles(vertices []Vertex, indices []uint16, img *Image, o
 	if img.isDisposed() {
 		panic("ebiten: the given image to DrawTriangles must not be disposed")
 	}
+	if img.isEmpty() {
+		return
+	}
 	if i.isDisposed() {
 		return
 	}
-
 	if i.isSubImage() {
 		panic("ebiten: render to a subimage is not implemented (DrawTriangles)")
 	}
@@ -296,6 +324,12 @@ func (i *Image) DrawTriangles(vertices []Vertex, indices []uint16, img *Image, o
 	}
 	// TODO: Check the maximum value of indices and len(vertices)?
 
+	if err := img.syncWithOriginalIfNeeded(); err != nil {
+		theUIContext.setError(err)
+		return
+	}
+	i.desyncSubImages()
+
 	if options == nil {
 		options = &DrawTrianglesOptions{}
 	}
@@ -309,18 +343,19 @@ func (i *Image) DrawTriangles(vertices []Vertex, indices []uint16, img *Image, o
 		filter = driver.Filter(img.filter)
 	}
 
-	b := img.Bounds()
+	w, h := img.Size()
-	bx0 := float32(b.Min.X)
+	bx0 := float32(0)
-	by0 := float32(b.Min.Y)
+	by0 := float32(0)
-	bx1 := float32(b.Max.X)
+	bx1 := float32(w)
-	by1 := float32(b.Max.Y)
+	by1 := float32(h)
 
+	dx, dy := float32(img.Bounds().Min.X), float32(img.Bounds().Min.Y)
 	vs := make([]float32, len(vertices)*graphics.VertexFloatNum)
 	for i, v := range vertices {
 		vs[i*graphics.VertexFloatNum] = v.DstX
 		vs[i*graphics.VertexFloatNum+1] = v.DstY
-		vs[i*graphics.VertexFloatNum+2] = v.SrcX
+		vs[i*graphics.VertexFloatNum+2] = v.SrcX - dx
-		vs[i*graphics.VertexFloatNum+3] = v.SrcY
+		vs[i*graphics.VertexFloatNum+3] = v.SrcY - dy
 		vs[i*graphics.VertexFloatNum+4] = bx0
 		vs[i*graphics.VertexFloatNum+5] = by0
 		vs[i*graphics.VertexFloatNum+6] = bx1
@@ -360,6 +395,8 @@ func (i *Image) DrawTrianglesWithShader(vertices []Vertex, indices []uint16, sha
 		panic("ebiten: len(indices) must be <= MaxIndicesNum")
 	}
 
+	i.desyncSubImages()
+
 	if options == nil {
 		options = &DrawTrianglesWithShaderOptions{}
 	}
@@ -374,16 +411,25 @@ func (i *Image) DrawTrianglesWithShader(vertices []Vertex, indices []uint16, sha
 			if v.isDisposed() {
 				panic("ebiten: the given image to DrawTriangles must not be disposed")
 			}
+			if v.isEmpty() {
+				// TODO: Fix this
+				panic("ebiten: zero-sized image for DrawTrianglesWithShader is not implemented so far")
+			}
+			if err := v.syncWithOriginalIfNeeded(); err != nil {
+				theUIContext.setError(err)
+				return
+			}
+
 			us = append(us, v.buffered)
 			if firstImage == nil {
 				firstImage = v
 			} else {
-				b := v.Bounds()
+				w, h := v.Size()
 				us = append(us, []float32{
-					float32(b.Min.X),
+					0,
-					float32(b.Min.Y),
+					0,
-					float32(b.Max.X),
+					float32(w),
-					float32(b.Max.Y),
+					float32(h),
 				})
 			}
 		default:
@@ -397,19 +443,20 @@ func (i *Image) DrawTrianglesWithShader(vertices []Vertex, indices []uint16, sha
 
 	var bx0, by0, bx1, by1 float32
 	if firstImage != nil {
-		b := firstImage.Bounds()
+		w, h := firstImage.Size()
-		bx0 = float32(b.Min.X)
+		bx0 = float32(0)
-		by0 = float32(b.Min.Y)
+		by0 = float32(0)
-		bx1 = float32(b.Max.X)
+		bx1 = float32(w)
-		by1 = float32(b.Max.Y)
+		by1 = float32(h)
 	}
 
+	dx, dy := float32(firstImage.Bounds().Min.X), float32(firstImage.Bounds().Min.Y)
 	vs := make([]float32, len(vertices)*graphics.VertexFloatNum)
 	for i, v := range vertices {
 		vs[i*graphics.VertexFloatNum] = v.DstX
 		vs[i*graphics.VertexFloatNum+1] = v.DstY
-		vs[i*graphics.VertexFloatNum+2] = v.SrcX
+		vs[i*graphics.VertexFloatNum+2] = v.SrcX - dx
-		vs[i*graphics.VertexFloatNum+3] = v.SrcY
+		vs[i*graphics.VertexFloatNum+3] = v.SrcY - dy
 		vs[i*graphics.VertexFloatNum+4] = bx0
 		vs[i*graphics.VertexFloatNum+5] = by0
 		vs[i*graphics.VertexFloatNum+6] = bx1
@@ -439,6 +486,8 @@ func (i *Image) SubImage(r image.Rectangle) image.Image {
 		return nil
 	}
 
+	// TODO: Check that SubImage cannot be called on a screen image.
+
 	r = r.Intersect(i.Bounds())
 	// Need to check Empty explicitly. See the standard image package implementations.
 	if r.Empty() {
@@ -451,17 +500,53 @@ func (i *Image) SubImage(r image.Rectangle) image.Image {
 		orig = i.original
 	}
 
+	if sub, ok := orig.subs[r]; ok {
+		return sub
+	}
+
+	// In the initial state, the image doesn't have its own pixels.
+	// Sync with its original image when necessary.
 	img := &Image{
-		buffered: i.buffered,
 		filter:   i.filter,
 		bounds:   r,
 		original: orig,
 	}
 	img.addr = img
+	orig.subs[img.Bounds()] = img
 
 	return img
 }
 
+func (i *Image) desyncSubImages() {
+	if i.isSubImage() {
+		panic("ebiten: desyncSubImages must be called on an original image")
+	}
+
+	for _, s := range i.subs {
+		if s.buffered == nil {
+			continue
+		}
+		s.buffered.MarkDisposed()
+		s.buffered = nil
+	}
+}
+
+func (i *Image) syncWithOriginalIfNeeded() error {
+	if !i.isSubImage() {
+		return nil
+	}
+	if i.buffered != nil {
+		return nil
+	}
+
+	x, y, width, height := i.bounds.Min.X, i.bounds.Min.Y, i.bounds.Dx(), i.bounds.Dy()
+	i.buffered = buffered.NewImage(width, height, false)
+	if err := i.buffered.CopyPixels(i.original.buffered, x, y, width, height); err != nil {
+		return err
+	}
+	return nil
+}
+
 // Bounds returns the bounds of the image.
 func (i *Image) Bounds() image.Rectangle {
 	if i.isDisposed() {
@@ -492,6 +577,11 @@ func (i *Image) At(x, y int) color.Color {
 	if !image.Pt(x, y).In(i.Bounds()) {
 		return color.RGBA{}
 	}
+
+	if i.isSubImage() {
+		return i.original.At(x, y)
+	}
+
 	pix, err := i.buffered.Pixels(x, y, 1, 1)
 	if err != nil {
 		theUIContext.setError(err)
@@ -515,20 +605,27 @@ func (i *Image) Set(x, y int, clr color.Color) {
 	if !image.Pt(x, y).In(i.Bounds()) {
 		return
 	}
+
 	if i.isSubImage() {
-		i = i.original
+		i.original.Set(x, y, clr)
+		return
 	}
 
 	r, g, b, a := clr.RGBA()
 	pix := []byte{byte(r >> 8), byte(g >> 8), byte(b >> 8), byte(a >> 8)}
 	if err := i.buffered.ReplacePixels(pix, x, y, 1, 1); err != nil {
 		theUIContext.setError(err)
+		return
 	}
+	i.desyncSubImages()
 }
 
 // Dispose disposes the image data.
 // After disposing, most of image functions do nothing and returns meaningless values.
 //
+// If the callee is a sub-image, Dispose disposes only the callee.
+// If the callee is not a sub-image, Dispose also diposes all its related sub-images.
+//
 // Calling Dispose is not mandatory. GC automatically collects internal resources that no objects refer to.
 // However, calling Dispose explicitly is helpful if memory usage matters.
 //
@@ -541,11 +638,25 @@ func (i *Image) Dispose() error {
 	if i.isDisposed() {
 		return nil
 	}
-	if i.isSubImage() {
+
-		return nil
+	// Get the bound before the image is disposed.
-	}
+	b := i.Bounds()
+
+	if i.buffered != nil {
 		i.buffered.MarkDisposed()
 		i.buffered = nil
+	}
+	i.disposed = true
+
+	// If a sub-image is disposed, dispose only this image.
+	if i.isSubImage() {
+		delete(i.original.subs, b)
+		return nil
+	}
+
+	for _, s := range i.subs {
+		s.Dispose()
+	}
 	return nil
 }
 
@@ -562,16 +673,35 @@ func (i *Image) Dispose() error {
 //
 // ReplacePixels always returns nil as of 1.5.0-alpha.
 func (i *Image) ReplacePixels(pix []byte) error {
+	// TODO: This is not very efficient. Fix this.
+	if i.isSubImage() {
+		i.original.replacePixels(pix, i.Bounds().Min.X, i.Bounds().Min.Y, i.Bounds().Dx(), i.Bounds().Dy())
+		return nil
+	}
+
+	w, h := i.Size()
+	i.replacePixels(pix, 0, 0, w, h)
+	return nil
+}
+
+func (i *Image) replacePixels(pix []byte, x, y, width, height int) {
 	i.copyCheck()
 
 	if i.isDisposed() {
-		return nil
+		return
 	}
-	r := i.Bounds()
+	if i.isEmpty() {
-	if err := i.buffered.ReplacePixels(pix, r.Min.X, r.Min.Y, r.Dx(), r.Dy()); err != nil {
+		return
+	}
+	if i.isSubImage() {
+		panic("ebiten: replacePixels must be called on an original image")
+	}
+
+	if err := i.buffered.ReplacePixels(pix, x, y, width, height); err != nil {
 		theUIContext.setError(err)
+		return
 	}
-	return nil
+	i.desyncSubImages()
 }
 
 // A DrawImageOptions represents options to render an image on an image.
@@ -627,6 +757,7 @@ func newImage(width, height int, filter Filter, volatile bool) *Image {
 		buffered: buffered.NewImage(width, height, volatile),
 		filter:   filter,
 		bounds:   image.Rect(0, 0, width, height),
+		subs:     map[image.Rectangle]*Image{},
 	}
 	i.addr = i
 	return i
@@ -649,6 +780,7 @@ func NewImageFromImage(source image.Image, filter Filter) (*Image, error) {
 		buffered: buffered.NewImage(width, height, false),
 		filter:   filter,
 		bounds:   image.Rect(0, 0, width, height),
+		subs:     map[image.Rectangle]*Image{},
 	}
 	i.addr = i
 

internal/buffered/image.go

@@ -205,7 +205,9 @@ func (i *Image) ReplacePixels(pix []byte, x, y, width, height int) error {
 		i.invalidatePendingPixels()
 
 		// Don't call (*mipmap.Mipmap).ReplacePixels here. Let's defer it to reduce GPU operations as much as
-		// posssible.
+		// posssible. This is a necessary optimization for sub-images: as sub-images are actually used and,
+		// have to allocate their region on a texture atlas, while their original image doesn't have to
+		// allocate its region on a texture atlas (#896).
 		copied := make([]byte, len(pix))
 		copy(copied, pix)
 		i.pixels = copied
@@ -234,6 +236,25 @@ func (i *Image) replacePendingPixels(pix []byte, x, y, width, height int) {
 	i.needsToResolvePixels = true
 }
 
+func (i *Image) CopyPixels(img *Image, x, y, width, height int) error {
+	if needsToDelayCommands() {
+		delayedCommands = append(delayedCommands, func() error {
+			i.CopyPixels(img, x, y, width, height)
+			return nil
+		})
+		return nil
+	}
+
+	pix, err := img.Pixels(x, y, width, height)
+	if err != nil {
+		return err
+	}
+	if err := i.ReplacePixels(pix, 0, 0, width, height); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (i *Image) DrawImage(src *Image, bounds image.Rectangle, a, b, c, d, tx, ty float32, colorm *affine.ColorM, mode driver.CompositeMode, filter driver.Filter) {
 	if i == src {
 		panic("buffered: Image.DrawImage: src must be different from the receiver")