search

CaffeineMC / sodium

A Minecraft mod designed to improve frame rates and reduce micro-stutter
Other
4.79k stars 811 forks source link

Modification of data precision should be optional #2004

Open Yefancy opened 1 year ago

Yefancy commented 1 year ago

https://github.com/CaffeineMC/sodium-fabric/blob/0a096a73ca962b27e1500a109d082d2df2481790/src/main/java/me/jellysquid/mods/sodium/client/render/chunk/vertex/format/impl/CompactChunkVertex.java#L32-L41

Issue

The precision of the modified data is too low, the color has no alpha anymore, and the light only goes from 0-15. I can see its value. but sometimes other mods require the original precision range to do additional things. For example, Shimmer will make fragments bloom by checking lights value bigger than 240. There is no way to make it compatible with sodium with the current modification.

Solution

I agree that this is a good improvement and will significantly reduce the overhead of data upload to the gpu. But can we make it optional?

IMP, at least the reduced precision of the uv will have a significant impact on the ultra-high resolution texture resourcepack. remove alpha really shouldn't be, it's risky. besides, I'm afraid of z-fighting issue by the precision of the position.

jellysquid3 commented 1 year ago

But can we make it optional?

Yes, the vertex format & encoding scheme used by the renderer is modular. The code needed to support it is already there, it's just that we haven't added a "full-precision" implementation yet. We'd accept a pull request to fix that if anyone is interested in working on it.

For example, Shimmer will make fragments bloom by checking lights value bigger than 240. There is no way to make it compatible with sodium with the current modification.

You could add an extra bit to the "material" attribute and check for that in the vertex/fragment shader. We currently use only 3 bits, which leaves 5 other bits available. But the problem of course is that we don't provide any way for mods to specify a custom material for their block models, so that would need to be changed first.

the reduced precision of the uv will have a significant impact on the ultra-high resolution texture resourcepack

I'm not sure this is true? Texture coordinates are still 16-bit, which gives them the ability to address every texel on a 65536x65536 atlas. Seeing that modern GPUs are actually limited to texture atlases of 16384x16384, this gives enough precision to access every sub-1/4th texel. Surely that is enough for all practical purposes, even for absurdly high-resolution texture packs?

remove alpha really shouldn't be, it's risky.

Sodium used to encode the "ambient occlusion shade factor" in the alpha component of the color value. We did not support mods changing it, and our pipeline would always replace whatever value you specified. We've never seen a mod break from the lack of an alpha-component.

besides, I'm afraid of z-fighting issue by the precision of the position.

The precision of the vertex position attribute did not change with Sodium 0.5.1... so it shouldn't be any worse than it was before.

Yefancy commented 1 year ago

Ok it makes sense. We tried to use the custom bits and it works well. Tyvm

embeddedt commented 1 year ago

https://github.com/embeddedt/embeddium/commit/99abfe1ae570e584aa5461a10249df443369defe implements this for Embeddium; the commit should be very straightforward to port to Sodium 0.5.x if anyone wants to clean it up/restructure to match upstream conventions and make a PR. An important note is that Iris seems to be hardcoded to assume the compact vertex format is the only one in use, so either the option should do nothing when Iris is installed, or Iris needs to add support for arbritrary vertex formats.