Intersections: finish making the debug capture opt-in, follow-up to #387

path_intersection.go

@@ -1556,14 +1556,7 @@ func (squares toleranceSquares) breakupCrossingSegments(n int, x float64) {
 					if square.Upper == nil {
 						if DebugPathIntersection {
 							// TODO: this happens sporadically, add to unit tests
-							w, err := os.CreateTemp("", "canvas-testcase-*.gob")
-							if err != nil {
-								log.Println("ERROR:", err)
-							} else {
-								gob.NewEncoder(w).Encode([]any{_ps, _qs, _op, _fillRule})
-								w.Close()
-							}
-							log.Println("NOTE: new test case written to", w.Name())
+							writePathIntersectionTestCase()
 						}
 						square.Upper = prev
 					}
@@ -1795,6 +1788,40 @@ var _ps, _qs Paths
 var _op pathOp
 var _fillRule FillRule
 
+// pathIntersectionTestCase is the input to a boolean path operation, as captured by
+// DebugPathIntersection. Its fields are exported because encoding/gob only encodes
+// exported fields; the type itself need not be.
+//
+// It is a concrete struct rather than the []any it used to be: gob cannot encode a
+// value through an interface unless its concrete type has been registered, so the
+// []any silently produced a 12-byte file holding nothing but a type descriptor —
+// Encode's error was dropped along with it. Decode one from inside this package with
+//
+//	var tc pathIntersectionTestCase
+//	err := gob.NewDecoder(f).Decode(&tc)
+type pathIntersectionTestCase struct {
+	Ps, Qs   Paths
+	Op       pathOp
+	FillRule FillRule
+}
+
+// writePathIntersectionTestCase saves the current boolean operation's input to a
+// temporary file, for the edge cases DebugPathIntersection exists to collect.
+func writePathIntersectionTestCase() {
+	w, err := os.CreateTemp("", "canvas-testcase-*.gob")
+	if err != nil {
+		log.Println("ERROR: could not create test case file:", err)
+		return
+	}
+	defer w.Close()
+	tc := pathIntersectionTestCase{Ps: _ps, Qs: _qs, Op: _op, FillRule: _fillRule}
+	if err := gob.NewEncoder(w).Encode(tc); err != nil {
+		log.Println("ERROR: could not write test case:", err)
+		return
+	}
+	log.Println("NOTE: new test case written to", w.Name())
+}
+
 func bentleyOttmann(ps, qs Paths, op pathOp, fillRule FillRule) Paths {
 	// TODO: add grid spacing argument
 	// TODO: add Intersects/Touches functions (return bool)
@@ -1810,7 +1837,11 @@ func bentleyOttmann(ps, qs Paths, op pathOp, fillRule FillRule) Paths {
 	// TODO: if overlapping segments can be detected earlier, we can just process left-events
 	//       and make the code simpler
 
-	_ps, _qs, _op, _fillRule = ps, qs, op, fillRule
+	// Only in debugging mode: these are package globals, so writing them
+	// unconditionally makes any two goroutines doing boolean path operations race.
+	if DebugPathIntersection {
+		_ps, _qs, _op, _fillRule = ps, qs, op, fillRule
+	}
 
 	// Implementation of the Bentley-Ottmann algorithm by reducing the complexity of finding
 	// intersections to O((n + k) log n), with n the number of segments and k the number of

path_intersection_test.go

@@ -1,8 +1,12 @@
 package canvas
 
 import (
+	"encoding/gob"
 	"fmt"
 	"math"
+	"os"
+	"path/filepath"
+	"sync"
 	"testing"
 	"time"
 
@@ -1553,3 +1557,65 @@ func TestPathRelate(t *testing.T) {
 	test.That(t, !MustParseSVGPath("L10 0L10 10L0 10z").Touches(MustParseSVGPath("")))
 	test.That(t, !MustParseSVGPath("L10 0L10 10L0 10z").Touches(MustParseSVGPath("M20 0L30 0L30 10L20 10z")))
 }
+
+func TestPathIntersectionTestCase(t *testing.T) {
+	// The case DebugPathIntersection captures must be readable back, or the files it
+	// asks users to send carry nothing. Encoding a []any of these values cannot work:
+	// gob will not encode a value through an interface whose concrete type has not
+	// been registered, so it failed and left only a 12-byte type descriptor on disk.
+	dir := t.TempDir()
+	t.Setenv("TMPDIR", dir)
+
+	_ps = Paths{MustParseSVGPath("M0 0H10V10H0z")}
+	_qs = Paths{MustParseSVGPath("M5 5H15V15H5z")}
+	_op, _fillRule = opAND, EvenOdd
+	defer func() { _ps, _qs, _op, _fillRule = nil, nil, opSettle, NonZero }()
+
+	writePathIntersectionTestCase()
+
+	entries, err := os.ReadDir(dir)
+	test.Error(t, err)
+	test.T(t, len(entries), 1)
+
+	f, err := os.Open(filepath.Join(dir, entries[0].Name()))
+	test.Error(t, err)
+	defer f.Close()
+
+	var got pathIntersectionTestCase
+	test.Error(t, gob.NewDecoder(f).Decode(&got))
+	test.T(t, len(got.Ps), 1)
+	test.T(t, len(got.Qs), 1)
+	test.T(t, got.Ps[0].String(), _ps[0].String())
+	test.T(t, got.Qs[0].String(), _qs[0].String())
+	test.T(t, got.Op, opAND)
+	test.T(t, got.FillRule, EvenOdd)
+}
+
+func TestPathIntersectionTestCaseNoTempDir(t *testing.T) {
+	// A temporary directory that cannot be written must be reported, not dereferenced:
+	// os.CreateTemp returns a nil file with its error, and (*os.File).Name has no nil
+	// check, so reporting the name outside the error branch panics inside a drawing
+	// call.
+	t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "does-not-exist"))
+	writePathIntersectionTestCase() // must not panic
+}
+
+func TestPathIntersectionConcurrent(t *testing.T) {
+	// Boolean operations must not share mutable package state: two goroutines doing
+	// them concurrently is ordinary use for a library. Run under -race this fails if
+	// bentleyOttmann writes its debugging globals unconditionally.
+	p := MustParseSVGPath("M0 0H10V10H0z")
+	q := MustParseSVGPath("M5 5H15V15H5z")
+
+	var wg sync.WaitGroup
+	for i := 0; i < 2; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < 20; j++ {
+				p.And(q)
+			}
+		}()
+	}
+	wg.Wait()
+}