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

Submitted by Harsha Manjula Mallikarjun (RBEI/ECF3) on June 28, 2018, 1:27 p.m.

Details

Message ID 1530192480-23334-5-git-send-email-harsha.manjulamallikarjun@in.bosch.com
State Changes Requested
Headers show
Series "Implement support for drm properties "GAMMA_LUT" and "CTM"" ( rev: 3 ) in Wayland

Commit Message

Harsha Manjula Mallikarjun (RBEI/ECF3) June 28, 2018, 1:27 p.m.
From: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>

v3:
--Terminate the compositor after test completion to fix stuck
  problem during make check
--Do not launch client binary as it is optional for the test.

Signed-off-by: Harsha M M <harsha.manjulamallikarjun@in.bosch.com>
---
 Makefile.am        |   7 +-
 tests/gamma-test.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 252 insertions(+), 1 deletion(-)
 create mode 100644 tests/gamma-test.c

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 551df2b..6b1c560 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1238,7 +1238,8 @@  shared_tests =					\
 module_tests =					\
 	plugin-registry-test.la			\
 	surface-test.la				\
-	surface-global-test.la
+	surface-global-test.la		\
+	gamma-test.la
 
 weston_tests =					\
 	bad_buffer.weston			\
@@ -1376,6 +1377,10 @@  nodist_libtest_client_la_SOURCES =				\
 libtest_client_la_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS) $(CAIRO_CFLAGS)
 libtest_client_la_LIBADD = libshared.la libtest-runner.la $(TEST_CLIENT_LIBS) $(CAIRO_LIBS)
 
+gamma_test_la_SOURCES = tests/gamma-test.c
+gamma_test_la_LIBADD = $(test_module_libadd)
+gamma_test_la_LDFLAGS = $(test_module_ldflags)
+gamma_test_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
 
 #
 # Internal tests - tests functionality of the testsuite itself
diff --git a/tests/gamma-test.c b/tests/gamma-test.c
new file mode 100644
index 0000000..d4782ff
--- /dev/null
+++ b/tests/gamma-test.c
@@ -0,0 +1,246 @@ 
+/*
+ * Copyright © 2018 Advanced Driver Information Technology, GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+#include "config.h"
+
+#include <stdint.h>
+#include <unistd.h>
+#include <signal.h>
+#include <string.h>
+#include <assert.h>
+
+#include "compositor.h"
+#include "compositor/weston.h"
+#include "shared/helpers.h"
+#include <wayland-server.h>
+
+struct gamma_tobject {
+	struct weston_compositor *w_compositor;
+	struct wl_listener output_add_listener;
+	struct wl_listener compositor_destroy;
+	struct wl_display *display;
+	struct wl_event_source *gamma_update_timer;
+	struct wl_list output_list;
+};
+
+struct output_obj {
+	struct weston_output *output;
+	struct wl_listener output_remove_listener;
+	float gamma;
+	bool test_complete;
+
+	struct wl_list link;
+};
+
+
+/* gamma change interval in milli seconds*/
+#define GAMMA_UPDATE_INTERVAL 1000
+#define COLOR_COMPONENT_DEPTH 16
+#define GET_GAMMA_COEFF(color, max_color, exp) pow((double)(color) /          \
+						   (double)(max_color),       \
+						   ((double)1.0 / (double)exp))
+
+static void
+compute_gamma_lut(float gamma, uint16_t gamma_sz,
+		uint16_t *r, uint16_t *g, uint16_t *b)
+{
+	double r_coeff;
+	double g_coeff;
+	double b_coeff;
+	uint32_t r_value;
+	uint32_t b_value;
+	uint32_t g_value;
+	uint32_t l_value = 0;
+	uint32_t max_value = (((uint32_t)1 << COLOR_COMPONENT_DEPTH) - 1);
+	uint16_t l_index;
+	uint16_t step_size;
+	uint16_t step_fraction;
+
+	step_size = max_value / (gamma_sz - 1);
+	step_fraction = max_value % (gamma_sz - 1);
+
+	for (l_index = 0; l_index < gamma_sz; l_index++)
+	{
+		/* Compute integral part and closest round up for contribution
+		* from fractional part */
+		l_value = l_index * step_size +
+                  (((l_index * step_fraction) + ((gamma_sz - 1) >> 1)) /
+                   (gamma_sz - 1));
+		r_coeff = GET_GAMMA_COEFF(l_value, max_value, 1.0/gamma);
+		g_coeff = GET_GAMMA_COEFF(l_value, max_value, 1.0/gamma);
+		b_coeff = GET_GAMMA_COEFF(l_value, max_value, 1.0/gamma);
+
+		r_value = (uint32_t)((r_coeff * (double)max_value) + (double)0.5);
+		g_value = (uint32_t)((g_coeff * (double)max_value) + (double)0.5);
+		b_value = (uint32_t)((b_coeff * (double)max_value) + (double)0.5);
+
+		r[l_index] = r_value;
+		g[l_index] = g_value;
+		b[l_index] = b_value;
+	}
+
+}
+
+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) {
+			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;
+		}
+	}
+
+	if (all_tests_complete) {
+		if (obj->gamma_update_timer) {
+			wl_event_source_remove(obj->gamma_update_timer);
+			obj->gamma_update_timer = NULL;
+		}
+		wl_display_terminate(obj->display);
+	}
+	else
+		wl_event_source_timer_update(obj->gamma_update_timer,
+					     GAMMA_UPDATE_INTERVAL);
+
+	return 0;
+}
+
+
+static
+void output_remove(struct wl_listener *listener, void *data)
+{
+	struct output_obj *op_obj;
+	op_obj = container_of(listener, struct output_obj,
+			output_remove_listener);
+	wl_list_remove(&op_obj->link);
+	free(op_obj);
+}
+
+
+static void
+output_add_gamma_test_loop(struct gamma_tobject *obj,
+			   struct weston_output *output)
+{
+	struct wl_event_loop *loop;
+	struct output_obj *op_obj = zalloc(sizeof(struct output_obj));
+
+	if (output->set_gamma && (output->gamma_size > 0)) {
+		op_obj->output_remove_listener.notify = output_remove;
+		wl_signal_add(&output->destroy_signal,
+			      &op_obj->output_remove_listener);
+		wl_list_insert(&obj->output_list, &op_obj->link);
+		/* Initialize with a number which is not multiple of
+		 * step size (0.3). This is to avoid divide by zero */
+		op_obj->gamma = -2.2;
+		op_obj->test_complete = false;
+		op_obj->output = output;
+	}
+
+	if (NULL == obj->gamma_update_timer) {
+		loop = wl_display_get_event_loop(obj->display);
+		obj->gamma_update_timer =
+			wl_event_loop_add_timer(loop,
+						gamma_update_event,
+						obj);
+		wl_event_source_timer_update(obj->gamma_update_timer,
+						     GAMMA_UPDATE_INTERVAL);
+	}
+}
+
+static
+void output_add(struct wl_listener *listener, void *data)
+{
+	struct weston_output *output = (struct weston_output *)data;
+	struct gamma_tobject *obj = container_of(listener, struct gamma_tobject,
+				output_add_listener);
+
+	output_add_gamma_test_loop(obj, output);
+}
+
+
+static
+void wet_compositor_destroy(struct wl_listener *listener, void *data)
+{
+	struct gamma_tobject *obj;
+	struct output_obj *op_obj;
+	struct output_obj *tmp;
+	obj = container_of(listener, struct gamma_tobject,
+			compositor_destroy);
+
+	wl_list_for_each_safe(op_obj, tmp, &obj->output_list, link) {
+		wl_list_remove(&op_obj->output_remove_listener.link);
+		wl_list_remove(&op_obj->link);
+		free(op_obj);
+	}
+
+	if (obj->gamma_update_timer)
+		wl_event_source_remove(obj->gamma_update_timer);
+
+	free(obj);
+}
+
+WL_EXPORT int
+wet_module_init(struct weston_compositor *compositor, int *argc, char *argv[])
+{
+	struct gamma_tobject *obj = zalloc(sizeof(struct gamma_tobject));
+	struct weston_output *output;
+
+	obj->output_add_listener.notify = output_add;
+	obj->w_compositor = compositor;
+	obj->compositor_destroy.notify = wet_compositor_destroy;
+	wl_signal_add(&compositor->destroy_signal, &obj->compositor_destroy);
+	wl_signal_add(&compositor->output_created_signal, &obj->output_add_listener);
+
+	wl_list_init(&obj->output_list);
+	obj->display = compositor->wl_display;
+
+	wl_list_for_each(output, &compositor->output_list, link)
+		output_add_gamma_test_loop(obj, output);
+
+	return 0;
+}

Comments

Hi Harsha,

On Thu, 28 Jun 2018 at 14:28, <harsha.manjulamallikarjun@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.

Also, lastly, is there a public libweston user for this new API? It
would be nice to have this be configurable.

Cheers,
Daniel
> -----Original Message-----

> From: Daniel Stone [mailto:daniel@fooishbar.org]

> Sent: Saturday, July 21, 2018 5:52 PM

> To: Harsha Manjula Mallikarjun (RBEI/ECF3)

> <Harsha.ManjulaMallikarjun@in.bosch.com>

> Cc: wayland <wayland-devel@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@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
Hi Harsha,

On Tue, 24 Jul 2018 at 10:11, Harsha Manjula Mallikarjun (RBEI/ECF3)
<Harsha.ManjulaMallikarjun@in.bosch.com> wrote:
> > 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?

That would be great, thankyou!

> > 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?

Ah no - always building the support is definitely the right thing to do!

What I meant was some support for loading the configuration of the
degamma LUT + CTM + gamma LUT, either from weston.ini, or udev
properties, or some other file. This way people could calibrate and
then statically configure systems to have the correct values when they
are not required to change dynamically.

Cheers,
Daniel