noise icon indicating copy to clipboard operation
noise copied to clipboard

Utils.makeInt32Range is implemented incorrectly

Open Barteks2x opened this issue 3 years ago • 2 comments

This has been found by DaPorkchop_ while working on another project

This method:

https://github.com/SpongePowered/noise/blob/213f04e0fc78bd19160c5700981b0620f45e9f98/src/main/java/org/spongepowered/noise/Utils.java#L114-L122

has been originally ported from the following C code in libnoise:

  inline double MakeInt32Range (double n)
  {
    if (n >= 1073741824.0) {
      return (2.0 * fmod (n, 1073741824.0)) - 1073741824.0;
    } else if (n <= -1073741824.0) {
      return (2.0 * fmod (n, 1073741824.0)) + 1073741824.0;
    } else {
      return n;
    }
  }

But because of operator precedence in java, the java code is actually equivalent to this

    public static double makeInt32Range(double n) {
        if (n >= 1073741824.0) {
            return ((2.0 * n) % 1073741824.0) - 1073741824.0;
        } else if (n <= -1073741824.0) {
            return ((2.0 * n) % 1073741824.0) + 1073741824.0;
        } else {
            return n;
        }
    }

Where in the original C code, modulo should take precedence.

For values of n between k * 1073741824 and k * 1073741824 + 536870912 for integer k >= 1 (and similar for negative range) this just happens to output the same values, but otherwise for n > 1073741824 the result will be always negative (and similarly for n < -1073741824 the result will be always positive) instead of repeating the entire range from -1073741824 to 1073741824.

Changing this now may be considered a breaking change as there could be terrain generators that would generate different terrain with this fixed.

Barteks2x avatar May 06 '21 20:05 Barteks2x