Page MenuHomeWildfire Games

Water shader improvements: fix some of the redness, fix edge-of-map showing up, improve entity-under-water, slight improvement to reflection edges.
ClosedPublic

Authored by wraitii on Apr 19 2017, 11:40 AM.

Details

Reviewers
vladislavbelov
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22297: Water GLSL shader improvements around reflections and whitespace fixes.
Summary

Updated summary:

-I've found a rather easy way to improve the "side of map" effect, where the water looked weird near the sides of the map. This re-uses the code to build "sides", basically. This is a big improvement on some maps.

-I've found a way to fix redness. Haven't really been able to see any red pixels (except for the "moving-the-camera" related bug, but that's a different thing). This fixes in particular Extinct Volcano (@elexis ).

-Slight improvement to reflection near the edge of stuff, by shifting the clipping plane down a tad (same thing done with refractions).

-Code improvements make entities under water (such as fish) look better.

Comparison screenshots:

Side of the map:

BeforeAfter

Fishes:

BeforeAfter
Test Plan

Start the game with real depth and refraction activated. Check out the water and the fishes in particular.

Should check all options but normally it should have smaller requirements the SVN, so I'm not toooo worried there.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
D359_water_improvements
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6658
Build 10969: Vulcan BuildJenkins
Build 10968: arc lint + arc unit

Event Timeline

wraitii created this revision.Apr 19 2017, 11:40 AM
wraitii edited the summary of this revision. (Show Details)
Vulcan added a subscriber: Vulcan.Apr 19 2017, 12:25 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/820/ for more details.

wraitii updated this revision to Diff 1373.Apr 20 2017, 10:18 AM
wraitii retitled this revision from Improve GLSL water shader refraction around fish. to Water shader improvements: fix redness, fix edge-of-map showing up, improve entity-under-water, slight improvement to reflection edges..
wraitii edited the summary of this revision. (Show Details)
wraitii edited the test plan for this revision. (Show Details)
wraitii added a subscriber: elexis.

See Updated summary

Owners added a subscriber: Restricted Owners Package.Apr 20 2017, 10:18 AM
wraitii edited the summary of this revision. (Show Details)Apr 20 2017, 10:20 AM

Tested in game, see the described improvement, notice nothing wrong on different maps with different settings.
System: linux

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/832/ for more details.

vladislavbelov added a subscriber: vladislavbelov.EditedApr 21 2017, 2:00 AM

It doesn't fix redness (Cycladic Archipelago):

Did you see/test my #2692?

vladislavbelov edited the summary of this revision. (Show Details)Apr 21 2017, 2:10 AM

Some of your screenshots look like the other water bug, which I don't know how to explain so far.

If you can make a patch file, I will help you test, thank you!

wraitii updated this revision to Diff 1538.Apr 29 2017, 5:32 PM

This also changes:
-Fix a bug with shadow on water that made the water overall brighter.
-If refractions are activated, use the skybox for reflections (cheap and nice, makes reflections less useful)
-Improve the non-refraction non-reflection reflection from a weird cyan to a nicer sky-like blend which looks much better overall.

Vladislav has confirmed the above redness is from a separate issue. This is thus RC.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/927/ for more details.

@vladislavbelov care to review this so it can get committed?

In D359#38593, @wraitii wrote:

@vladislavbelov care to review this so it can get committed?

Ok, I'll try to test it tomorrow.

(As it apparently wasn't mentioned yet but might help with testing,
about every map generation of Polar Sea has very obvious water rendering defects.
There are some water parts rendered from uninitialized data, presumably reflections from pixels that were never rendered.
Sometimes shows up as red, but after hiding and showing the affected area again, it instead renders incorrect pixels that were some time previously.)

(As it apparently wasn't mentioned yet but might help with testing,
about every map generation of Polar Sea has very obvious water rendering defects.
There are some water parts rendered from uninitialized data, presumably reflections from pixels that were never rendered.
Sometimes shows up as red, but after hiding and showing the affected area again, it instead renders incorrect pixels that were some time previously.)

Yeah that's a different issue, not fixed by this patch. I'm assuming it's linked to the scissoring of the water

Bumping, this is a HUGE improvement, particularly on lower settings.

bump @vladislavbelov or anyone else really.

It'd be good to add screenshots with a not deep water, and some objects in water, like towers or units. The patch looks good for me.

binaries/data/mods/public/shaders/glsl/water_high.fs
192

Some missing spaces here and above.

source/renderer/Renderer.cpp
1138

Why it's exactly 2.0f?

Comparison screenshots (also see above for other cases):

Reflections + Refractions enabled
(very few differences here except for the above screenshots)

Before:


After:

Before:


After:

Refractions enabled only
(here this basically makes this mode usable instead of extremely ugly)

Before:


After:

Before:


After:

Nothing enabled
(Here this improves the effect by removing the cyan tint)

Before:


After:

Before:


After:

(no noticeable difference when you only enable reflections).

wraitii updated this revision to Diff 5395.Jan 21 2018, 5:05 PM
wraitii retitled this revision from Water shader improvements: fix redness, fix edge-of-map showing up, improve entity-under-water, slight improvement to reflection edges. to Water shader improvements: fix some of the redness, fix edge-of-map showing up, improve entity-under-water, slight improvement to reflection edges..

Rebased. Plan to commit this next week.

wraitii updated this revision to Diff 5397.Jan 21 2018, 5:16 PM

Fix regression noticed by elexis.

vladislavbelov requested changes to this revision.Mar 24 2018, 5:52 PM

Screenshots and probably shaders changes aren't correct, because the vertex shader was wrong: D1402. Also we need to compare screenshots with the same position, the same angle and the same water settings.

This revision now requires changes to proceed.Mar 24 2018, 5:52 PM
wraitii updated this revision to Diff 6552.EditedMay 15 2018, 8:37 PM

Rebased.

Some more comparable screenshots. The fixed version is the one that doesn't look terrible.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/496/display/redirect

wraitii updated this revision to Diff 7197.Jan 2 2019, 5:06 PM

Rebased on top of rP22004 and rP22007.

Vulcan added a comment.Jan 2 2019, 7:05 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/877/

Imarok added a subscriber: Imarok.Apr 7 2019, 7:48 PM

@wraitii I guess you have overseen @vladislavbelov's inline comments. ;)

Stan added a subscriber: Stan.Apr 7 2019, 8:07 PM

Maybe he can request changes :)

wraitii updated this revision to Diff 7772.Apr 19 2019, 12:47 PM
wraitii marked an inline comment as done.

Fix all whitespace I've regex-ed in water_high.fs, address vlad's comments.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/WaterManager.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/renderer/Renderer.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1228/display/redirect

Stan added a comment.Apr 19 2019, 1:23 PM

Might want to commit the whitespace changes separately :)

I don't see the point of committing the whitespace separately in this instance to be honest.

source/renderer/Renderer.cpp
1138

Added a comment. The reason is that by clipping slightly above we generally have a bit more material "bleeding" in the reflection texture, and this looks better when distorting the texture for reflections, as there are fewer "holes" - since we don't render on both sides of most meshes.

wraitii updated this revision to Diff 8112.May 25 2019, 12:23 PM

Fixes some minor transparency issues. Will commit once tests pass.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/WaterManager.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/renderer/Renderer.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1475/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.May 25 2019, 1:09 PM
This revision was automatically updated to reflect the committed changes.