There was an issue with the water shader. It didn't cost the water height and always had set it to 15.0.
Details
- Reviewers
Stan wraitii - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22004: Clean up and fix height of water vertex shader.
- Run the game and open maps with water on different heights.
- Check that the water looks ok and not really worse.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/273/display/redirect
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
58 ↗ | (On Diff #6214) | Because we have a horizontal plane, defined by xOz. But the Y-axis looks upward. The normal has a different space (2D with additional params), so a_vertex.y isn't equal to normalCoords.y. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/274/display/redirect
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
79 ↗ | (On Diff #6215) | I didn't change the expression, you can compare it with the old one: ((0.15+a_waterInfo.r/1.15)) Why the original is so? Because a_waterInfo.r is in [0, 1] as I understand the code. |
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
79 ↗ | (On Diff #6215) | Yeah, I know I was just wondering if they didn't mess up somehow :) Sounds like magic numbers tho |
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
79 ↗ | (On Diff #6215) | There are many magic number in shaders sadly. We need to fix it somewhen. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/284/display/redirect
--scratch that--
I tested and this seems to work well. The wobblyness seems to be more consistent with this fix.
@aeonios My friend, will you continue to make other modifications? Or directly launch the patch, what is your choice? Well, you choose, my friend.
This does seem like a bug. Odd that I never noticed it.
Thanks for the patch :)
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
79 ↗ | (On Diff #6215) | These don't really have semantic value, I think. calling them WAVINESS_FIX_CONSTANT won't help the issue. |
3 ↗ | (On Diff #6228) | Useless |
24 ↗ | (On Diff #6228) | Useless |
45 ↗ | (On Diff #6228) | Useless |
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
3 ↗ | (On Diff #6228) | Do you think that it doesn't increase readability? |
binaries/data/mods/public/shaders/glsl/water_high.vs | ||
---|---|---|
3 ↗ | (On Diff #6228) | My opinion is that the name of the variables is explicit enough here. I don't care that much about it though so feel free to commit it with those if you prefer. |