Three.js: gradientMap GLSL is probably not correct?

[greggman]

Description of the problem

Looking at the gradientmap GLSL

this line

 vec2 coord = vec2( dotNL * 0.5 + 0.5, 0.0 );

this will not actually give a correct gradient. Imagine you have a 2x1 texture.

The correct gradient is here
       |             |
       V             V
+-------------+-------------+
|             |             |
+-------------+-------------+
^                           ^
|                           |
But the code above uses this. That means much of the sampling is outside gradient.

In other words you need to read the gradient from the center of the first texel to the center of the last texel. Past the center on either side is just solid color. The linear interpolation happens between texels which are defined by center points.

The correct code unfortunately requires the width of the gradient texture

 float gradientPos = dotNL * 0.5 + 0.5;
 float sampleLoc = 0.5 + (gradientWidth - 1) * gradientPos;
 vec2 coord = vec2(sampleLoc / gradientWidth, 0.0 );

Does anyone care?

Three.js version

• [X] Dev
• [X] r94

[mrdoob]

/ping @takahirox

[takahirox]

I'll look into a few weeks. Sorry, I've been busy lately.

[Mugen87]

I think this issue can be closed. According to Illustrative Rendering Illustrative Rendering in Team Fortress 2 page 30, the warping function which is used to derive texture coordinates from a ramp texture is defined as:

In code, that would be:

float dotNL = dot( normal, lightDirection );
float w = 0.5 * dotNL + 0.5;
vec3 rampColor =  texture2D( rampTex, vec2( w ) ).rgb;

So I think it's okay to stick to the current version, especially since the approach is properly documented.

[greggman]

You can prove the current one is wrong, all you have to do is make a 2 pixel ramp. Black and White. If it's correct it will look exactly the same as normal lighting. If it's wrong it won't. So try it and you'll see it's wrong as it is.

In fact I just wrote an article about it and show the issue

https://webglfundamentals.org/webgl/lessons/webgl-ramp-textures.html

The fact that it's in some other book doesn't mean they aren't wrong.

[Mugen87]

TBH, I've researched this topic for quite a while today and I found no other resource that does it like you presented here.

Especially the requirement for an additional uniform makes your approach inconvenient.

[greggman]

It's ridiculous to claim my method is wrong when it's easy to prove. That fact that so many other people got it wrong is irrelevant.

To put it another way. If you have a 2 pixel ramp (pink and green)

The ramp the code produces now is this

The ramp the code I'm suggesting produces is this

If you want to keep wrong because all the referecnes you found were wrong that's up to you. I'm just pointing out it's wrong.

[Mugen87]

It's ridiculous to claim my method is wrong when it's easy to prove

Um, did you actually read my comments? I did not say your method is wrong. Just inconvenient because of the introduction of an additional uniform which requires the texture's dimension.

[Mugen87]

BTW: I can't see this step yellowish/orangey color change of your second screenshot in this live demo:

https://jsfiddle.net/yz54sdn9/

How needs the demo changed so it can be reproduced?

Edit: Updated fiddle.

[greggman]

Sorry, I mis-interpreted

I've researched this topic for quite a while today and I found no other resource that does it like you

as claiming I did it wrong but I see that's not what it said.

Still, it is wrong as is.

The code has a value that goes from 0.0 to 1.0

      float w = 0.5 * dotNL + 0.5;

Using the existing code it is impossible to make a ramp that produces the same input. In other words

      float w = 0.5 * dotNL + 0.5;
      float r = texture2D(rampTexture, vec2(w)).r;

It is impossible for r to w over the ramp. Even if you make a 256x1 ramp with all values from 0 to 255 in the texture with the incorrect code it won't reproduce w. Where as with the corrected method, given a 2 pixel black and white image (or a 256x1) you'll get r == w over the entire range of w.

I get that the extra uniform is inconvenient but if you want to use ramps for anything you need to do the right thing else you get bad results and then wonder why when you tried to use a small ramp you got 50% more edge colors than you expected. The larger the ramp the less the problem will stick out but I'd argue it's more common to have ramps with only a few stops, the most common being 2 or 3.

[greggman]

The yellowish isn't the problem. The problem is the edges are too thick

the current ramp doesn't go from pink to green. It goes pink pink pink pink to green green green green

What a ramp should be

 0.0 0.1 0.2 0.3 0.4 0.5 0.6 0.7 0.8 0.9 1.0

What the current code is making

0.0 0.0 0.0 0.2 0.4 0.6 0.8 1.0 1.0 1.0 1.0

Some demos

This one shows plotting w on the left

float w = 0.5 * dotNL + 0.5;

followed by the current ramp in the middle and the fixed ramp on the right.

https://jsfiddle.net/greggman/e2d365y0/

This one shows a 3 stop ramp

https://jsfiddle.net/greggman/b74hq3tu/

[greggman]

Actually that second one is wrong ... sigh.... the problem is actually worse. If filtering is Linear then you need the math I'm proposing to get the correct ramp. If filtering is Nearest then you don't

[Mugen87]

Do we actually need THREE.LinearFilter for gradient maps in context of a toon/cel shading material? Maybe it's enough to just mention in the docs to always use THREE.NearestFilter for minFilter and magFilter?

[greggman]

Sure, although it's called gradientMap in GLSL. Maybe it needs a note there as well that's it's not correct for gradient maps.

[Mugen87]

Would be rampMap less confusing from your point of view?

[greggman]

toonMap? stepMap? Sorry I'm not trying to bike shed. I have no idea. ramps to me go like this

gradient maps are common in photoshop, illustrator, gIMP, etc...

[WestLangley]

FWIW, I like this suggestion: https://github.com/mrdoob/three.js/issues/16257#issuecomment-538018419.

[Mugen87]

@WestLangley Sounds interesting, yes. I mean introducing an alternative API for defining gradients.

However, @greggman seems to have issues with the term gradientMap itself. Because in cel shading, you are not rendering real color gradients but single tones.

[Mugen87]

I mean introducing an alternative API for defining gradients.

Sidenote: Apart from defining the amount of tones you could also allow the definition of the respective thresholds (which define when you switch from one tone to the next one).

[Mugen87]

Okay, the docs are updated to make clear that the engine assume nearest filtering when using gradient maps. Against this backdrop, the current GLSL code is correct. I suggest we discuss all further API and GLSL changes in #16257.

@greggman Thanks for the discussion here. It's good that we clarified the semantics of gradientMap and how it is intended to be used.