Discuss: Remove all Bump Mapping and Parallax Occlusion code

[lhofhansl]

As I look through the code - prompted by the inclusion of the bump-mapping test nodes into devtest - I'm coming more and more to the conclusion that the code should just be removed until we find someone who knows (and has time) to implement this for real.

Issues I've seen:

• normal maps are not translated from tangent space (i.e. are not adjusted for face's direction). There is an attempt it seems with calculating a tsEyeVec and tsLightVec in the vertex shader.
• wrong vectors used in the fragment shader
• diffuse light calculation is doubly wrong (uses world-eye vector, and should use the tangent-space light vector in the first place, diffuse light is independent of the position of the viewer)

Then there are other issues such as hardcoded lighting of the faces in the engine, which is at odds with lighting in the shader, since now you'd have to adjust for that.

I played around with this for a few hours, but it looks like there are other problems. Normals missing (or wrong), etc.

In the end, as much as it pains me (I'm a fan of better graphics), I think we might want to simply remove all bump mapping and related code, keep it simple. That way we also simplify the shader code and maybe provide a cleaner slate for trying again.

Thoughts?

[LoneWolfHT]

I don't use either of them. Looks ugly IMO

[kilbith]

TL;DR: RBA shaders is half-assed work and must be redone from scratch (example: tonemapping is a postprocessing thing normally)

[lhofhansl]

@kilbith I agree with the state of the shaders.

I'm trying to undo at least the parts that do not work, so we have less to redo.
With bump mapping et al code removed there really isn't much left, especially in the fragment shader.

Re: RBA (RIP)... I will say that he just didn't get to finish them. Someone (forgot who) showed me some video of his uncommitted work a while ago and that looked really impressive.

Related: Wasn't there some postprocessing PR? What happened to that one.

[kilbith]

Related: Wasn't there some postprocessing PR? What happened to that one.

#3623
#5913
#5932
#9895

Though none of them properly utilize the GPU framebuffers that I used in #6839.

[paramat]

👍

Bumpmapping looks so bad i think it is better to have none until it is done well than leave it in months or years waiting to be fixed and giving users a bad impression.
As you can see in #9241 i discovered very broken bumpmapping behaviour.
Few users actually use normalmap textures, most use the hideous 'generate normalmaps' which has already been removed, so most users will be losing bumpmapping anyway.

Parallax occlusion seemed to work and looked more acceptable to me. But i do not know the code as well as you do. The code may be closely linked to the bumpmapping code. It might help to make a fresh start with that too. So, i would support removing that too if you judge that to be best.

[lhofhansl]

Occlusion not exclusion. :)

Ok... I'll file a PR to remove all the code, including the UI and the various options passed to the shaders.

[lhofhansl]

@HybridDog Any opinion? (Since I missed #9241)
BTW, if we pass in the time of day it's not rocket science to calculate the position of the sun as a light source (at least for day-light).

[HybridDog]

I'm against the removal of Parallax Occlusion. In my opinion it is an interesting graphics feature and almost works correctly.

Interesting how the documentation reveals these shaders are actually 'experimental', unfinished and buggy. It is more of a mess than i realised. […]

I've recently added this documentation: #10096
Here is an issue for the fastfaces problem: #9243

[lhofhansl]

I was under the impression that parallax occlusion does not work without bump mapping.
Lemme look and try to disentangle the two.

Can parallax occlusion work without correct normals?

Edit: I got confused about normals. Parallax Occlusion can work without bump mapping.

I am still in favor removing both and starting again

[paramat]

I am still in favor removing both and starting again

Me too.

I did some testing of parallax occlusion. When more than 1 node away the node top face seems to appear ok, but note that the other node faces are inconsistent and much flatter, and partial pixels from the opposite edge are present:

Looking at the node top face, as your view angle decreases the height of the raised pixels decreases relative to their widths:

So the appearence is messy and has weird behaviour.

Up close the raised pixels 'lean' in odd ways, although this is much more subtle at typical distances, so not much of an issue. It is also not surprising since this is 'faked' 3D structure:

[HybridDog]

[…] and partial pixels from the opposite edge are present

I think this is supposed to happen unless the mod explicitly disables it for the tiles with tileable_vertical = false and/or tileable_horizontal = false.

Looking at the node top face, as your view angle decreases the height of the raised pixels decreases relative to their widths

This may be intentional, too. When the height decreases with the angle, the pixels from the opposite edges are less visible.

[paramat]

I think this is supposed to happen unless the mod explicitly disables it for the tiles with tileable_vertical = false and/or tileable_horizontal = false.

Yes. The wrapping pixels are needed for an area of nodes, but look wrong for one node. The 'tileable' flags force one behaviour or another, regardless of whether a node is alone or in an area of nodes, so they are not a complete solution.
However, this is an unsolvable problem, not bad shader coding.

This may be intentional, too.

I doubt it, sloppiness is far more likely.
It looks really bad. Even if it was intentional it was a poor decision, the wrapping pixels are less of a visual problem.

[lhofhansl]

#10487 is merged.

[lhofhansl]

@HybridDog Happy to work together to add graphics features back in.