Chisel-Team / ConnectedTexturesMod

Extensions to the vanilla model system, mainly for connected textures
http://chisel.team/ctm
GNU General Public License v2.0
125 stars 48 forks source link

[BUG] Rendering issue with Magnesium #168

Open PixVoxel opened 3 years ago

PixVoxel commented 3 years ago

When using Magnesium with CTM, All of CTM Blocks broken and disconnected. [Magnesium] 1.5 https://www.curseforge.com/minecraft/mc-mods/sodium-reforged

DrUltraLux commented 2 years ago

Can confirm. Also CTM emissive textures don't seem to work either.

ShibeTemple commented 2 years ago

I am experiencing the same issue. Connected textures randomly change.

ShibeTemple commented 2 years ago

Re-rendering chunks via F3+A fixes the issue.

tterrag1098 commented 2 years ago

I am inclined to think this is a Magnesium issue, but really I have no idea. The way CTM checks for neighbor blocks is very simple, so breaking it would require some very strange behavior changes. I leave it up to Magnesium to explain if/how I could fix it on my end.

WubbGmbaa commented 2 years ago

It is definitely a Magnesium issue. Rubidium seems to have a similar problem, so I expect it comes from Sodium upstream (all share the same codebase if I'm not mistaken ..?) Supposedly Rubidium mc1.16.5-0.2.6 fixes this according to its changelog on curseforge. Unfortunately, I haven't been able to get that version to run on my end in order to verify.

Here is the relevant Rubidium issue - https://github.com/Asek3/Rubidium/issues/69. Looks like there's a commit for the change that fixes it. I'd be tempted to try the same fix in Magnesium, would fork it if I thought my Java skills were adequate. I'll go post the same link on that Magnesium issue thread from above

Nevrai commented 2 years ago

I’m having the same issue with Rubidium. As previously mentioned, F3 + A to reload chunks is a temporary fix. I hope this is fixed.

greysondn commented 1 year ago

Chasing down #180, is the remark from Rubidium, here, relevant?

This appears to be caused by an issue in WorldSlice. Creating a new WorldSlice in ChunkRenderCacheLocal#init fixes this issue, though this would probably result in a fairly significant performance impact and an increase in memory usage. My guess is that the connected texture states aren't being saved or looked up correctly, or something along those lines.

This at least explains why it's fixed when reloading chunks, because new WorldSlice instances are created. I'll try to look into this more to see if it can be resolved, or if I can at least add more onto this issue.

Strangely enough, Connected Glass Mod works perfectly fine with Rubidium, so my only assumption is that something weird is happening with the way CTM handles block states, or at least the way it's stored in WorldSlice; it's hard to tell so far.

katubug commented 1 year ago

Just commenting to say that I'm also having this issue and would greatly appreciate an update if there is any! It seems like greysondn dug up something helpful?

For me this is occurring on - Minecraft 1.19.2 Forge 43.2.11 Rubidium 0.6.2b CTM 1.1.6+8

WubbGmbaa commented 1 year ago

Can confirm still an issue on 1.19.2 (with Rubidium). From what I gather,

something weird is happening with the way CTM handles block states, or at least the way it's stored in WorldSlice

Gotta look for the source of this in CTM, it would seem.

villainous-j commented 1 year ago

has anyone made any progress on identifying the underlying issue with this one?

embeddedt commented 1 year ago

I believe the underlying issue here is CTM's RegionCache relying on an identity comparison to determine when a new chunk is being rendered:

https://github.com/Chisel-Team/ConnectedTexturesMod/blob/32e5958d805810150bc43cc9c0982d74b7423ccb/src/main/java/team/chisel/ctm/client/util/RegionCache.java#L55

This assumption is valid in vanilla (as it allocates fresh objects) but Rubidium/Sodium use object pooling to mitigate a high allocation rate. After making a simple modification in Rubidium to ensure that a unique BlockGetter is provided each time a subchunk is built, the issue appears to be resolved from what I can see. The fix should be linked above if anyone who has the skills is interested in applying it to their MC version.

tterrag1098 commented 1 year ago

This is the change I was hoping for. I tested disabling that cache for non-vanilla classes, but I was worried the reflective check would negate any performance gains. I believe it is pretty much assumed that the readonly chunk views won't be modified after they are passed.