op/clip: don't accept open Paths for Outline

Outline represents a clipping operations that clips all drawing outside
a closed path. Before this change, paths not closed we're patched up by
adding an implicit line from the endpoint to the beginning.

These fixups are inefficient for a rare case, but acceptable because the
old renderer post-processes all paths anyway. However, the new compute
renderer don't need post-processing in most cases, making fixups too
expensive.

Given that clipping to an open path is fundamentally undefined and that
implicit fixup with a closing line segment is merely a way to force the
clip to be well-defined, this change adds a panic to Outline for Paths
that are not closed.

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

gpu/gpu.go

@@ -1385,37 +1385,15 @@ func (d *drawOps) buildVerts(aux []byte, tr f32.Affine2D, outline bool, stroke c
 
 	case outline:
 		// Outline path.
-		first := true
-		var firstPt, lastPt f32.Point
 		for len(aux) >= ops.QuadSize+4 {
 			d.qs.contour = bo.Uint32(aux)
 			quad := ops.DecodeQuad(aux[4:])
 			quad = quad.Transform(tr)
 
-			if first {
-				first = false
-				firstPt = quad.From
-				lastPt = quad.From
-			}
-			if quad.From != lastPt {
-				if lastPt != firstPt {
-					// Close outline before starting a new.
-					mid := firstPt.Add(lastPt).Mul(.5)
-					d.qs.splitAndEncode(ops.Quad{From: lastPt, To: firstPt, Ctrl: mid})
-				}
-				firstPt = quad.From
-			}
-			lastPt = quad.To
-
 			d.qs.splitAndEncode(quad)
 
 			aux = aux[ops.QuadSize+4:]
 		}
-		// Close last outline if necessary.
-		if !first && lastPt != firstPt {
-			mid := firstPt.Add(lastPt).Mul(.5)
-			d.qs.splitAndEncode(ops.Quad{From: lastPt, To: firstPt, Ctrl: mid})
-		}
 	}
 
 	fillMaxY(d.vertCache[startLength:])

gpu/internal/rendertest/clip_test.go

@@ -91,6 +91,7 @@ func TestPaintArc(t *testing.T) {
 		p.Arc(f32.Pt(-10, -20), f32.Pt(10, -5), math.Pi)
 		p.Line(f32.Pt(0, -10))
 		p.Line(f32.Pt(-50, 0))
+		p.Close()
 		clip.Outline{
 			Path: p.End(),
 		}.Op().Add(o)
@@ -112,6 +113,7 @@ func TestPaintAbsolute(t *testing.T) {
 		p.MoveTo(f32.Pt(20, 20))
 		p.LineTo(f32.Pt(80, 20))
 		p.QuadTo(f32.Pt(80, 80), f32.Pt(20, 80))
+		p.Close()
 		clip.Outline{
 			Path: p.End(),
 		}.Op().Add(o)

op/clip/clip.go

@@ -65,8 +65,11 @@ func (p Op) Add(o *op.Ops) {
 }
 
 type PathSpec struct {
-	spec  op.CallOp
-	quads uint32 // quads is the number Bézier segments in that path.
+	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
+	quads uint32 // quads is the number Bézier segments in the path.
 }
 
 // Path constructs a Op clip path described by lines and
@@ -78,6 +81,7 @@ type PathSpec struct {
 // data is stored directly in the Ops list supplied to Begin.
 type Path struct {
 	ops     *op.Ops
+	open    bool
 	contour int
 	pen     f32.Point
 	macro   op.MacroOp
@@ -102,6 +106,7 @@ func (p *Path) End() PathSpec {
 	c := p.macro.Stop()
 	return PathSpec{
 		spec:  c,
+		open:  p.open || p.pen != p.start,
 		quads: p.quads,
 	}
 }
@@ -114,6 +119,7 @@ func (p *Path) Move(delta f32.Point) {
 
 // MoveTo moves the pen to the specified absolute coordinate.
 func (p *Path) MoveTo(to f32.Point) {
+	p.open = p.open || p.pen != p.start
 	p.end()
 	p.pen = to
 	p.start = to
@@ -121,9 +127,6 @@ func (p *Path) MoveTo(to f32.Point) {
 
 // end completes the current contour.
 func (p *Path) end() {
-	if p.pen != p.start {
-		p.LineTo(p.start)
-	}
 	p.contour++
 }
 
@@ -369,8 +372,11 @@ func (p *Path) approxCubeTo(splits int, maxDist float32, ctrl0, ctrl1, to f32.Po
 	return splits
 }
 
-// Close closes the path.
+// Close closes the path contour.
 func (p *Path) Close() {
+	if p.pen != p.start {
+		p.LineTo(p.start)
+	}
 	p.end()
 }
 
@@ -382,6 +388,9 @@ 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,

op/clip/clip_test.go

@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: Unlicense OR MIT
+
+package clip
+
+import (
+	"testing"
+
+	"gioui.org/f32"
+	"gioui.org/op"
+)
+
+func TestOpenPathOutlinePanic(t *testing.T) {
+	defer func() {
+		if err := recover(); err == nil {
+			t.Error("Outline of an open path didn't panic")
+		}
+	}()
+	var p Path
+	p.Begin(new(op.Ops))
+	p.Line(f32.Pt(10, 10))
+	Outline{Path: p.End()}.Op()
+}