Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't set global uniforms in UpdateSurfaceData* #1477

Merged

Conversation

VReaperV
Copy link
Contributor

Picked from #1473.

@slipher
Copy link
Member

slipher commented Dec 27, 2024

I don't see the global flag in the constructor on u_UnprojectMatrix. (Maybe we should delete the liquid shader so we don't have to discuss that...)

@VReaperV VReaperV force-pushed the material-extraneous-uniforms branch from 45296ae to 8ebc163 Compare December 27, 2024 10:44
@VReaperV
Copy link
Contributor Author

VReaperV commented Dec 27, 2024

I don't see the global flag in the constructor on u_UnprojectMatrix.

Indeed. I have made it global now.

(Maybe we should delete the liquid shader so we don't have to discuss that...)

Then how would we render water?

@illwieckz
Copy link
Member

We want water to be fixed, not NUKED:

@slipher
Copy link
Member

slipher commented Jan 3, 2025

LGTM

(Maybe we should delete the liquid shader so we don't have to discuss that...)

Then how would we render water?

What do you mean? We don't have any assets using that shader so everything would continue working as before.

We want water to be fixed, not NUKED:

* [water shader is broken #645](https://github.com/DaemonEngine/Daemon/issues/645)

Then the issue can have a pointer to the old version of the code, if indeed it would be useful. Meanwhile we can delete it so it is not a drag on development, what with having to migrate APIs, fix GLSL compile errors, etc. in the dead code.

@illwieckz
Copy link
Member

illwieckz commented Jan 3, 2025

Water is basically a variant of both the heatHaze code and the mirror code, so 99% of the code is already there. And the missing 1% is things that had been broken when refactoring other things because we had no asset to test it.

We have a test map: https://github.com/UnvanquishedAssets/UnvanquishedTestAssets/tree/master/pkg/map-test-water_src.dpkdir

And the code is there, probably complete or near to be. The water code was never removed.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 3, 2025

Some tremulous maps have water. https://users.unvanquished.net/~sweet/pkg/map-citadel_0%2B0.dpk IIRC uses the water shader.

@VReaperV
Copy link
Contributor Author

VReaperV commented Jan 3, 2025

Anyway, if someone has more comments on liquid shaders, please make those in the relevant issues.

@VReaperV VReaperV merged commit 99fa09d into DaemonEngine:master Jan 3, 2025
9 checks passed
@VReaperV VReaperV deleted the material-extraneous-uniforms branch January 3, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants