Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shape gradients with percentage based coordination don't work correctly. #3665

Closed
artboard-studio opened this issue Feb 2, 2017 · 23 comments · Fixed by #5836
Closed

Shape gradients with percentage based coordination don't work correctly. #3665

artboard-studio opened this issue Feb 2, 2017 · 23 comments · Fixed by #5836

Comments

@artboard-studio
Copy link

Version

1.7.3

Test Case

https://jsfiddle.net/human_a/9xfa5u77/

Steps to reproduce

In my app, I am using the following function to turn angle (degrees based) to X and Y coordinations, but even using the examples from this page would not work if I hard code them.

const anglePI = (-parseInt(angle, 10)) * (Math.PI / 180)
const angleCoords = {
    'x1': Math.round(50 + Math.sin(anglePI) * 50) + '%',
    'y1': Math.round(50 + Math.cos(anglePI) * 50) + '%',
    'x2': Math.round(50 + Math.sin(anglePI + Math.PI) * 50) + '%',
    'y2': Math.round(50 + Math.cos(anglePI + Math.PI) * 50) + '%',
}

This function works perfectly fine and I am using the same technique to create SVG gradients for canvas background, while it works for the canvas background flawlessly, it does not work for shapes unless I use shape width and height which will limit my app for using different angles (I have a knob using which I can change the angle on mouse drag)

Expected Behavior

Percentage based coordinations should work the same as width and height based coords, but it does not produce correct gradients for shapes.

@asturur
Copy link
Member

asturur commented Feb 2, 2017

Well svg gradients are different from canvas one.
Can you show me a SVG with a gradient generated by this formula?
Once generated fabric should be able to import it correctly.
I just think that for fabric-canvas approach this formula is not right.

@artboard-studio
Copy link
Author

Alright, let's forget about my function, and just take a look at my fiddle, I am copying the coordinations from the fabricjs help docs and again it is not working

this is from fabric docs:

var redToOrangeGradient = { 
	x1: "0",  // For a vertical gradient, the x1 & y1 don't matter as long as they are the same
	y1: "100%",   // The top of the gradient will be at the top of the object
	x2: "100%",
	y2: "0%", // The bottom of the gradient will be at the bottom of the object
	colorStops: {
		0: "#FF0000",
		1: "#FFFF00"
	}
}

Take a look at the last example here: https://github.com/kangax/fabric.js/wiki/Working-with-gradients

P.S. In the docs it says shape gradients should be created using setGradientFill but it generate an error that the setGradientFill is not a function, however setGradient('fill', {...}) works fine so maybe it should be updated in the docs too, unless I am missing something here.

@asturur
Copy link
Member

asturur commented Feb 2, 2017

So the page has been updated a bit, not completely.

SetGradientFill has been removed prior to 1.4.0 and changed with setGradient. i think jsdocs mention it but the page not. Need to be addressed.

Other than that i see percentages produce an hard gradient, this is weird. i looked at the fiddle even before i did not notice the missing gradient because i was concentrated in the angle.
I ll check it.
you could start looking here https://github.com/kangax/fabric.js/blob/master/src/gradient.class.js#L391
i think setGradient('fill'...) use that.

I m planning a semplification of gradient because using x1,y1... is not comfortable. i would prefer to have a gradient angle option for linear gradient and default to full width/height. leaving gradient coords for who wants to make something different

@artboard-studio
Copy link
Author

Thank you for giving it a look

  1. It should be working with the percentage values, according to the docs and the source code of fabric, the rest (including my angle to points function) work perfectly fine as I tested.

  2. The idea of "simplification of gradients" sounds great, at least it could be like SVGs but if you can take to the next level so that it accepts CSS linear-gradient syntax it would be perfect.

Thanks anyways for always being responsive to the questions all the time.

@asturur
Copy link
Member

asturur commented Feb 2, 2017

i want something more simple. the current one respect the svg logic, users generally want something simpler.

@artboard-studio
Copy link
Author

I could not resolve the issue with percentage values since it is something done wrong inside fabric core, but there was a simple fix for my app, I change the angle to points function this, so now I am converting my percentage values to pixel based on the shape with/height, this works fine for me. Maybe the angle to point function would be useful for you too if you want to simplify the gradients for fabric so I share it with you too:

const { width, height } = selectedObject;
var anglePI = (-parseInt(angle, 10)) * (Math.PI / 180)
var angleCoords = {
    'x1': (Math.round(50 + Math.sin(anglePI) * 50) * width) / 100,
    'y1': (Math.round(50 + Math.cos(anglePI) * 50) * height) / 100,
    'x2': (Math.round(50 + Math.sin(anglePI + Math.PI) * 50) * width) / 100,
    'y2': (Math.round(50 + Math.cos(anglePI + Math.PI) * 50) * height) / 100,
}
selectedObject.setGradient('fill', {
    type: 'linear',
    x1: angleCoords.x1 || 0,
    y1: angleCoords.y1 || 0,
    x2: angleCoords.x2 || 0,
    y2: angleCoords.y2 || 0,
    colorStops: shapeStops
})
canvas.renderAll()

The reason I need this is that I have created a gradient color picker for react, and it has a draggable knob to set the angle, the return value of my gradient component can be used in CSS, SVG and now fabric shapes using the function above, I this comes handy to you too.

@asturur
Copy link
Member

asturur commented Feb 2, 2017

did you spot something wrong that you can point me at?

yes i need a react thing to make gradient. a screenshot is enough.

@artboard-studio
Copy link
Author

  1. The _convertPercentUnitsToValues function that you mentioned, checks for percentage values and sets the multFactor to 0.01 but I believe the percentage values should be transformed to pixel values based on the object.width to be accurate (which is what I am doing in the second version of angleCoords in my code). I haven't run any tests on the _convertPercentUnitsToValues yet and I might be wrong, if so please let me know.

  2. Do you need a screenshot of the gradient picker in my app or a screenshot of the code? If you need the code I could make a fiddle from my gradient picker and share it with you. I have to do it in my spare time since in my app I am using redux for store management, I have to transform it into setState because it would be redundant to share all the actions and reducers along with it.

I was planning on open sourcing it anyhow, since there are no other gradient pickers for React available on the web right now (there is one and it does not work)

@asturur
Copy link
Member

asturur commented Feb 3, 2017

no i need a screenshot of the interface to have an idea on how to realize one for a separate project.
Just to have an idea how to handle color position and quantity and visual representation. Then for sure the designers will want to modify it.

If you opensource it give me a link! If is not opensource i do not want to see code, since my project is commercial and i do not want to mess up with wrong things.

@artboard-studio
Copy link
Author

I hope this could be good enough.

screen shot 2017-02-03 at 13 31 40

I will send you the link as soon as I find the time to open source it with a proper license for personal and commercial use.

@sandor
Copy link

sandor commented Oct 17, 2017

@Human-a would you mind sharing some more infos on your usage of gradients? I would like to have a simple gradient control for a linear gradient with a color picker for the left and right color and ideally a numberinput for the gradient "angle"...
gradient

This is my code for creating the gradient so far:

$scope.addGradient = function (left, right) {
    var leftColor = document.getElementById('gradLeft').value;
    var rightColor = document.getElementById('gradRight').value;
    
    console.log(leftColor, rightColor);

    var grad = new fabric.Gradient({
        type: 'linear',
        coords: {
            x1: 0,
            y1: 0,
            x2: canvas.width,
            y2: canvas.height,
        },
        colorStops: [{
            color: leftColor,
            offset: 0,
        },
            {
                color: rightColor,
                offset: 1,
            }
        ]
    });
    canvas.backgroundColor = grad.toLive(canvas.contextContainer);
    canvas.renderAll();
}

@artboard-studio
Copy link
Author

@sandor Sure, here is how I did it (I am using ES6 syntax; if you are not familiar you can transpile it back to ES5 using babel)

const { colorStops, angle } = gradient; // Getting these values from my custom gradient colorpicker
const { width, height } = shape;
const shapeColorStops = {}

// My gradient picker is giving me percentage values for the angle (0-100)
// Using the code below I transform the angle to a radial value (0-360)
var anglePI = (-parseInt(angle, 10)) * (Math.PI / 180)
var angleCoords = {
    'x1': (Math.round(50 + Math.sin(anglePI) * 50) * width) / 100,
    'y1': (Math.round(50 + Math.cos(anglePI) * 50) * height) / 100,
    'x2': (Math.round(50 + Math.sin(anglePI + Math.PI) * 50) * width) / 100,
    'y2': (Math.round(50 + Math.cos(anglePI + Math.PI) * 50) * height) / 100,
}

// Perhaps you won't need to loop through your color stops if you only have 2 values
// In my case I can have infinite number of color stops, so I have to loop through them
Object.keys(colorStops).forEach((key,i) => {
    const leftPerc = (colorStops[key].leftPerc / 100) || 0;
    shapeColorStops[leftPerc] = colorStops[key].color;
})

// You are already familiar with this part, you have done it in your code too
layer.setGradient('fill', {
    type: 'linear',
    x1: angleCoords.x1 || 0,
    y1: angleCoords.y1 || 0,
    x2: angleCoords.x2 || 0,
    y2: angleCoords.y2 || 0,
    colorStops: shapeColorStops
})
canvas.renderAll()

As of the code the for custom gradient generator, I still haven't got the time to open source it. I am using a combination of react color component and a drag & drop component to be able to create infinite color stops and drag them around, using the same drag&drop lib for the angle too. The concept is very simple, you just have to work on the mechanics of it. Will open source it as soon as I can.

@sandor
Copy link

sandor commented Oct 18, 2017

@Human-a Excellent! Thank you very much! Since I have a numberinput that is dealing with values 0-360 is even more easier to implement. And yes, I have to take care only about two color-stops. I will try to make a short demo for fabricjs.com, since I am already dealing with Fabricjs v.2 beta... Thanks again!

@asturur
Copy link
Member

asturur commented Aug 11, 2019

So much time passed, i think this is now fixed.
The percentage for gradients should be supported, not in the form of 100% but more like from 0 to 1. ( or higher/lower, you get the point )

Verified with this code with the wip build:

canvas.getActiveObject()
var angle = 0;
var anglePI = (-parseInt(angle, 10)) * (Math.PI / 180)
var angleCoords = {
    'x1': (Math.round(50 + Math.sin(anglePI) * 50)) / 100,
    'y1': (Math.round(50 + Math.cos(anglePI) * 50)) / 100,
    'x2': (Math.round(50 + Math.sin(anglePI + Math.PI) * 50)) / 100,
    'y2': (Math.round(50 + Math.cos(anglePI + Math.PI) * 50)) / 100,
}
canvas.getActiveObject()
.setGradient('fill', {
    type: 'linear',
    gradientUnits: 'percentage',
    x1: angleCoords.x1 || 0,
    y1: angleCoords.y1 || 0,
    x2: angleCoords.x2 || 0,
    y2: angleCoords.y2 || 0,
    colorStops: { 0: 'black', 1: 'yellow' }
})
canvas.renderAll()

@lcswillems
Copy link

gradientUnits is not specified in the type of the options parameter of setGradient: http://fabricjs.com/docs/fabric.Object.html#setGradient

Is it normal?

I also don't understand what this sentence (in the setGradient doc) means:

Sets gradient (fill or stroke) of an object percentages for x1,x2,y1,y2,r1,r2 together with gradientUnits 'pixels', are not supported.

What is an "object percantages"?

@lcswillems
Copy link

Moreover, why setGradient is marked deprecated? What should I use instead?

@asturur
Copy link
Member

asturur commented Jan 7, 2020

setGradient is deprecated because is just a middleman between creating a gradient and assigning it to an object.

to avoid unnecessary code, since the library is already big, i prefer to remove the not useful pieces.
I think we should do:

var gradient = new fabric.Gradient({ ...allTheOptionsForAGradient });
shape.set('fill', gradient); // this will also invalidate the cache and redraw at next render

@asturur
Copy link
Member

asturur commented Jan 7, 2020

What we need is really more tutorials and examples.
I wish i had time and creativity for those.

@lcswillems
Copy link

Thank you for the fast answer! It is indeed a better way to do with set!

@asturur
Copy link
Member

asturur commented Jan 7, 2020

im not sure if you are making some demo or investigating the library for some application.
Soon enough there will be a major release with a new interface for controls.
Basic functionality will be mostly the same ( scaling flip won't be available in the first beta ) but all the rest will be in place, plus ability to customize controls.

So if you need some custom action from the corners, or more corners, refrain from writing crazy overrides today because they won't work with version 4.

@lcswillems
Copy link

I am currently building a website with a simple image editor inside.

This is great news a version 4 is coming! And thank you for alerting me :)

@satendra02
Copy link

satendra02 commented Jan 23, 2020

Hi @asturur

The above angle logic doesn't work well with stroke.

At first, it applies correctly on the object but after scaling up the stroke disappears

Note: It is working fine with Image object. But not working with Shapes (Rect, Circle, Triangle, etc..)

I am using the latest release V3.6.1
Here is the jsfiddle:
http://jsfiddle.net/satendra02nov/124vw7Ln/56/

@asturur
Copy link
Member

asturur commented Jan 25, 2020

There is definitely something wrong with my stroke logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants