Pepper
- Pepper_Bell
- JIRAUSER641735
- America/Los_Angeles
- Yes
- No
The
client-side onlymethodnet/minecraft/util/Mth#fastInvCubeRootdoes not work for negative numbers. This method is only used incom/mojang/blaze3d/vertex/PoseStack#scale when calculating the scaling for the normal matrix. If one or three of the passed arguments is negative, the normal matrix is not scaled correctly which can lead to incorrect lighting in certain situations.For example, fastInvCubeRoot(-1.0f) returns 2.189213E25, but the correct result is -1.0f.
A simple fix for this would be to check if the input is negative at the start of the method, and if it is multiply it by -1 and switch a boolean to true. Then, before returning the result, multiply it by -1 if the boolean is true.
The method com/mojang/blaze3d/vertex/PoseStack#scale is broken in some situations:
1. If one or three of the scales are negative, the normal matrix will not be scaled correctly as a result of net/minecraft/util/Mth#fastInvCubeRoot not working for negative numbers. The fix for this is to take the absolute value (Math.abs) of the argument passed into the fastInvCubeRoot method. (Fixing fastInvCubeRoot for negative numbers does not solve this issue because it causes the matrix to be inverted.)
2. If all scales are equal and negative, the normal matrix will not be scaled correctly. For this case, all normals should be inverted, but in reality nothing happens. The fix for this is to return after the normal matrix is inverted (normal.mul(-1.0f)).
Inverse Cube Root Method Broken for Negative NumbersNormal Matrix Scaling Broken in Some Situations
Eclipse
ForgeWorkspace andFabric Workspace
OpenJDK 11.0.8Eclipse
Forge / Fabric Workspace
OpenJDK 11.0.8 / 16.0.1
The method com/mojang/blaze3d/vertex/PoseStack#scale is broken in some situations:
1. If one or three of the scales are negative, the normal matrix will not be scaled correctly
as a result ofnet/minecraft/util/Mth#fastInvCubeRoot not working for negative numbers. The fix for this is to take the absolute value (Math.abs) of the argument passed into the fastInvCubeRoot method.(FixingfastInvCubeRoot for negative numbers does not solve this issue because it causes the matrix to be inverted.)2. If all scales are equal and negative, the normal matrix will not be scaled correctly. For this case, all normals should be inverted, but in reality nothing happens. The fix for this is to return after the normal matrix is inverted (normal.mul(-1.0f)).
The method com/mojang/blaze3d/vertex/PoseStack#scale is broken in some situations:
1. If one or three of the scales are negative, the normal matrix will not be scaled correctly. The fix for this is to take the absolute value (Math.abs) of the argument passed into the net/minecraft/util/Mth#fastInvCubeRoot method. Additionally, net/minecraft/util/Mth#fastInvCubeRoot does not work for negative numbers, but fixing this does not solve this issue because it causes the matrix to be inverted.
2. If all scales are equal and negative, the normal matrix will not be scaled correctly. For this case, all normals should be inverted, but in reality nothing happens. The fix for this is to return after the normal matrix is inverted (normal.mul(-1.0f)).
The method com/mojang/blaze3d/vertex/PoseStack#scale is broken in some situations:
1. If one or three of the scales are negative, the normal matrix will not be scaled correctly. The fix for this is to take the absolute value (Math.abs) of the argument passed into the net/minecraft/util/Mth#fastInvCubeRoot method. Additionally, net/minecraft/util/Mth#fastInvCubeRoot does not work for negative numbers, but fixing t
hisdoes not solve this issue because it causes the matrix to be inverted.2. If all scales are equal and negative, the normal matrix will not be scaled correctly. For this case, all normals should be inverted, but in reality nothing happens. The fix for this is to return after the normal matrix is inverted (normal.mul(-1.0f)).
The method com/mojang/blaze3d/vertex/PoseStack#scale is broken in some situations:
1. If one or three of the scales are negative, the normal matrix will not be scaled correctly. The fix for this is to take the absolute value (Math.abs) of the argument passed into the net/minecraft/util/Mth#fastInvCubeRoot method. Additionally, net/minecraft/util/Mth#fastInvCubeRoot does not work for negative numbers, but fixing it does not solve this issue because it causes the matrix to be inverted.
2. If all scales are equal and negative, the normal matrix will not be scaled correctly. For this case, all normals should be inverted, but in reality nothing happens. The fix for this is to return after the normal matrix is inverted (normal.mul(-1.0f)).
Eclipse
Forge / Fabric Workspace
OpenJDK 11.0.8/ 16.0.1Eclipse / IntelliJ IDEA
Forge / Fabric Workspace
OpenJDK 11 / 16 / 17
The class `net.minecraft.data.worldgen.BootstapContext` and the method `net.minecraft.core.RegistrySetBuilder$BuildState#bootstapContext` are not spelled correctly; they should be named `BootstrapContext` and `bootstrapContext` respectively. This is inferred to be a typo because every method that consumes a `BootstapContext` and is referenced in the `VanillaRegistries` class's `private static final RegistrySetBuilder BUILDER` is correctly named `bootstrap`.
The class net.minecraft.data.worldgen.BootstapContext and the method net.minecraft.core.RegistrySetBuilder$BuildState#bootstapContext are not spelled correctly; they should be named BootstrapContext and bootstrapContext respectively. This is inferred to be a typo because every method that consumes a BootstapContext and is referenced in the VanillaRegistries class's private static final RegistrySetBuilder BUILDER is correctly named bootstrap .
The bug
During the sniffer's sniffing animation, its snout inflates. However, the shading of it also changes, creating a clear line in its geometry and breaking the illusion. The snout may appear brighter or darker depending on the direction the sniffer is facing.
Code analysis
Code analysis by Pepper can be found in this comment.
In vanilla, the conditions for invalid values are usually met only when the model view PoseStack is scaled, but this stack's normal matrix does not matter since it is not uploaded to the shader or used in any other way.
The only places where the invalid values are actually used are in SignEditScreen#render, LoomScreen#renderBg, LoomScreen#renderPattern, and GameRenderer#renderItemActivationAnimation. GameRenderer#renderItemActivationAnimation is the only one that produces a visual effect. Right after being revived by a totem of undying, the totem in the activation animation will have lighting that does not transition smoothly as it spins. Applying my suggested changes fixes this.
MC-176864 is caused by a series of model matrix transformations reversing the vertex order for all item model quads. Minecraft renders items (that are not translucent block items) with culling enabled, meaning that quads only render on the side from which the vertex order is counter-clockwise. Since the order is reversed in the opposite hand, all quads render from the wrong side.
From my testing, however, it does seem like the lighting bugs described in that issue are directly related to this issue. The lighting was fixed once my suggested changes were applied.
I was able to create a Fabric mod that fixes this issue by changing Minecraft's code. Hopefully the way I resolved this issue and the following insights will be useful.
The issue was mostly fixed after the following change in com.mojang.blaze3d.vertex.PoseStack.
However, after this change, the top of nose got darker the more the nose was scaled up. This is because Minecraft does not normalize the normal vector after it is transformed by the normal matrix. When the fragment shader uses the vertex normal vector to calculate lighting, it is not unit length, which results in dark lighting. Normalizing the vector after it is transformed in net.minecraft.client.model.geom.ModelPart$Cube#compile fixes this issue, and thus completely resolves the initial issue.
Quads from block and item models always go through VertexConsumer#putBulkData. However, light_emission is only applied right after flat lighting and smooth lighting during block model buffering and ignored during item model buffering. Considering that this feature would have been easier to implement by applying the light value inside of VertexConsumer#putBulkData instead, the fact that items do not support light_emission seems intentional to me. However, I agree that items should support it.