Skip to content

Lerping Color Correctly #615

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

Merged
merged 2 commits into from
May 11, 2015
Merged

Conversation

njoubert
Copy link
Contributor

@njoubert njoubert commented May 9, 2015

RGB color values actually represent the square root of the displayed color, not the color itself. Your monitor squares all the color values before displaying it.

This means that when interpolating between two colors, one must first square the individual values before mixing, and then take the square root, as explained here: https://www.youtube.com/watch?v=LKnqECcg6Gw
I think this looks significantly better! This patch fixes lerpColor to the correct approach. Here is an example:

OLD lerpColor from rgb(0,255,0) to rgb(255,0,0)
screenshot 2015-05-08 20 55 45

NEW lerpColor from rgb(0,255,0) to rgb(255,0,0)
screenshot 2015-05-08 20 55 31

@njoubert njoubert force-pushed the correct_color_blend branch from 30fd5c3 to 25d5416 Compare May 9, 2015 04:09
@kevinbarabash
Copy link
Contributor

Nice code, awesome video. This is super cool!

@jedahan
Copy link
Contributor

jedahan commented May 11, 2015

That video rocks, and thanks for the improvement!

lmccart pushed a commit that referenced this pull request May 11, 2015
@lmccart lmccart merged commit a5f73c0 into processing:master May 11, 2015
@lmccart
Copy link
Member

lmccart commented May 11, 2015

thanks!!

@njoubert njoubert deleted the correct_color_blend branch May 11, 2015 15:34
@njoubert
Copy link
Contributor Author

Yay!! Glad I could contribute, i love this project and use p5 all the time!

@jedierikb
Copy link

So... does processing-java do this? Should it?

@njoubert
Copy link
Contributor Author

As far as I know, it does not, and yes it should!

Source: Ph.D in Computer Graphics ;)

-Niels

On Mon, May 11, 2015 at 12:21 PM, Erik Blankinship <notifications@github.com

wrote:

So... does processing-java do this? Should it?


Reply to this email directly or view it on GitHub
#615 (comment).

@jedahan
Copy link
Contributor

jedahan commented May 12, 2015

Would it be worth adding a comment about this to the source?

/*
RGB color values actually represent the square root of the displayed color, not the color itself.
Your monitor squares all the color values before displaying it.
For more info check out https://www.youtube.com/watch?v=LKnqECcg6Gw
*/```

@kadamwhite
Copy link
Contributor

@jedahan I would defer to @lmccart & Co on whether to link to that specific YouTube video or not (vs just naming the phenomenon it describes), but my personal rubric for open source projects is that if anybody involved asks if it is worth adding a particular comment, then it is. :)

@njoubert
Copy link
Contributor Author

Yea I'd say it's not a crazy idea to add a comment. That being said, I
don't see a single comment anywhere in the p5.js file, so perhaps @lmccart
and the like has a stronger idea about this.

@lmccart how do I add something to the docs about this? It seems like
having the lerpColor reference mention this would be good!
http://p5js.org/reference/#/p5/lerpColor

-Niels

On Tue, May 12, 2015 at 5:47 AM, K.Adam White notifications@github.com
wrote:

@jedahan https://github.com/jedahan I would defer to @lmccart
https://github.com/lmccart & Co on whether to link to that specific
YouTube video or not (vs just naming the phenomenon it describes), but my
personal rubric for open source projects is that if anybody involved asks
if it is worth adding a particular comment, then it is. :)


Reply to this email directly or view it on GitHub
#615 (comment).

@kadamwhite
Copy link
Contributor

@njoubert Comments are stripped from the built file: Comments (and code changes) should be made to the individual module files within the source directory, where each method gets a documentation comment block (which are parsed to create the reference material). lerpColor's documentation block is here, and this functionality could either be mentioned in that comment, in an in-line comment by the line affected, or both. Hope that helps!

@lmccart
Copy link
Member

lmccart commented May 12, 2015

thanks @kadamwhite! yes, I think a comment would be great, and toward the goal of learning, let's include the video link as well. @njoubert do you want to add this? while you're at it, would you mind also making your change to the src/ rather than the lib/ file? https://github.com/processing/p5.js/blob/master/src/color/creating_reading.js#L293

@njoubert
Copy link
Contributor Author

Yup I'm adding it now!

This also means my previous commit had a problem, since I didn't change the
creating_reading.js file, but directly edited p5.js, correct? I'm about to
submit another patch, would you mind checking that i touch the right files?
Thanks!

-Niels

On Tue, May 12, 2015 at 9:08 AM, Lauren McCarthy notifications@github.com
wrote:

thanks @kadamwhite https://github.com/kadamwhite! yes, I think a
comment would be great, and toward the goal of learning, let's include the
video link as well. @njoubert https://github.com/njoubert do you want
to add this?


Reply to this email directly or view it on GitHub
#615 (comment).

@lmccart
Copy link
Member

lmccart commented May 12, 2015

yes this is correct, sorry I missed it last time I merged, I'll take a close look when you submit this new patch.

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.

6 participants