[PATCH] Add initial color management framework code
Kristian Høgsberg
hoegsberg at gmail.com
Fri May 10 10:05:22 PDT 2013
On Fri, May 10, 2013 at 11:03:30AM -0400, Kristian Høgsberg wrote:
> On Thu, May 09, 2013 at 08:31:09PM +0100, Richard Hughes wrote:
> > ICC profiles can now be specified in weston.ini for each output, or a CMS
> > implementation can optionally loaded from a pluggable module.
> > ---
>
> Excellent, thanks for revising. Committed and pushed.
With a few changes... there were a few crashers in here, some of which
I triggered because I happen to have both eDP1 and LVDS output
sections in my weston.ini. We talked about using
wl_list_for_each_safe for destroying the outputs and configured
outputs in irc. Beyond that, there's also a problem with the config
API (and I'll concede that that API is very awkward): after handling a
section_done callback, you have to set the output_name and
output_icc_profile variables back to NULL, otherwise when you get the
next section_done callback you can't reliably test for NULL or not. I
had two configured_outputs get the same icc profile because of this,
which is how we ended up with the double free.
Finally the destroy order of outputs and compositor is a little
counter-intuitive: we emit the compositor destroy signal first and
then the output destroy signals. We probably want to switch that
around, but as it is, iterating cms->output_list from the output
destroy handler fails because cms is alread destroyed here. What I
did instead was to just remove struct cms_output, since we don't use
it for anything. We may want to bring them back so that we can reset
gamma on shutdown (right now I end up with a bluish kms console when I
exit weston), or maybe compositor-drm.c should make a copy of the
current ramp on start up and restore from that on shutdown.
Next, we should fix compositor-drm.c to make a copy of the current
gamma ramps so that it can reset it on vt-enter. Right now, if you vt
switch away to X and back, the gamma ramps are reset.
Kristian
> Kristian
>
> > configure.ac | 7 ++
> > src/Makefile.am | 12 +++
> > src/cms-helper.c | 134 ++++++++++++++++++++++++++++++++
> > src/cms-helper.h | 70 +++++++++++++++++
> > src/cms-static.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > src/compositor-drm.c | 3 +
> > src/compositor.h | 1 +
> > weston.ini | 3 +-
> > 8 files changed, 440 insertions(+), 1 deletion(-)
> > create mode 100644 src/cms-helper.c
> > create mode 100644 src/cms-helper.h
> > create mode 100644 src/cms-static.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index c535b28..b71038d 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -315,6 +315,13 @@ AC_MSG_NOTICE([Weston's native backend: $WESTON_NATIVE_BACKEND])
> > AC_DEFINE_UNQUOTED([WESTON_NATIVE_BACKEND], ["$WESTON_NATIVE_BACKEND"],
> > [The default backend to load, if not wayland nor x11.])
> >
> > +PKG_CHECK_MODULES(LCMS, lcms2,
> > + [have_lcms=yes], [have_lcms=no])
> > +if test "x$have_lcms" = xyes; then
> > + AC_DEFINE(HAVE_LCMS, 1, [Have lcms support])
> > +fi
> > +AM_CONDITIONAL(HAVE_LCMS, [test "x$have_lcms" = xyes])
> > +
> > WAYLAND_SCANNER_RULES(['$(top_srcdir)/protocol'])
> >
> > AC_CONFIG_FILES([Makefile
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index d33ebc5..92d9e38 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -94,6 +94,7 @@ moduledir = $(libdir)/weston
> > module_LTLIBRARIES = \
> > $(desktop_shell) \
> > $(tablet_shell) \
> > + $(cms_static) \
> > $(x11_backend) \
> > $(drm_backend) \
> > $(wayland_backend) \
> > @@ -251,6 +252,17 @@ tablet_shell_la_SOURCES = \
> > tablet-shell-server-protocol.h
> > endif
> >
> > +if HAVE_LCMS
> > +cms_static = cms-static.la
> > +cms_static_la_LDFLAGS = -module -avoid-version
> > +cms_static_la_LIBADD = $(COMPOSITOR_LIBS) $(LCMS_LIBS) ../shared/libshared.la
> > +cms_static_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LCMS_CFLAGS)
> > +cms_static_la_SOURCES = \
> > + cms-static.c \
> > + cms-helper.c \
> > + cms-helper.h
> > +endif
> > +
> > BUILT_SOURCES = \
> > screenshooter-server-protocol.h \
> > screenshooter-protocol.c \
> > diff --git a/src/cms-helper.c b/src/cms-helper.c
> > new file mode 100644
> > index 0000000..2c7b57f
> > --- /dev/null
> > +++ b/src/cms-helper.c
> > @@ -0,0 +1,134 @@
> > +/*
> > + * Copyright © 2013 Richard Hughes
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software and
> > + * its documentation for any purpose is hereby granted without fee, provided
> > + * that the above copyright notice appear in all copies and that both that
> > + * copyright notice and this permission notice appear in supporting
> > + * documentation, and that the name of the copyright holders not be used in
> > + * advertising or publicity pertaining to distribution of the software
> > + * without specific, written prior permission. The copyright holders make
> > + * no representations about the suitability of this software for any
> > + * purpose. It is provided "as is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <stdio.h>
> > +
> > +#ifdef HAVE_LCMS
> > +#include <lcms2.h>
> > +#endif
> > +
> > +#include "compositor.h"
> > +#include "cms-helper.h"
> > +
> > +#ifdef HAVE_LCMS
> > +static void
> > +weston_cms_gamma_clear(struct weston_output *o)
> > +{
> > + int i;
> > + uint16_t *red;
> > +
> > + if (!o->set_gamma)
> > + return;
> > +
> > + red = calloc(o->gamma_size, sizeof(uint16_t));
> > + for (i = 0; i < o->gamma_size; i++)
> > + red[i] = (uint32_t) 0xffff * (uint32_t) i / (uint32_t) (o->gamma_size - 1);
> > + o->set_gamma(o, o->gamma_size, red, red, red);
> > + free(red);
> > +}
> > +#endif
> > +
> > +void
> > +weston_cms_set_color_profile(struct weston_output *o,
> > + struct weston_color_profile *p)
> > +{
> > +#ifdef HAVE_LCMS
> > + cmsFloat32Number in;
> > + const cmsToneCurve **vcgt;
> > + int i;
> > + int size;
> > + uint16_t *red = NULL;
> > + uint16_t *green = NULL;
> > + uint16_t *blue = NULL;
> > +
> > + if (!o->set_gamma)
> > + return;
> > + if (!p) {
> > + weston_cms_gamma_clear(o);
> > + return;
> > + }
> > +
> > + weston_log("Using ICC profile %s\n", p->filename);
> > + vcgt = cmsReadTag (p->lcms_handle, cmsSigVcgtTag);
> > + if (vcgt == NULL || vcgt[0] == NULL) {
> > + weston_cms_gamma_clear(o);
> > + return;
> > + }
> > +
> > + size = o->gamma_size;
> > + red = calloc(size, sizeof(uint16_t));
> > + green = calloc(size, sizeof(uint16_t));
> > + blue = calloc(size, sizeof(uint16_t));
> > + for (i = 0; i < size; i++) {
> > + in = (cmsFloat32Number) i / (cmsFloat32Number) (size - 1);
> > + red[i] = cmsEvalToneCurveFloat(vcgt[0], in) * (double) 0xffff;
> > + green[i] = cmsEvalToneCurveFloat(vcgt[1], in) * (double) 0xffff;
> > + blue[i] = cmsEvalToneCurveFloat(vcgt[2], in) * (double) 0xffff;
> > + }
> > + o->set_gamma(o, size, red, green, blue);
> > + free(red);
> > + free(green);
> > + free(blue);
> > +#endif
> > +}
> > +
> > +void
> > +weston_cms_destroy_profile(struct weston_color_profile *p)
> > +{
> > + if (!p)
> > + return;
> > +#ifdef HAVE_LCMS
> > + cmsCloseProfile(p->lcms_handle);
> > +#endif
> > + free(p->filename);
> > + free(p);
> > +}
> > +
> > +struct weston_color_profile *
> > +weston_cms_create_profile(const char *filename,
> > + void *lcms_profile)
> > +{
> > + struct weston_color_profile *p;
> > + p = calloc(1, sizeof(struct weston_color_profile));
> > + p->filename = strdup(filename);
> > + p->lcms_handle = lcms_profile;
> > + return p;
> > +}
> > +
> > +struct weston_color_profile *
> > +weston_cms_load_profile(const char *filename)
> > +{
> > + struct weston_color_profile *p = NULL;
> > +#ifdef HAVE_LCMS
> > + cmsHPROFILE lcms_profile;
> > + lcms_profile = cmsOpenProfileFromFile(filename, "r");
> > + if (lcms_profile)
> > + p = weston_cms_create_profile(filename, lcms_profile);
> > +#endif
> > + return p;
> > +}
> > diff --git a/src/cms-helper.h b/src/cms-helper.h
> > new file mode 100644
> > index 0000000..a919a42
> > --- /dev/null
> > +++ b/src/cms-helper.h
> > @@ -0,0 +1,70 @@
> > +/*
> > + * Copyright © 2013 Richard Hughes
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software and
> > + * its documentation for any purpose is hereby granted without fee, provided
> > + * that the above copyright notice appear in all copies and that both that
> > + * copyright notice and this permission notice appear in supporting
> > + * documentation, and that the name of the copyright holders not be used in
> > + * advertising or publicity pertaining to distribution of the software
> > + * without specific, written prior permission. The copyright holders make
> > + * no representations about the suitability of this software for any
> > + * purpose. It is provided "as is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#ifndef _WESTON_CMS_H_
> > +#define _WESTON_CMS_H_
> > +
> > +#include "compositor.h"
> > +
> > +/* General overview on how to be a CMS plugin:
> > + *
> > + * First, some nomenclature:
> > + *
> > + * CMF: Color management framework, i.e. "Use foo.icc for device $bar"
> > + * CMM: Color management module that converts pixel colors, which is
> > + * usually lcms2 on any modern OS.
> > + * CMS: Color management system that encompasses both a CMF and CMM.
> > + * ICC: International Color Consortium, the people that define the
> > + * binary encoding of a .icc file.
> > + * VCGT: Video Card Gamma Tag. An Apple extension to the ICC specification
> > + * that allows the calibration state to be stored in the ICC profile
> > + * Output: Physical port with a display attached, e.g. LVDS1
> > + *
> > + * As a CMF is probably something you don't want or need on an embeded install
> > + * these functions will not be called if the icc_profile key is set for a
> > + * specific [output] section in weston.ini
> > + *
> > + * Most desktop environments want the CMF to decide what profile to use in
> > + * different situations, so that displays can be profiled and also so that
> > + * the ICC profiles can be changed at runtime depending on the task or ambient
> > + * environment.
> > + *
> > + * The CMF can be selected using the 'modules' key in the [core] section.
> > + */
> > +
> > +struct weston_color_profile {
> > + char *filename;
> > + void *lcms_handle;
> > +};
> > +
> > +void
> > +weston_cms_set_color_profile(struct weston_output *o,
> > + struct weston_color_profile *p);
> > +struct weston_color_profile *
> > +weston_cms_create_profile(const char *filename,
> > + void *lcms_profile);
> > +struct weston_color_profile *
> > +weston_cms_load_profile(const char *filename);
> > +void
> > +weston_cms_destroy_profile(struct weston_color_profile *p);
> > +
> > +#endif
> > diff --git a/src/cms-static.c b/src/cms-static.c
> > new file mode 100644
> > index 0000000..78d7643
> > --- /dev/null
> > +++ b/src/cms-static.c
> > @@ -0,0 +1,211 @@
> > +/*
> > + * Copyright © 2013 Richard Hughes
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software and
> > + * its documentation for any purpose is hereby granted without fee, provided
> > + * that the above copyright notice appear in all copies and that both that
> > + * copyright notice and this permission notice appear in supporting
> > + * documentation, and that the name of the copyright holders not be used in
> > + * advertising or publicity pertaining to distribution of the software
> > + * without specific, written prior permission. The copyright holders make
> > + * no representations about the suitability of this software for any
> > + * purpose. It is provided "as is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +#include "compositor.h"
> > +#include "cms-helper.h"
> > +
> > +struct cms_static {
> > + struct weston_compositor *ec;
> > + struct wl_listener destroy_listener;
> > + struct wl_listener output_created_listener;
> > + struct wl_list configured_output_list;
> > + struct wl_list output_list;
> > +};
> > +
> > +struct cms_configured_output {
> > + char *icc_profile;
> > + char *name;
> > + struct wl_list link;
> > +};
> > +
> > +struct cms_output {
> > + uint32_t id;
> > + struct cms_static *cms;
> > + struct wl_listener destroy_listener;
> > + struct wl_list link;
> > +};
> > +
> > +static void
> > +cms_output_destroy(struct cms_static *cms, struct weston_output *o)
> > +{
> > + struct cms_output *output;
> > +
> > + weston_log("cms-static: output %i removed\n", o->id);
> > +
> > + /* find profile from configured list */
> > + wl_list_for_each(output, &cms->output_list, link) {
> > + if (o->id == output->id) {
> > + wl_list_remove(&output->link);
> > + free(output);
> > + break;
> > + }
> > + }
> > +}
> > +
> > +static void
> > +cms_notifier_output_destroy(struct wl_listener *listener, void *data)
> > +{
> > + struct cms_output *output = container_of(listener, struct cms_output, destroy_listener);
> > + struct weston_output *o = (struct weston_output *) data;
> > + cms_output_destroy(output->cms, o);
> > +}
> > +
> > +static void
> > +cms_output_created(struct cms_static *cms, struct weston_output *o)
> > +{
> > + struct cms_configured_output *configured_output;
> > + struct cms_output *output;
> > + struct weston_color_profile *p;
> > +
> > + weston_log("cms-static: output %i [%s] created\n", o->id, o->name);
> > +
> > + /* find profile from configured list */
> > + wl_list_for_each(configured_output, &cms->configured_output_list, link) {
> > + if (strcmp (o->name, configured_output->name) == 0) {
> > + weston_log("cms-static: loading %s for %s\n",
> > + configured_output->icc_profile, o->name);
> > + p = weston_cms_load_profile(configured_output->icc_profile);
> > + weston_cms_set_color_profile(o, p);
> > + break;
> > + }
> > + }
> > +
> > + /* setup listener for when the output goes away */
> > + output = malloc(sizeof *output);
> > + memset(output, 0, sizeof *output);
> > + output->cms = cms;
> > + output->id = o->id;
> > + output->destroy_listener.notify = cms_notifier_output_destroy;
> > + wl_signal_add(&o->destroy_signal, &output->destroy_listener);
> > + wl_list_insert(&cms->output_list, &output->link);
> > +}
> > +
> > +static void
> > +cms_notifier_output_created(struct wl_listener *listener, void *data)
> > +{
> > + struct weston_output *o = (struct weston_output *) data;
> > + struct cms_static *cms = container_of(listener, struct cms_static, destroy_listener);
> > + cms_output_created(cms, o);
> > +}
> > +
> > +static void
> > +cms_module_destroy(struct cms_static *cms)
> > +{
> > + struct cms_configured_output *configured_output;
> > + struct cms_output *output;
> > +
> > + wl_list_for_each(output, &cms->output_list, link)
> > + free(output);
> > + wl_list_for_each(configured_output, &cms->configured_output_list, link) {
> > + free(configured_output->name);
> > + free(configured_output->icc_profile);
> > + free(configured_output);
> > + }
> > + free(cms);
> > +}
> > +
> > +static void
> > +cms_notifier_destroy(struct wl_listener *listener, void *data)
> > +{
> > + struct cms_static *cms = container_of(listener, struct cms_static, destroy_listener);
> > + cms_module_destroy(cms);
> > +}
> > +
> > +static char *output_icc_profile;
> > +static char *output_name;
> > +
> > +static void
> > +output_section_done(void *data)
> > +{
> > + struct cms_configured_output *configured_output;
> > + struct cms_static *cms = (struct cms_static *) data;
> > +
> > + if (output_name == NULL || output_icc_profile == NULL) {
> > + free(output_name);
> > + free(output_icc_profile);
> > + return;
> > + }
> > +
> > + weston_log("cms-static: output %s profile configured as %s\n",
> > + output_name, output_icc_profile);
> > +
> > + /* create an object used to store name<->profile data to avoid parsing
> > + * the config file every time a new output is added */
> > + configured_output = malloc(sizeof *configured_output);
> > + memset(configured_output, 0, sizeof *configured_output);
> > + configured_output->name = output_name;
> > + configured_output->icc_profile = output_icc_profile;
> > + wl_list_insert(&cms->configured_output_list, &configured_output->link);
> > +}
> > +
> > +WL_EXPORT int
> > +module_init(struct weston_compositor *ec,
> > + int *argc, char *argv[], const char *config_file)
> > +{
> > + struct cms_static *cms;
> > + struct weston_output *output;
> > +
> > + weston_log("cms-static: initialized\n");
> > +
> > + /* create local state object */
> > + cms = malloc(sizeof *cms);
> > + if (cms == NULL)
> > + return -1;
> > + memset(cms, 0, sizeof *cms);
> > +
> > + wl_list_init(&cms->output_list);
> > + wl_list_init(&cms->configured_output_list);
> > +
> > + /* parse config file */
> > + const struct config_key drm_config_keys[] = {
> > + { "name", CONFIG_KEY_STRING, &output_name },
> > + { "icc_profile", CONFIG_KEY_STRING, &output_icc_profile },
> > + };
> > +
> > + const struct config_section config_section[] = {
> > + { "output", drm_config_keys,
> > + ARRAY_LENGTH(drm_config_keys), output_section_done },
> > + };
> > +
> > + parse_config_file(config_file, config_section,
> > + ARRAY_LENGTH(config_section), cms);
> > +
> > + cms->destroy_listener.notify = cms_notifier_destroy;
> > + wl_signal_add(&ec->destroy_signal, &cms->destroy_listener);
> > +
> > + cms->output_created_listener.notify = cms_notifier_output_created;
> > + wl_signal_add(&ec->output_created_signal, &cms->output_created_listener);
> > +
> > + /* discover outputs */
> > + wl_list_for_each(output, &ec->output_list, link)
> > + cms_output_created(cms, output);
> > +
> > + return 0;
> > +}
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index f39096e..6cf1f76 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -29,6 +29,7 @@
> >
> > #include <errno.h>
> > #include <stdlib.h>
> > +#include <stdlib.h>
> > #include <ctype.h>
> > #include <string.h>
> > #include <fcntl.h>
> > @@ -1812,6 +1813,8 @@ create_output_for_connector(struct drm_compositor *ec,
> > wl_list_insert(ec->base.output_list.prev, &output->base.link);
> >
> > find_and_parse_output_edid(ec, output, connector);
> > + if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS)
> > + output->base.connection_internal = 1;
> >
> > output->base.origin = output->base.current;
> > output->base.start_repaint_loop = drm_output_start_repaint_loop;
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 7da6c48..069a1b4 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -200,6 +200,7 @@ struct weston_output {
> > void (*set_backlight)(struct weston_output *output, uint32_t value);
> > void (*set_dpms)(struct weston_output *output, enum dpms_enum level);
> >
> > + int connection_internal;
> > uint16_t gamma_size;
> > void (*set_gamma)(struct weston_output *output,
> > uint16_t size,
> > diff --git a/weston.ini b/weston.ini
> > index cd34044..49ba526 100644
> > --- a/weston.ini
> > +++ b/weston.ini
> > @@ -1,5 +1,5 @@
> > [core]
> > -#modules=desktop-shell.so,xwayland.so
> > +#modules=desktop-shell.so,xwayland.so,cms-static.so
> >
> > [shell]
> > background-image=/usr/share/backgrounds/gnome/Aqua.jpg
> > @@ -46,6 +46,7 @@ path=/usr/libexec/weston-keyboard
> > #name=LVDS1
> > #mode=1680x1050
> > #transform=90
> > +#icc_profile=/usr/share/color/icc/colord/Bluish.icc
> >
> > #[output]
> > #name=VGA1
> > --
> > 1.8.2.1
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list