HomeWildfire Games

Water GLSL shader improvements around reflections and whitespace fixes.
Concern RaisedrP22297

Description

Water GLSL shader improvements around reflections and whitespace fixes.

This improves refractions around entities close to the surface, such as fishes, by handling depth better and by clipping the water plane a little lower.

This uses the skybox for reflections when refractions are enabled but reflections are disabled, making it possible to play with reflections disabled without having super-ugly water (arguably a performance improvement).

Differential Revision: https://code.wildfiregames.com/D359

Event Timeline

vladislavbelov raised a concern with this commit.Jun 15 2019, 9:49 PM
vladislavbelov added a subscriber: vladislavbelov.
vladislavbelov added inline comments.
/ps/trunk/binaries/data/mods/public/shaders/glsl/water_high.fs
310

This block was removed (rP22039) and shouldn't present here.

This commit now has outstanding concerns.Jun 15 2019, 9:49 PM
elexis raised a concern with this commit.Jun 21 2019, 12:38 PM
elexis added a subscriber: elexis.

This uses the skybox for reflections when reflections are disabled

Which is a contradiction.
I suppose its a performance difference of several orders of magnitude between reflecting only the skybox and reflecting everything.
How does one address the contradiction instead of talking it away?
By rebranding the option to only speak about the reflection of entities.
That might be explained in options.json so that it's communicated to the graphics-settings-tuning user, but more importantly in the code definition of the option.

-If a player has refraction and reflection disabled, we return a gradient of blue based on the Y component.
-If a player has refraction OR reflection, we return a reflection of the actual skybox used.

It's also counterintuitive that the skybox is reflected if reflection OR refraction is enabled but that it is not reflected if both are disabled.

This allows the user to enable Skybox reflections by triggering the refractions option if reflections have been disabled, even more so without notification that this was made this weird.

It would be much cleaner with: Reflections: None, Sky, Everything.

vladislavbelov added inline comments.Jun 21 2019, 5:50 PM
/ps/trunk/source/renderer/Renderer.cpp
1140

How does it help? Any visual comparison?

This uses the skybox for reflections when reflections are disabled

Which is a contradiction.

It's not. Disabling reflections has never meant "show nothing". In fact, from the beginning reflections have been about entity reflection, so I guess your comment is correct. Again, I question your use of the concern option over a trac ticket.

/ps/trunk/source/renderer/Renderer.cpp
1140

There are pictures in the original diff which I believe show this. An easy way to test it is to look close to a ship's hull.

It would be much cleaner with: Reflections: None, Sky, Everything.

That'd be cool!