Skip to content
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

Array out of range #396

Closed
oxine opened this issue Mar 15, 2020 · 5 comments
Closed

Array out of range #396

oxine opened this issue Mar 15, 2020 · 5 comments
Assignees
Milestone

Comments

@oxine
Copy link
Contributor

oxine commented Mar 15, 2020

return ranfloat[perm_x[i] ^ perm_y[j] ^ perm_z[k]];

i j k can be minus value so would leads to error

@hollasch
Copy link
Collaborator

hollasch commented Mar 15, 2020

Original code was:

float noise(const vec3& p) const {

    float u = p.x() - floor(p.x());
    float v = p.y() - floor(p.y());
    float w = p.z() - floor(p.z());

    int i = int(4*p.x()) & 255;
    int j = int(4*p.y()) & 255;
    int k = int(4*p.z()) & 255;

    return ranfloat[perm_x[i] ^ perm_y[j] ^ perm_z[k]];
}

Current code (master; no changes in development branch) is:

double noise(const vec3& p) const {

    auto u = p.x() - floor(p.x());
    auto v = p.y() - floor(p.y());
    auto w = p.z() - floor(p.z());

    auto i = static_cast<int>(floor(p.x()));
    auto j = static_cast<int>(floor(p.y()));
    auto k = static_cast<int>(floor(p.z()));

    return ranfloat[perm_x[i] ^ perm_y[j] ^ perm_z[k]];
}

We lost the scaling by four, plus selection of the lower 8 bits, which handled negative values. Need to track down the evolution of this code.

@hollasch
Copy link
Collaborator

Well … that was fun. I had to go back to the archives for the original repository for The Next Week. All changes are in the new unified repository, but the combination of multiple streams of development makes tracking down the code development for this chunk non-trivial.

Anyway, this code has been broken from the very start in the repo when I first introduced it in commit d1ecb9d, in 2019-07-28. It was basically a transcription error on my part, and I'm a bit surprised that no-one's reported it until now (thank you @oxine!).

Will fix for upcoming v3.0.0 release.

@hollasch hollasch self-assigned this Mar 15, 2020
@hollasch hollasch added this to the v3 milestone Mar 15, 2020
@hollasch
Copy link
Collaborator

For the record, my original (bad) transcription was:

float noise(const vec3& p) const {
    float u = p.x() - floor(p.x());
    float v = p.y() - floor(p.y());
    float w = p.z() - floor(p.z());

    int i = floor(p.x());
    int j = floor(p.y());
    int k = floor(p.z());

    return ranfloat[perm_x[i] ^ perm_y[j] ^ perm_z[k]];
}

@hollasch
Copy link
Collaborator

@oxine — can you verify the following works? Thanks.

double noise(const vec3& p) const {
    auto u = p.x() - floor(p.x());
    auto v = p.y() - floor(p.y());
    auto w = p.z() - floor(p.z());

    auto i = static_cast<int>(4*p.x()) & 255;
    auto j = static_cast<int>(4*p.y()) & 255;
    auto k = static_cast<int>(4*p.z()) & 255;

    return ranfloat[perm_x[i] ^ perm_y[j] ^ perm_z[k]];
}

hollasch added a commit that referenced this issue Mar 16, 2020
The original code for the first version of the Perlin noise() function
in _The Next Week_ was incorrect. This returns the code to the original
behavior, but using static_cast instead of the original C-style cast.

Resolves #396
@oxine
Copy link
Contributor Author

oxine commented Mar 16, 2020

Verified,now it works well.

@oxine oxine closed this as completed Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants