Auburn / FastNoiseLite

Fast Portable Noise Library - C# C++ C Java HLSL GLSL JavaScript Rust Go
http://auburn.github.io/FastNoiseLite/
MIT License
2.77k stars 326 forks source link

Cellular Distance gives out-of-range results #141

Open ForeverZer0 opened 4 months ago

ForeverZer0 commented 4 months ago

I am uncertain if this is known issue, but the cellular distance algorithm will output values outside the range of of -1.0 and 1.0 under normal conditions, specifically with the "hybrid" distance function with my tests.

Minimal example using the C port:

#include <stdio.h>
#define FNL_IMPL
#include "FastNoiseLite.h"

#define SIZE 768

int main(int argc, const char **argv) {
    fnl_state state = fnlCreateState();
    state.seed = 1337;
    state.noise_type = FNL_NOISE_CELLULAR;
    state.frequency = 0.02f;
    state.cellular_distance_func = FNL_CELLULAR_DISTANCE_HYBRID;
    state.cellular_return_type = FNL_CELLULAR_RETURN_TYPE_DISTANCE2;
    state.cellular_jitter_mod = 1.0;

    int over = 0;
    int under = 0;
    for (int y = 0; y < SIZE; y++) {
        for (int x = 0; x < SIZE; x++) {
            float f = fnlGetNoise2D(&state, x, y);
            if (f > 1.0f) over++;
            if (f < -1.0f) under++;
        }
    }
    printf("TOTAL: %d, OVER: %d, UNDER: %d\n", SIZE * SIZE, over, under);
    return 0;
}

Output:

TOTAL: 589824, OVER: 77817, UNDER: 0

Other language ports output the same out-of-range values with equivalent settings. The web preview is coded defensively and clamps the values into range before conversion to an integer, but it is visually noticeable with the abundance of white in the output compared to similar configurations.

For the C port, this is the offending part of the algorithm on line 1647:

float newDistance = (FastAbs(vecX) + FastAbs(vecY)) + (vecX * vecX + vecY * vecY);

Assuming this is correct, would it be better to clamp it before returning the result from this function?

Auburn commented 4 months ago

Cellular output is actually never bounded, it's just normally falls within bounds unless using hybrid distance. I have done some work to add correct bounding to it, but since it is a output changing fix I'd want to bump the minor version at least