Respect the stride for efb copies when hashing them

[mimimi085181]

Before there was memory hashed that does not belong to the efb copy.

One problem with this right now is that the hash will be the same if the "blocks" of the efb copy are the same, but the order is different.

[phire]

Also, if the number of blocks is even and they all contain the same data (such as zeros), the hash will be zero.

Some kind of bit mixing is needed between each line. This hash doesn't need to be cyptographically secure so it could be anything. Rotating by like 13 would be fast but multiplying by a prime number like 397 would achieve better mixing of bits.

[phire]

Oh, rotating by a prime number would result in a cycle every 64 vertical blocks, which could lead to a 128 or 256 block high texture of repeated data canceling itself out.

Rotating by a non-prime number would result in much smaller cycles, from 32 down to 2 vertical blocks.

Mixing in i might help, but you might want to use a multiply.

[mimimi085181]

Don't i lose a few bits of the hash, everytime the multiplication happens? So in the end the last few hashes provide more information than the 1st half of the hashes?

[BhaaLseN]

Not necessarily, since you got the XOR in there too. It's intended to overflow there, with the only condition that the bits are scrambled enough to produce a fairly unique hash for a large range of inputs.
You could just add a few performance counters and see if it produces any significant amount of collisions, then adjust the constants/primes and/or algorithm if necessary.

[phire]

My analysis of the math shows "multiply by a prime" mixes bits up, but can't mix them down. But the hash function looks fine and you really shouldn't be worrying about improving it more without doing benchmarks and other tests.

But I think you should add a minimum to the samples per row of blocks. Right now your worst cast is one sample from the left edge of the image (one sample that only effects 32 bits of the hash with the crc32 code). Perhaps bump that up to a minimum of 4 samples per row of blocks.

[phire]

Also, you need to pass samples == 0 through as zero, or you will get the opposite of safe texture hashing (guaranteed minimum hash) when safe texture hashing is enabled.

[mimimi085181]

About the minimum number of samples per block. Imagine there are 512 blocks and the stride would be correct, samples == 128. What data would be hashed then? Using a low number of samples just results in useless data hashed right now.

[magumagu]

Don't i lose a few bits of the hash, everytime the multiplication happens?

Multiplying by 397 is actually a lossless operation. See https://en.wikipedia.org/wiki/Modular_multiplicative_inverse.

About the minimum number of samples per block. Imagine there are 512 blocks and the stride would be correct, samples == 128. What data would be hashed then?

If you wanted to actually honor the given sample count, you would take one sample every four blocks. Probably simpler to just force the number of samples to something sane, like samples = std::max(samples, blocks * 4);.

[mimimi085181]

I'm not going to increase the number of samples per block. If the used samples number is so low that only 1 sample is taken per block, it's correct. A low sample number can always end up in stupid patterns being hashed, like only some pixels on the left side. The proper way to fix this is to use a higher samples number and not force more samples. Or else, you would also have to increase the samples number for big(1024x1024) for fast hashing(128), because that's as stupid as 1 sample per block for "strided efb copies".

So, do i have to do something about CacheLinesPerRow() * 32 or can i leave it as it is?

[dolphin-emu-bot]

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

• ssbm-pointsize on ogl-lin-nv: diff
• xenoblade-menu on ogl-lin-nv: diff

_{^{automated-fifoci-reporter}}

[phire]

LGTM