Three.js: Introduce FogExp and consider refactoring+FogLinear

[EliasHasle]

In https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/ShaderChunk/fog_fragment.glsl.js, under the FOG_EXP2 ifdef:

float fogFactor = whiteCompliment( exp2( - fogDensity * fogDensity * fogDepth * fogDepth * LOG2 ) );

Is exp2(something*LOG2) intended here? LOG2 is 1/ln(2), and exp2 is the base 2 exponential, so the expression simplifies to exp(something).

I also wonder why the density and depth are squared. What is the physical analogy? For comparison, they are not squared in the implementation here: https://iquilezles.org/www/articles/fog/fog.htm

[WestLangley]

Source of the fog equations...

This: https://github.com/mrdoob/three.js/issues/61#issuecomment-623929

references this: http://www.geeks3d.com/20100228/fog-in-glsl-webgl/

which references this: http://ozone3d.net/tutorials/glsl_fog/p02.php

[EliasHasle]

...which references this: http://glprogramming.com/red/chapter06.html#name3 (and another, dead, link)

None of them explain _why_ there is a fogExp2, but it indeed appears to be a concept, alongside foxExp. The ozone3d source repeats the same complication error that I pointed out. Note that the glprogramming source does not. It defines fogExp2 as e^-(density*z)^2, which is equivalent and simpler. The usage of exp2 looks like a mistake, probably motivated by "Exp2" in the name, which apparently refers to the squaring.

I suggest adding a FogExp, which does no squaring.

[mrdoob]

So what would the glsl be for FOG_EXP?

[EliasHasle]

````

ifdef FOG_EXP

    float fogFactor = whiteComplement( exp( - fogDensity * fogDepth ) );

````

And FOG_EXP2 can be simplified to:

````

ifdef FOG_EXP2

    float fogFactor = 1.0 - exp( - fogDensity * fogDensity * fogDepth * fogDepth );

````

You don't even need to apply the whiteCompliment (should that be whiteComplement, no?), because the squaring makes the argument always non-positive and the exp always at most 1 (and still above 0).

[WestLangley]

FYI, Microsoft Fog Formulas (Direct3D 9).

Since there are no complaints, I would not change what is currently supported.

You can change the implementation if you want, as long as it produces the same result.

[EliasHasle]

The DirectX fog types are the same three: linear, exp and exp of square (so-called "exp2", but not using the exp2 function). I may perhaps throw together a PR simplifying the exp2 variant and introducing the simple exp variant, if @mrdoob isn't working on that already. (Probably not in a few days, though.)

[WestLangley]

I may perhaps ... introduce the simple exp variant

As I said, since there are no complaints, I would not introduce additional variants.

[EliasHasle]

Then I complain right now. 🙂 From what I can tell, simple exponential fog is the one of the three that makes the most physical sense. Here is a discussion of different types: http://in2gpu.com/2014/07/22/create-fog-shader/

[EliasHasle]

I consider making two PRs:

1. Simplify the FogExp2 shader code, correct the documentation and rename the fogExp parameter to the more accurate fogExp2. As far as I know, this parameter is used only internally, so shouldn't be a problem.
2. Introduce FogExp class and shader code that uses simple exponential decay. Add full support in all places where FogExp2 is currently mentioned, maybe even in examples. And docs.

Is the plan OK, @mrdoob?

[WestLangley]

Introduce FogExp class and shader code that uses simple exponential decay.

I see no compelling reason to add additional decay functions.

A simple exponential applies more fog close to the camera. ( It has very similar behavior to that of linear fog if fog.near is zero. )

[EliasHasle]

Exponential decay is more physically sensible. I am interested in approximations to photo-realism. Also, it seems to be usually (considering the sources linked in this discussion) mentioned as one of the three most basic types of fog. Even Babylon.js gets it right: https://doc.babylonjs.com/api/classes/babylon.scene#fogmode (is that "Bingo! Compelling." ? 😉 )

[mrdoob]

I think adding support for FOG_EXP is good.
Not sure how I feel about a new class for it, but it can be refactored later.

[EliasHasle]

What is the alternative to adding a new class? Using a single Fog class with a mode parameter?

whiteCompliment (should that be whiteComplement, no?)

(Yes. https://www.google.com/search?q=complement+vs+compliment)

[mrdoob]

whiteCompliment (should that be whiteComplement, no?)

(Yes. https://www.google.com/search?q=complement+vs+compliment)

Fixed. https://github.com/mrdoob/three.js/commit/969c06d471235b0122a55106b96eb2259a95d25b

Seems like only the fog use(d) that function btw.

[mrdoob]

What is the alternative to adding a new class? Using a single Fog class with a mode parameter?

Yeah... But it may just be simpler to make FogExp and then make FogExp2 extend it.

[EliasHasle]

... make FogExp and then make FogExp2 extend it.

Since they share the same constructor parameters? It looks like making FogExp2 a subclass, in addition to not representing a proper special case of FogExp, will save no code or time, and will make the tests less clean. The isFogExp test should only return true for FogExp, not for FogExp2, which is checked with isFogExp2.

[WestLangley]

@Mugen87 It may be a language barrier issue, but when you change the title so the PR and the issue have the same title, it is confusing... even though you add the "suggestion" label. This has happened several times before...

_Suggestion: Introduce FogExp_ - sounds like a suggestion.

_Introduce FogExp_ - sounds like a PR.

So in this case, I would retain the OP's title: Suggestion: Introduce FogExp and simplify FogExp2.

For clarity.

[Mugen87]

Just as you like.

Although I think you make it unnecessarily complicated. The tags are exactly for this use case (so people do not clutter the issue title). And I've never heard that a project at github uses a different nomenclature for their issue and PR titles....

[WestLangley]

I find this confusing...

issue tite:  "Support Foobar"     label: Suggestion

PR title:    "Support Foobar"     label: none

Of course, it is just my opinion, and it is no-big-deal. You are very good about being consistent on things, so I thought I bring it up. :-)

[EliasHasle]

"Suggestion: Introduce FogExp and simplify FogExp2" better summarizes the discussion. The issue spawned a PR, and is expected to spawn another one, which is related.

[mrdoob]

The isFogExp test should only return true for FogExp, not for FogExp2, which is checked with isFogExp2.

That's fair. Yeah, I guess we can create another class then.

[EliasHasle]

@mrdoob See #17355 for extended step 2.

[EliasHasle]

17355 also modifies the docs for Fog, removing the word "linear", because I discovered from the GLSL that it is indeed not linear, but a Cubic Hermite spline (smoothstep). If FOG_LINEAR is also desired, a natural hierarchy could look like this:

Fog RangeFog FogLinear FogSmooth DensityFog FogExp FogExp2

(Not sure about the naming with alternately prefixed and suffixed "Fog". This would be avoided if the lowest level were different modes instead.)

However, making Fog an abstract base class breaks backwards compatibility. :-/

A future extension that would fit in on the base class level would be a cubemap, to accompany or replace fog color (but requires accessing world coordinates). But the possibilities are endless, and advanced users may eventually wish to abandon any fixed presets.

Is there interest for a purely linear range fog, though, if anything for the sake of completeness? My personal appetite will be satisfied with FogExp and correct documentation of the existing methods.

[EliasHasle]

Closing this. Discussion continues in #17355 and #17420.