Three.js: overbright colors not mentioned in documentation

[thmasn]

Description of the problem

there is no mention of overbright values >1.0 being allowed in THREE.Color. Although they work completely fine and are an important feature imo (p.e. for emissive materials)

it simply states 'value between 0 and 1. Default is 1.'

maybe add 'bigger than 1 is handled as overbright'

http://threejs.org/docs/?q=standard#Reference/Math/Color

Three.js version

• [ ] Dev
• [x] r79

• [ ] ...

  Browser

• [x] All of them

• [ ] Chrome
• [ ] Firefox

• [ ] Internet Explorer

  OS

• [x] All of them

• [ ] Windows
• [ ] Linux
• [ ] Android
• [ ] IOS
  

  Hardware Requirements (graphics card, VR Device, ...)

[manthrax]

Wow yeah this is huge. Could have saved me a lot of time and hacks!!! 1up!

[rohan-deshpande]

Just came over here to make an issue out of this and you'd already done it! FYI to all others here is my SO question which @thmasn answered. I find it pretty weird that the docs specifically mention that THREE.Color only takes arguments between 0 and 1 when that is not really the case. I think the docs need to be updated.

[bhouston]

If you use colors outside of the 0-1 range with THREE.Color a bunch of things can break -- specifically saving to JSON And loading from JSON as it uses a hex representation that only supports between 0-1.

[rohan-deshpande]

Hmm ok that's good to know. Is there anything else? That's not a huge concern for my particular use case which is doing brightening or darkening animations at run time.

[bhouston]

I believe as well if you use it as vertex colors it is converted to bytes, or maybe it isn't. You'd have to check. I think all material color parameters are passed in as floats though so you are okay there.

[rohan-deshpande]

Yeah, I mean if that's the only issue I think it's more a JSON exporter/importer bug than a three bug. Fixing that shouldn't be too hard, just save/parse it as an array of floats or something.

[WestLangley]

THREE.Color outside the range 0-1 is not supported.

It is straight-forward to check the source for the methods that will be problematic. Conversion to/from HSL is one; ColorConverter is another. Gamma correction may be another.

[manthrax]

I think a useful discussion would be how to enable what this feature enables, before it's decided to restrict/remove it out of hand. Namely overbright without resorting to multiple overlays+additive blending. a pretty widely used technique.

Custom shader support is pretty janky at present, or at least has been last I dug into it.. i.e. getting a custom shader working that interacts properly with the existing lighting/shadowing model.
Let alone with sprite material which is already a custom shader...

[rohan-deshpande]

Agreed, "not supported" is neither helpful nor relevant to this issue/discussion. The docs explicitly say that THREE.Color only accepts values between 0 and 1, this is actually not true. Brightening and darkening are super useful especially to games so this should be considered as a feature to add in IMO.

[mrdoob]

If you use colors outside of the 0-1 range with THREE.Color a bunch of things can break -- specifically saving to JSON And loading from JSON as it uses a hex representation that only supports between 0-1.

Maybe we could change that to an array...?

[rohan-deshpande]

@mrdoob yep! I suggested that above hehe that would totally work IMO.

[WestLangley]

@rohan-deshpande said

"not supported" is neither helpful nor relevant to this issue/discussion.

All the Color methods work correctly when the values are in the range [0, 1]. Outside that range, not all methods work correctly. Hence, values outside that range are not supported.

The docs explicitly say that THREE.Color only accepts values between 0 and 1, this is actually not true.

It is true. You are free to pass in any value you want, but there is no guarantee the Color methods will work correctly if you do so.

[rohan-deshpande]

@WestLangley if a method doesn't throw an exception when an "unsupported" argument is passed and even returns a different result based on that argument, then it saying it only accepts a specific argument that doesn't include that value is completely false. I'd be very interested to see the unit test you have written for THREE.Color that supports your claim.

[manthrax]

I disagree, West. Saying that it only accepts values in a range indicates that it will perform clamping.. which it doesnt.. and in fact, sending values out of that range enables new useful behavior. Knowing about this could have saved me time in the past, working around that percieved limit by making additional source art, or canvas tricks. Just a note in the docs a la "setting values out of these ranges may produce desirable results (like overbright) but cannot be guaranteed across all platforms blahblah. Unless anyone has a more "legit" solution for the over/underbright technique?

[looeee]

This is an issue that I feel needs clarification - three.js currently does not warn when parameters are outside the expected range (except in a couple of places). It would be pretty simple to implement warnings / clamping to expected values in constructors, but it would add a lot of code that is not strictly necessary, might affect performance and the expected ranges are generally well documented.

I'm in favour of leaving it as is, but if so perhaps it should be noted somewhere as an "official" three.js coding convention. @mrdoob, @WestLangley, @Mugen87 do you guys have any thoughts on this? How do other libraries handle this?

[mrdoob]

Maybe we can change the wording... "recommended values" instead of "accepts value in range"?

[looeee]

Yes, or perhaps "the valid range is..." which I think is the wording I already used in a few places.

Actually going over the docs I take back saying that ranges are well documented. They are not mentioned at all in a lot of cases, just the default value. I guess that is a separate issue though...

I'm trying to think where would be the best place to note this, what do you think of adding a FAQ regarding ranges?

[WestLangley]

Values outside the supported range may produce unexpected or incorrect results.

what do you think of adding a FAQ regarding ranges?

I am not a fan having a separate document for that.

[looeee]

@WestLangley there's already a FAQ page: https://threejs.org/docs/#Manual/Introduction/FAQ
I meant adding a question to that.

[Mugen87]

IMO, this issue can be closed. The documentation already states that RGB color values have to be in the range of [0,1]. This automatically implies that values outside the range are invalid.

Since this issue was the only one in the last three years about this topic, an entry at the FAQ site seems not necessary to me (since it's not a frequently asked question).