[PATCH v3 weston 4/5] tests: add test for setting gamma

[Harsha Manjula Mallikarjun (RBEI/ECF3)]

> -----Original Message-----
> From: Daniel Stone [mailto:daniel at fooishbar.org]
> Sent: Saturday, July 21, 2018 5:52 PM
> To: Harsha Manjula Mallikarjun (RBEI/ECF3)
> <Harsha.ManjulaMallikarjun at in.bosch.com>
> Cc: wayland <wayland-devel at lists.freedesktop.org>
> Subject: Re: [PATCH v3 weston 4/5] tests: add test for setting gamma
> 
> Hi Harsha,
> 
> On Thu, 28 Jun 2018 at 14:28, <harsha.manjulamallikarjun at in.bosch.com>
> wrote:
> > +static int
> > +gamma_update_event(void *data)
> > +{
> > +       struct gamma_tobject *obj;
> > +       struct output_obj *op_obj;
> > +       uint16_t *r, *g, *b;
> > +       bool all_tests_complete = true;
> > +
> > +       obj = (struct gamma_tobject *)data;
> > +
> > +       wl_list_for_each(op_obj, &obj->output_list, link) {
> > +               /*compute Gamma lookup table*/
> > +               if (!op_obj->test_complete) {
> 
> As a small style point, please make this 'if (op_obj->test_complete)
> [newline] continue;'. This removes one level of indentation from the
> branch.
> 
> > +                       op_obj->gamma += 0.3;
> > +                       if (op_obj->gamma > 2.1) {
> > +                               op_obj->test_complete = true;
> > +                               /*End the test with gamma set to 1*/
> > +                               op_obj->gamma = 1.0;
> > +                       }
> > +                       r = malloc(sizeof(uint16_t) * op_obj->output->gamma_size *
> 3);
> > +                       g = &r[op_obj->output->gamma_size];
> > +                       b = &g[op_obj->output->gamma_size];
> > +
> > +                       compute_gamma_lut(op_obj->gamma, op_obj->output-
> >gamma_size,
> > +                                                         r, g, b);
> > +
> > +                       op_obj->output->set_gamma(op_obj->output, op_obj-
> >output->gamma_size,
> > +                                       r, g, b);
> > +
> > +                       free(r);
> > +                       all_tests_complete=false;
> > +               }
> > +       }
> 
> So, I understand this would be a manual test where the user would
> visually verify that the gamma is changing in the way they would
> expect? I have no problem with that, but it would be nice to see this
> documented with an example as to what the user should be seeing with
> each step.
An application showing gamma value on screen along with the image would
be very nice I thought. Due to lack of time I did this simple plugin which
just changes Gamma/CTM irrespective of any application that is running. User
has to run an app manually though.
I can document with comments in code what is expected on display, with 
each step increment in CTM and gamma values. Would this be fine?

> 
> Also, lastly, is there a public libweston user for this new API? 
There is a plan to use this from wayland-ivi-extension. Then a demo
app will also come along :-).

> It would be nice to have this be configurable.
I did not get this point. Do you mean that the inclusion of this test would be
configurable during build time? OR this feature of Gamma and CTM itself needs
to included based on configuration? 

> 
> Cheers,
> Daniel

Best Regards,
Harsha