fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
29.07k stars 3.52k forks source link

Stroke with gradient "percentage" not working properly #7381

Open blucobalto opened 3 years ago

blucobalto commented 3 years ago

Hi, stroke not working properly when set: gradient percentage, strokeUniform = false, canvas.zoom != 1

Thanks a lot, Blu

Version

4.3.1 - 4.6.0

Test Case

http://jsfiddle.net/yth81xL7/

Information about environment

Nodejs or browser? Browser Which browsers? Firefox, Chrome, Edge

Steps to reproduce

Set stroke with gradient percentage. Set strokeUniform = false. set canvas.zoom =! 1 (ex: 0.3). Change width/height with corner handles.

Expected Behavior

Stroke working properly

Actual Behavior

Stroke not working properly Senzanome

asturur commented 3 years ago

this seems wrong and needs a fix

asturur commented 3 years ago

Did it start in 4.3.1? or also before?

blucobalto commented 3 years ago

Hi Asturur, I tried to test the code with version 3.6.6 and 3.4.0 and I have the same problem. While with version 3.0.0 I think the "percentage" setting does not work because it does not draw the gradient as I expect.

Thanks, Blu

ShaMan123 commented 2 years ago

https://github.com/fabricjs/fabric.js/blob/3feb8660a4d047f06ed392af5de2897eca722155/src/shapes/object.class.js#L1543

This is a POC, by no means a solution, just something that might be a solution: http://jsfiddle.net/5g2arwfk/45/

There's a need to calculate a scaling factor that scales down the object so that stroke can remain the same size

EDIT: I may have got it wrong

ShaMan123 commented 2 years ago

I think this is complicated more than strokeUniform

asturur commented 2 years ago

seems zoom fault

hoomanaskari commented 2 years ago

Has there been any solutions for this issue?

hoomanaskari commented 2 years ago

Okay after a little while of experimenting stuff, I realized that by simplifying the _applyPatternForTransformedGradient a bit we can solve this issue for gradients. Here is what I came up with so far, not perfect, but it works. I am not comfortable with the radial gradient though, and I may be wrong but this does not cover patterns for strokes!

/**
* This function try to patch the missing gradientTransform on canvas gradients.
* transforming a context to transform the gradient, is going to transform the stroke too.
* we want to transform the gradient but not the stroke operation, so we create
* a transformed gradient on a pattern and then we use the pattern instead of the gradient.
* this method has drwabacks: is slow, is in low resolution, needs a patch for when the size
* is limited.
* @private
* @param {CanvasRenderingContext2D} ctx Context to render on
* @param {fabric.Gradient} filler a fabric gradient instance
*/
fabric.Object.prototype._applyPatternForTransformedGradient = function (ctx, filler) {
  if (filler.gradientUnits === 'percentage' && filler.type === 'linear') {
    filler = fabric.util.object.clone(filler);

    filler.coords.x1 = ((filler.coords.x1 * this.width) - (this.width / 2)) + (filler.offsetX || 0);
    filler.coords.y1 = ((filler.coords.y1 * this.height) - (this.height / 2)) + (filler.offsetY || 0);
    filler.coords.x2 = ((filler.coords.x2 * this.width) - (this.width / 2)) + (filler.offsetX || 0);
    filler.coords.y2 = ((filler.coords.y2 * this.height) - (this.height / 2)) + (filler.offsetY || 0);
  } else if (filler.gradientUnits === 'percentage' && filler.type === 'radial') {
    filler = fabric.util.object.clone(filler);

    filler.coords.x1 = ((filler.coords.x1 * this.width) - (this.width / 2)) + (filler.offsetX || 0);
    filler.coords.y1 = ((filler.coords.y1 * this.height) - (this.height / 2)) + (filler.offsetY || 0);
    filler.coords.x2 = ((filler.coords.x2 * this.width) - (this.width / 2)) + (filler.offsetX || 0);
    filler.coords.y2 = ((filler.coords.y2 * this.height) - (this.height / 2)) + (filler.offsetY || 0);

    filler.coords.r1 *= (this.width + this.height) / 2;
    filler.coords.r2 *= (this.width + this.height) / 2;
  }

  ctx.strokeStyle = filler.toLive(ctx);
};
zhe-he commented 3 months ago

I also encountered this issue, and I fixed it this way. I hope it helps others. demo

The left side is the reference rectangle, the middle side is the bug rectangle, and the right side is the fixed rectangle.

asturur commented 3 months ago
 fabric.FabricObject.prototype._applyPatternForTransformedGradient.call(this, ctx, filler);
    const zoom = this.canvas?.getZoom() ?? 1;
    ctx.lineWidth *= zoom;
  }

i m not sure how this works with text. The _applyPatternForTransformedGradient is indeed problematic and not easy to solve.

There are some improvements around the repository in examples and tentaive prs, but in general if you are hitting that function you will have issues of some sort, either graphical or performance related.