From 11bb86166a5b29eb95f1180d3add751fae4e7edc Mon Sep 17 00:00:00 2001
From: Pierre Curto <pierre.curto@gmail.com>
Date: Sun, 19 Dec 2021 17:09:54 +0100
Subject: [PATCH] op/clip: automatically close Path in Outlines

Unclosed path segments in Path will be automatically
closed by a line.

Fixes: https://todo.sr.ht/~eliasnaur/gio/320
Signed-off-by: Pierre Curto <pierre.curto@gmail.com>
---
 gpu/compute.go                                |  15 +++--
 gpu/gpu.go                                    |   6 ++
 .../refs/TestGapsInPath/Outline.png           | Bin 0 -> 408 bytes
 .../rendertest/refs/TestGapsInPath/Stroke.png | Bin 0 -> 488 bytes
 gpu/internal/rendertest/render_test.go        |  54 ++++++++++++++++++
 gpu/internal/rendertest/util_test.go          |  16 +++++-
 internal/scene/scene.go                       |  25 +++++++-
 internal/stroke/stroke.go                     |   2 +
 op/clip/clip.go                               |  22 ++++---
 op/clip/clip_test.go                          |  11 ----
 10 files changed, 123 insertions(+), 28 deletions(-)
 create mode 100644 gpu/internal/rendertest/refs/TestGapsInPath/Outline.png
 create mode 100644 gpu/internal/rendertest/refs/TestGapsInPath/Stroke.png

diff --git a/gpu/compute.go b/gpu/compute.go
index ee6e127c..625658c7 100644
--- a/gpu/compute.go
+++ b/gpu/compute.go
@@ -1186,12 +1186,19 @@ func min(p1, p2 f32.Point) f32.Point {
 	return p
 }
 
-func (enc *encoder) encodePath(verts []byte) {
-	for len(verts) >= scene.CommandSize+4 {
+func (enc *encoder) encodePath(verts []byte, fillMode int) {
+	for ; len(verts) >= scene.CommandSize+4; verts = verts[scene.CommandSize+4:] {
 		cmd := ops.DecodeCommand(verts[4:])
+		if cmd.Op() == scene.OpGap {
+			if fillMode != scene.FillModeNonzero {
+				// Skip gaps in strokes.
+				continue
+			}
+			// Replace them by a straight line in outlines.
+			cmd = scene.Line(scene.DecodeGap(cmd))
+		}
 		enc.scene = append(enc.scene, cmd)
 		enc.npathseg++
-		verts = verts[scene.CommandSize+4:]
 	}
 }
 
@@ -2109,7 +2116,7 @@ func encodeOp(viewport image.Point, absOff image.Point, enc *encoder, texOps []t
 		if len(cl.path) == 0 {
 			enc.rect(cl.state.bounds)
 		} else {
-			enc.encodePath(cl.path)
+			enc.encodePath(cl.path, fillMode)
 		}
 		if i != 0 {
 			enc.beginClip(cl.union.Add(absOfff))
diff --git a/gpu/gpu.go b/gpu/gpu.go
index dd2161d2..0bd15c69 100644
--- a/gpu/gpu.go
+++ b/gpu/gpu.go
@@ -1344,6 +1344,12 @@ func decodeToOutlineQuads(qs *quadSplitter, tr f32.Affine2D, pathData []byte) {
 			q.Ctrl = q.From.Add(q.To).Mul(.5)
 			q = q.Transform(tr)
 			qs.splitAndEncode(q)
+		case scene.OpGap:
+			var q stroke.QuadSegment
+			q.From, q.To = scene.DecodeGap(cmd)
+			q.Ctrl = q.From.Add(q.To).Mul(.5)
+			q = q.Transform(tr)
+			qs.splitAndEncode(q)
 		case scene.OpQuad:
 			var q stroke.QuadSegment
 			q.From, q.Ctrl, q.To = scene.DecodeQuad(cmd)
diff --git a/gpu/internal/rendertest/refs/TestGapsInPath/Outline.png b/gpu/internal/rendertest/refs/TestGapsInPath/Outline.png
new file mode 100644
index 0000000000000000000000000000000000000000..aa0a7749307cffe20070cb4ef5d1e2a78fe15571
GIT binary patch
literal 408
zcmeAS@N?(olHy`uVBq!ia0vp^4Is?H1|$#LC7xzrV2t;4aSW-L^XAGy&ISVk)<DtU
z`H_meFElyNROc}MH+WO~=S$D*_cGsezt`^G6~>>S%XoxS!H=PksYBRd4#OiB0rdt8
r(u5j}<S8?NY@6w;@f|M%!~g$+oUDvGmdS>|5NGgo^>bP0l+XkKfva(K

literal 0
HcmV?d00001

diff --git a/gpu/internal/rendertest/refs/TestGapsInPath/Stroke.png b/gpu/internal/rendertest/refs/TestGapsInPath/Stroke.png
new file mode 100644
index 0000000000000000000000000000000000000000..1420f6b7ae73562e325f458e595e1a3501ab1c3b
GIT binary patch
literal 488
zcmeAS@N?(olHy`uVBq!ia0vp^4Is?H1|$#LC7xzrU|jF%;uumf=gn<LFKtH&w~PN<
zx2xU#e`byI^@(B!7fz6J+4aaaJtjc@WUJiO|B(e}V$z?bK7F%T*7$hpv!dr`uKk{O
z@A#Sh#_#_AD$F<DUj4l6{`SrLWA(~875o?qnL30W<}f^B5m0ZiU_3&Ckazy8zM0!+
zR{s5KX@76;CySETwbs$Hb*CPme&4-5`}nzyJKpcyJ3r#b#t+Zm?c?9}b<2G{IcEy&
kBio_$)41b2Fw*}2-xIGOWnGxr4&*R+y85}Sb4q9e0MrVs)c^nh

literal 0
HcmV?d00001

diff --git a/gpu/internal/rendertest/render_test.go b/gpu/internal/rendertest/render_test.go
index 9ba14799..5809ca63 100644
--- a/gpu/internal/rendertest/render_test.go
+++ b/gpu/internal/rendertest/render_test.go
@@ -357,6 +357,60 @@ func TestImageRGBA(t *testing.T) {
 	})
 }
 
+func TestGapsInPath(t *testing.T) {
+	ops := new(op.Ops)
+	var p clip.Path
+	p.Begin(ops)
+	// Unclosed square 1
+	p.MoveTo(f32.Point{X: 10})
+	p.LineTo(f32.Point{X: 40})
+	p.LineTo(f32.Point{X: 40, Y: 30})
+	p.LineTo(f32.Point{X: 10, Y: 30})
+
+	// Unclosed square 2
+	p.MoveTo(f32.Point{X: 50})
+	p.LineTo(f32.Point{X: 80})
+	p.LineTo(f32.Point{X: 80, Y: 30})
+	p.LineTo(f32.Point{X: 50, Y: 30})
+
+	spec := p.End()
+
+	t.Run("Stroke", func(t *testing.T) {
+		run(t,
+			func(ops *op.Ops) {
+				stack := clip.Stroke{
+					Path:  spec,
+					Width: 2,
+				}.Op().Push(ops)
+				paint.ColorOp{Color: color.NRGBA{R: 255, A: 255}}.Add(ops)
+				paint.PaintOp{}.Add(ops)
+				stack.Pop()
+			},
+			func(r result) {
+				r.expect(10, 20, color.RGBA{})
+				r.expect(50, 20, color.RGBA{})
+			},
+		)
+	})
+
+	t.Run("Outline", func(t *testing.T) {
+		run(t,
+			func(ops *op.Ops) {
+				stack := clip.Outline{Path: spec}.Op().Push(ops)
+				paint.ColorOp{Color: color.NRGBA{R: 255, A: 255}}.Add(ops)
+				paint.PaintOp{}.Add(ops)
+				stack.Pop()
+			},
+			func(r result) {
+				r.expect(10, 20, colornames.Red)
+				r.expect(20, 20, colornames.Red)
+				r.expect(50, 20, colornames.Red)
+				r.expect(60, 20, colornames.Red)
+			},
+		)
+	})
+}
+
 // lerp calculates linear interpolation with color b and p.
 func lerp(a, b f32color.RGBA, p float32) f32color.RGBA {
 	return f32color.RGBA{
diff --git a/gpu/internal/rendertest/util_test.go b/gpu/internal/rendertest/util_test.go
index 6f3f688c..a19f3026 100644
--- a/gpu/internal/rendertest/util_test.go
+++ b/gpu/internal/rendertest/util_test.go
@@ -11,6 +11,7 @@ import (
 	"image/draw"
 	"image/png"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strconv"
 	"testing"
@@ -143,11 +144,20 @@ func multiRun(t *testing.T, frames ...frameT) {
 
 func verifyRef(t *testing.T, img *image.RGBA, frame int) (ok bool) {
 	// ensure identical to ref data
-	path := filepath.Join("refs", t.Name()+".png")
-	if frame != 0 {
-		path = filepath.Join("refs", t.Name()+"_"+strconv.Itoa(frame)+".png")
+	var path string
+	if frame == 0 {
+		path = t.Name()
+	} else {
+		path = t.Name() + "_" + strconv.Itoa(frame)
 	}
+	path = filepath.Join("refs", path+".png")
 	if *dumpImages {
+		if err := os.MkdirAll(filepath.Dir(path), 0766); err != nil {
+			if !os.IsExist(err) {
+				t.Error(err)
+				return
+			}
+		}
 		saveImage(t, path, img)
 		return true
 	}
diff --git a/internal/scene/scene.go b/internal/scene/scene.go
index 21fd92cb..03588587 100644
--- a/internal/scene/scene.go
+++ b/internal/scene/scene.go
@@ -18,7 +18,7 @@ type Op uint32
 
 type Command [sceneElemSize / 4]uint32
 
-// GPU commands from scene.h
+// GPU commands from piet/scene.h in package gioui.org/shaders.
 const (
 	OpNop Op = iota
 	OpLine
@@ -31,6 +31,7 @@ const (
 	OpEndClip
 	OpFillImage
 	OpSetFillMode
+	OpGap
 )
 
 // FillModes, from setup.h.
@@ -56,6 +57,9 @@ func (c Command) String() string {
 	case OpLine:
 		from, to := DecodeLine(c)
 		return fmt.Sprintf("line(%v, %v)", from, to)
+	case OpGap:
+		from, to := DecodeLine(c)
+		return fmt.Sprintf("gap(%v, %v)", from, to)
 	case OpQuad:
 		from, ctrl, to := DecodeQuad(c)
 		return fmt.Sprintf("quad(%v, %v, %v)", from, ctrl, to)
@@ -107,6 +111,16 @@ func Line(start, end f32.Point) Command {
 	}
 }
 
+func Gap(start, end f32.Point) Command {
+	return Command{
+		0: uint32(OpGap),
+		1: math.Float32bits(start.X),
+		2: math.Float32bits(start.Y),
+		3: math.Float32bits(end.X),
+		4: math.Float32bits(end.Y),
+	}
+}
+
 func Cubic(start, ctrl0, ctrl1, end f32.Point) Command {
 	return Command{
 		0: uint32(OpCubic),
@@ -206,6 +220,15 @@ func DecodeLine(cmd Command) (from, to f32.Point) {
 	return
 }
 
+func DecodeGap(cmd Command) (from, to f32.Point) {
+	if cmd[0] != uint32(OpGap) {
+		panic("invalid command")
+	}
+	from = f32.Pt(math.Float32frombits(cmd[1]), math.Float32frombits(cmd[2]))
+	to = f32.Pt(math.Float32frombits(cmd[3]), math.Float32frombits(cmd[4]))
+	return
+}
+
 func DecodeQuad(cmd Command) (from, ctrl, to f32.Point) {
 	if cmd[0] != uint32(OpQuad) {
 		panic("invalid command")
diff --git a/internal/stroke/stroke.go b/internal/stroke/stroke.go
index f60dab06..f7cad17d 100644
--- a/internal/stroke/stroke.go
+++ b/internal/stroke/stroke.go
@@ -637,6 +637,8 @@ func decodeToStrokeQuads(pathData []byte) StrokeQuads {
 				Quad:    q,
 			}
 			quads = append(quads, quad)
+		case scene.OpGap:
+			// Ignore gaps for strokes.
 		case scene.OpQuad:
 			var q QuadSegment
 			q.From, q.Ctrl, q.To = scene.DecodeQuad(cmd)
diff --git a/op/clip/clip.go b/op/clip/clip.go
index ed8ef178..5b4dad09 100644
--- a/op/clip/clip.go
+++ b/op/clip/clip.go
@@ -90,9 +90,6 @@ func (s Stack) Pop() {
 
 type PathSpec struct {
 	spec op.CallOp
-	// open is true if any path contour is not closed. A closed contour starts
-	// and ends in the same point.
-	open bool
 	// hasSegments tracks whether there are any segments in the path.
 	hasSegments bool
 	bounds      image.Rectangle
@@ -109,7 +106,6 @@ type PathSpec struct {
 // data is stored directly in the Ops list supplied to Begin.
 type Path struct {
 	ops         *ops.Ops
-	open        bool
 	contour     int
 	pen         f32.Point
 	macro       op.MacroOp
@@ -136,10 +132,10 @@ func (p *Path) Begin(o *op.Ops) {
 
 // End returns a PathSpec ready to use in clipping operations.
 func (p *Path) End() PathSpec {
+	p.gap()
 	c := p.macro.Stop()
 	return PathSpec{
 		spec:        c,
-		open:        p.open || p.pen != p.start,
 		hasSegments: p.hasSegments,
 		bounds:      boundRectF(p.bounds),
 		hash:        p.hash.Sum64(),
@@ -157,12 +153,23 @@ func (p *Path) MoveTo(to f32.Point) {
 	if p.pen == to {
 		return
 	}
-	p.open = p.open || p.pen != p.start
+	p.gap()
 	p.end()
 	p.pen = to
 	p.start = to
 }
 
+func (p *Path) gap() {
+	if p.pen != p.start {
+		// A closed contour starts and ends in the same point.
+		// This move creates a gap in the contour, register it.
+		data := ops.Write(p.ops, scene.CommandSize+4)
+		bo := binary.LittleEndian
+		bo.PutUint32(data[0:], uint32(p.contour))
+		p.cmd(data[4:], scene.Gap(p.pen, p.start))
+	}
+}
+
 // end completes the current contour.
 func (p *Path) end() {
 	p.contour++
@@ -331,9 +338,6 @@ type Outline struct {
 
 // Op returns a clip operation representing the outline.
 func (o Outline) Op() Op {
-	if o.Path.open {
-		panic("not all path contours are closed")
-	}
 	return Op{
 		path:    o.Path,
 		outline: true,
diff --git a/op/clip/clip_test.go b/op/clip/clip_test.go
index 405c6552..4d32b98e 100644
--- a/op/clip/clip_test.go
+++ b/op/clip/clip_test.go
@@ -15,17 +15,6 @@ import (
 )
 
 func TestPathOutline(t *testing.T) {
-	t.Run("unclosed path", func(t *testing.T) {
-		defer func() {
-			if err := recover(); err == nil {
-				t.Error("Outline of an open path didn't panic")
-			}
-		}()
-		var p clip.Path
-		p.Begin(new(op.Ops))
-		p.Line(f32.Pt(10, 10))
-		clip.Outline{Path: p.End()}.Op()
-	})
 	t.Run("closed path", func(t *testing.T) {
 		defer func() {
 			if err := recover(); err != nil {