Normal matrix scaling broken in some situations
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)).
Created Issue:
Inverse Cube Root Method Broken for Negative Numbers
The client-side only method net/minecraft/util/Mth#fastInvCubeRoot does not work for negative numbers. This method is only used in com/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.
Environment
Eclipse
Forge Workspace and Fabric Workspace
OpenJDK 11.0.8
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
relates to
NormalMatrixScalingBroken inSomeSituationsNormal matrix scaling broken in some situations
relates to
relates to
relates to
As of 20w05a the issue seems fixed. Probably related to the fix of MC-225170.
Does this affect anything in vanilla gameplay?
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.
Relates of MC-176864?
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.