Crash When Disabling OptiFine Shaders (Reloading Resources?)
ChloeDawn opened this issue ยท 24 comments
The game crashes when I try to disable shaders (which reloads all resources). I know this is probably an issue with OptiFine but I thought I should report it regardless, as it may be easily resolvable on your end.
This crash occurred whilst there were scaffolds placed in the world, near my position.
Mod version: 0.5.80
Forge version: 1.12.2-14.23.1.2562
Crash report: crash-2017-12-09_14.32.00-client.txt
This is very much an OptiFine issue. I might look into it, but it is not a priority, I'm afraid.
I'm going to call @sp614x here - I have no idea why this would work before resource reloading but not after.
The model being called here is loaded with ModelLoaderRegistry.getModel(location); in TextureStitchEvent.Pre, and the ModelScaffold instance is set in ModelBakeEvent.
Why is the IBakedModel.getQuads() trying to bake models?
The models have to be baked when the resources are loaded, not when rendering chunks.
at pl.asie.charset.module.misc.scaffold.ModelScaffold.bake(ModelScaffold.java:34)
at pl.asie.charset.lib.render.model.ModelFactory.getModel(ModelFactory.java:115)
at pl.asie.charset.lib.render.model.ModelFactory.getModel(ModelFactory.java:128)
at pl.asie.charset.lib.render.model.ModelFactory.func_188616_a(ModelFactory.java:137)
at net.minecraft.client.renderer.BlockModelRenderer.func_187498_b(BlockModelRenderer.java:106)
at net.minecraftforge.client.model.pipeline.ForgeBlockModelRenderer.func_187498_b(ForgeBlockModelRenderer.java:107)
at net.minecraft.client.renderer.BlockModelRenderer.func_187493_a(BlockModelRenderer.java:76)
at net.minecraft.client.renderer.BlockModelRenderer.func_178267_a(BlockModelRenderer.java:55)
@sp614x The models are generated dynamically based on the material used to build the scaffold with, which can in theory be any ItemStack but is in general limited to wooden planks in Charset itself. I cannot create a list of all possible model combinations for scaffolds, barrels, etc. - such a list would very quickly outgrow the RAM capabilities of any Minecraft player.
Maybe you are caching the DefaultVertexFormats.BLOCK or ITEM formats. The shaders replace the BLOCK and ITEM formats with extended versions with more fields. They are restored to vanilla formats when shaders are disabled.
The crash is caused by mismatching vertex formats:
"Can't bake vanilla models to the format that doesn't fit into the default one: format: 4 elements: 3,Position,Float 4,Vertex Color,Unsigned Byte 2,UV,Float 2,UV,Short"
@sp614x No. I always grab it from DefaultVertexFormats.BLOCK, in this case in ModelScaffold.bake() itself.
Are DefaultVertexFormats entries under OptiFine no longer marked as final? If they still are, it might be the JVM doing something weird...
Currently OF changes the vertex format and then triggers a resource reload. At this moment some chunks updates may still be running on the worker threads and they may have a race condition.
I think there's a good way to check this.
@InsomniaKitten Could you reproduce the crash and send a complete console log? If sp614x's theory is right, the crash occurs before the resource reload is completed; the fix would then be to clean/stall all the worker threads before the vertex format is changed.
There may be some pending chunk updates when OF changes the vertex format.
Usually this is not a problem as models are expected to have pre-baked vertex data which only is uploaded to the GPU.
@sp614x Even then, the baking would happen at the moment of the render call - are you implying ChunkRenderWorkers are not stalled long enough and thus cause a bit of a race condition?
It looks like OF is not properly updating Attributes.DEFAULT_BAKED_FORMAT after disabling shaders.
Alright. I'll leave this open because this is just a hypothesis - could you keep me updated on whether or not that was the root cause?
Updated OptiFine 1.12.2_HD_U_C8
- fixed crash when disabling shaders (Charset #165)
It turned out that the Attributes.DEFAULT_BAKED_FORMAT was not reset after disabling the shaders (legacy from the old shaders mod). This somehow works with all other mods, but makes problems when models are baked on the chunk loading thread (no idea why). Fixed the default format to be reset and now everything is OK.
The crash is easy to reproduce and does not seem to be a race condition:
- Enable shaders
- Place scaffold block
- Turn shaders OFF
- Crash
It also crashes when rendering the scaffold block in the creative inventory.
This is the place that crashes:
https://github.com/MinecraftForge/MinecraftForge/blob/1.12.x/src/main/java/net/minecraftforge/client/model/ModelLoader.java#L434
The crash complains that the given vertex format does not match the Attributes.DEFAULT_BAKED_FORMAT.
OptiFine replaces the DEFAULT_BAKED_FORMAT when enabling/disabling shaders:
https://github.com/MinecraftForge/MinecraftForge/blob/1.12.x/src/main/java/net/minecraftforge/client/model/Attributes.java#L32
Looks like the scaffold model is using an older (cached) version of the vertex format.
It doesn't crash when enabling shaders because the updated Attributes.DEFAULT_BAKED_FORMAT is larger than the (cached) vanilla used by the model.
It crashes only on disable, then Attributes.DEFAULT_BAKED_FORMAT is vanilla and the model uses the (cached) extended format.
The cached vertex format is updated at some point because only enabling and disabling shaders and then placing a scaffold block still crashes. This would not be the case if the model was caching permanently the vertex format.
The ModelScaffold.bake() is using the static cached scaffoldModel which is probably not refreshed after resources are reloaded.
@sp614x Hm. TextureStitchEvent.Pre is technically called during every resource reload, isn't it? At least in Forge. Then, at some point later in the timeline, ModelBakeEvent is called...
The reason I do it this way is because getModelWithTextures also marks the textures used by the model as required to be loaded into the atlas, which is handy but - I am aware - a bit hacky.
Does enabling/disabling shaders not cause the texture atlas to be re-stitched?
On shaders enable/disable all resources are reloaded - texture atlas, sprites, textures, models, etc.
The TextureStitchEvent.Pre is called before the texture atlas is reloaded.
Maybe you can add some debug code and see if the scaffoldModel is really reloaded on shaders enable/disable and which vertex format it is using. The vanila vertex data is 28 bytes, the shaders vertex data is 56 bytes, easy to check.
In that case, the TextureStitchEvent.Pre should be called and reload the model, no? The only place which could then hold the model object cached is ModelLoaderRegistry, as I do not provide my own cache: source.
It could be that the ModelLoaderRegistry's IModel cache is not cleared by that point, however. I think it has one of its own. The vanilla loader might also have some kind of cache - haven't looked at that in a while.
Maybe the VanillaLoader.LoadingCache is the problem:
https://github.com/MinecraftForge/MinecraftForge/blob/1.12.x/src/main/java/net/minecraftforge/client/model/ModelLoader.java#L817
@sp614x Very likely! I think resetting it at the very beginning of the resource reload process might fix this.
Can you add some debug code and see if the scaffoldModel is really reloaded on shaders enable/disable and which vertex format it is using. The vanila vertex data is 28 bytes, the shaders vertex data is 56 bytes, easy to check.
When is the scaffoldModel refreshed and when is addCustomModels() being called?
Debug code? Sure, but I'm afraid not today. I have an exam tomorrow and I might not be able to find time.
It's just a SubscribeEvent, so the method is called when Forge posts a TextureStitchEvent.Pre event on the EVENT_BUS. The scaffoldModel is refreshed via ModelLoaderRegistry.getModel() in that same method, addCustomModels().