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

Add gradient feature. #164

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add gradient feature. #164

wants to merge 3 commits into from

Conversation

aurium
Copy link

@aurium aurium commented Oct 21, 2019

Methods for two color models: gradientRGB and gradientHSL.
The gradientHSL supports clokwise and counterclokwise hue rotations.

Methods for two color models: gradientRGB and gradientHSL.
The gradientHSL supports clokwise and counterclokwise hue rotations.
@Qix-
Copy link
Owner

Qix- commented Oct 21, 2019

What's the point of this? Why not use .mix()? seems like this is gold plating.

@aurium
Copy link
Author

aurium commented Oct 21, 2019

To do a gradient, mix is more computationally costly.

@aurium
Copy link
Author

aurium commented Oct 21, 2019

The Travis errors are not related to my patch... Node 0.12 still a valid target?

Copy link
Owner

@Qix- Qix- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that some of the above below (github keeps changing) comments apply to both functions.


I did a benchmark and the gradient function is indeed faster for steps >= 4.

const Color = require('./color');
const benchmark = require('benchmark');

const srcColor = new Color('#FF0000');
const destColor = new Color('#00FF00');

new benchmark.Suite()
	.add('mix3', function () {
		const gradient = [
			srcColor,
			srcColor.mix(destColor, 0.5),
			destColor
		];
	})
	.add('grad3', function () {
		const gradient = srcColor.gradientRGB(destColor, 3);
	})
	.on('cycle', function(event) {
		console.log(String(event.target));
	})
	.on('complete', function() {
		console.log('Fastest is ' + this.filter('fastest').map('name'));
	})
	.run({ 'async': true });


new benchmark.Suite()
	.add('mix4', function () {
		const gradient = [
			srcColor,
			srcColor.mix(destColor, 0.33),
			srcColor.mix(destColor, 0.66),
			destColor
		];
	})
	.add('grad4', function () {
		const gradient = srcColor.gradientRGB(destColor, 4);
	})
	.on('cycle', function(event) {
		console.log(String(event.target));
	})
	.on('complete', function() {
		console.log('Fastest is ' + this.filter('fastest').map('name'));
	})
	.run({ 'async': true });


new benchmark.Suite()
	.add('mix5', function () {
		const gradient = [
			srcColor,
			srcColor.mix(destColor, 0.25),
			srcColor.mix(destColor, 0.5),
			srcColor.mix(destColor, 0.75),
			destColor
		];
	})
	.add('grad5', function () {
		const gradient = srcColor.gradientRGB(destColor, 5);
	})
	.on('cycle', function(event) {
		console.log(String(event.target));
	})
	.on('complete', function() {
		console.log('Fastest is ' + this.filter('fastest').map('name'));
	})
	.run({ 'async': true });

index.js Outdated Show resolved Hide resolved
index.js Show resolved Hide resolved
index.js Show resolved Hide resolved
@Qix-
Copy link
Owner

Qix- commented Oct 25, 2019

The Travis errors are not related to my patch... Node 0.12 still a valid target?

No, don't worry about CI. I'll let you know if there are any actionable items. This package needs to be refreshed sometime soon anyway - 0.12 will be removed, but not before this PR is merged unfortunately.

* Do not require ECMA6.
* Do not round channel values.
* Do not compute small and obivious gradients.
@aurium
Copy link
Author

aurium commented Nov 29, 2019

Hi @Qix-, is this ok? Do you need any help with this repo?

@Qix-
Copy link
Owner

Qix- commented Nov 30, 2019

See https://github.com/Qix-/color/pull/158#issuecomment-558352861 on why changes take forever for me to merge.

@aurium
Copy link
Author

aurium commented Dec 2, 2019

There is a channel where you share your thoughts about this? A page at @Qix-/color/wiki maybe?
Do you use Telegram? You can find me with this same @name there.

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

Successfully merging this pull request may close these issues.

2 participants