[igt-dev] [RFC PATCH 5/7] igt/color: Add SW color transform functionality

[Harry Wentland]

On 2023-09-18 04:02, Pekka Paalanen wrote:
> On Fri, 15 Sep 2023 15:50:52 -0400
> Harry Wentland <harry.wentland at amd.com> wrote:
> 
>> On 2023-09-15 10:52, Pekka Paalanen wrote:
>>> On Fri, 8 Sep 2023 11:03:13 -0400
>>> Harry Wentland <harry.wentland at amd.com> wrote:
>>>   
>>>> In order to test color we want to compare a HW (KMS) transform
>>>> with a SW transform. This introduces color transform for an
>>>> sRGB EOTF but this can be extended to other transforms. Code is
>>>> borrowed from Skia.
>>>>
>>>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>>>> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
>>>> Cc: Pekka Paalanen <pekka.paalanen at collabora.com>
>>>> Cc: Simon Ser <contact at emersion.fr>
>>>> Cc: Harry Wentland <harry.wentland at amd.com>
>>>> Cc: Melissa Wen <mwen at igalia.com>
>>>> Cc: Jonas Ådahl <jadahl at redhat.com>
>>>> Cc: Sebastian Wick <sebastian.wick at redhat.com>
>>>> Cc: Shashank Sharma <shashank.sharma at amd.com>
>>>> Cc: Alexander Goins <agoins at nvidia.com>
>>>> Cc: Joshua Ashton <joshua at froggi.es>
>>>> Cc: Michel Dänzer <mdaenzer at redhat.com>
>>>> Cc: Aleix Pol <aleixpol at kde.org>
>>>> Cc: Xaver Hugl <xaver.hugl at gmail.com>
>>>> Cc: Victoria Brekenfeld <victoria at system76.com>
>>>> Cc: Daniel Vetter <daniel at ffwll.ch>
>>>> Cc: Uma Shankar <uma.shankar at intel.com>
>>>> Cc: Naseer Ahmed <quic_naseer at quicinc.com>
>>>> Cc: Christopher Braga <quic_cbraga at quicinc.com>
>>>> ---
>>>>  lib/igt_color.c | 330 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  lib/igt_color.h | 105 +++++++++++++++
>>>>  lib/igt_fb.c    |   6 +-
>>>>  lib/igt_fb.h    |   2 +
>>>>  lib/meson.build |   1 +
>>>>  5 files changed, 441 insertions(+), 3 deletions(-)
>>>>  create mode 100644 lib/igt_color.c
>>>>  create mode 100644 lib/igt_color.h
> 
> ...
> 
>>>> +/*
>>>> + * A transfer function mapping encoded values to linear values,
>>>> + * represented by this 7-parameter piecewise function:
>>>> + *
>>>> + *   linear = sign(encoded) *  (c*|encoded| + f)       , 0 <= |encoded| < d
>>>> + *          = sign(encoded) * ((a*|encoded| + b)^g + e), d <= |encoded|  
>>>
>>> The code you have does not actually do the extended form (use of sign
>>> and absolute value), but clamps the result to [0.0, 1.0].
>>>   
>>
>> Yes, the intention (and I didn't write this but copied it) is to be able to use
>> this to describe most (all?) standard transfer functions. See
>> https://github.com/google/skia/blob/main/include/core/SkColorSpace.h
> 
> My complaint is about doc and implementation disagreeing.
> 
> xvYCC and integer-encoded scRGB would need an extended range if anyone
> wanted to test those, if they even fit here. I also do not understand
> how it can be possible to express PQ EOTF or HLG OETF with this
> parameterisation. Maybe it's an approximation on a very limited domain?
> 

I have a feeling Skia maps everything to [0.0, 1.0] and agree that this
wouldn't work in the cases you mention (PQ would work with an additional
scaling of the values to 125.0).

What's here seems to work fine for sRGB but I'll have to hook this all up
to amdgpu and test both sRGB and PQ at the very least.

Harry

> 
> Thanks,
> pq
> 
>>>> + *
>>>> + * (A simple gamma transfer function sets g to gamma and a to 1.)
>>>> + */
>>>> +struct igt_color_tf {
>>>> +    float g, a,b,c,d,e,f;
>>>> +};
>>>> +
>>>> +const struct igt_color_tf srgb_tf = {2.4f, (float)(1/1.055), (float)(0.055/1.055), (float)(1/12.92), 0.04045f, 0, 0};
>>>> +
>>>> +/* end of Skia-based code */
>>>> +
>>>> +typedef struct igt_pixel {
>>>> +	float r;
>>>> +	float g;
>>>> +	float b;
>>>> +} igt_pixel_t;  
>>>
>>> Thanks,
>>> pq  
>>
>