Text fill render issue when gradient unit is percentage

[amirhossein92]

After setting the gradient on fill property of the text, the element is not rendered correctly. I tried in Kitchensink and the Automated Visual Test, and in both of them gradient is not working if the gradientUnits is set to percentage.

Version

5.2.1

Test Case

• Automated Test: 5- Text: Text percentage gradient

Information about environment

browser - chrome

Steps To Reproduce

1. applied the following object as gradient:

sample of gradient object:
{ "type": "radial", "coords": { "x1": 1, "y1": 0.5, "x2": 1, "y2": 0.5, "r1": 0, "r2": 1 }, "colorStops": [ { "color": "red", "offset": 0 }, { "color": "blue", "offset": 1 } ], "offsetX": 0, "offsetY": 0, "gradientUnits": "percentage", }

[ShaMan123]

let me check if this happens on #8154

[ShaMan123]

managed to reproduce on master

Many visual tests have too permissive diff values. Now that we refactored how testing works we might enforce a stricter diff.

I will address this in the PR

[ShaMan123]

stricter

I am getting uncertain results. Sometimes is renders as expected and sometimes it is blank. I will look into the loading logic.

[ShaMan123]

Didn't find the reason yet. But It seems text rendering is to blame

[ShaMan123]

@amirhossein92 I wonder if it's fabric or not
I'm seeing it on chrome, which browser are you using?
Could you try to reproduce without fabric?

[amirhossein92]

I tried that on chrome. I am not sure if its fabric or not, but I should mention that I tried older version of fabric
(3.6.6) and this problem does not exists.

this is the result of visual test in Safari:

[ShaMan123]

on Safari what versions did you test?

[amirhossein92]

I tested on this:
Safari Version 15.5 (17613.2.7.1.8)

[ShaMan123]

I meant fabric version 🙃

[ShaMan123]

BTW if you want we just merged a major improvement to the test suite...
So update from upstream/master and then you can

npm test -- --dev -l

and it will launch the browser with visual tests
instead of using fabricjs.com
see contribution guide

[amirhossein92]

I meant fabric version 🙃

Oh sorry, 😄 the screenshot of test for safari is the latest fabric version (5.2.1)

[ShaMan123]

Could you try to reproduce without fabric?

[amirhossein92]

Do you mean by css? here is a sample for text gradient. it renders correctly in both chrome and safari, but if you change the -webkit-background-clip to background-clip, chrome renders incorrectly.

[ShaMan123]

bare html canvas

[ShaMan123]

I came across forceGLPutImageData, is it related?
Try setting to true is so

[ShaMan123]

any progress?

[asturur]

I came across forceGLPutImageData, is it related? Try setting to true is so

sorry i m a bit lage. forceGLPutImageData is a webgl thing for old hardware that we could or could not deprecate. I m not sure which is the situation all around now

[carior]

I also reproduced that the gradientUnits set percentage of gradient fill is not shown。
May I ask if there is another way to set the percentage?

[iocoker]

This is definitely a Chrome issue, it renders fine in Firefox and Edge. Perhaps a Skia bug

@carior You can try this code, it's a hack that works. It basically bypasses the pattern route _applyPatternGradientTransformText and creates a CanvasGradient with the right coordinates. I'm not sure why they went that route for a gradient.

`

fabric.Text.prototype.handleFiller = function(ctx, property, filler) {
	
	let offsetX, offsetY;
	if (filler && filler.toLive) {

		if (filler.gradientUnits === 'percentage' && !filler.patternTransform) {

			offsetX = -this.width / 2;
			offsetY = -this.height / 2;
			ctx.translate(offsetX, offsetY);

			let coords = { 
				x1: this.width * filler.coords.x1,
				y1: this.height * filler.coords.y1,
				x2: this.width * filler.coords.x2,
				y2: this.height * filler.coords.y2
			};

			let gradient = ctx.createLinearGradient(coords.x1, coords.y1, coords.x2, coords.y2);
			
			for (let i = 0; i < filler.colorStops.length; i++) {
				let stop = filler.colorStops[i];
				gradient.addColorStop(stop.offset, stop.color);
			}
			ctx[property] = gradient;
			return { offsetX: offsetX, offsetY: offsetY };

		} else if (filler.gradientTransform || filler.patternTransform) {
			// need to transform gradient in a pattern.
			// this is a slow process. If you are hitting this codepath, and the object
			// is not using caching, you should consider switching it on.
			// we need a canvas as big as the current object caching canvas.
			offsetX = -this.width / 2;
			offsetY = -this.height / 2;
			ctx.translate(offsetX, offsetY);

			ctx[property] = this._applyPatternGradientTransformText(filler);
			return { offsetX: offsetX, offsetY: offsetY };
		} else {
			// is a simple gradient or pattern
			ctx[property] = filler.toLive(ctx, this);
			console.log(ctx[property]);
			return this._applyPatternGradientTransform(ctx, filler);
		}
	} else {
		// is a color
		ctx[property] = filler;
	}
	return { offsetX: 0, offsetY: 0 };
}

`

[somq]

This is definitely a Chrome issue, it renders fine in Firefox and Edge. Perhaps a Skia bug

@carior You can try this code, it's a hack that works. It basically bypasses the pattern route _applyPatternGradientTransformText and creates a CanvasGradient with the right coordinates. I'm not sure why they went that route for a gradient.

`

fabric.Text.prototype.handleFiller = function(ctx, property, filler) {
	
	let offsetX, offsetY;
	if (filler && filler.toLive) {

		if (filler.gradientUnits === 'percentage' && !filler.patternTransform) {

			offsetX = -this.width / 2;
			offsetY = -this.height / 2;
			ctx.translate(offsetX, offsetY);

			let coords = { 
				x1: this.width * filler.coords.x1,
				y1: this.height * filler.coords.y1,
				x2: this.width * filler.coords.x2,
				y2: this.height * filler.coords.y2
			};

			let gradient = ctx.createLinearGradient(coords.x1, coords.y1, coords.x2, coords.y2);
			
			for (let i = 0; i < filler.colorStops.length; i++) {
				let stop = filler.colorStops[i];
				gradient.addColorStop(stop.offset, stop.color);
			}
			ctx[property] = gradient;
			return { offsetX: offsetX, offsetY: offsetY };

		} else if (filler.gradientTransform || filler.patternTransform) {
			// need to transform gradient in a pattern.
			// this is a slow process. If you are hitting this codepath, and the object
			// is not using caching, you should consider switching it on.
			// we need a canvas as big as the current object caching canvas.
			offsetX = -this.width / 2;
			offsetY = -this.height / 2;
			ctx.translate(offsetX, offsetY);

			ctx[property] = this._applyPatternGradientTransformText(filler);
			return { offsetX: offsetX, offsetY: offsetY };
		} else {
			// is a simple gradient or pattern
			ctx[property] = filler.toLive(ctx, this);
			console.log(ctx[property]);
			return this._applyPatternGradientTransform(ctx, filler);
		}
	} else {
		// is a color
		ctx[property] = filler;
	}
	return { offsetX: 0, offsetY: 0 };
}

`

What about a PR/merge & release? It has been almost a year...

[asturur]

pr merge of what?

[somq]

pr merge of what?

@iocoker hotfix above. Does that make sense?