Review: move image comparison functionality from idiff into ImageBufAlgo

[lgritz]

I needed this for another app I was tinkering around with over the weekend, so took the opportunity to move this handy image comparison out of being local to idiff.cpp, and info the main library in the ImageBufAlgo namespace.

[robert-matusewicz]

will this comment be helpfull in the future? I think it can be removed;)

[lgritz]

No, I think that's an accident, I'll remove it.

[jeremyselan]

Looks good to me. Only minor issue is that we get a compilation warning:

/usr/lib64/gcc/x86_64-unknown-linux/4.4.4/../../../../include/c++/4.4.4/bits/stl_vector.h: In function ‘int OpenImageIO_SPI::v30::ImageBufAlgo::compare_Yee(const OpenImageIO_SPI::v30::ImageBuf&, const OpenImageIO_SPI::v30::ImageBuf&, float, float)’:

/usr/lib64/gcc/x86_64-unknown-linux/4.4.4/../../../../include/c++/4.4.4/bits/stl_vector.h:626: warning: array subscript is above array bounds

(It's always had this warning, but I didnt care when it was in the binary. Now that it's in the core library we should probably address it). I'll poke and see if the bug is obvious.

[jeremyselan]

The warning is from this line,

        for (int i = 0; i < LAPLACIAN_MAX_LEVELS - 2; i++)
            F_mask[i] = mask(contrast[i] * contrast_sensitivity(cpd[i], adapt)); 

Investigating further...

[jeremyselan]

oops - thats not the line. please hold. sorry for the spam

[jeremyselan]

Aha. Compilers are so smart.

In LaplacianPyramid, std::min (lev, LAPLACIAN_MAX_LEVELS) (line 63) should actually read std::min (lev, LAPLACIAN_MAX_LEVELS-1).

(This is a clamping to make sure you cant make a pyramid request off the end of the array, so the max valid index is size-1). Pending this change, lgtm.

[lgritz]

Great, I will fix this before doing the commit. Thanks for tracking down.

[lgritz]

Squashed and merged.