svg.draw.js icon indicating copy to clipboard operation
svg.draw.js copied to clipboard

Removing points in a Polyline is rewriten by svg.draw.js

Open dotnetCarpenter opened this issue 7 years ago • 10 comments

Take the following code snippet, where peek(lines) returns the current Polyline and info() just prints to console.log:

function cancel() {
	const l = peek(lines)
	const points = l.attr('points').replace(/(\s\d+,\d+){2}$/, '')

	info(l.attr('points'))
	info(l.array().value.reduce( (a1,a2) => a1.concat(a2) ).join(' '))

	info(points)
	l.attr('points', points)
	l.array(new SVG.PointArray(
		l.array().value.slice(0, l.array().value.length - 3)
	))

	info(l.attr('points'))
	info(l.array().value.reduce( (a1,a2) => a1.concat(a2) ).join(' '))

	/*peek(lines)
		.draw('cancel')*/
}

Live example: https://github.com/dotnetCarpenter/mua

Double click on the SVG element and add some points (single click) and then press ctrl + c.

If you look in the devtool console, you can see the points, both via the points attribute and via the .array() method. If I remove the last two points (the last is for animation and the second last is the last clicked point), and update both the polyline's points attribute and via the polyline's .array() method, svg.draw.js will still remember the point array and recreate the removed points.

dotnetCarpenter avatar Apr 04 '17 20:04 dotnetCarpenter

The corresponding code to this behavior should be in the calc function. However as you can see:

        calc:function (e) {
            var arr = this.el.array().valueOf();
            arr.pop();

            if (e) {
                var p = this.transformPoint(e.clientX, e.clientY);
                arr.push(this.snapToGrid([p.x, p.y]));
            }
            
            this.el.plot(arr);

        },

The array is pulled from the element every time. Nothing is cached. Iam not entirely sure what you are doing there anyway:

        // why dont you use `plot` which updates the internal array
	l.attr('points', points)

       // `array()` is no setter. When you want to set the array use plot again
	l.array(new SVG.PointArray(
		l.array().value.slice(0, l.array().value.length - 3)
	))

So I guess the fix to your problem is: Use plot! (or delete the array with clear() after changing points directly on the element - but no - just use plot)

Code that should work:

        // no need to use PointArray - the parse method also accepts arrays
	l.plot(l.array().valueOf().slice(0, -2)) // slice accepts negative indices as well

Fuzzyma avatar Apr 05 '17 06:04 Fuzzyma

You're right!

Got a version up now where ctrl + c removes the last point and last circle. The only syntax I could get working for removing the circles (calling .drawCircles() with less points), is l.draw('drawCircles', points.slice(0, -1)).

The entire cancel function then becomes:

function cancel() {
	const l = peek(lines)

	if(!l) return

	// remove last point
	const points = l.array().valueOf().slice(0, -1)

	// update polyline with new points
	l.plot(points)

	// redraw circles with new points
	//l.draw.plugins.polyline.drawCircles.call(l.draw.plugins.polyline, points)
	l.draw('drawCircles', points.slice(0, -1))

	if( l.array().valueOf().length === 1 ) {
		l.draw('cancel')
		lines.pop()
	}
}

Should .draw('drawCircles') also be documented? Could be in the same section as we're talking about in #16.

Anyway, I think that using .plot() to update the path/points should be clear. If you want, I'll think of a way to describe it and create a PR?

dotnetCarpenter avatar Apr 05 '17 18:04 dotnetCarpenter

Mh did you try calling the update function instead of the drawCircle function? I feel like this circle thingy should not be public api.

Fuzzyma avatar Apr 05 '17 18:04 Fuzzyma

I just did a really quick test with l.draw('update', points.slice(0, -1)) and Chrome gives me Error: attribute points: Expected number, "177,110 291,50 NaN,NaN". and Firefox give me TypeError: Value being assigned to SVGPoint.x is not a finite floating-point value.

I'll be back in an hour to investigate further...

dotnetCarpenter avatar Apr 05 '17 18:04 dotnetCarpenter

Update does not expect an argument. Don't pass the points!

Fuzzyma avatar Apr 05 '17 18:04 Fuzzyma

I tried that and it does nothing. Basically I need to remove the last point from .array().valueOf() and the last 2 points when redrawing the circles.

// remove last point
const points = l.array().valueOf().slice(0, -1)

// update polyline with new points
l.plot(points)

// redraw circles with new points
l.draw('drawCircles', points.slice(0, -1))

// update the polyline to end at the mouse position
l.draw('update')

I'm pretty sure that it works by .plot() removing the last point in _memory, .draw('drawCircles', points.slice(0, -1)) removes all circles and draw only N-2 points (the animation point and the last clicked point) and lastly .draw('update') in turn calls calc() which updates the polyline to the mouse pointer.

The only other way I can think of, that let me do this, is by creating a plug-in for svg.draw.js.

dotnetCarpenter avatar Apr 05 '17 20:04 dotnetCarpenter

I guess I will remove the need to pass any argument to drawCircles. It will just pull the needed points from the element instead. That would make drawCircles less confusing and could be part of the public api

Fuzzyma avatar Apr 11 '17 10:04 Fuzzyma

Unfortunately that will not work. I have to (as in it is my use-case) remove 2 points when drawing the circles or it looks weird. The circle for the last animation point will be drawn, if it doesn't have one point less. So I need an API where I can change the circles. To that end, .draw('drawCircles', points) worked well. So please don't remove it.

I can upload a demo, if you need to see it.

dotnetCarpenter avatar Apr 11 '17 19:04 dotnetCarpenter

If you clone https://github.com/dotnetcarpenter/mua/ you can open index.html and see the issue. In the master branch it's working (with current svg.draw.js release) and in the newDraw branch (with the version in your master branch and without sending points to drawCircles) it looks weird when you use ctrl +z to undo the last point.

dotnetCarpenter avatar Apr 11 '17 20:04 dotnetCarpenter

Well the parameter free version would just draw all points except the last. This would work, wouldn't it?

Fuzzyma avatar Sep 11 '17 15:09 Fuzzyma