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

Feature suggestion: Support mixing with different blend modes #99

Open
loilo opened this issue Dec 7, 2016 · 9 comments
Open

Feature suggestion: Support mixing with different blend modes #99

loilo opened this issue Dec 7, 2016 · 9 comments

Comments

@loilo
Copy link

loilo commented Dec 7, 2016

I was about to write my own color library with a nicely clean API which in the end I found to resemble your approach very closely. So I screwed that over and instead would like to propose one big feature I wanted to have in my toolkit but that's missing in this library.

What

I planned a feature that allows mixing of colors not only with the standard "source-over" algorithm but with all the standard blend modes that Photoshop and the W3C are presenting.

Who

It was a little tricky to find the algorithms for all the blend modes with alpha channel taken into account but I invested quite some time in researching and eventually was successful. So I already completely implemented all the blend modes in TypeScript—it shouldn't be too hard to transfer the implementations to this library. I'd be happy to attach a PR.

How

Now comes the tricky part: What I'm not sure about is how to—in case you're positive about this feature at all—integrate that thing into the existing API.

The feature would probably need an all new .blend() method as a kind of "enhanced" .mix() because

a) every architectural approach I took to adjust the .mix() parameters for this feature introduced some kind of design flaw.
b) using the existing .mix() at all would only work sanely if it by default handled color mixing exactly like the normal blend mode does—which isn't the case. That would require calculating the resulting alpha in a different way than just averaging the alphas of the mixed colors.

You got any thoughts on this? Do you like the idea? What would be your approach with the API facet?

@Qix-
Copy link
Owner

Qix- commented Dec 8, 2016

I think that's a great idea and would love to see a PR submitted for it :)

As for the API design, this kind of goes right along with how I wanted 2.x to look.

Since the library has been growing at a steadily increasing rate over the last 6 months or so (both here and over at color-convert and color-string), the API is beginning to look a bit... ragged. Lots of naming overlaps and confusion make for a not-so-intuitive API.

The 1.x rewrite tended to some of that, but if we were to incorporate these changes (which I totally agree belong in this library!) then the API is going to need to be ready for them.

One of the things I was thinking was to have something similar to:

Color.rgb(146, 166, 277).hsl.string()
Color('#FF0000').hsl.hue();
Color.cmyk(50, 50, 50, 100).blend.multiply(Color.rgb(184, 55, 210));

I feel like that API is much more intuitive and readable and helps 'namespace' operations out into logical units that are both self documenting as well as exclusive to the particular color format or operation being worked with.

All of the blending operations would be in Color().mix or Color().blend (I like mix better myself) and would be accessed like Color().mix.add(...), Color().mix.subtract(...), Color().mix.pinLight(...), etc.

Thoughts?

@loilo
Copy link
Author

loilo commented Dec 8, 2016

I really like idea for the API. Making blend modes properties of .mix would even allow to keep compatibility with the current state without a semver breaking change.

@Qix-
Copy link
Owner

Qix- commented Dec 8, 2016

True, though it would make the API really unshapely - plus, the approach for such an API would require using getters which can't be added onto functions.

@loilo
Copy link
Author

loilo commented Dec 8, 2016

I wouldn't have a problem with the unshapely part—the existing behaviour of .mix as a function could just be deprecated/removed with the next major release.

Regarding getters: to be honest that's a quirk of JavaScript I didn't even know about. Are you referencing the Color(...).hsl etc?

I didn't have that problem in my own API approach since I completely namespaced conversions out, like Color(...).as.hsl (though I hadn't decided yet if I wanted to use as or to).

@Qix-
Copy link
Owner

Qix- commented Dec 8, 2016

So with parameters like that, you have to be able to bind them back to the original color object. You can do it by constructing those properties right then and there (which is expensive and wasteful) or you can defer execution with a getter. I generally opt for the latter.

@loilo
Copy link
Author

loilo commented Dec 8, 2016

Mh, yeah—I don't have too much insight into browser perf stuff (though constructor-defined properties being slower than prototype-defined ones is something I've already heard of).

So how'd you like to have the blend modes attached in the PR? As of now I'd have assigned them to the .mix on instantiation but with that caveat in mind what would you suggest instead?

@loilo
Copy link
Author

loilo commented Dec 8, 2016

Also what I noticed while working on this:

The blending algorithms are about 300+ lines of code.
Since they are completely decoupled from the color library—they really just take two { r, g, b, a } values plus a mode string and return another { r, g, b, a }—I'd tend to source this functionality out into a separate module, just like color-convert and color-string. are color-blend would probably be natural.

Thoughts on this?

@loilo
Copy link
Author

loilo commented Dec 13, 2016

So I just did what I thought about last, I made this feature a standalone package.

It's very straightforward and would be easy to put into the color library. We should still agree on a way to implement it into the API though.

@Qix-
Copy link
Owner

Qix- commented Dec 13, 2016

I'd tend to source this functionality out into a separate module

I agree. I'm still not sure on how the API should look, but I'll leave this open so I can think about it.

Thanks for the great package by the way, I could see my self using it in the future.

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

No branches or pull requests

2 participants