DashArray wrong with strokeUniform

[awehring]

Version

3.4.0
(also in version 2.7.0)

Test Case

https://jsfiddle.net/awehring/osv8Lr2k/23/

Information about environment

Firefox 70.0
Edge 44.18362.387.0

Steps to reproduce

Define a DashArray for a Stroke of a Rect, Circle, Triangle or other object with strokeUniform = false.
Define a DashArray for a Stroke of an identical object with strokeUniform = true.
Compare the two objects.

Expected Behavior

The strokes for both objects should look the same.

Actual Behavior

The strokes for the second object (yellow) are about 5 times as long as for the first (green).

[awehring]

I found a (partial) workaround: Divide the DashArray parameters by the strokeWidth when strokeUniform == true, like

obj.set({strokeDashArray: [15 / obj.strokeWidth, 7.5 / obj.strokeWidth]});

It works well for Circle, Triangle, Ellipse. Not so well for Rect.

This hint might also help with bugfixing.

[korzo89]

I will try to fix it as a part of PR #5950, since it touches a common area of problems.

[asturur]

Deleted my comment, the bug here is that i have no memory of why the strokeWidth is playing a role in the stroke dash array.

I hope there are some comments in the code or in the PR that introduced the feature

[asturur]

https://github.com/fabricjs/fabric.js/pull/5473/files#diff-840d0c62989de96492c73f1de1f574d0R1340

This is the code.
I'm not sure is a bug or not, i'm really undecided.

[asturur]

i think the idea of that code was to avoid having strokeDashes of 1 with big stroke width that would look awful, but eventually that is not a problem of the strokeUniform, nor a problem of fabricJS.

I'll meet the guy that wrote that code later, i ll ask him if he remember

[asturur]

@korzo89 opene a separate PR, this is an easier fix, while i have no idea what we should do with the other one. Start with removing the code i linked, and let's see what is the end result.

[awehring]

asturur, I think you are right. The length of the strokes / dashes schould fit to the strokeWidth. But this should be handled in the application not in fabrcs.js, I think.

For strokeWidth between 0.5px and 30px I came up with a squareroot relation. Including the workaround that I mentioned above, this is my code.
Feel free to copy it, if it is usefull in anybodies app.

`
// make DashArray
function makeDashArray (pty_linestyle, stroke_width, uniscaling) {

	var strokelength = 0;	// (number) stroke length
	var pauselength = 0;	// (number) pause length
	var uniscaling_fix = 1;	// (number) Bugfix for strokeUniform == true;
	var strokeDashArray = null;	// (Array of numbers) fabric.js DashArray


	// Bugfix: for fabric.js  Issue #5939
	if (uniscaling) {
		uniscaling_fix = stroke_width;
	}

	// Rounding  up for strokeWidth < 1 px
	stroke_width = Math.ceil(stroke_width);

	switch (pty_linestyle) {
		case "linestyle_line":
			strokeDashArray = [1, 0];
			break;
		case "linestyle_stroke":
			pauselength = Math.max((6 * Math.sqrt(stroke_width) - 4), 2);
			strokelength = 2 * pauselength;
			strokeDashArray = [strokelength / uniscaling_fix, pauselength / uniscaling_fix];
			break;
		case "linestyle_dash":
			strokelength = (6 * Math.sqrt(stroke_width) - 4);
			pauselength = strokelength;
			strokeDashArray = [strokelength / uniscaling_fix, pauselength / uniscaling_fix];
			break;
		default:
			break;
	}
	return (strokeDashArray);
}; // --- End function makeDashArray() ---

`