Page MenuHome

Proposed Fix T67939: GPencil Noise modifier wrong random calculation
ClosedPublic

Authored by Antonio Vazquez (antoniov) on Thu, Aug 1, 10:54 AM.

Details

Summary

There were several problems in the old random calculation:

* Different result in the viewport and render.
* Noise "pop" in some frames.
* Random number was calculated every time the file was opened, so get different results.

Now, instead to calculate the random numbers when n number of frames changed, the random values are calculated whith a seed by group of frames and stroke.

Also, a new Seed parameter has been added and this adds more control in the noise generated. This value can be animated and get more variations.

As result of these changes, some variables have been removed from Noise modifier struct too.

Diff Detail

Repository
rB Blender

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.EditedThu, Aug 1, 11:41 AM

Seed can be 0 by default for consistency with other seed values in Blender.

Precomputing 20 random values with BLI_array_frand and storing them in the modifier is unnecessarily complicated, just compute them in the modifier evaluation function, it's not that expensive.

If you want to remove the limit of 20 values, you can also add this function to BLI_hash.h and not compute any array in advance.

BLI_INLINE float BLI_hash_int_2d_01(unsigned int kx, unsigned int ky)
{
  return (float)BLI_hash_int_2d(kx, ky) * (1.0f / (float)0xFFFFFFFF);
}

float random_value = BLI_hash_int_2d_01(seed, idx);
This revision now requires changes to proceed.Thu, Aug 1, 11:41 AM

@Brecht Van Lommel (brecht) Before I compute the random values in Evalute (deformStroke) and the values where missing in render...this is the reason why I compute in Init.

Before, the random numbers depended on the current time and pointer values, which are different in the render.

In your new code, gpmd->rnd only depends on the value of gpmd->seed. So as far as I can tell computing it in deformStroke should work fine.

Ok, I will make the changes and test again.

  • Remove unnecessarily complicated code and generate same seed for render and viewport by stroke.

@Brecht Van Lommel (brecht) I always thought in Blender the random generator depended of time (internally) and not only of seed. Now the code is more easy and fast with your idea.

Brecht Van Lommel (brecht) requested changes to this revision.Thu, Aug 1, 5:49 PM
Brecht Van Lommel (brecht) added inline comments.
source/blender/gpencil_modifiers/intern/MOD_gpencilnoise.c
157

Doing a simple multiplication is not the right way to combined two random seeds. In this case, at frame 0 the random number will be the same for all modifiers.

The easiest way to avoid that is to use BLI_hash_int_2d((sc_frame / mmd->step) + gps->totpoints, mmd->seed + 1).

This revision now requires changes to proceed.Thu, Aug 1, 5:49 PM
  • Added BLI_hash_int_2d(() to calculate seed
Antonio Vazquez (antoniov) marked an inline comment as done.Thu, Aug 1, 5:58 PM
Antonio Vazquez (antoniov) added inline comments.
source/blender/gpencil_modifiers/intern/MOD_gpencilnoise.c
157

I have never used before the random number in Blender and I did not know this function.

I have changed the code as you suggest.

This revision is now accepted and ready to land.Thu, Aug 1, 6:02 PM