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.

Sign up for GitHub

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

Jump to bottom

Thread Safety change in ItemBlockRenderType.setRenderLayer for fluids #8911

Closed
warjort opened this issue Jul 27, 2022 · 2 comments
Closed

Thread Safety change in ItemBlockRenderType.setRenderLayer for fluids #8911

warjort opened this issue Jul 27, 2022 · 2 comments
Labels
1.19 Regression This request demonstrates something missing in a newer version that was present in a prior version. Rendering This request deals with rendering. These must be treated with care, and tend to progress slowly.

Comments

@warjort
Copy link

warjort commented Jul 27, 2022

Minecraft Version: 1.19

Forge Version: 41.1.0

Logs: https://forums.minecraftforge.net/topic/114540-i-dont-understand-why-mekanism-generators-gives-an-error-before-that-it-worked-fine/#comment-508017

Steps to Reproduce:

  1. Install Mekanism and Mekanism Generators that do setRenderLayer() for fluids concurrently
    https://github.com/mekanism/Mekanism/blob/9baddb9d7f033f9e001c6a22e5e423ec53ee2b23/src/main/java/mekanism/client/ClientRegistration.java#L256

Description of issue:
Somewhere along the way in the client refactoring using setRenderLayer() for fluids seems to have become non-threadsafe?

1.18

MinecraftForge/patches/minecraft/net/minecraft/client/renderer/ItemBlockRenderTypes.java.patch

Line 104 in 1dacb88

+ synchronized (fluidRenderChecks) {

1.19

MinecraftForge/patches/minecraft/net/minecraft/client/renderer/ItemBlockRenderTypes.java.patch

Line 115 in 3c1126a

+ FLUID_RENDER_TYPES.put(net.minecraftforge.registries.ForgeRegistries.FLUIDS.getDelegateOrThrow(fluid), type);

I don't see this being explicitly mentioned or discussed in the #8685 #8786 patches?
@warjort warjort added the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Jul 27, 2022
@sciwhiz12 sciwhiz12 added Regression This request demonstrates something missing in a newer version that was present in a prior version. Rendering This request deals with rendering. These must be treated with care, and tend to progress slowly. 1.19 labels Jul 27, 2022
pupnewfster added a commit to mekanism/Mekanism that referenced this issue Jul 27, 2022
@pupnewfster
Set fluid render layers on main thread as it isn't thread safe anymore …
c58fad8 
…MinecraftForge/MinecraftForge#8911
@warjort
Copy link
Author

warjort commented Jan 13, 2023

Looks like this was fixed in this change? 2d3ee8a

@AterAnimAvis
Copy link
Contributor

Looks to be,
Closed by #9065

@autoforge autoforge bot removed the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
1.19 Regression This request demonstrates something missing in a newer version that was present in a prior version. Rendering This request deals with rendering. These must be treated with care, and tend to progress slowly.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@warjort @AterAnimAvis @sciwhiz12