op/paint: make every ImageOp unique

The gioui.org/commit/74407a50d598bfd27e8f8e48b6832cc5df04de77
added a NewImageOp constructor that always copies the supplied
image. It does that for two reasons:

First, the image.Image reference is used in the image=>texture
map of cached textures. Without a copy, we wouldn't detect a
modified image even if a new ImageOp was created.

Second, we don't want the program to touch the image while the GPU
is uploading it.

The second reason was removed in a previous change that blocks
FrameEvent.Frame until we're done with the operations, including
uploading images to the GPU.

The first reason is easily fixed by using a unique per ImageOp,
as pointed out by Alessandro Arzilli.

This change switches to using the unique key. Alessandro's patch
avoids the copy when possible.

Signed-off-by: Elias Naur <mail@eliasnaur.com>

app/internal/gpu/gpu.go

@@ -80,10 +80,10 @@ type drawState struct {
 	rect  bool
 	z     int
 
-	// Current ImageOp image and size, if any.
+	matType materialType
-	img     *image.RGBA
+	// Current paint.ImageOp
-	imgSize image.Point
+	image imageOpData
-	// Current ColorOp, if any.
+	// Current paint.ColorOp, if any.
 	color color.RGBA
 }
 
@@ -125,6 +125,12 @@ type clipOp struct {
 	bounds f32.Rectangle
 }
 
+// imageOpData is the shadow of paint.ImageOp.
+type imageOpData struct {
+	src    *image.RGBA
+	handle interface{}
+}
+
 func (op *clipOp) decode(data []byte) {
 	if opconst.OpType(data[0]) != opconst.TypeClip {
 		panic("invalid op")
@@ -145,29 +151,29 @@ func (op *clipOp) decode(data []byte) {
 	}
 }
 
-func decodeImageOp(data []byte, refs []interface{}) (*image.RGBA, image.Point) {
+func decodeImageOp(data []byte, refs []interface{}) imageOpData {
-	bo := binary.LittleEndian
 	if opconst.OpType(data[0]) != opconst.TypeImage {
 		panic("invalid op")
 	}
-	sz := image.Point{
+	handle := refs[1]
-		X: int(int32(bo.Uint32(data[1:]))),
+	if handle == nil {
-		Y: int(int32(bo.Uint32(data[5:]))),
+		panic("nil handle")
+	}
+	return imageOpData{
+		src:    refs[0].(*image.RGBA),
+		handle: handle,
 	}
-	return refs[0].(*image.RGBA), sz
 }
 
-func decodeColorOp(data []byte) paint.ColorOp {
+func decodeColorOp(data []byte) color.RGBA {
 	if opconst.OpType(data[0]) != opconst.TypeColor {
 		panic("invalid op")
 	}
-	return paint.ColorOp{
+	return color.RGBA{
-		Color: color.RGBA{
+		R: data[1],
-			R: data[1],
+		G: data[2],
-			G: data[2],
+		B: data[3],
-			B: data[3],
+		A: data[4],
-			A: data[4],
-		},
 	}
 }
 
@@ -746,11 +752,11 @@ loop:
 			aux = nil
 			auxKey = ops.Key{}
 		case opconst.TypeColor:
-			op := decodeColorOp(encOp.Data)
+			state.matType = materialColor
-			state.img = nil
+			state.color = decodeColorOp(encOp.Data)
-			state.color = op.Color
 		case opconst.TypeImage:
-			state.img, state.imgSize = decodeImageOp(encOp.Data, encOp.Refs)
+			state.matType = materialTexture
+			state.image = decodeImageOp(encOp.Data, encOp.Refs)
 		case opconst.TypePaint:
 			op := decodePaintOp(encOp.Data)
 			off := state.t.Transform(f32.Point{})
@@ -808,15 +814,20 @@ func expandPathOp(p *pathOp, clip image.Rectangle) {
 
 func (d *drawState) materialFor(cache *resourceCache, rect f32.Rectangle, off f32.Point, clip image.Rectangle) material {
 	var m material
-	if d.img == nil {
+	switch d.matType {
+	case materialColor:
 		m.material = materialColor
 		m.color = gamma(d.color.RGBA())
 		m.opaque = m.color[3] == 1.0
-	} else {
+	case materialTexture:
 		m.material = materialTexture
 		dr := boundRectF(rect.Add(off))
+		sz := d.image.src.Bounds().Size()
 		sr := f32.Rectangle{
-			Max: f32.Point{X: float32(d.imgSize.X), Y: float32(d.imgSize.Y)},
+			Max: f32.Point{
+				X: float32(sz.X),
+				Y: float32(sz.Y),
+			},
 		}
 		if dx := float32(dr.Dx()); dx != 0 {
 			// Don't clip 1 px width sources.
@@ -832,16 +843,16 @@ func (d *drawState) materialFor(cache *resourceCache, rect f32.Rectangle, off f3
 				sr.Max.Y -= (float32(dr.Max.Y-clip.Max.Y)*sdy + dy/2) / dy
 			}
 		}
-		tex, exists := cache.get(d.img)
+		tex, exists := cache.get(d.image.handle)
 		if !exists {
 			t := &texture{
-				src: d.img,
+				src: d.image.src,
 			}
-			cache.put(d.img, t)
+			cache.put(d.image.handle, t)
 			tex = t
 		}
 		m.texture = tex.(*texture)
-		m.uvScale, m.uvOffset = texSpaceTransform(sr, d.img.Bounds().Size())
+		m.uvScale, m.uvOffset = texSpaceTransform(sr, sz)
 	}
 	return m
 }

internal/opconst/ops.go

@@ -34,7 +34,7 @@ const (
 	TypeTransformLen    = 1 + 4*2
 	TypeLayerLen        = 1
 	TypeRedrawLen       = 1 + 8
-	TypeImageLen        = 1 + 4*4
+	TypeImageLen        = 1
 	TypePaintLen        = 1 + 4*4
 	TypeColorLen        = 1 + 4
 	TypeAreaLen         = 1 + 1 + 4*4
@@ -74,8 +74,10 @@ func (t OpType) Size() int {
 
 func (t OpType) NumRefs() int {
 	switch t {
-	case TypeMacro, TypeImage, TypeKeyInput, TypePointerInput, TypeProfile:
+	case TypeMacro, TypeKeyInput, TypePointerInput, TypeProfile:
 		return 1
+	case TypeImage:
+		return 2
 	default:
 		return 0
 	}

op/paint/paint.go

@@ -20,6 +20,10 @@ type ImageOp struct {
 	color   color.RGBA
 	src     *image.RGBA
 	size    image.Point
+
+	// handle is a key to uniquely identify this ImageOp
+	// in a map of cached textures.
+	handle interface{}
 }
 
 // ColorOp sets the material to a constant color.
@@ -49,8 +53,9 @@ func NewImageOp(src image.Image) ImageOp {
 		})
 		draw.Draw(dst, src.Bounds(), src, image.Point{}, draw.Src)
 		return ImageOp{
-			src:  dst,
+			src:    dst,
-			size: sz,
+			size:   sz,
+			handle: new(int),
 		}
 	}
 }
@@ -66,11 +71,8 @@ func (i ImageOp) Add(o *op.Ops) {
 		}.Add(o)
 		return
 	}
-	data := o.Write(opconst.TypeImageLen, i.src)
+	data := o.Write(opconst.TypeImageLen, i.src, i.handle)
 	data[0] = byte(opconst.TypeImage)
-	bo := binary.LittleEndian
-	bo.PutUint32(data[1:], uint32(i.size.X))
-	bo.PutUint32(data[5:], uint32(i.size.Y))
 }
 
 func (c ColorOp) Add(o *op.Ops) {