Implement correct handling of Curves / Segments synchronization, improve CurveLocation linking to Curves through their linked Segments, and preserve Curves in Path#split() calls.

src/path/Curve.js

@@ -411,9 +411,8 @@ var Curve = this.Curve = Base.extend(/** @lends Curve# */{
 	
 			// Insert it in the segments list, if needed:
 			if (this._path) {
-				// Insert at the end if this curve is a closing curve
+				// Insert at the end if this curve is a closing curve of a
-				// of a closed path, since otherwise it would be inserted
+				// closed path, since otherwise it would be inserted at 0.
-				// at 0
 				if (this._segment1._index > 0 && this._segment2._index == 0) {
 					this._path.add(segment);
 				} else {

src/path/CurveLocation.js

@@ -42,6 +42,11 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 */
 	initialize: function(curve, parameter, point, distance) {
 		this._curve = curve;
+		// Also store references to segment1 and segment2, in case path
+		// splitting / dividing is going to happen, in which case the segments
+		// can be used to determine the new curves, see #getCurve(true)
+		this._segment1 = curve._segment1;
+		this._segment2 = curve._segment2;
 		this._parameter = parameter;
 		this._point = point;
 		this._distance = distance;
@@ -55,7 +60,7 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 */
 	getSegment: function() {
 		if (!this._segment) {
-			var curve = this._curve,
+			var curve = this.getCurve(),
 				parameter = this.getParameter();
 			if (parameter == 0) {
 				this._segment = curve._segment1;
@@ -80,7 +85,17 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @type Curve
 	 * @bean
 	 */
-	getCurve: function() {
+	getCurve: function(/* uncached */) {
+		if (!this._curve || arguments[0]) {
+			// If we're asked to get the curve uncached, access current curve
+			// objects through segment1 / segment2. Since path splitting or
+			// dividing might have happened in the meantime, try segment1's
+			// curve, and see if _point lies on it still, otherwise assume it's
+			// the curve before segment2.
+			this._curve = this._segment1.getCurve();
+			if (this._curve.getParameterOf(this._point) == null)
+				this._curve = this._segment2.getPrevious().getCurve();
+		}
 		return this._curve;
 	},
 
@@ -91,7 +106,8 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getPath: function() {
-		return this._curve && this._curve._path;
+		var curve = this.getCurve();
+		return curve && curve._path;
 	},
 
 	/**
@@ -102,7 +118,8 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getIndex: function() {
-		return this._curve && this._curve.getIndex();
+		var curve = this.getCurve();
+		return curve && curve.getIndex();
 	},
 
 	/**
@@ -113,7 +130,7 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getOffset: function() {
-		var path = this._curve && this._curve._path;
+		var path = this.getPath();
 		return path && path._getOffset(this);
 	},
 
@@ -125,9 +142,9 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getCurveOffset: function() {
-		var parameter = this.getParameter();
+		var curve = this.getCurve(),
-		return parameter != null && this._curve
+			parameter = this.getParameter();
-				&& this._curve.getLength(0, parameter);
+		return parameter != null && curve && curve.getLength(0, parameter);
 	},
 
 	/**
@@ -139,9 +156,10 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getParameter: function(/* uncached */) {
-		if ((this._parameter == null || arguments[0])
+		if ((this._parameter == null || arguments[0]) && this._point) {
-				&& this._curve && this._point)
+			var curve = this.getCurve(arguments[0] && this._point);
-			this._parameter = this._curve.getParameterOf(this._point);
+			this._parameter = curve && curve.getParameterOf(this._point);
+		}
 		return this._parameter;
 	},
 
@@ -153,8 +171,10 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getPoint: function() {
-		if (!this._point && this._curve && this._parameter != null)
+		if (!this._point && this._parameter != null) {
-			this._point = this._curve.getPoint(this._parameter);
+			var curve = this.getCurve();
+			this._point = curve && curve.getPoint(this._parameter);
+		}
 		return this._point;
 	},
 
@@ -165,9 +185,9 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getTangent: function() {
-		var parameter = this.getParameter();
+		var parameter = this.getParameter(),
-		return parameter != null && this._curve
+			curve = this.getCurve();
-				&& this._curve.getTangent(parameter);
+		return parameter != null && curve && curve.getTangent(parameter);
 	},
 
 	/**
@@ -177,9 +197,9 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	 * @bean
 	 */
 	getNormal: function() {
-		var parameter = this.getParameter();
+		var parameter = this.getParameter(),
-		return parameter != null && this._curve
+			curve = this.getCurve();
-				&& this._curve.getNormal(parameter);
+		return parameter != null && curve && curve.getNormal(parameter);
 	},
 
 	/**
@@ -193,11 +213,13 @@ CurveLocation = Base.extend(/** @lends CurveLocation# */{
 	},
 
 	divide: function() {
-		return this._curve ? this._curve.divide(this.getParameter(true)) : null;
+		var curve = this.getCurve();
+		return curve && curve.divide(this.getParameter(true));
 	},
 
 	split: function() {
-		return this._curve ? this._curve.split(this.getParameter(true)) : null;
+		var curve = this.getCurve();
+		return curve && curve.split(this.getParameter(true));
 	},
 
 	/**

src/path/Path.js

@@ -89,7 +89,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 			// Clockwise state becomes undefined as soon as geometry changes.
 			delete this._clockwise;
 			// Curves are no longer valid
-			if (this._curves != null) {
+			if (this._curves) {
 				for (var i = 0, l = this._curves.length; i < l; i++) {
 					this._curves[i]._changed(/*#=*/ Change.GEOMETRY);
 				}
@@ -114,7 +114,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 	setSegments: function(segments) {
 		this._selectedSegmentState = 0;
 		this._segments.length = 0;
-		// Make sure new curves are calculated next time we call getCurves()
+		// Calculate new curves next time we call getCurves()
 		delete this._curves;
 		this._add(Segment.readAll(segments));
 	},
@@ -149,11 +149,8 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 		var curves = this._curves,
 			segments = this._segments;
 		if (!curves) {
-			var length = segments.length;
+			var length = this._getCurveCount();
-			// Reduce length by one if it's an open path:
+			curves = this._curves = new Array(length);
-			if (!this._closed && length > 0)
-				length--;
-			this._curves = curves = new Array(length);
 			for (var i = 0; i < length; i++)
 				curves[i] = Curve.create(this, segments[i],
 					// Use first segment for segment2 of closing curve
@@ -162,6 +159,8 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 		// If we're asked to include the closing curve for fill, even if the
 		// path is not closed for stroke, create a new uncached array and add
 		// the closing curve. Used in Path#contains()
+		// TODO: This is not consistent with the filling in Illustrator. 
+		// I suggest to only fill closed paths (lehni).
 		if (arguments[0] && !this._closed && this._style._fillColor) {
 			curves = curves.concat([
 				Curve.create(this, segments[segments.length - 1], segments[0])
@@ -170,6 +169,12 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 		return curves;
 	},
 
+	_getCurveCount: function() {
+		var length = this._segments.length;
+		// Reduce length by one if it's an open path:
+		return !this._closed && length > 0 ? length - 1 : length;
+	},
+
 	/**
 	 * The first Curve contained within the path.
 	 *
@@ -218,11 +223,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 			this._closed = closed;
 			// Update _curves length
 			if (this._curves) {
-				var length = this._segments.length;
+				var length = this._curves.length = this._getCurveCount();
-				// Reduce length by one if it's an open path:
-				if (!closed && length > 0)
-					length--;
-				this._curves.length = length;
 				// If we were closing this path, we need to add a new curve now
 				if (closed)
 					this._curves[length - 1] = Curve.create(this,
@@ -277,6 +278,10 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 	 */
 	_add: function(segs, index) {
 		// Local short-cuts:
+/*#*/ if (options.debug) {
+		var beforeSegments = this._segments.length,
+			beforeCurves = this._curves && this._curves.length;
+/*#*/ }
 		var segments = this._segments,
 			curves = this._curves,
 			amount = segs.length,
@@ -308,24 +313,49 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 			// Insert somewhere else
 			segments.splice.apply(segments, [index, 0].concat(segs));
 			// Adjust the indices of the segments above.
-			for (var i = index + amount, l = segments.length; i < l; i++) {
+			for (var i = index + amount, l = segments.length; i < l; i++)
 				segments[i]._index = i;
-			}
 		}
 		// Keep the curves list in sync all the time in case it as requested
 		// already.
-		if (curves) {
+		if (curves || segs._curves) {
+			if (!curves)
+				curves = this._curves  = [];
 			// We need to step one index down from the inserted segment to
 			// get its curve, except for the first segment.
-			// TODO: 
+			var from = index > 0 ? index - 1 : index,
-			if (index > 0)
+				start = from,
-				index--;
+				to = Math.min(from + amount, this._getCurveCount());
-			// Insert a new curve as well and update the curves above
+			if (segs._curves) {
-			for (var i = index, l = index + amount; i < l; i++)
+				// Reuse removed curves.
-				curves.splice(i, 0, Curve.create(this, segments[i],
+				curves.splice.apply(curves, [from, 0].concat(segs._curves));
-						segments[i + 1] || segments[0]));
+				start += segs._curves.length;
+			}
+			// Insert new curves, but do not initialize them yet, since
+			// #_adjustCurves() handles all that for us.
+			for (var i = start; i < to; i++)
+				curves.splice(i, 0, Base.create(Curve));
 			// Adjust segments for the curves before and after the removed ones
-			this._adjustCurves(index - 1, index + amount);
+			this._adjustCurves(from, to);
+/*#*/ if (options.debug) {
+			var count = this._getCurveCount();
+			console.log('add',
+				'id', this._id,
+				'from', from,
+				'to', to,
+				'amount', amount,
+				'start', start,
+				'BEFORE:',
+				'segments', beforeSegments,
+				'curves', beforeCurves,
+				'AFTER:',
+				'segments', segments.length,
+				'curves', this._curves.length,
+				count != this._curves.length ? '(SHOULD BE ' + count + ')' : '',
+				'segs', segs.length,
+				'segs._curves', segs._curves && segs._curves.length
+			);
+/*#*/ }
 		}
 		this._changed(/*#=*/ Change.GEOMETRY);
 		return segs;
@@ -566,11 +596,16 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 	 * // Select the path, so we can see its segments:
 	 * path.selected = true;
 	 */
-	removeSegments: function(from, to) {
+	removeSegments: function(from, to/*, includeCurves */) {
 		from = from || 0;
 		to = Base.pick(to, this._segments.length);
+/*#*/ if (options.debug) {
+		var beforeSegments = this._segments.length,
+			beforeCurves = this._curves && this._curves.length;
+/*#*/ }
 		var segments = this._segments,
 			curves = this._curves,
+			count = segments.length, // segment count before removal
 			removed = segments.splice(from, to - from),
 			amount = removed.length;
 		if (!amount)
@@ -581,21 +616,43 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 			if (segment._selectionState)
 				this._updateSelection(segment, segment._selectionState, 0);
 			// Clear the indices and path references of the removed segments
-			segment._index = segment._path = undefined;
+			delete segment._index;
+			delete segment._path;
 		}
 		// Adjust the indices of the segments above.
 		for (var i = from, l = segments.length; i < l; i++)
 			segments[i]._index = i;
 		// Keep curves in sync
 		if (curves) {
-			// If we're removing the last segment, remove the last curve. Also
+			// If we're removing the last segment, remove the last curve (the
-			// take into account closed paths, which have one curve more than
+			// one to the left of the segment, not to the right, as normally).
-			// segments.
+			// Also take into account closed paths, which have one curve more
-			var index = from == segments.length + (this._closed ? 1 : 0)
+			// than segments.
-					? from - 1 : from;
+			var index = to == count + (this._closed ? 1 : 0) ? from - 1 : from,
-			curves.splice(index, amount);
+				curves = curves.splice(index, amount);
+/*#*/ if (options.debug) {
+			console.log('remove',
+				'id', this._id,
+				'from', from,
+				'to', to,
+				'amount', amount,
+				'index', index,
+				'BEFORE:',
+				'segments', beforeSegments,
+				'curves', beforeCurves,
+				'AFTER:',
+				'segments', segments.length,
+				'curves', this._curves.length,
+				'curves removed', curves.length
+			);
+/*#*/ }
+			// Return the removed curves as well, if we're asked to include
+			// them, but exclude the first curve, since that's shared with the
+			// previous segment and does not connect the returned segments.
+			if (arguments[2])
+				removed._curves = curves.slice(1);
 			// Adjust segments for the curves before and after the removed ones
-			this._adjustCurves(index - 1, index + amount);
+			this._adjustCurves(index, index);
 		}
 		this._changed(/*#=*/ Change.GEOMETRY);
 		return removed;
@@ -604,16 +661,24 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 	/**
 	 * Adjusts segments of curves before and after inserted / removed segments.
 	 */
-	_adjustCurves: function(left, right) {
+	_adjustCurves: function(from, to) {
 		var segments = this._segments,
 			curves = this._curves,
 			curve;
+		for (var i = from; i < to; i++) {
+			curve = curves[i];
+			curve._path = this;
+			curve._segment1 = segments[i];
+			curve._segment2 = segments[i + 1] || segments[0];
+		}
 		// If it's the first segment, correct the last segment of closed
 		// paths too:
-		if (curve = curves[this._closed && left == -1 ? segments.length - 1 : left])
+		if (curve = curves[this._closed && from === 0 ? segments.length - 1
-			curve._segment2 = segments[left + 1] || segments[0];
+				: from - 1])
-		if (curve = curves[right])
+			curve._segment2 = segments[from] || segments[0];
-			curve._segment1 = segments[right];
+		// Fix the segment after the modified range, if it exists
+		if (curve = curves[to])
+			curve._segment1 = segments[to];
 	},
 
 	/**
@@ -801,6 +866,15 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 
 	// DOCS: split(index, parameter) / split(offset) / split(location)
 	split: function(index, parameter) {
+/*#*/ if (options.debug) {
+		console.log('split',
+			'id:', this._id,
+			'index:', index,
+			'param:', parameter
+		);
+/*#*/ }
+		if (parameter === null)
+			return;
 		if (arguments.length == 1) {
 			var arg = index;
 			// split(offset), convert offset to location
@@ -824,25 +898,30 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 				curves[index++].divide(parameter);
 			}
 			// Create the new path with the segments to the right of given
-			// parameter, which are removed from the current path.
+			// parameter, which are removed from the current path. Pass true
-			var segs = this.removeSegments(index, this._segments.length);
+			// for includeCurves, since we want to preserve and move them to
-			// If the path is closed, open it and move the segments round,
+			// the new path through _add(), allowing us to have CurveLocation
-			// otherwise create two paths.
+			// keep the connection to the new path through moved curves. 
+			var segs = this.removeSegments(index, this._segments.length, true),
+				path;
 			if (this._closed) {
+				// If the path is closed, open it and move the segments round,
+				// otherwise create two paths.
 				this.setClosed(false);
-				this.insertSegments(0, segs);
+				// Just have path point to this. The moving around of segments
-				// Add bginning segment again at the end, since we opened a
+				// will happen below.
-				// closed path.
+				path = this;
-				this.addSegment(segs[0]);
-				return this;
 			} else if (index > 0) {
-				// Add dividing segment again
+				// Pass true for _cloning, in case of CompoundPath, to avoid 
-				this.addSegment(segs[0]);
+				// reversing of path direction, which would mess with segs!
-				var path = new Path(segs);
+				// Use _clone to copy over all other attributes, including style
-				// Copy over all other attributes, including style
+				path = this._clone(new Path().insertAbove(this, true));
-				this._clone(path);
-				return path;
 			}
+			path._add(segs, 0);
+			// Add dividing segment again. In case of a closed path, that's the
+			// beginning segment again at the end, since we opened it.
+			this.addSegment(segs[0]);
+			return path;
 		}
 		return null;
 	},
@@ -1050,7 +1129,7 @@ var Path = this.Path = PathItem.extend(/** @lends Path# */{
 		var curves = this.getCurves();
 		for (var i = 0, l = curves.length; i < l; i++) {
 			var loc = curves[i].getLocationOf(point);
-			if (loc != null)
+			if (loc)
 				return loc;
 		}
 		return null;

src/path/Segment.js

@@ -57,6 +57,7 @@ var Segment = this.Segment = Base.extend(/** @lends Segment# */{
 		var count = arguments.length,
 			createPoint = SegmentPoint.create,
 			point, handleIn, handleOut;
+		// TODO: Use Point.read or Point.readNamed to read these?
 		if (count == 0) {
 			// Nothing
 		} else if (count == 1) {