LambdAurora / LambDynamicLights

A dynamic lights mod for Minecraft on Fabric Loader.
https://modrinth.com/mod/lambdynamiclights
Other
397 stars 98 forks source link

Fix Sodium 0.5 incompatibility #178

Closed BluSpring closed 1 year ago

BluSpring commented 1 year ago

Issue: #175

In Sodium 0.5, they appear to have moved from Vanilla's LightmapTextureManager to using LightDataAccess. However, this brings in the caveat of not using Vanilla's WorldRenderer#getLightmapCoordinates, which LambDynLights relies on to provide dynamic light data to the renderer.

This PR creates a mixin into LightDataAccess that passes the dynamic light data onto Sodium. This unfortunately looks like the only way to both provide this lighting data and make it limited to the renderer alone. An injection into BlockRenderView#getLightLevel may also be plausible to provide that light data, but it would disrupt the world's light data for both Vanilla and mods.

WARNING: I observed large amounts of chunk flickering on my integrated GPU after making this change, so I suspect that more work will have to be done for this change to actually be viable.

BluSpring commented 1 year ago

i accidentally hit commit & push before realizing a better idea-

Kichura commented 1 year ago

Decided to give it a play-test on my radeon dGPU + iGPU and i cannot seem to reproduce the flickering chunks, is it intel-related?

BluSpring commented 1 year ago

Decided to give it a play-test on my radeon dGPU + iGPU and i cannot seem to reproduce the flickering chunks, is it intel-related?

I don't use Intel, the flickering chunks were on my Radeon Vega 8. Though this was in debugging mode in a development environment, so that might also change things.

Kichura commented 1 year ago

Decided to give it a play-test on my radeon dGPU + iGPU and i cannot seem to reproduce the flickering chunks, is it intel-related?

I don't use Intel, the flickering chunks were on my Radeon Vega 8. Though this was in debugging mode in a development environment, so that might also change things.

Possibly although i will attempt to run a second play-test using an Intel iGPU instead to see what comes out.

Kichura commented 1 year ago

Decided to give it a play-test on my radeon dGPU + iGPU and i cannot seem to reproduce the flickering chunks, is it intel-related?

I don't use Intel, the flickering chunks were on my Radeon Vega 8. Though this was in debugging mode in a development environment, so that might also change things.

Possibly although i will attempt to run a second play-test using an Intel iGPU instead to see what comes out.

Did the play-test and i still can't note flickerings from moving around with an torch although there are minor delays in certain places although hard to spot.

caproven commented 1 year ago

As an avid fan & user of the mod, thank you for making a PR! I tested this myself (AMD Ryzen cpu + Nvidia gpu) and didn't observe any flickering. Dynamic lights don't seem as "fluid" as before when the subject is moving, giving a subtle stuttering effect. This was mostly observed with flame arrows but is noticeable for common scenarios like the player holding a torch.

joestr commented 1 year ago

As an avid fan & user of the mod, thank you for making a PR! I tested this myself (AMD Ryzen cpu + Nvidia gpu) and didn't observe any flickering. Dynamic lights don't seem as "fluid" as before when the subject is moving, giving a subtle stuttering effect. This was mostly observed with flame arrows but is noticeable for common scenarios like the player holding a torch.

Can confirm. No flickering but the lighting is not that fluid anymore.

Energobro commented 1 year ago

There is flickering, rtx 3050 ti

LambdAurora commented 1 year ago

Please run ./gradlew applyLicenses and commit to fix the build.

BluSpring commented 1 year ago

Please run ./gradlew applyLicenses and commit to fix the build.

done

TheRealJake12 commented 1 year ago

yeah I'm getting delayed lighting as well. Also the lighting looks different? Idk. Intel Iris XE integrated gpu btw https://github.com/LambdAurora/LambDynamicLights/assets/84357907/320efe56-0bea-48ff-ac0c-429529890841

NotDarkn commented 1 year ago

currently using your build of LambDynamicLights and it does seem to work with Sodium 0.5, however the only issue I've found atm is that it's not as fluid as it should be. No flickering though, and I'm on an iGPU 6000.

BluSpring commented 1 year ago

seems a very common factor is the update fluidity. making a RETURN injection into ArrayLightDataCache and/or HashLightDataCache's get methods instead of LightDataAccess#compute might be able to resolve this.. i'm gonna go try that.

BluSpring commented 1 year ago

I'm unable to properly verify if this commit will improve light update fluidity, can someone help verify? An outstanding problem that I am noticing with light update fluidity is that the sub-chunk (16x16x16) below you will update slower compared to your current sub-chunk, this appears to be a little quirk with Sodium 0.5 though. I'm not sure how that can be dealt with.

TheRealJake12 commented 1 year ago

I'm unable to properly verify if this commit will improve light update fluidity, can someone help verify? An outstanding problem that I am noticing with light update fluidity is that the sub-chunk (16x16x16) below you will update slower compared to your current sub-chunk, this appears to be a little quirk with Sodium 0.5 though. I'm not sure how that can be dealt with.

gonna try it in a few hours gimme a lil bit

TheRealJake12 commented 1 year ago

I'm unable to properly verify if this commit will improve light update fluidity, can someone help verify? An outstanding problem that I am noticing with light update fluidity is that the sub-chunk (16x16x16) below you will update slower compared to your current sub-chunk, this appears to be a little quirk with Sodium 0.5 though. I'm not sure how that can be dealt with.

gonna try it in a few hours gimme a lil bit

lighting still feels a little delayed but is better I think. The light still looks weird though. https://github.com/LambdAurora/LambDynamicLights/assets/84357907/0119c7bd-85ce-4005-8624-e2ed7c7c9229

LambdAurora commented 1 year ago

I have tested this PR, and I am noticing significant delays in light propagation, and the light pattern is also very strange. Someone might need to do further research and maybe find another injection point, or at least the cause of the weirdness.

MUKSC commented 1 year ago

The light delay issue was introduced in this commit, and the strange light pattern issue was introduced in this commit, if that helps.

MUKSC commented 1 year ago

I realized that the light delay issue is actually a precision issue. In the original behavior, it tries to save more decimals as you can see here. But in this PR, it's casting double to int in here to replace block light, so it loses all decimals here.

I think what we can do is either:

  1. Inject to ArrayLightDataCache.get and save block position, then inject to LightDataAccess.getLightMap and use saved block position.
  2. Redirect all calls to LightDataAccess.getLightMap/getEmissiveLightmap and use captured local variables to know block position.

Method 1 is hacky way and easily break with something like this:

int word1 = cache.get(x1, y1, z1);
int word2 = cache.get(x2, y2, z2);
int lm1 = getLightmap(word1);
int lm2 = getLightmap(word2);

Method 2 is more safer way, but it would be a complete mess.

Both methods are pretty bad, so it might be better to send PR to Sodium to address this. A potential solution could be to modify LightDataAccess to cache the lightmap as well.

redmine4404 commented 1 year ago

i don't really understand how all of this work but I have the impression it's a sync issue, it's more visible when turn off smooth lightning :

https://github.com/LambdAurora/LambDynamicLights/assets/96315018/9a892d5f-9f90-41ac-807b-8139e8790c14

donington commented 1 year ago

I suspect the weird lighting is because luminance is not being updated. The original lightmap MUKSC referenced above was updating both the block light and luminance.

I pushed a pull request to BluSpring a couple of days ago since my git is rusty and I didn't want to clutter things with a failed attempt. Which I did fail and just fixed thanks to UnablePath noticing my failed push attempt.

I have a branch with an updated lighting that utilizes Sodium's system a bit further. I can't seem to post a test release properly because I haven't remembered how to add a tag properly yet, but the code compiles and looks a lot smoother on my end. If anyone who is able to compile can test that would be great as I'd love to see this fixed on the latest version.

BluSpring commented 1 year ago

In my previous commit, I had just noticed something particularly interesting: The flat lighting data did not match the data without Sodium.

Without Sodium: image

With Sodium + my PR: image

In @donington's branch, I had noticed these things on my end:

In my latest commit, I decided to try reading further in understanding how Sodium handled lighting in both pipelines, and realized something: It still uses LightmapTextureManager#pack in some places. This made me realize that LambDynLights' getLightmapWithDynamicLight method could still be used, and would hopefully solve some issues with the light fluidity. After some time moving things around and testing, I finally got those changes functioning, with the flat lighting data to be matching again.

image

There is however one final problem: The smooth lighting data still generates some weird patterns, and I'm currently unsure as to what is causing it. image

However, from what I can tell, the light updating seems to be much smoother now, at least on my side.

LambdAurora commented 1 year ago

Now that I'm seeing the top-down view of the weird light pattern, it's genuinely fun to see? I think at this point it's not bothering me as much if the delay issue is fixed.

Could you please undo the copyright header changes for previously existing files? The dates are supposed to be the dates of the creation of the file, and the gradle plugin seems to have gotten confused and not recognized the files.

BluSpring commented 1 year ago

Done. When I ran the applyLicenses task (I'm on Windows), the copyright symbol didn't seem to be added correctly either, and I had to fix it by manually doing a Replace All in VS Code, which is why there were two commits for it.

MUKSC commented 1 year ago

Nice! Btw I'll leave here a potential cause of light pattern issue. Maybe we can fully fix the issue once this is fixed. https://github.com/CaffeineMC/sodium-fabric/issues/2004

embeddedt commented 1 year ago

Btw I'll leave here a potential cause of light pattern issue. Maybe we can fully fix the issue once this is fixed.

Drive-by comment, as I am not familiar with the LDL code, but if it previously assumed it could use all 8 bits of the lightmap coordinates, that is no longer the case in Sodium 0.5 - you can only use the top 4. Providing something like 0xef for the coordinate will result in Sodium interpreting it as 0xe instead of 0xf, causing dark patches.

BluSpring commented 1 year ago

LDL does in fact use all 8 bits, and the bottom 4 being unusable was what I had assumed, and that does appear to be the case with the smooth lighting pipeline, hence why I switched to just directly modifying the lightmap value returns, but what I do find interesting is that the flat lighting pipeline doesn't appear to really abide by this rule, aside from the very edge of the light area.