Fix off-by-one in compressing/decompressing texture coordinates #4355
Conversation
|
A note about the spec changes that loosen up the tolerance.... Previously, values like 0.25 and 0.5 could be represented exactly because the value was multiplied by a nice power of two, 4096. Now that it's multiplied by 4095, those values can no longer be represented exactly, so we need an epsilon that actually represents the quantization we're doing. So what was wrong with multiplying by 4096, cause that seems better, right? Well, basically because if we multiply by 4096, |
| @@ -303,8 +304,9 @@ define([ | |||
| //>>includeEnd('debug'); | |||
|
|
|||
| var temp = compressed / 4096.0; | |||
| result.x = Math.floor(temp) / 4096.0; | |||
| result.y = temp - Math.floor(temp); | |||
| var xZeroTo4095 = Math.floor(temp); | |||
There was a problem hiding this comment.
maybe var xZeroTo4095 = compressed >> 12 etc?
There was a problem hiding this comment.
Yeah that's a good point, we could do this whole thing as integers and only make it a float at the end.
There was a problem hiding this comment.
The reason I did it this way was to mirror what was done in the shader which doesn't have any bitwise operations, but its up to you.
|
@bagnell can you please review since you wrote the original code? |
|
@kring This looks good to me. Thanks for the fix. Let me know if you plan on making the integer change. If not, I'll merge this. |
|
Thanks Dan. Yeah, I can see the wisdom in writing similar code in javascript and in GLSL, so I'm happy to leave it the way it is. I merged in master so this should be ready. |
This removes the need for the check for 1.0 and avoids the potential for encoding the coordinates totally wrong for a value that is not 1.0 but is very close to 1.0. More discussion in #4103.