Page MenuHomeWildfire Games

Do not send Reflection and Refraction matrices to shader when not needed
ClosedPublic

Authored by Silier on Jun 5 2019, 11:43 AM.

Details

Summary

We should avoid to send not needed data into shaders as much as possible.

Test Plan

enable reflection/refraction
move camera
disable them
move camera
enable them
check that effects are correctly applied

Diff Detail

Event Timeline

Silier created this revision.Jun 5 2019, 11:43 AM
Owners added a subscriber: Restricted Owners Package.Jun 5 2019, 11:43 AM

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

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

wraitii requested changes to this revision.EditedJun 5 2019, 1:32 PM
wraitii added a subscriber: wraitii.
  • Indenting the water shader is wrong (the #ifs don't necessarily correspond to real ifs)

Edit: got confused about one point.

This revision now requires changes to proceed.Jun 5 2019, 1:32 PM
Silier added a comment.EditedJun 5 2019, 2:18 PM

I would leave indenting, because it is easier to read.
Used already in this file and some others: terrain_common.vs, terrain_common.fs, model_waterfall.vs, model_waterfall.fs, model_water.fs::get_shadow, model_common.*

wraitii resigned from this revision.Jun 5 2019, 2:28 PM

Resigning to un-request revision (hopefully)

This revision now requires review to proceed.Jun 5 2019, 2:28 PM
Stan added a subscriber: Stan.Jun 5 2019, 2:39 PM

Angen could just have requested review :)

vladislavbelov commandeered this revision.Jun 5 2019, 4:24 PM
vladislavbelov added a reviewer: Silier.
vladislavbelov added a subscriber: vladislavbelov.

I agree that it's harder to read the code without indents for some people. Only objection that I'm thinking about, if you remove #if with leaving the code you need to fix the indents again that also means you rewrite the blame again.

vladislavbelov added a comment.EditedJun 5 2019, 4:24 PM

WAT, why I commandeered it? Sorry anyway! @Angen could you take it back?

Silier commandeered this revision.Jun 5 2019, 4:26 PM
Silier edited reviewers, added: vladislavbelov; removed: Silier.

@Angen The "real" indentation of the if L315-330 conflicts with the #if indentation. That's one reason not to indent I think.

Silier updated this revision to Diff 8324.Jun 5 2019, 5:44 PM
Silier added inline comments.
binaries/data/mods/public/shaders/glsl/water_high.fs
269

just an idea to unify some lines

wraitii added inline comments.Jun 5 2019, 5:49 PM
binaries/data/mods/public/shaders/glsl/water_high.fs
272

Why is this removed?

Vulcan added a comment.Jun 5 2019, 6:13 PM

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

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

Will test and commit after, this is a good idea

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

ok so it's merged with the same code below. LGTM I suppose.

@Angen How is this going, my friend? Is there a good solution released? I can test it for you.

@gameboy not related to water problem

@Angen When will there be a solution to the water problem?

Vladislav told he will look into his older patch, I tried to but cannot solve it, unless we remove something but it might lower performance.

@Angen If you can launch this patch that may degrade performance, let me test it. I want to test this patch to help you.

Stan added a comment.Jun 20 2019, 3:45 PM

@wraitii Thoughts on this ?

Apparently when you resign things no longer show up in your review queue

wraitii accepted this revision.Sep 3 2019, 9:56 AM

Gonna accept to make it pop again, and I'll commit this later.

This revision is now accepted and ready to land.Sep 3 2019, 9:56 AM
wraitii updated this revision to Diff 9917.Sep 22 2019, 4:47 PM

Rebased before committing

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/288/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/803/display/redirect