From 884e7d27e2283bf7254e8bf68be53e20634cb081 Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Thu, 11 Mar 2021 08:09:07 +0100 Subject: [PATCH] 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 --- gpu/gpu.go | 22 ---------------------- gpu/internal/rendertest/clip_test.go | 2 ++ op/clip/clip.go | 21 +++++++++++++++------ op/clip/clip_test.go | 22 ++++++++++++++++++++++ 4 files changed, 39 insertions(+), 28 deletions(-) create mode 100644 op/clip/clip_test.go diff --git a/gpu/gpu.go b/gpu/gpu.go index 41604fa0..c5e02590 100644 --- a/gpu/gpu.go +++ b/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:]) diff --git a/gpu/internal/rendertest/clip_test.go b/gpu/internal/rendertest/clip_test.go index 20696726..a9b86d3f 100644 --- a/gpu/internal/rendertest/clip_test.go +++ b/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) diff --git a/op/clip/clip.go b/op/clip/clip.go index 9a9c3088..d32da87b 100644 --- a/op/clip/clip.go +++ b/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, diff --git a/op/clip/clip_test.go b/op/clip/clip_test.go new file mode 100644 index 00000000..8021fc2f --- /dev/null +++ b/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() +}