XeGTAO icon indicating copy to clipboard operation
XeGTAO copied to clipboard

bhos_laine_karras_permutation - there could be a significant problem with this hash.

Open CptLucky8 opened this issue 3 years ago • 1 comments

https://github.com/GameTechDev/XeGTAO/blob/0d177ce06bfa642f64d8af4de1197ad1bcb862d4/Source/Rendering/Shaders/vaNoise.hlsl#L432

Hi,

I don't know if this is relevant in how it is used with XeGTAO, but when reviewing some of the references cited in the source code, there was one in particular which might be important. In effect, the article author suggests using this instead:

x ^= x * 0x3d20adea
x += seed
x *= (seed >> 16) | 1
x ^= x * 0x05526c56
x ^= x * 0x53a22864

Matt Pharr identified a significant problem with this hash when testing it in PBRT. In short, if you run the following code which buckets outputs based on their lowest 8 bits:

any_constant = 123
buckets = [0, 0, 0, 0, ...]
loop for as long as you like:
    seed = random()
    buckets[owen_hash(any_constant, seed) & 0xff] += 1

...almost half of the buckets end up empty, and there is significant bias even in the buckets that aren't. Since you need to reverse the bits for Owen scrambling, those lowest bits become the highest bits, and it can end up being a real problem.7

7- Interestingly, if you do both scrambling and shuffling, like I am in Psychopath, the practical impacts of this don't manifest in any obvious way (which is why I didn't notice it). It turns out that shuffling is really good at hiding problems with scrambling. But if you do only scrambling—no shuffling—this problem shows up as subtle but noticeable artifacts at low sample counts.

CptLucky8 avatar Oct 07 '22 19:10 CptLucky8

Hi CptLucky8 - thank you so much for noting this!

The good thing is that it isn't used for the XeGTAO, so it doesn't affect the sample itself really (although it is used for ground truth computation, so the (offline) training might be ever so slightly affected).

I'm no longer at Intel so I'll see if I can do a PR or figure out who's responsible for maintaining the code now :)

fstrugar avatar Oct 11 '22 13:10 fstrugar