dmtrKovalenko / odiff

The fastest pixel-by-pixel image visual difference tool in the world.
MIT License
1.96k stars 73 forks source link

Why is threshold multiplied twice? #52

Closed makew0rld closed 3 years ago

makew0rld commented 3 years ago

https://github.com/dmtrKovalenko/odiff/blob/626840954a8910aa87561d5551a9a2cd2b64289c/src/Diff.re#L30

I'm confused why this line is like this. I'd assume the threshold (from 0 to 1) only needs to be multiplied once?

dmtrKovalenko commented 3 years ago

Its MAX_YIQ_POSSIBLE_DELTA * threshold^2 where YIQ delta is just max costant between 2 colors (black and white)

If you have more questions about diffing algorithm feel free to ask here or open a new discussion

makew0rld commented 2 years ago

Right, but why is threshold put to the power of two? Thanks.

dmtrKovalenko commented 2 years ago

Honestly to be in sync with pixelmatch, but in fact setting threshold to real 0.5 makes no sense because it will say that yellow is equal to red

makew0rld commented 2 years ago

I found these parts of pixelmatch that explain this:

Code: https://github.com/mapbox/pixelmatch/blob/2b03b635cffb9525bcefee4cc7329f98e375cff5/index.js#L43-L45

Threshold issue: https://github.com/mapbox/pixelmatch/issues/3

Threshold commit that puts it to a power of two: https://github.com/mapbox/pixelmatch/commit/5bfdf6cf6f565da75f1fc50f69d2a10279904b44#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346

So the reason threshold is squared is because the YIQ delta is also squared. You could square root the YIQ delta to get a more accurate delta and not need to square the threshold, but it would only waste resources.