Skip to content
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

Fix Sodium 0.5 incompatibility #178

Merged
merged 11 commits into from
Aug 30, 2023
Merged

Fix Sodium 0.5 incompatibility #178

merged 11 commits into from
Aug 30, 2023

Conversation

BluSpring
Copy link
Contributor

@BluSpring BluSpring commented Aug 12, 2023

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
Copy link
Contributor Author

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

@Kichura
Copy link

Kichura commented Aug 12, 2023

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
Copy link
Contributor Author

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
Copy link

Kichura commented Aug 12, 2023

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
Copy link

Kichura commented Aug 12, 2023

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
Copy link

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
Copy link
Contributor

joestr commented Aug 13, 2023

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
Copy link

There is flickering, rtx 3050 ti

@LambdAurora
Copy link
Owner

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

@BluSpring
Copy link
Contributor Author

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

done

@TheRealJake12
Copy link

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor Author

BluSpring commented Aug 17, 2023

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
Copy link

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
Copy link

TheRealJake12 commented Aug 17, 2023

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
Copy link
Owner

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
Copy link
Contributor

MUKSC commented Aug 19, 2023

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

@MUKSC
Copy link
Contributor

MUKSC commented Aug 20, 2023

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
Copy link

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 :

2023-08-29.00-09-13_H.264.mp4

@donington
Copy link

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
Copy link
Contributor Author

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:

  • The light update fluidity was slightly improved, but not by much.
  • The strange light patterns were still occurring.
  • The flat lighting data was still very different from the data without Sodium.

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
Copy link
Owner

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
Copy link
Contributor Author

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.

Copy link
Owner

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this PR has reached a good enough stage for me to merge it in in a couple of hours.

I might rework a little bit some details to loosen up the requirement to have Sodium 0.5 for compilation to work through the power of aggressive Mixin Pseudo.

Otherwise, congratulations on the PR and huge thanks to everyone who contributed to it, I don't think I would have had the courage to figure out this solution!

@LambdAurora LambdAurora added bug Something isn't working good first issue Good for newcomers sodium The issue is related to Sodium. to be merged labels Aug 29, 2023
@MUKSC
Copy link
Contributor

MUKSC commented Aug 29, 2023

Nice!
Btw I'll leave here a potential cause of light pattern issue.
Maybe we can fully fix the issue once this is fixed.
CaffeineMC/sodium#2004

@LambdAurora LambdAurora merged commit 8c2c010 into LambdAurora:1.20 Aug 30, 2023
@embeddedt
Copy link

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers sodium The issue is related to Sodium. to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.