CaffeineMC / sodium

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

Core Shader Sodium Directive Definition #2772

Closed Neylz closed 1 month ago

Neylz commented 1 month ago

Request Description

Some resource packs are relying on core shaders in order to provide a better user experience and create new visual effects.

Pack makers are aware of the incompatibilities between core shaders and sodium, and #2206 was perfect for bypassing the Sodium's warning. However, this doesn't permit us to provide a clean end-user experience because Sodium may break some parts of the shaders anyways.

Thus, a short term solution to core shaders not being supported, would be to provide a header definition such as #define SODIUM to each core shader before being interpreted. Like how GLSL ES does that with the GL_ES definition.

This feature would permit to handle Sodium edge cases and then just provide the vanilla (or custom) behavior instead of having a visual breaking effects for the end user. Here is a use case example:

void main() {
    gl_Position = ProjMat * ModelViewMat * vec4(Position, 1.0);

    vertexDistance = fog_distance(Position, FogShape);
    vertexColor = minecraft_mix_light(Light0_Direction, Light1_Direction, Normal, Color) * texelFetch(Sampler2, UV2 / 16, 0);
    texCoord0 = UV0;
    texCoord1 = UV1;
    texCoord2 = UV2;

    #ifndef SODIUM
        customStuffUnsupportedBySodium();
    #endif
}
jellysquid3 commented 1 month ago

There are no plans to extend core shaders to expose this information, or provide servers with any option to detect Sodium.

jellysquid3 commented 1 month ago

Even if we were to provide this option, the hacks that server owners are using to implement functionality will continue to break with Sodium, in ways that are not possible to workaround.

We do not want commercial Minecraft servers to be under the impression that anything they are doing is somehow supported or stable, and we especially don't want them coming after us when suddenly their shaders no longer work with a new release of the mod.

Neylz commented 1 month ago

The point of this feature request isn't at all to be able to provide workarounds to make things work or to support core shaders. I perfectly understand where the issue comes in terms of technical implementation and why it isn't possible to support them.

However, the issue here is that core shaders are part of the creative process when making a resource pack, whether it targets maps or servers. And nowdays, Sodium is very commonly used by the playerbase, breaking stuff and making things appearing differently for these players. And some of them doesn't have the required setup to play wihtout Sodium, so they are forced to have a broken experience.

Again, this feature request doesn't asks for reliability between versions, or any core shaders support; the wiki disclaimer has been very clear. And at the same time, Sodium has the chance to have the core shader developers who are willing to spend (a lot of) time to guaranty the compatibility for Sodium users. So i don't get this position of not providing the necessary tools to handle the things your mod is breaking under the pretext that "it will break later".

In the current situation, things are just looking broken. Try changing the fog distance 2 seconds, and yipee only entities will get the fog updated breaking visually the immersion. And I don't get why this would be considered as a "working as intended" when your mod is revamping the whole rendering system. Inevitably it breaks things, and this ticket is all about providing core shaders devs a way to fix visual stuff.

jellysquid3 commented 1 month ago

We don't want people relying on the internal details of the renderer, because it makes people dependent on things which are not clearly defined, and it greatly increases the amount of technical burden we have. Core shader replacement is not a good solution to any problem -- it's a kludge that makes it very difficult for us to improve the mod.

Right now, if you replace the shader code in Sodium in a way that isn't compatible or otherwise incorrect, the game crashes. And it won't be resource pack authors that are handling those bug reports, because users are going to see that it was Sodium which failed to load shaders, and then promptly report bugs with us for a problem we can do nothing about.

jellysquid3 commented 1 month ago

The resource pack community needs to engineer better solutions to their problems, instead of pushing the burden onto mod authors to ensure compatibility with interfaces which are not even supported by Mojang. Were there agreed upon standards for things like custom entity models which didn't involve hacking every core shader and trashing the game's performance, we would consider differently.

But right now, we just don't have the resources to engineer these solutions, especially while the mod is in a state of flux. And even if it was not the case that our rendering engine was still developing rapidly, we still would not want to create a platform that other people depend on, because that isn't something we signed up for, and it is a lot of responsibility.

jellysquid3 commented 1 month ago

Also, to the technical merit of what you ask for: Merely being able to detect that Sodium is running would not be sufficient, because you would need to conditionally handle every version of the mod, and ensure that your shader can adapt to whichever version of the mod installed. We change the shader interfaces often, because there is continual improvement to be made in every update of the mod.

jellysquid3 commented 1 month ago

There is also a lot of other discussion that has already been had on the topic with issue https://github.com/CaffeineMC/sodium-fabric/issues/1569.

Neylz commented 1 month ago

I see, I don't agree with the first part, tho I definitely get the point of the second part now.

Would that be reasonable to request for a boolean in pack.mcmeta defaulted to false which would force Sodium to use the vanilla core shaders instead of the ones provided by the pack? Having nothing is better than having broken elements and would push the players to disable Sodium to play the maps. Such as:

{
    "pack": { "pack_format": 34, "description": "RP" },
    "sodium": {
        "disable_shaders": true
    }
}

PS:

The resource pack community needs to engineer better solutions to their problems, instead of pushing the burden onto mod authors to ensure compatibility with interfaces which are not even supported by Mojang.

I was agreeing with you before this, but this sounds very off. Core shaders permits things which are just impossible to do differently. This is not a bad engineering issue, that is just a "it is not possible to do that differently in vanilla". I wish core shaders would be supported with official doc, no changes between each version, etc... But that is not the case, and we have to deal with it. Also, don't get me wrong, I am not blaming you and other contributors for breaking core shaders. If there was something to blame, that would be Mojang's code, because it isn't normal that a vaste part of the community is relying on community mods for having decent performances in game.

Thus that is why this ticket was about adding a way for shaders developers to support the effort you are doing on Sodium by taking the time to handle the janky cases we could face specifically for Sodium users.

Were there agreed upon standards for things like custom entity models which didn't involve hacking every core shader and trashing the game's performance, we would consider differently.

Strongly agreeing with this, but that's Mojang's job. Tho it will take a lot of time before we can see this.

However this sentence looks very biased, please note that core shaders aren't only heavy packs like objmc or CEM-S. Core shaders are used for every-day things like:

It isn't because it is in a core shader that it is necessary laggy. That's like adding a new model to the game with a resource pack, if you have too much vertices, your game will lag... Welp, that's the same thing with a shader, you can do things reasonably or not.

But right now, we just don't have the resources to engineer these solutions, especially while the mod is in a state of flux. And even if it was not the case that our rendering engine was still developing rapidly, we still would not want to create a platform that other people depend on, because that isn't something we signed up for, and it is a lot of responsibility.

Gotcha 👍

Also, to the technical merit of what you ask for: Merely being able to detect that Sodium is running would not be sufficient, because you would need to conditionally handle every version of the mod, and ensure that your shader can adapt to whichever version of the mod installed. We change the shader interfaces often, because there is continual improvement to be made in every update of the mod.

Makes sense and doesn't seems reasonable to define an integer within the SODIUM definition. That is comforting me in the idea of just adding something which permits to just disable shaders from the resource pack.

There is also a lot of other discussion that has already been had on the topic with issue #1569.

Yes i already read all of that, and it isn't my goal to come back onto what has been said.

jellysquid3 commented 1 month ago

Would that be reasonable to request for a boolean in pack.mcmeta defaulted to false which would force Sodium to use the vanilla core shaders instead of the ones provided by the pack? Having nothing is better than having broken elements and would push the players to disable Sodium to play the maps.

No, we are not going to provide resource packs with a method to disable the entire mod. Because then every server is going to do this (when in most cases the resource pack only provides optional features), and players will not be happy with that outcome.

There is already the unfortunate situation that many resource packs on the internet are based on templates which pointlessly include a copy of Minecraft's core shaders, even when they're not used for anything (as doing so makes it easier for authors to simply edit the file they want.) No doubt, these templates will start enabling the option to disable Sodium's renderer by default, which again is a terrible situation.

MCRcortex commented 1 month ago

I think they mean the resource packs shaders, not sodium

Neylz commented 1 month ago

uh, not asking for disabling the mod, but for disabling the core shaders provided by the RP and default them by the Minecraft's vanilla's core shaders

jellysquid3 commented 1 month ago

It could be possible to do that, but I do not know how to bend the resource pack system to do what it is you are asking without rather intrusive patches. If someone else were to work on the problem and submit a pull request, we may consider adding it.

Neylz commented 1 month ago

Okay that's noted thanks 👍

zegevlier commented 3 weeks ago

Hey there. Due to a recent update in our server, many people updated their Sodium version, breaking our use of core shaders. We would love to be able to support people continuing to use Sodium on our server because it's a fantastic mod. I fully understand where you're coming from in this thread, and the last thing I want is to create more of a burden on you.

If someone were to make a PR to pass the current sodium version into the core shaders, would you be open to accepting it? This would let us continue to fix our shaders, just like we do across Minecraft version, even if Sodium changes its optimisations in the future.

Neylz commented 3 weeks ago

I thought about this before my second suggestion. However this isn't possible easily because Sodium uses semantic versioning and GLSL doesn't supports strings. Thus that would mean that only major, minor and patch parts could be included. Even with this defining 3 separated definitions seems a bit janky. From that, two possible choices, or 3 definitions are added, or the values are compacted into a single int, such that #define SODIUM_VERSION <major * 10^6 + minor * 10^3 + patch>. I think this second option is more reliable.

About the mcmeta's disable_shaders it is indeed pretty hard to implement this with minimal changes.

Tho, about the original feature request, in the latest snapshots, Mojang added net.minecraft.client.renderer.ShaderDefines which would make the addition of SODIUM_VERSION definition extremely trivial. If the addition would be considered if a PR is submitted, I'd be willing to do that once 1.21.2 is out.

douira commented 3 weeks ago

Adding a define for the version is easy, that's not a difficult problem and not the reason why it hasn't been implemented. (it would either just increment each time we change the shaders or be a random ID that requires exact matching)

@zegevlier I'm confused why an update to sodium would break your use of core shaders considering sodium doesn't even load resource pack core shaders. You might be experiencing the vertex order change that was recently reported and fixed here.

Neylz commented 3 weeks ago

ok, definitely a loss for a whole community. Just mentioned that in case it has been refused because "not planned" as the original answer states it.

jellysquid3 commented 3 weeks ago

It was refused because of a long list of technical reasons, which led to the reason of it not being planned for implementation.

jellysquid3 commented 3 weeks ago

I'm not actually convinced that resource pack authors want to be handling every single version of Sodium in their shaders, because it's a lot of work, and the shaders will keep breaking with each release of the mod. It is also difficult to ensure that when an incompatible version of Sodium is detected, that shader authors will be careful enough to disable all their functionality at compile time, so their shader behaves behave identically to Minecraft.

Which is to say that in practice, this doesn't really help anyone. It won't stop users from seeing broken content in multiplayer servers (i.e. when they update the mod), and it won't make things any easier for resource pack authors.

douira commented 3 weeks ago

Here's the history of every time we changed the shaders, which would require attention from consumers of the shader format: 1 https://github.com/CaffeineMC/sodium-fabric/commits/dev/src/main/resources/assets/sodium/shaders 2 https://github.com/CaffeineMC/sodium-fabric/commits/dev/common/src/main/resources/assets/sodium/shaders

Neylz commented 3 weeks ago

it's a lot of work

You are talking about a community which is desperate enough to use unsupported features to create better experiences lol

About the number of versions to support, it still is a very reasonable number of versions per MC release, and it is possible with definitions to only change parts of the code so it is okay.

zegevlier commented 3 weeks ago

@zegevlier I'm confused why an update to sodium would break your use of core shaders considering sodium doesn't even load resource pack core shaders. You might be experiencing the vertex order change that was recently reported and fixed here.

Apologies for the confusion, that is indeed the issue we are experiencing. I had seen that issue but misunderstood the final outcome. Glad to see it's been fixed, thank you for that. AFAICT this change hasn't made it out to a pre-release yet, so I'll keep an eye on that.

I'm not actually convinced that resource pack authors want to be handling every single version of Sodium in their shaders, because it's a lot of work, and the shaders will keep breaking with each release of the mod.

In our broken shader, the fix would have been about 10 minutes of work, if we had a way to detect that sodium was being used. Had, hypothetically, a version indicator in shaders been implemented in a previous version, we could have had this resolved for our player base within an hour of the first report.

For some more context: We recently announced we would drop support for several old versions our player base uses, which caused a lot of people to simultaneously update their Minecraft and mod versions, causing a flood of reports of texture issues from people using Sodium. We had a similar case while ago where Minecraft updated this order. There we were able to find a way to differentiate between the old and new version inside the shader and handle this change gracefully. It would be great if this could be possible with Sodium as well, should this break again in the future.

douira commented 3 weeks ago

The correct response to the vertex ordering issue was to make a bug report and have sodium update it. That bug was reported only two weeks ago but by someone running sodium 0.5.8, a version that's been released for 7 months. The vertex ordering issue is somewhat separate from the terrain rendering issue, in that sodium doesn't change entity shaders, it just optimizes how data is fed to the vanilla shader (which can be replaced by resource packs).