Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

mix weight parameter has been switched #442

Closed
jakepruitt opened this issue Apr 18, 2016 · 3 comments
Closed

mix weight parameter has been switched #442

jakepruitt opened this issue Apr 18, 2016 · 3 comments

Comments

@jakepruitt
Copy link

So it looks like the recent fix to mix in the RGB color space switched the functionality of the mix function's weight parameter.

Functionality change:

old: mix(blue,purple,100) = blue
new: mix(blue,purple,100) = purple

This is not detectable with 50% mixes, so it was pretty easy to slip through the cracks of the tests. Also, the documentation doesn't explicitly say how this functionality should work. It is breaking people's styles though, like @xrwang's style here: mapbox/mapbox-studio-classic#1553.

@springmeyer @nebulon42 - would love to hear your opinions :)

@pnorman
Copy link
Contributor

pnorman commented Apr 18, 2016

Right now there's unquestionably a bug in that the alpha is alpha1*p + alpha2*(1-p) while the color is color1*(1-p)+color2*p
(Assuming you define addition and multiplication of colours sensibly)

I checked with with less and it just defines it as "a percentage balance point between the two colors, defaults to 50%" (less/less-docs#409)

However, less does define tint(color,weight) = mix(white, color, weight). Assuming tint(color, 0%) = color, then mix with weight=0 is 0% of the first colour and 100% of the second colour, which is the reverse of chroma.js's chroma.mix.

The relevant line should be something like chroma.mix(color2.toString(), color1.toString(), p, 'rgb'); or chroma.mix(color1.toString(), color2.toString(), 1-p, 'rgb'); with a comment that chroma.mix is the opposite order from less and cartocss. Right now it's var mix = chroma.mix(color1.toString(), color2.toString(), p, 'rgb');

@nebulon42
Copy link
Collaborator

nebulon42 commented Apr 19, 2016

@jakepruitt thanks for reporting and @pnorman thanks for the background research. I had to get rid of the chroma.mix function since it behaved differently than LESS/SASS regarding mixing RGBA colours. I have now expanded the test cases and the mix function should work like in carto 0.15.3. You're welcome to verify that this is the case.

@tmcw @springmeyer I have prepared everything for a 0.16.1 release. Please sign off on it and tag/publish, thanks.

springmeyer pushed a commit to mapbox/mapbox-studio-classic that referenced this issue Apr 19, 2016
@springmeyer
Copy link

Thank you @nebulon42! @xrwang just tested and confirmed this is fixed. So I just tagged and published v0.16.1.

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

No branches or pull requests

4 participants