Blood Magic

Blood Magic

90M Downloads

Demon Crystal Cluster causing serious render lag

tommyTT opened this issue ยท 10 comments

commented

Issue Description:

Having Demon Crystal Cluster grow in a chunk causes serious render lag after a recent update to the mod.

What happens:

Whenever you are in the vicinity of a crystal cluster the game experiences those micro stutters in the rendering and everything isn't smooth any more. If you remove the cluster everything immediately returns to normal.

What you expected to happen:

The cluster shouldn't affect the rendering that much, especially since it happens constantly and not only when a cluster actually grows.

Steps to reproduce:

  1. Put down a crystal cluster in a chunk.
  2. Look around in the world when near the cluster. Maybe even pull up the render graph (I'm using the Sampler mod for that).
  3. Now remove the cluster and repeat step 2.

Analysis

I looked a bit further into the code since this only became an issue in a recent version of the mod and found this addition to the tile entity code of the cluster. When I uncomment this update call in my dev environment, everything is smooth again but the original problem of course returns. Forcing a render update every 20 ticks even if the cluster didn't grow causes those render issues and isn't the solution to the problem. Looking further into the code I found that the growth method tries to cause that update already but since the block state never actually changes, the same block state gets syncronized to the client again, therefore not updating the rendering. I then looked into how Astral Sorcery e.g. does the crystal growth and changed that method to look like this:

public void checkAndGrowCrystal() {
        if (progressToNextCrystal >= 1 && internalCounter % 100 == 0) {
            progressToNextCrystal--;
            crystalCount++;

            IBlockState state = getWorld().getBlockState(pos);
            IBlockState newState = state.getActualState(getWorld(), pos);
            getWorld().setBlockState(pos, newState);
      }
 }

Now everything updates like it should when a cluster grows without causing those constant render updates. Since I'm no expert at Modding and especially rendering, I don't know if this is a good solution for this mod. It should however demonstrate the underlying problem that was the intent of that bugfix.


Affected Versions (Do not use "latest"):

  • BloodMagic-1.12.2-2.3.3-101
  • Minecraft: 1.12.2
  • Forge: 14.23.4.2765
  • Other mods:
    Guide-API-1.12-2.1.7-62
    jei_1.12.2-4.13.1.220
    sampler-1.73
commented

PR in progress.

commented

My fault! I'll look into it.

Am I allowed to use that code snippet?
Or, if it already fixes the problem, you should make a PR, it's all yours in that case.

commented

I cannot replicate (or rather am unable to see) the symptoms of the issue.
Can you please provide a picture of the setup and or tools you're using to monitor it?

It would make it easier to find an optimal solution.

With current code:
image

With your suggestion & markBlockRangeForRenderUpdate every 100 ticks:
image

By the way I have NO idea how to read these things.

It's also very likely that my java args have a say in this.
fpslimit_wait stabilizes around 30-35%, gameRenderer is in both between 55-65%

I should probably add that my method of taking the screenshots is most likely flawed and not that great to show it but you get the idea, I cannot see a difference which is why I need more info and while I can make a PR, I cannot test if it makes a difference. Since you are having the issue and got it corrected, you have more insight on how to optimize it.

Fact is that your code doesn't break the reason why I made the change (instant update on adding crystals) so I'd say you should just make a PR and grab that nice shit-brown color for discord.

commented

I'm no expert either, but here are a few screenshots and explanations when I run the current head revision from github in my dev environment. The pie chart is not really that interesting for this problem.

  1. This is the debug graph without anything in the chunk, in fact, I just spawned into a completely new and random world:
    2018-10-03_17 18 27
    You see the smooth bars that don't have any spikes, which means that everything is basically running as it should.

  2. Now this is the bar when I put down one crystal cluster:
    2018-10-03_17 18 41
    See all those spikes? Each spike here means that a lot more information needs to be rerendered than before. This isn't bad per se since I added something into the chunk, but at the moment nothing really happens to that new tile, so it should behave like in screenshot 1 until there is a reason to update. In my setup here, the effects are rather limited since nothing expensive is around that requires a lot of rendering time. But in my custom mod pack, I have a mob farm and a lot of other machines around the area where the clusters grow and the stuttering gets worse and worse. I don't know the internals of why it is happening, but it is happening.

  3. Now this is the graph without those rendering updates but with my proposed changes, so the rendering only updates when the crystal grew:
    2018-10-03_17 19 29
    It is back to the way the first situation rendered.

This is my exact change to the code:
crystalrendering.patch.txt

commented

I see. I'll walk over it and see if there's something else that can be improved and then make the PR if you don't do it.

Your patch removes the markBlockRangeForRenderUpdate which is required to render new crystal spikes immediatly, however, it can be just applied once instead of every 20 ticks to work.

commented

I don't think it changes anything at all if markBlockRangeForRenderUpdate is not removed.
However, that is required to get the visual update on crystal growth (unless you find a better solution).

commented

I made a small video in my world where there is a lot happening around the crystals. Depending on your system you can see the micro stuttering happening (at 1080p60) when the crystal is out as well as the graph go crazy.

The markBlockRangeForRenderUpdate is no longer needed in my solution since I'm setting the blockstate in the world. This method does a block update as well as syncing that change to the client which causes the client to update the rendering without needing a manual update.

From what I understand this will cause a chunk update which is very costly and that's why I didn't make a PR. I don't think this is a problem but I don't know for sure either, I'm just going off what other mods are doing.

commented

When I tested your solution, it didn't update the crystal rendering. Try adding a crystal manually (with your solution) in creative mode. For me, it didn't give it a new spike.

Though to be honest the whole solution (my initial solution) is not thought through. I just tried to get it working and didn't notice any performance impact so it was done for me.

If it gives a visual update when adding the crystal, make a PR, otherwise a better solution should be looked for.

commented

It looks good to me: https://www.youtube.com/watch?v=v6PEN3bfBdc

I will make a PR, then you guys can decide how to progress further. I'm not sure if the markDirty and notifyUpdate methods need to be called, it doesn't look like it, but that needs to be verified before merging the PR.

commented

I'll check it again with growing the crystal "naturally" instead of adding them. Please update BlockDemonCrystal accordingly to have the visual update when adding crystals manually with creative mode.

The problem I was facing was that it would only update on every second growth and show the past state.

I added the manual adding with creative mode to test faster but it might just have ended up in me producing inefficient code because I didn't add the block update in the block code (where it is added manually) but only in the TE code (where I would have had to wait for it to grow to see it).