Little Tiles Issues
SheltieChan opened this issue · 26 comments
For some reason when this mod and Little Tiles are both installed, Lighting updates cause the LittleTiles blocks to dissapear and re-appear randomly.
After doing some testing with only this mod, Little Tiles, and Optifine (With the Specified Options Selected), Better Portals is the cause.
Mods Used -
1.12.2-forge1.12.2-14.23.5.2836
CreativeCore_v1.9.68_mc1.12.2
LittleTiles_v1.5.0-pre168_mc1.12.2
littleopener-1.0.1
Forgelin-1.8.4
betterportals-0.3.7
OptiFine_1.12.2_HD_U_E3
preview_OptiFine_1.12.2_HD_U_F4_pre3
Example Footage -
https://youtu.be/zYzLkenTR2s
https://youtu.be/sXQXQKqYLH0
This looks like an issue which would need to be fixed in LittleTiles. I'd suggest you report it over there as well.
And just to verify: This happens with only Forge + BP (+ Forgelin) + LT (+ CreativeCore) as well, so no Optifine required, correct? (cause that kind of bug could definitely be caused by OF as well)
From quickly skimming over some of LT's code, this part might be relevant:
https://github.com/CreativeMD/LittleTiles/blob/d12e317f99aa5423768967a16dd2db73cf5b4d53/src/main/java/com/creativemd/littletiles/client/render/cache/RenderingThread.java#L162
Accessing mc.world
from another thread in vanilla is sketchy at best (chance of getting the wrong instance is fairly low but generally non-zero), with BP it's almost certainly broken (BP swaps between different mc.world
s multiple times per frame).
Hey, actually I wanted to comment here before you :D
LT generates the render cache in another thread and adds it to the chunk vertex buffer just before it's uploaded (source). I'm pretty sure something gets messed up there.
So it would be good to know what BetterPortals changes about that.
BP re-implements the ChunkRenderDispatcher
(source) to support multiple worlds (formerly called views).
It cannot just be swapped out as needed like other parts (e.g. RenderGlobal) because it allocates a fixed number of worker threads which it doesn't share with other instances and tries not to use too much rendering time (and twice not too much time is far too much time).
The only notable difference would be that during uploadChunk
, MC may not be in the state which the chunk represents. That's because chunk uploads are only run once, during the primary render pass of each frame and switching to the correct state from there is both difficult and unnecessary.
The only thing in LT's uploadChunk
method affected by this is the call to mc.getRenderViewEntity()
to sort for transparency. Nothing I'd worry about since transparency sorting is an unsolved problem in BP in general.
Another thing which BP fundamentally changes is the ViewFrustum
(source) to allow rendering of possibly distant and/or overlapping views of the same world. The vanilla one has a fixed size array of RenderChunk
s around the player, BP instead maintains a map of all render chunks and creates arrays for various player positions on demand.
In particular that also means that the active RenderChunk
array is always the one from the last call to updateChunkPositions
.
If this is called from outside the rendering code (or before updateChunkPositions
is called), then it'll potentially return the wrong chunk. Not sure if that's actually the issue since LT seems to be storing the RenderChunk in the tile entity after it has first fetched it (so it shouldn't flicker, should it?)
I'm not sure what the best way of fixing this would be since MC relies on getRenderChunk
to wrap around and only return results from within the render distance around the player (iirc).
@ShetlandSan Does this happen only when there's a portal around or does it even happen in a newly generated flat world?
It happens in newly generated flat worlds, as well as worlds that I hadn't even generated a Nether Portal in yet.
Sorry for the late response.
@ShetlandSan
And just to verify: This happens with only Forge + BP (+ Forgelin) + LT (+ CreativeCore) as well, so no Optifine required, correct? (cause that kind of bug could definitely be caused by OF as well)
@CreativeMD No because that's never really what it did in the first place.
getRenderChunk
returns a RenderChunk
from within view distance around the player. In particular it even returns ones within the view distance even if you pass it coordinates outside the view distance. So depending on which view was rendered last (see my previous post), you're potentially not getting the RenderChunk you were hoping for.
So, while that is an issue in general, it's probably not this issue since this issue happens even when there are no portals around (and with no portals around, i.e. only a single render pass, it should behave just like vanilla).
@Johni0702 LT uses ViewFrustum.getRenderChunk(BlockPos)
to get the render chunk at the given block position. Does this still work with your mod?
So i've been doing some checking, and it seems that having Optifine installed causes BetterPortals and Little Tiles to get angry at each other.
Little Tiles and Optifine on their own have no issues, and BetterPortals and Optifine (When I put in the settings you outline in the CurseForge page) have no issues either.
It's only when the three of them try to mix that we see this issue.
Unsure if this helps, But here's some footage of me trying to get the bug to happen from every angle I can make it happen at.
@ShetlandSan I'm unable to reproduce this with OF F4_pre3, BP 0.3.7, LT 1.5.0-pre168 and CreativeCore 1.9.68. Not even after resetting all video settings and/or with Dynamic Lighting set to Fancy.
I could not reproduce it either. @ShetlandSan which Optifine version are you using (there are two versions in your first post)?
https://www.dropbox.com/s/yoh5lzb5wyyw470/Goo.zip?dl=1
@ShetlandSan send me the world and I could reproduce this issue. It feels like there are plenty of chunk updates and sometimes stuff from LT is not uploaded to the buffer. So the problem must be somewhere here. There multiple options where it could go wrong:
- the uploadChunk method is not called for some reason (maybe a different chunk render dispatcher is in us) <-- seems unlikely
- the same buffer is used upload the chunk render data. LT marks each buffer with an additional field
added
. If the same buffer is used again, it will be skipped.
I don't know what BetterPortals changes about it, but there no issues without.
Hmm... I've updated all mods to exactly the versions you stated (including CMDCam, just in case) but still no luck. My Forge is version 2847 as recommended by OF.
The only thing I do notice is that my FPS take a significant dip (solid 60 → ~30±15) when I move around with Fancy Dynamic Lights and a torch in my hand. I know my system to be slow at uploading data to its GPU, so that would make sense. If the issue is indeed some kind of race condition, that might also be the reason why I'm consistently not experiencing it (and why it's so much more consistent in OP's second video than in yours).
Oh, I've just noticed that MC runs all chunk updates on the main thread for me (I've only passed through two cores to my gaming vm), that would certainly also explain my inability to reproduce it if it's indeed a race condition. I'll see if I can force it to somehow run with background threads and report back if that changes anything.
Indeed, after forcing MC via debugger to run two chunk worker threads, I'm now able to easily reproduce the issue.
Edit: Also happens with Dynamic Lights: Fast
Edit: Even happens with Dynamic Lights: OFF
when placing torches nearby
Edit: Also without CMDCam (no surprise) and also without OF
Edit: Also happens if I just place/remove bits anywhere else in the chunk. Doesn't happen when placing/removing normal blocks though.
Interesting ... to be honest I have no idea how this can happen.
Is there any case a chunk could be uploaded without calling the vanilla method (source)? Otherwise it must be the parameters.
@CreativeMD OK, I think I've got a pretty good basic understanding of what's going on.
It basically boils down to the fact LittleChunkDispatcher.uploadChunk
assumes to be called right from the ChunkRenderWorker
thread which built that particular RenderChunk
. LCD.uC
then sets the added
flag and thereby ignores that particular chunk when it's later called again on the main thread where MC uploads the buffers.
In the process of building a chunk, LT collects all the tile entities in that particular chunk in RenderChunk.tileEntities
(via onDoneRendering
), then uses it in LCD.uC
and subsequently discards it because it no longer needs it after it's done building (this will become important later). Note how this all happens on one ChunkRenderWorker
thread.
Now, the important difference with BP is that BP does all the worker thread scheduling by itself (note the else-case in the source you posted), so the vanilla ChunkRenderDispatcher.uploadChunk
is only called once the task has gone through that scheduler and has arrived on the main thread.
A race condition appears when the same chunk is twice in quick succession:
- The first compile task is dispatched, the
ChunkRenderWorker
collects LT tile entities inRenderChunk.tileEntities
, then hands the task off to BP's scheduler for uploading which might take a bit (who knows what the main thread is currently up to). - While the first task waits, the second is processed (maybe even by the same worker thread), the worker also collects LT tile entities and stores the result in
RenderChunk.tileEntities
(likely identical result to the previous one), it then hands the task off to BP's scheduler for uploading. - The main thread finally arrives and BP calls the vanilla
uploadChunk
for the first task. This in turn callsLCD.uC
which adds the geometry for allRenderChunk.tileEntities
to the buffer and then clears that list and finally MC uploads the buffers as usual. So far, so normal. - BP calls the vanilla
uploadChunk
for the second task.LCD.uC
finds an emptyRenderChunk.tileEntities
, adds no TEs to the buffer finally MC uploads the buffers as usual and we've got the result we were looking for: No LT TEs are visible in that chunk.
Now, how do we best go about fixing this? As I see it the options are either:
- Add an explicit call to
LCD.uC
in BP non-main-thread code path inViewChunkRenderDispatcher.uploadChunk
- Change LT's transformer to instead call
LCD.uC
fromChunkRenderWorker.processTask
(where vanilla usually callsuploadChunk
from)
IMO the second option is the more idiomatic one because it is where LCD.uC
implicitly expects to be called from.
@CreativeMD Did you install any additional mods? As is, with the mods listed in my previous post (except BP 0.3.7.1), I'm still unable to reproduce the issue.
While the initial flickering blocks in the first video are missing the textures for me (cause I don't have that mod installed), they're persistent and don't flicker, regardless of Dynamic Lights setting.
My mod list:
- CreativeCore_v1.9.70_mc1.12.2
- LittleTiles_v1.5.0-pre170_mc1.12.2
- CMDCam_v1.4.4_mc1.12.2 <-- don't think that one does change anything
- Forgelin-1.8.4
- betterportals-0.3.7.1
- OptiFine_1.12.2_HD_U_F4
I couldn't reproduce it either, until I started to shake my mouse and jump around. It eventually started to flicker. It would either remain visible or invisible once I stopped moving my mouse. A block update fixed it again (or moving the mouse heavily).
Also make sure to have dynamic lights on fancy and hold a torch in hand. I will try to record a video of it.
Oh wow, interesting. Makes a lot of sense. Some time ago I left the list untouched, but I was concerned about memory leaks (the tileentities caused that the world wasn't garbage collected in the main menu).
I could use a queue which stores a lists of tileentities, instead of a single list. So multiple updates of the same chunks wouldn't be a problem at all (or an integer which will be incremented every time).
You did some very good research! I will implement a solution for it soon and get back to you. Thank you very much for all your work and effort!
I fixed it, new version should be available in a few minutes. Thank you both very much for your help! Good to see it being fixed.