-
Notifications
You must be signed in to change notification settings - Fork 812
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
Downgrade quad materials by scanning their textures #2666
Downgrade quad materials by scanning their textures #2666
Conversation
…t and not performant. Mods should instead use FRAPI or Forge APIs to assign multiple materials to the quads in their models such that each quad only renders as the most demanding material it requires.
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Outdated
Show resolved
Hide resolved
...ain/java/net/caffeinemc/mods/sodium/mixin/features/textures/mipmaps/SpriteContentsMixin.java
Outdated
Show resolved
Hide resolved
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Outdated
Show resolved
Hide resolved
I didn't investigate this fully in the code review, but: When downgrading materials, we must be careful to use a material which has the same alpha-cutoff and mip-mapping behavior. The only thing we should be changing is the render pass the geometry is placed into. |
I think the abstract you describe here is fine, though. This is very similar to a patch set I wrote, but due to recent events (as we've discussed) it's difficult for me to get access to that code. Things like Grass blocks contribute to a significant number of fragments in the overall scene, and this patch would make it so the base dirt layer to no longer rendered with alpha-testing enabled, which is likely a decent performance win. |
change injection of both mixins to not conflict by using WrapOperation, change the downgrade method to use passes, but then also add code to change the material bits' alpha cutoff parameter
I've resolved the comments in the latest commit. This required changing some things to accept the material as bits instead of the material object since I need to change the alpha cutoff parameter independently since otherwise a downgraded translucent material has a zero cutoff which doesn't work. I hope it's kinda ok the way it is now. Thanks to @LlamaLad7 for helping me rewrite the mixin. |
It might also be a good idea to do a performance comparison in both rendering and meshing times to see what impact downgrading has on those. |
remove ternary that gives different (but functionally identical) alpha cutoff parameters
…ad's UV coordinates and by sanity checking the quad against the sprite before downgrading
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Show resolved
Hide resolved
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Show resolved
Hide resolved
# Conflicts: # common/src/main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
...c/main/java/net/caffeinemc/mods/sodium/mixin/features/textures/scan/SpriteContentsMixin.java
Show resolved
Hide resolved
...c/main/java/net/caffeinemc/mods/sodium/mixin/features/textures/scan/SpriteContentsMixin.java
Outdated
Show resolved
Hide resolved
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Outdated
Show resolved
Hide resolved
...main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Outdated
Show resolved
Hide resolved
These changes look good to me now, outside of my comments regarding code style. Thanks for your effort. |
address review comments in the sprite contents mixin
I've refactored it into a separate method as far as possible, and added some documentation to both the mixin and the quad processing. |
This seems fine in my testing, so we can merge it for 0.6.0-beta.2 if you can rebase the pull request on /dev. |
# Conflicts: # common/src/main/java/net/caffeinemc/mods/sodium/client/render/chunk/compile/pipeline/BlockRenderer.java
Merged dev into the branch, you can squash merge the PR. |
Adds texture scanning to the sprite contents mixing to check if any of the pixels in a sprite are fully transparent or even translucent. This information is then used during block rendering to "downgrade" a quad's material to either cutout or solid, if no translucent or transparent pixels are present in the quad's sprite, respectively.
The benefit of material downgrading is that it for one, has a performance benefit since cutout and translucent materials are somewhat more expensive to render. It also helps reduce the load on the translucency sorting system if it can avoid non-translucent quads from being declared translucent and subsequently incorporated in the translucency sorting calculations.
It does not separately scan each quad's area in sprites that are used by multiple quads since this is complex and/or unlikely to perform well. Concrete suggestions of mod-compatible and performant approaches to per-quad texture scanning are welcome. Mods should instead use FRAPI or Forge APIs to assign multiple materials to the quads in their models such that each quad only renders as the most demanding material it requires.