-
-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
Casts missing parentheses and causing precision loss #29
Comments
The affected method is As decompiled by FF, it looks like this: public BufferBuilder func_181663_c(float p_181663_1_, float p_181663_2_, float p_181663_3_) {
int i = this.field_178997_d * this.field_179011_q.func_177338_f() + this.field_179011_q.func_181720_d(this.field_181678_g);
switch(this.field_181677_f.func_177367_b()) {
case FLOAT:
this.field_179001_a.putFloat(i, p_181663_1_);
this.field_179001_a.putFloat(i + 4, p_181663_2_);
this.field_179001_a.putFloat(i + 8, p_181663_3_);
break;
case UINT:
case INT:
this.field_179001_a.putInt(i, (int)p_181663_1_);
this.field_179001_a.putInt(i + 4, (int)p_181663_2_);
this.field_179001_a.putInt(i + 8, (int)p_181663_3_);
break;
case USHORT:
case SHORT:
this.field_179001_a.putShort(i, (short)((int)p_181663_1_ * 32767 & '\uffff'));
this.field_179001_a.putShort(i + 2, (short)((int)p_181663_2_ * 32767 & '\uffff'));
this.field_179001_a.putShort(i + 4, (short)((int)p_181663_3_ * 32767 & '\uffff'));
break;
case UBYTE:
case BYTE:
this.field_179001_a.put(i, (byte)((int)p_181663_1_ * 127 & 255));
this.field_179001_a.put(i + 1, (byte)((int)p_181663_2_ * 127 & 255));
this.field_179001_a.put(i + 2, (byte)((int)p_181663_3_ * 127 & 255));
}
this.func_181667_k();
return this;
} As disassembled by javap, it looks like this:public net.minecraft.client.renderer.BufferBuilder func_181663_c(float, float, float); descriptor: (FFF)Lnet/minecraft/client/renderer/BufferBuilder; flags: ACC_PUBLIC Code: stack=4, locals=5, args_size=4 0: aload_0 1: getfield #99 // Field field_178997_d:I 4: aload_0 5: getfield #101 // Field field_179011_q:Lnet/minecraft/client/renderer/vertex/VertexFormat; 8: invokevirtual #106 // Method net/minecraft/client/renderer/vertex/VertexFormat.func_177338_f:()I 11: imul 12: aload_0 13: getfield #101 // Field field_179011_q:Lnet/minecraft/client/renderer/vertex/VertexFormat; 16: aload_0 17: getfield #300 // Field field_181678_g:I 20: invokevirtual #329 // Method net/minecraft/client/renderer/vertex/VertexFormat.func_181720_d:(I)I 23: iadd 24: istore 4 26: getstatic #332 // Field net/minecraft/client/renderer/BufferBuilder$1.field_210254_a:[I 29: aload_0 30: getfield #298 // Field field_181677_f:Lnet/minecraft/client/renderer/vertex/VertexFormatElement; 33: invokevirtual #336 // Method net/minecraft/client/renderer/vertex/VertexFormatElement.func_177367_b:()Lnet/minecraft/client/renderer/vertex/VertexFormatElement$EnumType; 36: invokevirtual #339 // Method net/minecraft/client/renderer/vertex/VertexFormatElement$EnumType.ordinal:()I 39: iaload 40: tableswitch { // 1 to 7 1: 84 2: 125 3: 125 4: 169 5: 169 6: 239 7: 239 default: 303 } 84: aload_0 85: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 88: iload 4 90: fload_1 91: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer; 94: pop 95: aload_0 96: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 99: iload 4 101: iconst_4 102: iadd 103: fload_2 104: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer; 107: pop 108: aload_0 109: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 112: iload 4 114: bipush 8 116: iadd 117: fload_3 118: invokevirtual #343 // Method java/nio/ByteBuffer.putFloat:(IF)Ljava/nio/ByteBuffer; 121: pop 122: goto 303 125: aload_0 126: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 129: iload 4 131: fload_1 132: f2i 133: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer; 136: pop 137: aload_0 138: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 141: iload 4 143: iconst_4 144: iadd 145: fload_2 146: f2i 147: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer; 150: pop 151: aload_0 152: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 155: iload 4 157: bipush 8 159: iadd 160: fload_3 161: f2i 162: invokevirtual #347 // Method java/nio/ByteBuffer.putInt:(II)Ljava/nio/ByteBuffer; 165: pop 166: goto 303 169: aload_0 170: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 173: iload 4 175: fload_1 176: f2i 177: sipush 32767 180: imul 181: ldc_w #484 // int 65535 184: iand 185: i2s 186: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer; 189: pop 190: aload_0 191: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 194: iload 4 196: iconst_2 197: iadd 198: fload_2 199: f2i 200: sipush 32767 203: imul 204: ldc_w #484 // int 65535 207: iand 208: i2s 209: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer; 212: pop 213: aload_0 214: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 217: iload 4 219: iconst_4 220: iadd 221: fload_3 222: f2i 223: sipush 32767 226: imul 227: ldc_w #484 // int 65535 230: iand 231: i2s 232: invokevirtual #351 // Method java/nio/ByteBuffer.putShort:(IS)Ljava/nio/ByteBuffer; 235: pop 236: goto 303 239: aload_0 240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 243: iload 4 245: fload_1 246: f2i 247: bipush 127 249: imul 250: sipush 255 253: iand 254: i2b 255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 258: pop 259: aload_0 260: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 263: iload 4 265: iconst_1 266: iadd 267: fload_2 268: f2i 269: bipush 127 271: imul 272: sipush 255 275: iand 276: i2b 277: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 280: pop 281: aload_0 282: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 285: iload 4 287: iconst_2 288: iadd 289: fload_3 290: f2i 291: bipush 127 293: imul 294: sipush 255 297: iand 298: i2b 299: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 302: pop 303: aload_0 304: invokespecial #357 // Method func_181667_k:()V 307: aload_0 308: areturn StackMapTable: number_of_entries = 5 frame_type = 252 /* append */ offset_delta = 84 locals = [ int ] frame_type = 40 /* same */ frame_type = 43 /* same */ frame_type = 251 /* same_frame_extended */ offset_delta = 69 frame_type = 255 /* full_frame */ offset_delta = 63 locals = [ class net/minecraft/client/renderer/BufferBuilder ] stack = [] LineNumberTable: line 464: 0 line 465: 26 line 467: 84 line 468: 95 line 469: 108 line 470: 122 line 473: 125 line 474: 137 line 475: 151 line 476: 166 line 479: 169 line 480: 190 line 481: 213 line 482: 236 line 485: 239 line 486: 259 line 487: 281 line 490: 303 line 491: 307 LocalVariableTable: Start Length Slot Name Signature 0 309 0 this Lnet/minecraft/client/renderer/BufferBuilder; 0 309 1 p_181663_1_ F 0 309 2 p_181663_2_ F 0 309 3 p_181663_3_ F 26 283 4 lvt_4_1_ I Since this is a lot of text, I'll focus just on just the Here's the same disassembly, trimmed:public net.minecraft.client.renderer.BufferBuilder func_181663_c(float, float, float); Code: // Computing i - snip // switch 40: tableswitch { // 1 to 7 // snip cases we don't care about 6: 239 // for UBYTE jump to 239 7: 239 // for BYTE jump to 239 default: 303 } // snip code for cases we don't care about 239: aload_0 240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 243: iload 4 245: fload_1 246: f2i 247: bipush 127 249: imul 250: sipush 255 253: iand 254: i2b 255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 258: pop 259: aload_0 260: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 263: iload 4 265: iconst_1 266: iadd 267: fload_2 268: f2i 269: bipush 127 271: imul 272: sipush 255 275: iand 276: i2b 277: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 280: pop 281: aload_0 282: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer; 285: iload 4 287: iconst_2 288: iadd 289: fload_3 290: f2i 291: bipush 127 293: imul 294: sipush 255 297: iand 298: i2b 299: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer; 302: pop // Exiting the method 303: aload_0 304: invokespecial #357 // Method func_181667_k:()V 307: aload_0 308: areturn And now to dissect a tiny section of that specifically -- one call to ; Put `this` onto the stack
; Stack: [this]
239: aload_0
; Remove `this` from the stack and put `this.byteBuffer` onto it
; Stack: [this.byteBuffer]
240: getfield #61 // Field field_179001_a:Ljava/nio/ByteBuffer;
; Load local variable 4 (which is i, or alternatively lvt_4_1_), and put it onto the stack
; Stack: [this.byteBuffer, i]
243: iload 4
; Load local variable 1 (which is x, or alternatively p_181663_1_), and put it onto the stack.
; Stack: [this.byteBuffer, i, x]
245: fload_1
; Convert the top of the stack from a float to an int and put it into the stack
; Stack: [this.byteBuffer, i, ((int)x)]
246: f2i
; Put the number 127 onto the stack
; Stack: [this.byteBuffer, i, ((int)x), 127]
247: bipush 127
; Take the top two values from the stack and perform an integer multiplication
; and then put that onto the stack
; Stack: [this.byteBuffer, i, (((int)x) * 127)]
249: imul
; Take the number 255 and put it onto the stack
; Stack: [this.byteBuffer, i, (((int)x) * 127), 255]
250: sipush 255
; And the two top values together and put the result onto the stack
; Stack: [this.byteBuffer, i, ((((int)x) * 127) & 255)]
253: iand
; Convert the top of the stack from an int into a byte and put it onto the stack
; Stack: [this.byteBuffer, i, ((byte)((((int)x) * 127) & 255))]
254: i2b
; Call `this.byteBuffer.put`. Two arguments are popped off
; and then the object to call it on.
; The resultant call is `this.byteBuffer.put(i, ((byte)((((int)x) * 127) & 255)))`
; `ByteBuffer.put` puts the result back onto the stack, so:
; Stack: [this.byteBuffer]
255: invokevirtual #354 // Method java/nio/ByteBuffer.put:(IB)Ljava/nio/ByteBuffer;
; Discard the return value of put
; Stack: []
280: pop This shows that the cast in the bytecode happens before the multiplication, and the original code actually does have it. So it's a mistake on Mojang's end, not a bug with FernFlower. I'm mostly ignorant on the actual rendering code, so I need to ask: how would this manifest itself ingame? Is there any way that it could empirically be confirmed that this is a vanilla issue? |
There's an image here: MinecraftForge/MinecraftForge#2437 |
Could it be FernFlower causing this bug? MinecraftForge/MinecraftForge#2321
The
(int)
cast needs to be applied to the entirep_181663_1_ * 127
expression, but the parentheses are left out so that it only castsp_181663_1_
, changing the calculation.The text was updated successfully, but these errors were encountered: