noise-rs icon indicating copy to clipboard operation
noise-rs copied to clipboard

Scale Factor calculations are incorrect

Open mfiano opened this issue 3 years ago • 9 comments

Some time after the 0.7 release, something broke the billow generator. The examples which make use of it, such as slime and granite, look absolutely nothing like they did in 0.7 or in libnoise. I'm not sure what is the problem yet...been debugging for a while now. I'll post back if I find time to dive any deeper.

mfiano avatar Feb 25 '21 06:02 mfiano

Ok I tracked down the issue. The cause is the commit that attempts to calculate the scaling factors of the fractal-based generators. The C++ code's examples, which were translated verbatim over to this project, actually depend on the output range of billow being [-0.5, 1.5].

I completely agree that attempting to move everything into the [-1, 1] range is the correct thing to do, but I must also mention that the way you calculate the scale factor for some of the fractal generators is completely wrong, and will not work for all persistence/octave/attenuation values.

I will present some correct scaling factor functions, though forgive my use of pseudo-code as I am not very great with Rust yet:

// billow and fbm scaling factor calculation
result = 0.0
loop for i from 0 to octave-count - 1
    result += pow(persistence, i)
result

// basic-multi scaling factor calculation
result = 0.0
loop for i from 0 to octave-count - 1
    result = 1.0 <then> (result + (result * pow(persistence, i))) // different for first iteration
result

// hybrid-multi scaling factor calculation
result = persistence
loop from 1 to octave-count - 1
    amplitude = persistence <then> amplitude * persistence // different for first iteration
    weight = result <then> max(weight, 1) // different for first iteration
    signal = amplitude
    weight *= signal
    result += weight
result

// ridged-multi scaling factor calculation
result = 0.0
loop from 0 to octave-count
    amplitude = 1.0 <then> amplitude * persistence // first iteration is different
    weight = 1.0 <then> clamp(signal / attenuation, 0, 1) // first iteration is different
    signal = weight * amplitude
    result += signal
2 / result

These should all result in output ranges that are [-1.0, 1.0] regardless of persistence (or attenuation in the case of ridged-multi), after they are translated properly to Rust.

However, the examples will surely need to be rewritten, likely by adding a ScaleBias modifier in the case of billow that multiplies the output by 1 and adds 0.5 bias, since that is what the libnoise examples expect the range to be.

mfiano avatar Feb 25 '21 07:02 mfiano

One last note. The wood texture example was ported over slightly wrongly. Because libnoise samples the z coordinate for the y image coordinates, the y stretching of 0.25 actually has to be performed on the z axis in noise-rs. You will see how this shows you the grain of the wood, which you cannot see it after rotation with the current axis.

mfiano avatar Feb 25 '21 07:02 mfiano

It's great to get a second set of eyes on this code!

After looking at your suggestion, and taking another look at the scale factor code, I can definitely agree that my code is wrong, and it looks like your scaling factor functions would be correct. Just to make sure, and to show the work, I'll go through a derivation for Billow.

I think to determine the exact equation needed for the scale factor, we'll have to look at the maximum values generated by each fractal generator.

The Work

For Billow, the maximum values for each octave, assuming that the value returned from NoiseFn::get() is always 1, would look like the following:

Billow 1st Octave 2nd Octave 3rd Octave 4th Octave 5th Octave 6th Octave
MaxValueofOctave 0.5 0.75 0.825 0.9375 0.9688 0.9844

The scale factors that are needed for each octave in order to scale the result into the {-1, 1} range is simply 1 / MaxValueofOctave. Looking at the sequence of values, it can be determined that the formula for the maximum value at a certain octave can be expressed as:

However, this only holds true if persistence = 0.5. If the persistence value is something else, like 0.3, then the Max Value of each octave is different:

Persistence = 0.3 1st Octave 2nd Octave 3rd Octave 4th Octave 5th Octave 6th Octave
MaxValueofOctave 0.3 0.39 0.417 0.4251 0.4275 0.4283

This leads to the more general equation below:

Now, since the scale factor for the result would be 1 / MaxValue, we can write the whole formula as follows:

I'm putting the divide here so that it only has to be calculated once, and the actual use of the value can be a multiply which is slightly faster.

or in Rust:

fn calc_scale_factor(persistence: f64, octaves: usize) -> f64 {
    let mut denom = 0.0;

    for n in 1..=octaves {
        denom += persistence.powi(n as i32);
    };

    1.0 / denom
}

In a more functional form, this function would be:

fn calc_scale_factor(persistence: f64, octaves: usize) -> f64 {
    let denom = (1..=octaves).fold(0.0, |acc, x| acc + persistence.powi(x as i32));

    1.0 / denom
}

Implementing this corrected calc_scale_factor function in the library still results in weird images, but a closer look reveals that

// Scale the amplitude appropriately for this frequency.
    signal *= self.persistence.powi(x as i32);

should actually be

// Scale the amplitude appropriately for this frequency.
    signal *= self.persistence.powi((x as i32) + 1);

since start with x = 0 results in the first loop having a multipler here of persistence0 = 1 instead of persistence1 = persistence

After fixing this, the images look much more like they did before the addition of the scale_factor.

So again, thanks for looking into this and getting me to take another, closer look at the implementation.

I will be taking another look at the scale_factors in the other fractal generators as well, and doing the write-ups here.

Razaekel avatar Feb 26 '21 02:02 Razaekel

No problem! I will have to analyze your math closer when I get some time, but it sounds logical at a quick glance.

One other thing that I feel I should mention, is the Perlin noise implementation is very questionable to me.

I can see why you are doing the different hashing method to keep the table 256-long without wrapping lookups - that's fine. However, the scaling factors are wrong. Your implementation of Perlin is Perlin "Improved Noise", not classic Perlin. It's only Classic Perlin that has an output range of +/- sqrt(n/4). Improved Perlin has an output range of [-1, 1] for 2D and 3D. 1D Improved is 4x that, so multiply by 0.25. and 4D, I haven't quite worked out after a lot of investigation, and is closer to [-1.2, 1.2]. I would recommend giving Perlin a look over again for 2D and 3D to do without the scaling factors, because you certainly shouldn't need to scale (and then clip off values with a clamp) if they are implemented correctly. Again, only classic Perlin outputs [-sqrt(n/4), sqrt(n/4)] for 2D and 3D.

Edit: I just had to check my implementation of Perlin Improved directly translated from Ken's code, to confirm I wasn't misremembering, and indeed what I said is correct. 10 million random samples produces a range of [-1, 1] (or close to it by a hundredth of error or so) for 2D and 3D.

Also I apologize for all the criticism under one issue. Maybe we should rename this issue, or I should open up new ones?

mfiano avatar Feb 26 '21 02:02 mfiano

In regards to the range of Perlin Noise, I used the following link as the reference that determined the range.

The range of Perlin noise - Digital Freepen

According to it, if the vectors of the gradients are of unit length, then the range of values would be +/- sqrt(n/4), regardless of whether it's Classic or Improved. It does mention the caveat that if the vector is not of unit length, then the result should also be scaled to the unit length.

Razaekel avatar Feb 26 '21 03:02 Razaekel

Yes you have to read that article carefully. You probably read:

"TL;DR: The range is [−√N/4,√N/4] where N is the number of dimensions, ...

without noticing the one place where it notes improved perlin doesn't use unit gradient vectors

mfiano avatar Feb 26 '21 03:02 mfiano

Ignore that if your implementation is not affected here. I did notice that it was not returning +/- 1 by a little bit without the scale factor though. More like 0.9 - 0.95

mfiano avatar Feb 26 '21 03:02 mfiano

Ok, we finished analyzing Perlin noise. Feel free to use any data you find to orient and scale them with good distributions: https://github.com/3b/noise-range-test/blob/main/perlin-improved.md

mfiano avatar Mar 10 '21 23:03 mfiano

The examples which make use of it, such as slime and granite, look absolutely nothing like they did in 0.7 or in libnoise.

This got me wondering what the granite texture looks like with all older versions. So I did that:

v0.3.0

texturegranite-v0 3 0

v0.4.0

texturegranite-v0 4 0

v0.4.1

texturegranite-v0 4 1

v0.5.0

texture_granite_planar-v0 5 0

v0.5.1

texture_granite_planar-v0 5 1

v0.6.0

texture_granite_planar-v0 6 0

v0.7.0

texture_granite_planar-v0 7 0

92ec3c1 (Current develop branch)

texture_granite_planar-92ec3c1

6920df5 (Currently open PR to fix this)

texture_granite_planar-6920df5


IMHO, v0.4.1 looks nice, but it is too dark and has a closer resemblance to marble than granite. Every iteration beyond this version does not look like granite at all. But you're right, the current develop branch is the worst.

Here is a reference photo for comparison, which is my best guess at the look it is trying to achieve. Apparently, this is called "chima pink granite".

chima-pink-4-555x415

parasyte avatar Feb 09 '22 14:02 parasyte