Merge pull request #864 from pharo-graphics/852-FlattenBezier

Flatten bezier when first or last pairs of control points are too close

Author: tinchodias

src/Bloc-Alexandrie-Tests/BlSpaceFixture.class.st

@@ -1872,6 +1872,28 @@ BlSpaceFixture >> buildVariantsOfBezier4CPs [
 		(BlBezierCurveGeometry controlPoints: { 5@0. 15@30. 25@10. 35@40 })
 ]
 
+{ #category : #fixtures }
+BlSpaceFixture >> buildVariantsOfBezier4CPsWithCloseFirstAndSecond [
+	"Regression test for the stroke-outline self-intersection fix.
+	The first two control points (P0 and P1) are separated by only 1 unit,
+	much less than the stroke half-width (5), so the De Casteljau safe path
+	is activated at the start of the curve."
+
+	self buildVariantsOfLinelikeGeometry:
+		(BlBezierCurveGeometry controlPoints: { 5@0. 5@1. 25@10. 35@40 })
+]
+
+{ #category : #fixtures }
+BlSpaceFixture >> buildVariantsOfBezier4CPsWithCloseThirdAndFourth [
+	"Regression test for the stroke-outline self-intersection fix.
+	The last two control points (P2 and P3) are separated by only 1 unit,
+	much less than the stroke half-width (5), so the De Casteljau safe path
+	is activated at the end of the curve."
+
+	self buildVariantsOfLinelikeGeometry:
+		(BlBezierCurveGeometry controlPoints: { 5@0. 15@30. 35@39. 35@40 })
+]
+
 { #category : #fixtures }
 BlSpaceFixture >> buildVariantsOfCircleSector [
 

src/Bloc-Alexandrie/BlBezierCurveGeometry.extension.st

@@ -4,58 +4,65 @@ Extension { #name : #BlBezierCurveGeometry }
 BlBezierCurveGeometry >> aeApplyTo: aeCanvas element: aBlElement [
 
 	aeCanvas pathFactory: [ :cairoContext |
-		self aeDrawAccordingToControlPointsOn: cairoContext ]
-
+		self aeDrawAccordingToControlPointsOn: cairoContext strokeWidth: aBlElement border width ]
 ]
 
 { #category : #'*Bloc-Alexandrie' }
-BlBezierCurveGeometry >> aeDrawAccordingToControlPointsOn: cairoContext [
-	"Based on RSAthensRenderer>>#buildBezierPath:"
+BlBezierCurveGeometry >> aeDrawAccordingToControlPointsOn: cairoContext strokeWidth: strokeWidth [
+	"Based on RSAthensRenderer>>#buildBezierPath:
+
+	For the cubic (4-point) case we choose between two rendering strategies:
+
+	  REGULAR PATH  — cairo_curve_to (cairo_curve_to).
+	    Cairo's native bezier stroking is fast and correct when the local
+	    radius of curvature at both endpoints is larger than the stroke
+	    half-width.
 
-	controlPoints size = 2 ifTrue: [ 
+	  SAFE PATH  — De Casteljau polyline (flattenedCubicVerticesDo:).
+	    When one of the end control segments (P0→P1 or P2→P3) is shorter
+	    than halfWidth, the radius of curvature at that endpoint is smaller
+	    than the stroke radius.  Cairo then builds a self-intersecting stroke
+	    outline polygon, which it fills incorrectly — producing a visible blob
+	    or fin artifact.  Emitting explicit lineTo: calls instead makes Cairo
+	    use its polyline stroking code, which handles inner corners with a
+	    concave vertex join and never self-intersects."
+
+	controlPoints size = 2 ifTrue: [
 		cairoContext
 			lineFrom: controlPoints first
 			to: controlPoints second.
 		^ self ].
 
-	controlPoints size = 3 ifTrue: [ 
+	controlPoints size = 3 ifTrue: [
 		cairoContext
 			moveTo: controlPoints first;
 			quadraticCurveFromCurrentPoint: controlPoints first
 				via: controlPoints second
 				to: controlPoints third.
 		^ self ].
 
-	controlPoints size = 4 ifTrue: [ 
-		cairoContext
-			moveTo: controlPoints first;
-			curveVia: controlPoints second
-				  via: controlPoints third
-				   to: controlPoints fourth.
+	controlPoints size = 4 ifTrue: [
+		| p0 p1 p2 p3 halfWidth |
+		p0 := controlPoints first.
+		p1 := controlPoints second.
+		p2 := controlPoints third.
+		p3 := controlPoints fourth.
+		halfWidth := strokeWidth / 2.0.
+
+		((p1 distanceTo: p0) < halfWidth
+			or: [ (p2 distanceTo: p3) < halfWidth ])
+			ifTrue: [
+				"Safe path: De Casteljau flattening avoids self-intersecting stroke outline."
+				self flattenedCubicVerticesDo: [ :x :y |
+					cairoContext lineToX: x y: y ] ]
+			ifFalse: [
+				"Regular path: Cairo's native bezier stroke is correct here."
+				cairoContext
+					moveTo: p0;
+					curveVia: p1
+					      via: p2
+					       to: p3 ].
 		^ self ].
 
 	self notYetImplemented
 ]
-
-{ #category : #'*Bloc-Alexandrie' }
-BlBezierCurveGeometry >> asPolylineVertices [
-
-	| c b a via1 via2 start end |
-	controlPoints size = 2 ifTrue: [ 
-		^ Array
-			with: controlPoints first
-			with: controlPoints second ].
-
-	start := controlPoints first.
-	via1 := controlPoints second.
-	via2 := controlPoints third.
-	end := controlPoints size = 3
-       ifTrue: [ via2 ]
-       ifFalse: [ controlPoints fourth ].
-	c := 3 * (via1 - start).
-	b := 3 * (via2 - via1) - c.
-	a := end - start - c - b.
-
-	^ (0.0 to: 1.0 by: 0.2) collect: [ :t | 
-		  a * (t raisedTo: 3) + (b * (t * t)) + (c * t) + start ]
-]

src/Bloc/BlBezierCurveGeometry.class.st

@@ -25,6 +25,43 @@ BlBezierCurveGeometry >> = anObject [
 	^ self controlPoints = anObject controlPoints
 ]
 
+{ #category : #geometry }
+BlBezierCurveGeometry >> asPolylineVertices [
+	"Answer an Array of Points approximating the curve, for hit-testing.
+
+	For the cubic (4-point) case this reuses the same De Casteljau-flattened
+	cache that the renderer uses, so both share a single computation and a
+	single allocation.
+
+	For the quadratic (3-point) case a coarse uniform sample is sufficient
+	for hit-testing purposes."
+
+	| c b a via1 via2 start end |
+
+	controlPoints size = 2 ifTrue: [
+		^ Array
+			with: controlPoints first
+			with: controlPoints second ].
+
+	controlPoints size = 4 ifTrue: [
+		| result |
+		result := OrderedCollection new.
+		self flattenedCubicVerticesDo: [ :x :y | result add: x @ y ].
+		^ result asArray ].
+
+	"size = 3: treat as degenerate cubic (CP2 = CP3) and sample uniformly."
+	start := controlPoints first.
+	via1 := controlPoints second.
+	via2 := controlPoints third.
+	end := via2.
+	c := 3 * (via1 - start).
+	b := 3 * (via2 - via1) - c.
+	a := end - start - c - b.
+
+	^ #(0.0 0.2 0.4 0.6 0.8 1.0) collect: [ :t |
+		a * (t raisedTo: 3) + (b * (t * t)) + (c * t) + start ]
+]
+
 { #category : #'geometry testing' }
 BlBezierCurveGeometry >> containsPoint: aPoint alreadyInGeometryBoundsOf: aBlElement [
 	"Answer whether aPoint is contained by this geometry.
@@ -33,10 +70,10 @@ BlBezierCurveGeometry >> containsPoint: aPoint alreadyInGeometryBoundsOf: aBlEle
 
 	| lines ir |
 	ir := aBlElement border width / 2.
-	lines := self asPolylineVertices. "TODO: cache"
+	lines := self asPolylineVertices.
 
 	^ (1 to: lines size - 1) anySatisfy: [ :i |
-			aPoint 
+			aPoint
 				onLineFrom: (lines at: i)
 				to: (lines at: i + 1)
 				within: ir ]
@@ -57,6 +94,96 @@ BlBezierCurveGeometry >> controlPoints: aCollection [
 	self releaseCache
 ]
 
+{ #category : #private }
+BlBezierCurveGeometry >> flattenCubicFrom: p0 cp1: p1 cp2: p2 to: p3 into: aCollection depth: depth [
+	"Private helper for flattenedCubicVerticesDo:.
+	Recursively subdivides the cubic Bezier p0-p1-p2-p3 using De Casteljau and
+	appends each new vertex to aCollection (p0 is assumed to be already present).
+
+	Flatness criterion: the perpendicular distance from each interior control
+	point to the chord p0→p3 must be ≤ 1.0 user-space unit (≈ 1 pixel).
+	Working with squared distances avoids a sqrt per call.
+
+	A depth limit of 16 prevents unbounded recursion on degenerate inputs
+	(e.g. all four points coincident)."
+
+	| chordX chordY chordLenSq d1Sq d2Sq q0 q1 q2 r0 r1 mid |
+
+	depth > 16 ifTrue: [
+		aCollection add: p3.
+		^ self ].
+
+	"Chord vector p0→p3."
+	chordX := p3 x - p0 x.
+	chordY := p3 y - p0 y.
+	chordLenSq := (chordX * chordX) + (chordY * chordY).
+
+	"Negligibly short chord → the whole sub-curve is sub-pixel; close it."
+	chordLenSq < 1.0e-8 ifTrue: [
+		aCollection add: p3.
+		^ self ].
+
+	"Perpendicular-distance² from each interior control point to the chord.
+	  d = |cross(chord, p - p0)| / |chord|   →   d² = cross² / chordLenSq"
+	d1Sq := ((p1 x - p0 x) * chordY - ((p1 y - p0 y) * chordX)) squared
+	        / chordLenSq.
+	d2Sq := ((p2 x - p0 x) * chordY - ((p2 y - p0 y) * chordX)) squared
+	        / chordLenSq.
+
+	"Flat enough? tolerance = 1.0 px  →  tolerance² = 1.0"
+	(d1Sq max: d2Sq) <= 1.0 ifTrue: [
+		aCollection add: p3.
+		^ self ].
+
+	"Subdivide at t = 0.5 using De Casteljau."
+	q0 := (p0 + p1) * 0.5.
+	q1 := (p1 + p2) * 0.5.
+	q2 := (p2 + p3) * 0.5.
+	r0 := (q0 + q1) * 0.5.
+	r1 := (q1 + q2) * 0.5.
+	mid := (r0 + r1) * 0.5.
+
+	self
+		flattenCubicFrom: p0 cp1: q0 cp2: r0 to: mid
+		into: aCollection
+		depth: depth + 1.
+	self
+		flattenCubicFrom: mid cp1: r1 cp2: q2 to: p3
+		into: aCollection
+		depth: depth + 1
+]
+
+{ #category : #geometry }
+BlBezierCurveGeometry >> flattenedCubicVerticesDo: aBlock [
+	"Evaluate aBlock with (x, y) for each vertex of the De Casteljau-flattened
+	cubic Bezier path, including the start point p0.
+
+	The flattened vertex array is computed lazily on first call and stored in
+	pathCache geometryPath.  The cache is cleared automatically whenever
+	releaseCache is called (e.g. from controlPoints:), which also clears the
+	inherited strokedBounds cache.
+
+	Usage:
+	  self flattenedCubicVerticesDo: [:x :y | canvas lineToX: x y: y]"
+
+	| vertices |
+	vertices := pathCache geometryPath ifNil: [
+		| result |
+		result := OrderedCollection with: controlPoints first.
+		self
+			flattenCubicFrom: controlPoints first
+			cp1: controlPoints second
+			cp2: controlPoints third
+			to: controlPoints fourth
+			into: result
+			depth: 0.
+		result := result asArray.
+		pathCache geometryPath: result.
+		result ].
+
+	vertices do: [ :each | aBlock value: each x value: each y ]
+]
+
 { #category : #'geometry bounds' }
 BlBezierCurveGeometry >> geometryBounds: aBlBounds [
 

src/Bloc/BlPathCache.class.st

@@ -54,7 +54,7 @@ BlPathCache >> geometryPath [
 { #category : #accessing }
 BlPathCache >> geometryPath: anObject [
 
-	geometryPath := nil
+	geometryPath := anObject
 ]
 
 { #category : #'session management' }