Engine-Room / Flywheel

A modern engine for modded Minecraft.
MIT License
205 stars 52 forks source link

Add More Uniforms #231

Closed Kneelawk closed 4 months ago

Kneelawk commented 4 months ago

This PR

This pr implements many uniforms based on the uniforms provided by FREX.

Kneelawk commented 4 months ago

And with that, I think this PR is ready for review. Let me know if you think anything is unnecessary or if you think this is missing any uniforms.

Jozufozu commented 4 months ago
  • Is it necessary to have separate uniforms for matrix inverses if they can be computed manually in the shader? If each shader recalculating the inverse is a performance issue, then the inverses can be kept as standard variables but they can be filled in by an internal shader instead of taking up space in the uniform buffer.

We've used about 1kb of the guaranteed 16kb of space in the frame uniform buffer. Space is not a concern.

Calculating the inverses on the CPU is the standard way to do this because it's by far the simplest solution. I doubt whatever marginal gains of doing the inversion on the GPU would outweigh the complexity.

Jozufozu commented 4 months ago
  • It would be more consistent if all programs received all uniforms. It makes sense that the cull shader shouldn't need fog values, but I also don't see a reason to not provide them.

I agree, but the fog parameters can change across the frame, right? If it's just the color changing then yeah I don't see an issue, but if the distance/shape is different then which version does the cull shader see? If we were to try and cull instances that were fully occluded by fog would we get any false positives?