add yee's perceptual metric

add yee's perceptual metric

R=bsalomon@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9803

[Zach Reizner, 2013-06-27 15:31:00]

There is giant chunk of math in this CL. The algorithm is taken from
http://pdiff.sourceforge.net/metric.html if you feel like checking it, but don't
feel bad about just skimming those parts.

[bsalomon, 2013-06-27 18:31:15]

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
File experimental/skpdiff/SkPMetric.cpp (right):

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
experimental/skpdiff/SkPMetric.cpp:54: template<class T>
It seems like the template param should be restricted to be an Image2D.

Maybe make T be one of float, RGB, or LAB and instead of:

T** image;

have

Image2D<T>** image;

But also does this class add much value over an array or Image2Ds? The name
makes it sound like a volume texture, but I don't think that's really what it
is.

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
experimental/skpdiff/SkPMetric.cpp:89: void adobergb_to_cielab(float r, float g,
float b, LAB* lab) {
Does this mean the input is in Adobe RGB?
http://en.wikipedia.org/wiki/Adobe_RGB_color_space

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
experimental/skpdiff/SkPMetric.cpp:211: if (nx < 0) {
should these four ifs be done inside the vertical/horizontal if/else?

You mirror at the borders?

[Zach Reizner, 2013-06-27 18:48:49]

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
File experimental/skpdiff/SkPMetric.cpp (right):

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
experimental/skpdiff/SkPMetric.cpp:54: template<class T>
On 2013/06/27 18:31:15, bsalomon wrote:
> It seems like the template param should be restricted to be an Image2D.
> 
> Maybe make T be one of float, RGB, or LAB and instead of:
> 
> T** image;
> 
> have
> 
> Image2D<T>** image;
> 
> But also does this class add much value over an array or Image2Ds? The name
> makes it sound like a volume texture, but I don't think that's really what it
> is.
> 

I like the idea of moving the template params.

The reason I have this class over an array is for bounds checking. I was having
a hard time debugging my segmentation faults, so I created this and the Image2D.
As an added benefit, I didn't have to pass so array lengths to the functions
below, and my code wasn't littered with complex pointer arithmetic.

https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
experimental/skpdiff/SkPMetric.cpp:89: void adobergb_to_cielab(float r, float g,
float b, LAB* lab) {
On 2013/06/27 18:31:15, bsalomon wrote:
> Does this mean the input is in Adobe RGB?
> http://en.wikipedia.org/wiki/Adobe_RGB_color_space

The input is assumed to be Adobe RGB. Adobe RGB has a defined white point that
matches many typical monitors.

[bsalomon, 2013-06-28 14:44:08]

On 2013/06/27 18:48:49, Zach Reizner wrote:
>
https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
> File experimental/skpdiff/SkPMetric.cpp (right):
> 
>
https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
> experimental/skpdiff/SkPMetric.cpp:54: template<class T>
> On 2013/06/27 18:31:15, bsalomon wrote:
> > It seems like the template param should be restricted to be an Image2D.
> > 
> > Maybe make T be one of float, RGB, or LAB and instead of:
> > 
> > T** image;
> > 
> > have
> > 
> > Image2D<T>** image;
> > 
> > But also does this class add much value over an array or Image2Ds? The name
> > makes it sound like a volume texture, but I don't think that's really what
it
> > is.
> > 
> 
> I like the idea of moving the template params.
> 
> The reason I have this class over an array is for bounds checking. I was
having
> a hard time debugging my segmentation faults, so I created this and the
Image2D.
> As an added benefit, I didn't have to pass so array lengths to the functions
> below, and my code wasn't littered with complex pointer arithmetic.

We do have array classes :) (e.g. SkAutoTArray, SkTArray)

> 
>
https://codereview.chromium.org/18066004/diff/2001/experimental/skpdiff/SkPMe...
> experimental/skpdiff/SkPMetric.cpp:89: void adobergb_to_cielab(float r, float
g,
> float b, LAB* lab) {
> On 2013/06/27 18:31:15, bsalomon wrote:
> > Does this mean the input is in Adobe RGB?
> > http://en.wikipedia.org/wiki/Adobe_RGB_color_space
> 
> The input is assumed to be Adobe RGB. Adobe RGB has a defined white point that
> matches many typical monitors.

Gotcha

[Zach Reizner, 2013-06-28 14:50:50]

The advantage of Image3D is that it initializes all its slices to be the same
size.

[bsalomon, 2013-06-28 15:00:28]

On 2013/06/28 14:50:50, Zach Reizner wrote:
> The advantage of Image3D is that it initializes all its slices to be the same
> size.

Ok, given my understanding of how this is being used, I still think "3D" is
somewhat misleading (implying volumetric data).

[jvanverth1, 2013-06-28 15:05:03]

On 2013/06/28 15:00:28, bsalomon wrote:
> On 2013/06/28 14:50:50, Zach Reizner wrote:
> > The advantage of Image3D is that it initializes all its slices to be the
same
> > size.
> 
> Ok, given my understanding of how this is being used, I still think "3D" is
> somewhat misleading (implying volumetric data).

Yeah, I'd suggest Image2DArray, or even just ImageArray.

[bsalomon, 2013-06-28 15:23:17]

On 2013/06/28 15:10:52, Zach Reizner wrote:

lgtm

[Zach Reizner, 2013-06-28 15:34:58]

Committed patchset #4 manually as r9803 (presubmit successful).