Re: [RFC wayland-protocols] Color management protocol

[Richard Hughes]

On 13 January 2017 at 14:56, Florian Höch  wrote:
> .. also guarantee that nothing else will be shown on that specific output

FWIW, I'd be fine just displaying a color swatch on the entire screen
with no window decorations; the only reason we use a composited window
in gnome-color-manager is so the user knows where the put the sensor.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC wayland-protocols] Color management protocol

[Richard Hughes]

On 20 December 2016 at 07:22, Graeme Gill  wrote:
> Or be prepared to re-visit Wayland's fundamental design decisions
> if they turn out to be based on a false premise.

I don't think that's fair. I think Wayland is the opportunity to upset
the status quo with color management and do correctly what what never
possible with X11.

Lets be honest for a moment. How many applications support color
management on the Linux desktop? We're asking application authors to
understand things like blending spaces, source and destination
profiles, vcgt, overlapping windows on different crtc's and horrible
concepts like that.

As a framework guy, _and_ an app developer I just want to tag one
surface with a colorspace and then let the compositor figure out all
the grotty details. Anything more and the application author will just
decide it's not worth the bother. To calibrate we just ask for a
surface that's not going to be tampered with, but we don't want to
optimize for this super-uncommon case.

Concepts like _ICC_PROFILE are not going to happen in Wayland, and I
really thing a "late" bound color workflow is what Wayland should be
working towards. We can't live in the 1990's any longer.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Fix mutex hang in colord on output removal

[Richard Hughes]

On 15 January 2015 at 14:40, Olivier Fourdan ofour...@redhat.com wrote:
 ocms is taken from the container now (that was the casting error), so no
 need for the lookup from cms as we already have it.

Ohh of course, makes sense.

Signed-off-by: Richard Hughes rich...@hughsie.com

Pekka, can you pull this into master please. Thanks!

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Fix mutex hang in colord on output removal

[Richard Hughes]

On 8 January 2015 at 14:40, Olivier Fourdan ofour...@redhat.com wrote:
 This is because of a cast error, the type of container should be
 cms_output and not cms_colord.

Good catch!

 -   ocms = g_hash_table_lookup(cms-devices, device_id);
 -   if (!ocms) {
 -   weston_log(colord: failed to delete device\n);
 -   goto out;
 -   }

How come this is removed too? I think I'd rather it stayed so we show
a warning rather than explode in a ball of fire.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 6/6] image demo client: Add support for color management

[Richard Hughes]

On 13 October 2014 18:40, Niels Ole Salscheider
niels_...@salscheider-online.de wrote:
 - Attach a false-color ICC profile to the surface
 +const unsigned char icc_profile[798] =

Hey, looks really good. Are you using a channel-swapped test profile?
If so, there are example profiles in
/usr/share/color/icc/colord/SwappedRedAndGreen.icc that might be
easier than embedding it. Up to you.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC PATCH 1/6] Add colorcorrection protocol

[Richard Hughes]

On 31 March 2014 08:46, Pekka Paalanen ppaala...@gmail.com wrote:
 how much data can an ICC profile be?

Printer profiles can be several megabytes in size, but display
profiles (what will be seen here) are usually in the 20-30kb size
range.

 Also, what if a client has several surfaces all with the same ICC
 profile, would it not be useful to have some notion of re-using an
 already sent and parsed color profile? Otherwise I would imagine lots
 of overhead if every surface has a private copy of the profile
 sent over the wire, parsed, and stored in the compositor's renderer.

I don't think typically they'll be many different profiles in use, on
a typical system most things will just be (assumed) sRGB to 'n'
display profiles, where n is the number of outputs.

 Is that a reasonable requirement for all compositors that support
 per-output ICC profiles?

I think it's a very little amount of work, IMO doing it centrally
makes a lot of sense as some bits are tricky to do correctly.

Overall, I'm very happy to see someone pick up this work. Thanks.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [RFC PATCH 1/6] Add colorcorrection protocol

[Richard Hughes]

On 31 March 2014 12:13, Kai-Uwe Behrmann ku.b-l...@gmx.de wrote:
 In many cases a client is not remotely able to compute per output
 regions without much information about scaling, positioning, (warping?)

To explain a little here, different outputs need different ICC
profiles (and thus a different color table) that usually have
different primaries to adapt to. I have a wide gamut DreamColor
attached to my low-gamut T510 laptop. Putting this logic in the
compositor makes it two order of magnitude simpler when you have more
than one output attached.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Wayland and Weston 1.4 alpha snapshot (1.3.91) available

[Richard Hughes]

On 17 December 2013 08:17, Kristian Høgsberg hoegsb...@gmail.com wrote:
 We made it to Dec 16th and it's time to put out an alpha release and
 start winding down the feature work

Awesome news, but a few little typos:
http://wayland.freedesktop.org/releases.html


The 1.3.91 version of Wayland and Weston was released on December
16th, 2013. This release is an alpha release, for 1.4 Release
announcement.
wayland-1.3.0.tar.xz
weston-1.3.0.tar.xz


You want to s/1.3.0/1.3.91 in both the displayed text and the
hyperlink. It's also more correct to say that they were released
(rather than was, which implies just one tarball) -- but now I'm
just being picky :)

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: weston never calls drmModeCrtcSetGamma() by default

[Richard Hughes]

On 2 December 2013 03:24, Michel Dänzer mic...@daenzer.net wrote:
 No colour management is fine, but a 16bpp gamma ramp (let alone an 8bpp
 palette) just doesn't look very good in 32bpp. :)

If you can tell me how to adjust the color depth of the console I can
probably rustle up a patch to just set a linear ramp at startup.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Wayland and Weston 1.2.0 released

[Richard Hughes]

On 13 July 2013 07:13, Kristian Høgsberg hoegsb...@gmail.com wrote:
 We have a 1.2.0 release of Wayland and Weston:

Awesome news. Builds now available for Fedora Rawhide and Fedora 19.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Wayland color management protocol proposal

[Richard Hughes]

On 19 June 2013 03:52, John Kåre Alsaker john.kare.alsa...@gmail.com wrote:
 Here is my current color management protocol for Wayland clients. The
 basic idea is that the compositor sends two color spaces to the
 clients.

If we back up a little first, what's the problem we're trying to solve?

* We already have the ICC profile filename from the CMS plugin for the
output that sets the calibration sate
* We want to convert the pixels on a surface from assumed-sRGB to the
chosen display profile
* We want there to be no conversion if the display profile is not set
or if the display profile is set to sRGB
* We want to be able to poke out regions of a surface for specific
users, e.g. if a video player or image editor is already doing CM
itself, or if a calibration tool is running that needs to output
colors unmolested
* We perhaps want to mark out regions of a surface that have a
predictable or well known color space, to offload conversion from my
Nikon D60 camera profile to the output space on the GPU, rather than
the CPU.
* Ideally we want to composite the surfaces together using linear
light where possible

I think addressing these in small chunks will let us define the
protocol to be as simple as possible. I'm also keen on keeping the
workflow ICC-only, trying to do anything else will lead to madness.

Also, as a small point, we can't just ignore the VCGT tag as it forms
part of the calibration state, and the characterisation is only valid
for that state. I agree it makes sense to produce profiles without a
VCGT when we have full screen color correction on most hardware, but
until then we have to honor them.

I think what it makes most sense for Raphael to start on is to take
the ICC profile filename as specified in the CMS plugin, and do the
assumed-sRGB - output transform in a GPU shader. That's quite a task
in itself, and trying to think about everything all at once and
talking protocol changes is bewildering for my little head, let alone
someone new to the project :)

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 1/2] cms-colord: Fix build after the API change 'Honor XDG_CONFIG_DIRS'

[Richard Hughes]

---
 src/cms-colord.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cms-colord.c b/src/cms-colord.c
index 33f23b2..af6b5fa 100644
--- a/src/cms-colord.c
+++ b/src/cms-colord.c
@@ -478,7 +478,7 @@ colord_cms_output_destroy(gpointer data)
 
 WL_EXPORT int
 module_init(struct weston_compositor *ec,
-   int *argc, char *argv[], const char *config_file)
+   int *argc, char *argv[])
 {
gboolean ret;
GError *error = NULL;
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 2/2] cms-colord: Warn if reading or writing to the FD failed

[Richard Hughes]

This also fixes a compile warning when building the tarball.
---
 src/cms-colord.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/cms-colord.c b/src/cms-colord.c
index af6b5fa..6056407 100644
--- a/src/cms-colord.c
+++ b/src/cms-colord.c
@@ -127,6 +127,7 @@ static void
 update_device_with_profile_in_idle(struct cms_output *ocms)
 {
gboolean signal_write = FALSE;
+   ssize_t rc;
struct cms_colord *cms = ocms-cms;
 
colord_idle_cancel_for_output(cms, ocms-o);
@@ -139,7 +140,9 @@ update_device_with_profile_in_idle(struct cms_output *ocms)
/* signal we've got updates to do */
if (signal_write) {
gchar tmp = '\0';
-   write(cms-writefd, tmp, 1);
+   rc = write(cms-writefd, tmp, 1);
+   if (rc == 0)
+   weston_log(colord: failed to write to pending fd);
}
 }
 
@@ -365,6 +368,7 @@ colord_dispatch_all_pending(int fd, uint32_t mask, void 
*data)
 {
gchar tmp;
GList *l;
+   ssize_t rc;
struct cms_colord *cms = data;
struct cms_output *ocms;
 
@@ -387,7 +391,9 @@ colord_dispatch_all_pending(int fd, uint32_t mask, void 
*data)
g_mutex_unlock(cms-pending_mutex);
 
/* done */
-   read(cms-readfd, tmp, 1);
+   rc = read(cms-readfd, tmp, 1);
+   if (rc == 0)
+   weston_log(colord: failed to read from pending fd);
return 1;
 }
 
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] Add a colord implementation of a CMS plugin for weston

[Richard Hughes]

This allows users to change the assigned display profile in GNOME (using
gnome-control-center) or KDE (using colord-kde) and also allows the profiling
tools to correctly inhibit the calibration state whilst measuring the native
screen response.
---
 configure.ac |  10 +
 src/Makefile.am  |  11 ++
 src/cms-colord.c | 550 +++
 weston.ini   |   2 +-
 4 files changed, 572 insertions(+), 1 deletion(-)
 create mode 100644 src/cms-colord.c

diff --git a/configure.ac b/configure.ac
index b71038d..109afee 100644
--- a/configure.ac
+++ b/configure.ac
@@ -277,6 +277,16 @@ AC_ARG_ENABLE(tablet-shell,
 AM_CONDITIONAL(ENABLE_TABLET_SHELL,
   test x$enable_tablet_shell = xyes)
 
+# CMS modules
+AC_ARG_ENABLE(colord,
+  AS_HELP_STRING([--disable-colord],
+ [do not build colord CMS support]),,
+ enable_colord=yes)
+AM_CONDITIONAL(ENABLE_COLORD,
+  test x$enable_colord = xyes)
+if test x$enable_colord = xyes; then
+  PKG_CHECK_MODULES(COLORD, colord = 0.1.8)
+fi
 
 AC_ARG_ENABLE(wcap-tools, [  --disable-wcap-tools],, enable_wcap_tools=yes)
 AM_CONDITIONAL(BUILD_WCAP_TOOLS, test x$enable_wcap_tools = xyes)
diff --git a/src/Makefile.am b/src/Makefile.am
index 92d9e38..2d71e48 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -95,6 +95,7 @@ module_LTLIBRARIES =  \
$(desktop_shell)\
$(tablet_shell) \
$(cms_static)   \
+   $(cms_colord)   \
$(x11_backend)  \
$(drm_backend)  \
$(wayland_backend)  \
@@ -261,6 +262,16 @@ cms_static_la_SOURCES =\
cms-static.c\
cms-helper.c\
cms-helper.h
+if ENABLE_COLORD
+cms_colord = cms-colord.la
+cms_colord_la_LDFLAGS = -module -avoid-version
+cms_colord_la_LIBADD = $(COMPOSITOR_LIBS) $(COLORD_LIBS)
+cms_colord_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(COLORD_CFLAGS)
+cms_colord_la_SOURCES =\
+   cms-colord.c\
+   cms-helper.c\
+   cms-helper.h
+endif
 endif
 
 BUILT_SOURCES =\
diff --git a/src/cms-colord.c b/src/cms-colord.c
new file mode 100644
index 000..33f23b2
--- /dev/null
+++ b/src/cms-colord.c
@@ -0,0 +1,550 @@
+/*
+ * 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 fcntl.h
+#include unistd.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include colord.h
+
+#include compositor.h
+#include cms-helper.h
+
+struct cms_colord {
+   struct weston_compositor*ec;
+   CdClient*client;
+   GHashTable  *devices; /* key = device-id, value = 
cms_output */
+   GHashTable  *pnp_ids; /* key = pnp-id, value = 
vendor */
+   gchar   *pnp_ids_data;
+   GMainLoop   *loop;
+   GThread *thread;
+   GList   *pending;
+   GMutex   pending_mutex;
+   struct wl_event_source  *source;
+   int  readfd;
+   int  writefd;
+   struct wl_listener   destroy_listener;
+   struct wl_listener   output_created_listener;
+};
+
+struct cms_output {
+   CdDevice*device;
+   guint32

Re: [PATCH 2/2] Move the EDID parsing to its own file

[Richard Hughes]

On 4 May 2013 00:16, Kai-Uwe Behrmann k...@gmx.de wrote:
 +struct weston_edid_color_Yxy {
 +   double Y;
 +   double x;
 +   double y;
 +};
 Why is the Y value set when it is all about primaries. No one will ever use
 that other than for assuming 1.0 .

I assumed a standard type would be more useful than a custom type. If
it stays as Yxy, then colord can use it without having to do a:

var2-Y = 1.0;
var2-x = var1-x;
var2-y = var1-y;

...every time.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] Add initial color management framework code

[Richard Hughes]

On 3 May 2013 11:53, Pekka Paalanen ppaala...@gmail.com wrote:
 + o-set_gamma(o, size, red, red, red);
 Three times red? :-)

Good catch, thanks.

 + p = calloc(sizeof(struct weston_color_profile), 1);
 Calloc arguments are swapped.

All fixed.

 + weston_cms_destroy_profile(output-color_profile);
 Was this call supposed to be replaced by an output destroy signal
 listener? IIRC there were some such comments before.

I think krh meant the cms plugin should listen for the devices being
created and destroyed. I think the color_profile structure is small
enough not to need the complexity of a callback, and I'm not sure we
want to teach profiles about devices if you see what I mean. It's
really not much different to calling any of the other functions that
deallocate per-output state. The CMS plugin implements all of the
signals we talked about.

New patch attached. Thanks for the review.

Richard


0001-Add-initial-color-management-framework-code.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 5/5] Move the optional output name property from drm_output to weston_output

[Richard Hughes]

In the future the CMS plugins will need to read the config file and setup a list
of hardcoded names to ICC profiles.
---
 src/compositor-drm.c | 11 ---
 src/compositor.c |  1 +
 src/compositor.h |  1 +
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index db4db33..b6a7ae4 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -146,7 +146,6 @@ struct drm_edid {
 struct drm_output {
struct weston_output   base;
 
-   char *name;
uint32_t crtc_id;
int pipe;
uint32_t connector_id;
@@ -1073,7 +1072,6 @@ drm_output_destroy(struct weston_output *output_base)
weston_output_destroy(output-base);
wl_list_remove(output-base.link);
 
-   free(output-name);
free(output);
 }
 
@@ -1696,7 +1694,7 @@ create_output_for_connector(struct drm_compositor *ec,
else
type_name = UNKNOWN;
snprintf(name, 32, %s%d, type_name, connector-connector_type_id);
-   output-name = strdup(name);
+   output-base.name = strdup(name);
 
output-crtc_id = resources-crtcs[i];
output-pipe = i;
@@ -1731,7 +1729,7 @@ create_output_for_connector(struct drm_compositor *ec,
configured = NULL;
 
wl_list_for_each(temp, configured_output_list, link) {
-   if (strcmp(temp-name, output-name) == 0) {
+   if (strcmp(temp-name, output-base.name) == 0) {
if (temp-mode)
weston_log(%s mode \%s\ in config\n,
temp-name, temp-mode);
@@ -1785,7 +1783,7 @@ create_output_for_connector(struct drm_compositor *ec,
output-base.current = current-base;
 
if (output-base.current == NULL) {
-   weston_log(no available modes for %s\n, output-name);
+   weston_log(no available modes for %s\n, output-base.name);
goto err_free;
}
 
@@ -1835,7 +1833,7 @@ create_output_for_connector(struct drm_compositor *ec,
  ec-base.primary_plane);
 
weston_log(Output %s, (connector %d, crtc %d)\n,
-  output-name, output-connector_id, output-crtc_id);
+  output-base.name, output-connector_id, output-crtc_id);
wl_list_for_each(m, output-base.mode_list, link)
weston_log_continue(  mode %dx%d@%.1f%s%s%s\n,
m-width, m-height, m-refresh / 1000.0,
@@ -1860,7 +1858,6 @@ err_free:
drmModeFreeCrtc(output-original_crtc);
ec-crtc_allocator = ~(1  output-crtc_id);
ec-connector_allocator = ~(1  output-connector_id);
-   free(output-name);
free(output);
 
return -1;
diff --git a/src/compositor.c b/src/compositor.c
index c1acd50..a6610e6 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -2908,6 +2908,7 @@ weston_output_destroy(struct weston_output *output)
 
wl_signal_emit(output-destroy_signal, output);
 
+   free(output-name);
pixman_region32_fini(output-region);
pixman_region32_fini(output-previous_damage);
output-compositor-output_id_pool = ~(1  output-id);
diff --git a/src/compositor.h b/src/compositor.h
index 63d1127..7da6c48 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -156,6 +156,7 @@ enum dpms_enum {
 
 struct weston_output {
uint32_t id;
+   char *name;
 
void *renderer_state;
 
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 5/5] Move the optional output name property from drm_output to weston_output

[Richard Hughes]

On 2 May 2013 13:49, Kai-Uwe Behrmann k...@gmx.de wrote:
 Why will come a need for a CMS to read hardcoded names from the config file?

Well, colord won't, but not everybody wants/needs colord, e.g. on an
embedded display where a color profile is never likely to change. For
this case I've added a cms-static plugin[1] that reads the weston.conf
config file like this:

[output]
name=LVDS1
icc_profile=/usr/share/color/icc/colord/Bluish.icc

So the static CMS plugin needs the LVDS1 name so it can check the
weston_output when the created callback gets called. Note, in the
colord plugin we also set the LVDS1-type name as metadata on the
device, although it's not required.

Richard.

[1] krh didn't want this directly in the compositor-drm file, which makes sense.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH] Add initial color management framework code

[Richard Hughes]

ICC profiles can now be specified in weston.ini for each output, or a CMS
implementation can optionally loaded from a pluggable module.
---
 configure.ac |   7 ++
 src/Makefile.am  |  13 +++-
 src/cms-static.c | 211 +++
 src/cms.c| 143 ++
 src/cms.h|  65 
 src/compositor-drm.c |   2 +
 src/compositor.c |   2 +
 src/compositor.h |  11 +++
 weston.ini   |   3 +-
 9 files changed, 454 insertions(+), 3 deletions(-)
 create mode 100644 src/cms-static.c
 create mode 100644 src/cms.c
 create mode 100644 src/cms.h

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..4ff1dec 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,8 +9,8 @@ AM_CPPFLAGS =   \
-DIN_WESTON
 
 weston_LDFLAGS = -export-dynamic
-weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
-weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
+weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) 
$(LCMS_CFLAGS)
+weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) $(LCMS_LIBS) \
$(DLOPEN_LIBS) -lm ../shared/libshared.la
 
 weston_SOURCES =   \
@@ -20,6 +20,8 @@ weston_SOURCES =  \
compositor.h\
filter.c\
filter.h\
+   cms.c   \
+   cms.h   \
screenshooter.c \
screenshooter-protocol.c\
screenshooter-server-protocol.h \
@@ -94,6 +96,7 @@ moduledir = $(libdir)/weston
 module_LTLIBRARIES =   \
$(desktop_shell)\
$(tablet_shell) \
+   $(cms_static)   \
$(x11_backend)  \
$(drm_backend)  \
$(wayland_backend)  \
@@ -251,6 +254,12 @@ tablet_shell_la_SOURCES =  \
tablet-shell-server-protocol.h
 endif
 
+cms_static = cms-static.la
+cms_static_la_LDFLAGS = -module -avoid-version
+cms_static_la_LIBADD = $(COMPOSITOR_LIBS) ../shared/libshared.la
+cms_static_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
+cms_static_la_SOURCES = cms-static.c
+
 BUILT_SOURCES =\
screenshooter-server-protocol.h \
screenshooter-protocol.c\
diff --git a/src/cms-static.c b/src/cms-static.c
new file mode 100644
index 000..69671a3
--- /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.h
+
+struct cms_static {
+   struct weston_compositor*ec;
+   struct wl_listener   destroy_listener;
+   struct wl_listener

Re: [PATCH] Parse the color management parts of the EDID

[Richard Hughes]

edid.[c|h] is actually what I had initially, want me to do something
like that? Something like a:

struct edid_info;
int edid_parse(struct edid_info *info, const uint8_t *data, size_t length);

Richard

On 2 May 2013 21:57, John Kåre Alsaker john.kare.alsa...@gmail.com wrote:
 EDID parsing should probably be moved out of compositor-drm since other
 backends can provide EDIDs too.


 On Thu, May 2, 2013 at 10:38 PM, Richard Hughes hughsi...@gmail.com wrote:

 This code was originally written by Soren Sandmann sandm...@redhat.com
 and was
 found here: http://people.gnome.org/~ssp/randr/edid-parse.c

 The code has been in use in gnome-settings-daemon for a couple of years
 now and
 is used to generate an Auto-EDID ICC profile if the screen has not been
 profiled
 with a calibration device.

 This will be used by the CMS plugins in the future, and so it makes sense
 being
 common code in compositor-drm.
 ---
  src/compositor-drm.c | 56
 
  1 file changed, 56 insertions(+)

 diff --git a/src/compositor-drm.c b/src/compositor-drm.c
 index f39096e..e4a1919 100644
 --- a/src/compositor-drm.c
 +++ b/src/compositor-drm.c
 @@ -136,11 +136,22 @@ struct drm_fb {
 void *map;
  };

 +struct drm_color_Yxy {
 +   double Y;
 +   double x;
 +   double y;
 +};
 +
  struct drm_edid {
 char eisa_id[13];
 char monitor_name[13];
 char pnp_id[5];
 char serial_number[13];
 +   struct drm_color_Yxy whitepoint;
 +   struct drm_color_Yxy primary_red;
 +   struct drm_color_Yxy primary_green;
 +   struct drm_color_Yxy primary_blue;
 +   double gamma;
  };

  struct drm_output {
 @@ -1558,6 +1569,31 @@ edid_parse_string(const uint8_t *data, char text[])
  #define EDID_OFFSET_SERIAL 0x0c

  static int
 +edid_get_bit(int in, int bit)
 +{
 +   return (in  (1  bit))  bit;
 +}
 +
 +static int
 +edid_get_bits(int in, int begin, int end)
 +{
 +   int mask = (1  (end - begin + 1)) - 1;
 +   return (in  begin)  mask;
 +}
 +
 +static double
 +edid_decode_fraction(int high, int low)
 +{
 +   double result = 0.0;
 +   int i;
 +
 +   high = (high  2) | low;
 +   for (i = 0; i  10; ++i)
 +   result += edid_get_bit(high, i) * pow(2, i - 10);
 +   return result;
 +}
 +
 +static int
  edid_parse(struct drm_edid *edid, const uint8_t *data, size_t length)
  {
 int i;
 @@ -1579,6 +1615,26 @@ edid_parse(struct drm_edid *edid, const uint8_t
 *data, size_t length)
 edid-pnp_id[2] = 'A' + (data[EDID_OFFSET_PNPID + 1]  0x1f) - 1;
 edid-pnp_id[3] = '\0';

 +   /* get native gamma */
 +   if (data[0x17] == 0xff)
 +   edid-gamma = -1.0;
 +   else
 +   edid-gamma = (data[0x17] + 100.0) / 100.0;
 +
 +   /* get primaries and whitepoint */
 +   edid-primary_red.Y = 1.0f;
 +   edid-primary_red.x = edid_decode_fraction(data[0x1b],
 edid_get_bits(data[0x19], 6, 7));
 +   edid-primary_red.y = edid_decode_fraction(data[0x1c],
 edid_get_bits(data[0x19], 5, 4));
 +   edid-primary_green.Y = 1.0f;
 +   edid-primary_green.x = edid_decode_fraction(data[0x1d],
 edid_get_bits(data[0x19], 2, 3));
 +   edid-primary_green.y = edid_decode_fraction(data[0x1e],
 edid_get_bits(data[0x19], 0, 1));
 +   edid-primary_blue.Y = 1.0f;
 +   edid-primary_blue.x = edid_decode_fraction(data[0x1f],
 edid_get_bits(data[0x1a], 6, 7));
 +   edid-primary_blue.y = edid_decode_fraction(data[0x20],
 edid_get_bits(data[0x1a], 4, 5));
 +   edid-whitepoint.Y = 1.0f;
 +   edid-whitepoint.x = edid_decode_fraction(data[0x21],
 edid_get_bits(data[0x1a], 2, 3));
 +   edid-whitepoint.y = edid_decode_fraction(data[0x22],
 edid_get_bits(data[0x1a], 0, 1));
 +
 /* maybe there isn't a ASCII serial number descriptor, so use this
 instead */
 serial_number = (uint32_t) data[EDID_OFFSET_SERIAL + 0];
 serial_number += (uint32_t) data[EDID_OFFSET_SERIAL + 1] * 0x100;
 --
 1.8.2.1

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel


___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 2/2] Move the EDID parsing to its own file

[Richard Hughes]

 weston_output_update_matrix(struct weston_output *output)
 {
float magnification;
diff --git a/src/compositor.h b/src/compositor.h
index 7da6c48..6f15350 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -34,6 +34,7 @@ extern C {
 
 #include version.h
 #include matrix.h
+#include edid.h
 #include config-parser.h
 
 #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
@@ -181,6 +182,7 @@ struct weston_output {
int disable_planes;
 
char *make, *model, *serial_number;
+   struct weston_edid_info edid;
uint32_t subpixel;
uint32_t transform;

@@ -766,6 +768,9 @@ weston_output_update_zoom(struct weston_output *output, 
uint32_t type);
 void
 weston_output_update_matrix(struct weston_output *output);
 void
+weston_output_set_edid(struct weston_output *output,
+  const uint8_t *data, size_t length);
+void
 weston_output_move(struct weston_output *output, int x, int y);
 void
 weston_output_init(struct weston_output *output, struct weston_compositor *c,
diff --git a/src/edid.c b/src/edid.c
new file mode 100644
index 000..f591e74
--- /dev/null
+++ b/src/edid.c
@@ -0,0 +1,175 @@
+/*
+ * 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 ctype.h
+#include math.h
+#include stdint.h
+#include stdio.h
+#include string.h
+#include unistd.h
+
+#include edid.h
+
+static void
+weston_edid_parse_string(const uint8_t *data, char text[])
+{
+   int i;
+   int replaced = 0;
+
+   /* this is always 12 bytes, but we can't guarantee it's null
+* terminated or not junk. */
+   strncpy(text, (const char *) data, 12);
+
+   /* remove insane chars */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (text[i] == '\n' ||
+   text[i] == '\r') {
+   text[i] = '\0';
+   break;
+   }
+   }
+
+   /* ensure string is printable */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (!isprint(text[i])) {
+   text[i] = '-';
+   replaced++;
+   }
+   }
+
+   /* if the string is random junk, ignore the string */
+   if (replaced  4)
+   text[0] = '\0';
+}
+
+#define EDID_DESCRIPTOR_ALPHANUMERIC_DATA_STRING   0xfe
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_NAME   0xfc
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_SERIAL_NUMBER  0xff
+#define EDID_OFFSET_DATA_BLOCKS0x36
+#define EDID_OFFSET_LAST_BLOCK 0x6c
+#define EDID_OFFSET_PNPID  0x08
+#define EDID_OFFSET_SERIAL 0x0c
+
+static int
+edid_get_bit(int in, int bit)
+{
+   return (in  (1  bit))  bit;
+}
+
+static int
+edid_get_bits(int in, int begin, int end)
+{
+   int mask = (1  (end - begin + 1)) - 1;
+   return (in  begin)  mask;
+}
+
+static double
+edid_decode_fraction(int high, int low)
+{
+   double result = 0.0;
+   int i;
+
+   high = (high  2) | low;
+   for (i = 0; i  10; ++i)
+   result += edid_get_bit(high, i) * pow(2, i - 10);
+   return result;
+}
+
+int
+weston_edid_parse(struct weston_edid_info *edid,
+ const uint8_t *data, size_t length)
+{
+   int i;
+   uint32_t serial_number;
+
+   /* check header */
+   if (length  128)
+   return -1;
+   if (data[0] != 0x00 || data[1] != 0xff)
+   return -1;
+
+   /* decode the PNP ID from three 5 bit words packed into 2 bytes
+* /--08--\/--09--\
+* 7654321076543210
+* |\---/\---/\---/
+* R  C1   C2   C3 */
+   edid-pnp_id[0] = 'A' + ((data[EDID_OFFSET_PNPID + 0]  0x7c) / 4) - 1;
+   edid-pnp_id[1] = 'A' + ((data[EDID_OFFSET_PNPID + 0

[PATCH 2/4] Add a output_created_signal on weston_compositor

[Richard Hughes]

---
 src/compositor.c | 2 ++
 src/compositor.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/src/compositor.c b/src/compositor.c
index 0214eed..c1acd50 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3076,6 +3076,7 @@ weston_output_init(struct weston_output *output, struct 
weston_compositor *c,
output-global =
wl_display_add_global(c-wl_display, wl_output_interface,
  output, bind_output);
+   wl_signal_emit(c-output_created_signal, output);
 }
 
 static void
@@ -3154,6 +3155,7 @@ weston_compositor_init(struct weston_compositor *ec,
wl_signal_init(ec-show_input_panel_signal);
wl_signal_init(ec-hide_input_panel_signal);
wl_signal_init(ec-seat_created_signal);
+   wl_signal_init(ec-output_created_signal);
ec-launcher_sock = weston_environment_get_fd(WESTON_LAUNCHER_SOCK);
 
ec-output_id_pool = 0;
diff --git a/src/compositor.h b/src/compositor.h
index 3b08f29..ce63fc2 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -313,6 +313,7 @@ struct weston_compositor {
struct wl_signal hide_input_panel_signal;
 
struct wl_signal seat_created_signal;
+   struct wl_signal output_created_signal;
 
struct wl_event_loop *input_loop;
struct wl_event_source *input_loop_source;
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 3/4] Add a set_gamma vfunc on weston_output

[Richard Hughes]

---
 src/compositor-drm.c | 24 
 src/compositor.h |  7 +++
 2 files changed, 31 insertions(+)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index c8016cd..857aeed 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -545,6 +545,27 @@ drm_output_render(struct drm_output *output, 
pixman_region32_t *damage)
 }
 
 static void
+drm_output_set_gamma(struct weston_output *output_base,
+uint16_t size, uint16_t *r, uint16_t *g, uint16_t *b)
+{
+   int rc;
+   struct drm_output *output = (struct drm_output *) output_base;
+   struct drm_compositor *compositor = (struct drm_compositor *) 
output-base.compositor;
+
+   /* check */
+   if (output_base-gamma_size != size)
+   return;
+   if (!output-original_crtc)
+   return;
+
+   rc = drmModeCrtcSetGamma(compositor-drm.fd,
+output-crtc_id,
+size, r, g, b);
+   if (rc)
+   weston_log(set gamma failed: %m\n);
+}
+
+static void
 drm_output_repaint(struct weston_output *output_base,
   pixman_region32_t *damage)
 {
@@ -1799,6 +1820,9 @@ create_output_for_connector(struct drm_compositor *ec,
output-base.set_dpms = drm_set_dpms;
output-base.switch_mode = drm_output_switch_mode;
 
+   output-base.gamma_size = output-original_crtc-gamma_size;
+   output-base.set_gamma = drm_output_set_gamma;
+
weston_plane_init(output-cursor_plane, 0, 0);
weston_plane_init(output-fb_plane, 0, 0);
 
diff --git a/src/compositor.h b/src/compositor.h
index ce63fc2..63d1127 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -198,6 +198,13 @@ struct weston_output {
uint32_t backlight_current;
void (*set_backlight)(struct weston_output *output, uint32_t value);
void (*set_dpms)(struct weston_output *output, enum dpms_enum level);
+
+   uint16_t gamma_size;
+   void (*set_gamma)(struct weston_output *output,
+ uint16_t size,
+ uint16_t *r,
+ uint16_t *g,
+ uint16_t *b);
 };
 
 struct weston_xkb_info {
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 4/4] Include config.h in compositor-drm.c

[Richard Hughes]

---
 src/compositor-drm.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 857aeed..db4db33 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -21,6 +21,10 @@
  * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
 #define _GNU_SOURCE
 
 #include errno.h
-- 
1.8.2.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 2/3] Add a 'serial' property on weston_output

[Richard Hughes]

On 24 April 2013 12:42, Emilio Pozuelo Monfort poch...@gmail.com wrote:
 This is exposed in the weston SDK (compositor.h is a public header). This 
 patch
 breaks the ABI, but I don't think we have promised to keep it stable for now.

I didn't know we offered any king of ABI/API stability in Weston. If
we do, we probably need to make quite a few items on that struct
private and add getters to avoid pain in the future.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCHv3 0/3] Combined EDID parsing and CMS patchset

[Richard Hughes]

These three patches are the combined work of the EDID feature and
the CMS feature. With all three patches applied to master output devices are
registered with colord. Color correction and profiling works when using
clients such as colord-kde or gnome-color-manager.

In the future, this framework will allow us to do per-output color transforms
to automatically convert from a defined color space to the output space.

All suggestions and recommendations have been implemented from past reviews.

Richard Hughes (3):
  Extract and parse the EDID when outputs are added
  Add initial color management framework code
  Add a colord implementation of a CMS plugin for weston

 configure.ac |  17 ++
 src/Makefile.am  |  15 +-
 src/cms.c| 106 +
 src/cms.h|  84 ++
 src/colord.c | 440 +++
 src/compositor-drm.c | 296 +-
 src/compositor.c |   7 +
 src/compositor.h |  23 ++-
 weston.ini   |   2 +
 9 files changed, 985 insertions(+), 5 deletions(-)
 create mode 100644 src/cms.c
 create mode 100644 src/cms.h
 create mode 100644 src/colord.c

-- 
1.8.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 1/3] Extract and parse the EDID when outputs are added

[Richard Hughes]

At the moment we're only extracting interesting strings. We have to be quite
careful parsing the EDID data, as vendors like to do insane things.

The original EDID parsing code was written by me for gnome-color-manager.
---
 src/compositor-drm.c | 149 +++
 src/compositor.h |   2 +-
 2 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index da1ba79..c8016cd 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -25,6 +25,7 @@
 
 #include errno.h
 #include stdlib.h
+#include ctype.h
 #include string.h
 #include fcntl.h
 #include unistd.h
@@ -131,6 +132,13 @@ struct drm_fb {
void *map;
 };
 
+struct drm_edid {
+   char eisa_id[13];
+   char monitor_name[13];
+   char pnp_id[5];
+   char serial_number[13];
+};
+
 struct drm_output {
struct weston_output   base;
 
@@ -139,6 +147,7 @@ struct drm_output {
int pipe;
uint32_t connector_id;
drmModeCrtcPtr original_crtc;
+   struct drm_edid edid;
 
int vblank_pending;
int page_flip_pending;
@@ -1486,6 +1495,143 @@ drm_output_fini_pixman(struct drm_output *output)
}
 }
 
+static void
+edid_parse_string(const uint8_t *data, char text[])
+{
+   int i;
+   int replaced = 0;
+
+   /* this is always 12 bytes, but we can't guarantee it's null
+* terminated or not junk. */
+   strncpy(text, (const char *) data, 12);
+
+   /* remove insane chars */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (text[i] == '\n' ||
+   text[i] == '\r') {
+   text[i] = '\0';
+   break;
+   }
+   }
+
+   /* ensure string is printable */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (!isprint(text[i])) {
+   text[i] = '-';
+   replaced++;
+   }
+   }
+
+   /* if the string is random junk, ignore the string */
+   if (replaced  4)
+   text[0] = '\0';
+}
+
+#define EDID_DESCRIPTOR_ALPHANUMERIC_DATA_STRING   0xfe
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_NAME   0xfc
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_SERIAL_NUMBER  0xff
+#define EDID_OFFSET_DATA_BLOCKS0x36
+#define EDID_OFFSET_LAST_BLOCK 0x6c
+#define EDID_OFFSET_PNPID  0x08
+#define EDID_OFFSET_SERIAL 0x0c
+
+static int
+edid_parse(struct drm_edid *edid, const uint8_t *data, size_t length)
+{
+   int i;
+   uint32_t serial_number;
+
+   /* check header */
+   if (length  128)
+   return -1;
+   if (data[0] != 0x00 || data[1] != 0xff)
+   return -1;
+
+   /* decode the PNP ID from three 5 bit words packed into 2 bytes
+* /--08--\/--09--\
+* 7654321076543210
+* |\---/\---/\---/
+* R  C1   C2   C3 */
+   edid-pnp_id[0] = 'A' + ((data[EDID_OFFSET_PNPID + 0]  0x7c) / 4) - 1;
+   edid-pnp_id[1] = 'A' + ((data[EDID_OFFSET_PNPID + 0]  0x3) * 8) + 
((data[EDID_OFFSET_PNPID + 1]  0xe0) / 32) - 1;
+   edid-pnp_id[2] = 'A' + (data[EDID_OFFSET_PNPID + 1]  0x1f) - 1;
+   edid-pnp_id[3] = '\0';
+
+   /* maybe there isn't a ASCII serial number descriptor, so use this 
instead */
+   serial_number = (uint32_t) data[EDID_OFFSET_SERIAL + 0];
+   serial_number += (uint32_t) data[EDID_OFFSET_SERIAL + 1] * 0x100;
+   serial_number += (uint32_t) data[EDID_OFFSET_SERIAL + 2] * 0x1;
+   serial_number += (uint32_t) data[EDID_OFFSET_SERIAL + 3] * 0x100;
+   if (serial_number  0)
+   sprintf(edid-serial_number, %lu, (unsigned long) 
serial_number);
+
+   /* parse EDID data */
+   for (i = EDID_OFFSET_DATA_BLOCKS;
+i = EDID_OFFSET_LAST_BLOCK;
+i += 18) {
+   /* ignore pixel clock data */
+   if (data[i] != 0)
+   continue;
+   if (data[i+2] != 0)
+   continue;
+
+   /* any useful blocks? */
+   if (data[i+3] == EDID_DESCRIPTOR_DISPLAY_PRODUCT_NAME) {
+   edid_parse_string(data[i+5],
+ edid-monitor_name);
+   } else if (data[i+3] == 
EDID_DESCRIPTOR_DISPLAY_PRODUCT_SERIAL_NUMBER) {
+   edid_parse_string(data[i+5],
+ edid-serial_number);
+   } else if (data[i+3] == 
EDID_DESCRIPTOR_ALPHANUMERIC_DATA_STRING) {
+   edid_parse_string(data[i+5],
+ edid-eisa_id);
+   }
+   }
+   return 0;
+}
+
+static void
+find_and_parse_output_edid(struct drm_compositor *ec,
+  struct drm_output *output,
+  drmModeConnector 

[PATCH 2/3] Add initial color management framework code

[Richard Hughes]

ICC profiles can now be specified in weston.ini for each output, or a CMS
implementation can optionally loaded from a pluggable module.
---
 configure.ac |   7 +++
 src/Makefile.am  |   6 ++-
 src/cms.c| 106 +
 src/cms.h|  84 +
 src/compositor-drm.c | 147 ++-
 src/compositor.c |   7 +++
 src/compositor.h |  21 
 weston.ini   |   2 +
 8 files changed, 376 insertions(+), 4 deletions(-)
 create mode 100644 src/cms.c
 create mode 100644 src/cms.h

diff --git a/configure.ac b/configure.ac
index d5fea9d..1552d73 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..7489086 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -9,8 +9,8 @@ AM_CPPFLAGS =   \
-DIN_WESTON
 
 weston_LDFLAGS = -export-dynamic
-weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
-weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
+weston_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS) 
$(LCMS_CFLAGS)
+weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) $(LCMS_LIBS) \
$(DLOPEN_LIBS) -lm ../shared/libshared.la
 
 weston_SOURCES =   \
@@ -20,6 +20,8 @@ weston_SOURCES =  \
compositor.h\
filter.c\
filter.h\
+   cms.c   \
+   cms.h   \
screenshooter.c \
screenshooter-protocol.c\
screenshooter-server-protocol.h \
diff --git a/src/cms.c b/src/cms.c
new file mode 100644
index 000..1acc680
--- /dev/null
+++ b/src/cms.c
@@ -0,0 +1,106 @@
+/*
+ * 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.h
+
+WL_EXPORT void
+weston_cms_set_color_profile(struct weston_output *o,
+struct weston_color_profile *p)
+{
+   if (o-color_profile == p)
+   return;
+   if (o-color_profile)
+   weston_cms_destroy_profile(o-color_profile);
+   o-color_profile = p;
+   if (o-updated_color_profile)
+   o-updated_color_profile(o);
+}
+
+WL_EXPORT int
+weston_cms_output_added(struct weston_output *o,
+   enum weston_color_manager_flags flags)
+{
+   struct weston_compositor *ec = o-compositor;
+   if (!ec-cms)
+   return 0;
+   if (!ec-cms-output_added)
+   return 0;
+   return ec-cms-output_added(o, flags);
+}
+
+WL_EXPORT int
+weston_cms_output_removed(struct weston_output *o)
+{
+   struct weston_compositor *ec = o-compositor;
+   if (!ec-cms)
+   return 0;
+   if (!ec-cms-output_removed)
+   return 0;
+   return ec-cms-output_removed(o);
+}
+
+WL_EXPORT void
+weston_cms_destroy_profile(struct weston_color_profile *p

[PATCH 3/3] Add a colord implementation of a CMS plugin for weston

[Richard Hughes]

This allows users to change the assigned display profile in GNOME (using
gnome-control-center) or KDE (using colord-kde) and also allows the profiling
tools to correctly inhibit the calibration state whilst measuring the native
screen response.
---
 configure.ac|  10 ++
 src/Makefile.am |   9 ++
 src/colord.c| 440 
 3 files changed, 459 insertions(+)
 create mode 100644 src/colord.c

diff --git a/configure.ac b/configure.ac
index 1552d73..20011d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -277,6 +277,16 @@ AC_ARG_ENABLE(tablet-shell,
 AM_CONDITIONAL(ENABLE_TABLET_SHELL,
   test x$enable_tablet_shell = xyes)
 
+# CMS modules
+AC_ARG_ENABLE(colord,
+  AS_HELP_STRING([--disable-colord],
+ [do not build colord CMS support]),,
+ enable_colord=yes)
+AM_CONDITIONAL(ENABLE_COLORD,
+  test x$enable_colord = xyes)
+if test x$enable_colord = xyes; then
+  PKG_CHECK_MODULES(COLORD, colord = 0.1.8)
+fi
 
 AC_ARG_ENABLE(wcap-tools, [  --disable-wcap-tools],, enable_wcap_tools=yes)
 AM_CONDITIONAL(BUILD_WCAP_TOOLS, test x$enable_wcap_tools = xyes)
diff --git a/src/Makefile.am b/src/Makefile.am
index 7489086..513b446 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -96,6 +96,7 @@ moduledir = $(libdir)/weston
 module_LTLIBRARIES =   \
$(desktop_shell)\
$(tablet_shell) \
+   $(cms_colord)   \
$(x11_backend)  \
$(drm_backend)  \
$(wayland_backend)  \
@@ -253,6 +254,14 @@ tablet_shell_la_SOURCES =  \
tablet-shell-server-protocol.h
 endif
 
+if ENABLE_COLORD
+cms_colord = colord.la
+colord_la_LDFLAGS = -module -avoid-version
+colord_la_LIBADD = $(COMPOSITOR_LIBS) $(COLORD_LIBS)
+colord_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(COLORD_CFLAGS)
+colord_la_SOURCES = colord.c
+endif
+
 BUILT_SOURCES =\
screenshooter-server-protocol.h \
screenshooter-protocol.c\
diff --git a/src/colord.c b/src/colord.c
new file mode 100644
index 000..1a7fce0
--- /dev/null
+++ b/src/colord.c
@@ -0,0 +1,440 @@
+/*
+ * 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 fcntl.h
+#include unistd.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include colord.h
+
+#include compositor.h
+#include cms.h
+
+struct colord_color_manager {
+   struct weston_color_manager  base;
+   CdClient*client;
+   GHashTable  *devices;
+   GMainLoop   *loop;
+   GThread *thread;
+   GList   *pending;
+   GMutex   pending_mutex;
+   struct wl_event_source  *source;
+   int  readfd;
+   int  writefd;
+};
+
+static inline struct colord_color_manager *
+to_colord_cm(struct weston_color_manager *base)
+{
+   return container_of(base, struct colord_color_manager, base);
+}
+
+typedef struct {
+   struct weston_output*o;
+   struct weston_color_profile *p;
+} ColordIdleHelper;
+
+static void
+colord_idle_destroy_helper(ColordIdleHelper *helper)
+{
+   g_slice_free(ColordIdleHelper, helper);
+}
+
+static gint
+colord_idle_find_output_cb(gconstpointer a, gconstpointer b)
+{
+   ColordIdleHelper *helper = (ColordIdleHelper *) a;
+   struct weston_output *o = (struct weston_output *) b;
+   return helper-o == o ? 0

Re: [PATCH 3/3] Parse the EDID when outputs are added

[Richard Hughes]

On 23 Apr 2013 16:36, Rob Bradford robert.bradf...@intel.com wrote:
 Please can you clarify what I think this statement implies. That is that
 you are certifying that you have the rights to license this code under
 the MIT license as carried by Weston.

Correct, i.e. I read the VESA EDID specification document. :)
Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH 2/3] Add a 'serial' property on weston_output

[Richard Hughes]

On 23 April 2013 16:37, Rob Bradford robert.bradf...@intel.com wrote:
 When I read the commit message I wondered why does he need a serial
 for the wl_output. I was of course thinking about the wayland use of
 serial as a sequence number.

Ooops, my bad.

 Are you intending to expose this through the wayland protocol? If so
 have you considered how this ambiguity might be avoided.

serial_number?

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Add simple EDID parsing to Weston

[Richard Hughes]

At the moment weston doesn't know anything about EDID blobs. To make
the CMS functionality complete we have to tell the CM the display
attributes, e.g. make, model and serial. This allows us to show
something nice in the calibration UI, and also allows us to match a
specific _display_ to the color profile, not just the _output_ that
it's sitting on. Getting the serial is important as there might be
more than one monitor of the same type attached which is quite common.

I've attached 3 patches for initial review. They don't depend on my
CMS patchset to not block one on the other. They probably need
detailed review for correctness/coding style as this is code ported
from GNOME.

Comments welcome, thanks.

Richard


0001-Extract-the-EDID-blob-when-adding-a-DRM-output.patch
Description: Binary data


0002-Add-a-serial-property-on-weston_output.patch
Description: Binary data


0003-Parse-the-EDID-when-outputs-are-added.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 1/3] Extract the EDID blob when adding a DRM output

[Richard Hughes]

---
 src/compositor-drm.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index da1ba79..61ef97e 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -139,6 +139,7 @@ struct drm_output {
int pipe;
uint32_t connector_id;
drmModeCrtcPtr original_crtc;
+   drmModePropertyBlobPtr edid_blob;
 
int vblank_pending;
int page_flip_pending;
@@ -1011,6 +1012,9 @@ drm_output_destroy(struct weston_output *output_base)
(struct drm_compositor *) output-base.compositor;
drmModeCrtcPtr origcrtc = output-original_crtc;
 
+   if (output-edid_blob)
+   drmModeFreePropertyBlob(output-edid_blob);
+
if (output-backlight)
backlight_destroy(output-backlight);
 
@@ -1499,6 +1503,7 @@ create_output_for_connector(struct drm_compositor *ec,
drmModeEncoder *encoder;
drmModeModeInfo crtc_mode;
drmModeCrtc *crtc;
+   drmModePropertyPtr property;
int i;
char name[32];
const char *type_name;
@@ -1642,6 +1647,27 @@ create_output_for_connector(struct drm_compositor *ec,
 
wl_list_insert(ec-base.output_list.prev, output-base.link);
 
+   /* find the EDID blob */
+   for (i = 0; i  connector-count_props  !output-edid_blob; i++) {
+   property = drmModeGetProperty(ec-drm.fd, connector-props[i]);
+   if (!property)
+   continue;
+   if ((property-flags  DRM_MODE_PROP_BLOB) 
+   !strcmp(property-name, EDID)) {
+   output-edid_blob = drmModeGetPropertyBlob(ec-drm.fd,
+  
connector-prop_values[i]);
+   }
+   drmModeFreeProperty(property);
+   }
+
+   /* parse the EDID blob */
+   if (output-edid_blob) {
+   weston_log(Got EDID blob %p of size %u.\n,
+  output-edid_blob-data,
+  output-edid_blob-length);
+   /* FIXME: actually parse EDID */
+   }
+
output-base.origin = output-base.current;
output-base.start_repaint_loop = drm_output_start_repaint_loop;
output-base.repaint = drm_output_repaint;
-- 
1.8.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 2/3] Add a 'serial' property on weston_output

[Richard Hughes]

---
 src/compositor-drm.c | 1 +
 src/compositor.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 61ef97e..a454676 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -1522,6 +1522,7 @@ create_output_for_connector(struct drm_compositor *ec,
output-base.subpixel = drm_subpixel_to_wayland(connector-subpixel);
output-base.make = unknown;
output-base.model = unknown;
+   output-base.serial = unknown;
wl_list_init(output-base.mode_list);
 
if (connector-connector_type  ARRAY_LENGTH(connector_type_names))
diff --git a/src/compositor.h b/src/compositor.h
index 1e999a6..fa6162c 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -178,7 +178,7 @@ struct weston_output {
uint32_t frame_time;
int disable_planes;
 
-   char *make, *model;
+   char *make, *model, *serial;
uint32_t subpixel;
uint32_t transform;

-- 
1.8.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH 3/3] Parse the EDID when outputs are added

[Richard Hughes]

At the moment we're only extracting interesting strings. We have to be quite
careful parsing the EDID data, as vendors like to do insane things.

The original EDID parsing code was written by me for gnome-color-manager.
---
 src/compositor-drm.c | 166 ++-
 1 file changed, 165 insertions(+), 1 deletion(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index a454676..4f37447 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -25,6 +25,7 @@
 
 #include errno.h
 #include stdlib.h
+#include ctype.h
 #include string.h
 #include fcntl.h
 #include unistd.h
@@ -131,6 +132,14 @@ struct drm_fb {
void *map;
 };
 
+struct drm_edid {
+   char *monitor_name;
+   char *serial_number;
+   char *eisa_id;
+   char *pnp_id;
+   /* other things can be added as required */
+};
+
 struct drm_output {
struct weston_output   base;
 
@@ -140,6 +149,7 @@ struct drm_output {
uint32_t connector_id;
drmModeCrtcPtr original_crtc;
drmModePropertyBlobPtr edid_blob;
+   struct drm_edid *edid;
 
int vblank_pending;
int page_flip_pending;
@@ -1005,6 +1015,16 @@ static void
 drm_output_fini_pixman(struct drm_output *output);
 
 static void
+drm_edid_destroy(struct drm_edid *edid)
+{
+   free(edid-monitor_name);
+   free(edid-serial_number);
+   free(edid-eisa_id);
+   free(edid-pnp_id);
+   free(edid);
+}
+
+static void
 drm_output_destroy(struct weston_output *output_base)
 {
struct drm_output *output = (struct drm_output *) output_base;
@@ -1014,6 +1034,8 @@ drm_output_destroy(struct weston_output *output_base)
 
if (output-edid_blob)
drmModeFreePropertyBlob(output-edid_blob);
+   if (output-edid)
+   drm_edid_destroy (output-edid);
 
if (output-backlight)
backlight_destroy(output-backlight);
@@ -1490,6 +1512,131 @@ drm_output_fini_pixman(struct drm_output *output)
}
 }
 
+static char *
+edid_parse_string (const uint8_t *data)
+{
+   char *text;
+   int i;
+   int replaced = 0;
+
+   /* this is always 12 bytes, but we can't guarantee it's null
+* terminated or not junk. */
+   text = strndup ((const char *) data, 12);
+
+   /* remove insane chars */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (text[i] == '\n' ||
+   text[i] == '\r')
+   text[i] = '\0';
+   }
+
+   /* nothing left? */
+   if (text[0] == '\0') {
+   free (text);
+   text = NULL;
+   goto out;
+   }
+
+   /* ensure string is printable */
+   for (i = 0; text[i] != '\0'; i++) {
+   if (!isprint (text[i])) {
+   text[i] = '-';
+   replaced++;
+   }
+   }
+
+   /* if the string is random junk, ignore the string */
+   if (replaced  4) {
+   free (text);
+   text = NULL;
+   goto out;
+   }
+out:
+   return text;
+}
+
+#define EDID_DESCRIPTOR_ALPHANUMERIC_DATA_STRING   0xfe
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_NAME   0xfc
+#define EDID_DESCRIPTOR_DISPLAY_PRODUCT_SERIAL_NUMBER  0xff
+#define EDID_OFFSET_DATA_BLOCKS0x36
+#define EDID_OFFSET_LAST_BLOCK 0x6c
+#define EDID_OFFSET_PNPID  0x08
+#define EDID_OFFSET_SERIAL 0x0c
+
+static int
+edid_parse (struct drm_edid *edid, const uint8_t *data, size_t length)
+{
+   char *tmp;
+   int i;
+   int rc = 0;
+   uint32_t serial;
+
+   /* check header */
+   if (length  128) {
+   rc = -1;
+   goto out;
+   }
+   if (data[0] != 0x00 || data[1] != 0xff) {
+   rc = -1;
+   goto out;
+   }
+
+   /* decode the PNP ID from three 5 bit words packed into 2 bytes
+* /--08--\/--09--\
+* 7654321076543210
+* |\---/\---/\---/
+* R  C1   C2   C3 */
+   edid-pnp_id = malloc(4);
+   edid-pnp_id[0] = 'A' + ((data[EDID_OFFSET_PNPID+0]  0x7c) / 4) - 1;
+   edid-pnp_id[1] = 'A' + ((data[EDID_OFFSET_PNPID+0]  0x3) * 8) + 
((data[EDID_OFFSET_PNPID+1]  0xe0) / 32) - 1;
+   edid-pnp_id[2] = 'A' + (data[EDID_OFFSET_PNPID+1]  0x1f) - 1;
+   edid-pnp_id[3] = '\0';;
+
+   /* maybe there isn't a ASCII serial number descriptor, so use this 
instead */
+   serial = (int32_t) data[EDID_OFFSET_SERIAL+0];
+   serial += (int32_t) data[EDID_OFFSET_SERIAL+1] * 0x100;
+   serial += (int32_t) data[EDID_OFFSET_SERIAL+2] * 0x1;
+   serial += (int32_t) data[EDID_OFFSET_SERIAL+3] * 0x100;
+   if (serial  0) {
+   edid-serial_number = malloc(9);
+   sprintf (edid-serial_number, %li, (long int) serial);
+   }
+
+   /* parse EDID data */
+   for 

[rebased] Add a color management framework to weston

[Richard Hughes]

I've rebased my cms patches from 1.0.6 to master/1.1.0. Two patches
attached. I've been testing them using gnome-color-manager, but you
can equally do something like:

colormgr device-add-profile
/org/freedesktop/ColorManager/devices/weston_drm_0
/org/freedesktop/ColorManager/profiles/icc_4b416c56cc6c3322a30b6d48548b449f
colormgr device-add-profile
/org/freedesktop/ColorManager/devices/weston_drm_0
/org/freedesktop/ColorManager/profiles/icc_a5bdfcca60078dce523c5acc8d1cbaad

Then, to make the display a test blue color:

colormgr device-make-profile-default
/org/freedesktop/ColorManager/devices/weston_drm_0
/org/freedesktop/ColorManager/profiles/icc_4b416c56cc6c3322a30b6d48548b449f

And to restore the screen back to sRGB with linear gamma ramps:

colormgr device-make-profile-default
/org/freedesktop/ColorManager/devices/weston_drm_0
/org/freedesktop/ColorManager/profiles/icc_a5bdfcca60078dce523c5acc8d1cbaad

Richard.


0001-Add-initial-color-management-framework-code.patch
Description: Binary data


0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [patches] Add a color management framework to weston

[Richard Hughes]

On 5 April 2013 09:53, Richard Hughes hughsi...@gmail.com wrote:
 * GLib really wants to return all signals (colord_device_changed_cb)
 on the main thread, not on the thread that's running the loop.

As discussed on IRC, it seems I was wrong. I've attached both patches
(v6) for review. I've switched the coldplug onto the cms thread, and
switched to using wl_event_loop_add_fd() to batch up the updates.

I've got to move onto other stuff next week, but I'd be really
interested if anyone has inputs on how the sub-surface gamut mapping
using shaders is going to work. The main complication seems to be that
it's per-output, rather than per-surface. I've played with 1bit masks
in the past, but this made the shader quite complicated. Ideas very
welcome. Thanks.

Richard


0001-Add-initial-color-management-framework-code.patch
Description: Binary data


0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [patches] Add a color management framework to weston

[Richard Hughes]

On 5 April 2013 20:23, Thomas Daede daede...@umn.edu wrote:
 Color correction is most important for artists doing work in something
 like the GIMP. Programs like this (as of GIMP 3.0, at least) generally
 work in higher bit depths

For GIMP I was going to just allow the app to opt-out a sub-surface
from any kind of color management. i.e. do it all in the app.

 The compositor should be able to draw N polygons for the N intersections of 
 a surface with each output, each using a different shader?
 You absolutely do not want to put an if based on a pixel value into a glsl 
 shader. It is really slow.

 This is the correct way, I think. Splitting surfaces by monitor also
 has other unrelated benefits, like being able to sync separately to
 each monitor - it probably should go somewhere else.

Right, I'll look into per-output shader code next week.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [patches] Add a color management framework to weston

[Richard Hughes]

On 4 April 2013 07:58, John Kåre Alsaker john.kare.alsa...@gmail.com wrote:
 The weston_color_manager struct could lose the state field

All fixed in the newest patchset, thanks for your detailed review.

 I don't see a GLib event loop, so I'm curious to how the signals work.

As discussed on IRC, I've used a thread for this. If wayland-glib can
do what we need in the future, I'll switch to using that if the dep is
okay.

New patches attached. Comments and further review welcomed as usual. Thanks.

Richard


0001-Add-initial-color-management-framework-code.patch
Description: Binary data


0002-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [patches] Add a color management framework to weston

[Richard Hughes]

On 4 April 2013 19:21, Kai-Uwe Behrmann k...@gmx.de wrote:
 Parsing of ICC profiles inside weston is not nice. That would be better
 abstracted out to lay inside the CMS plugins. Those CMS plugins can link
 against lcms or whatever CMM they prefere.

Well, we could certainly add some abstraction to get 3 allocated gamma
ramps, although I don't think we should aim to abstract everything
without a good reason. It's not likely a different cms will want to
load the vcgt calibration data in a different way after all. It's
basically table data.

 Are you aiming to do spectral imaging in weston?

No, that would be silly.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [patches] Add a color management framework to weston

[Richard Hughes]

On 4 April 2013 19:08, Bill Spitzak spit...@gmail.com wrote:
 Are clients supposed to tell the compositor what color space their buffer
 is, and the compositor then converts to the output's color space as part of
 the compositing step?

At the moment I'm just implementing the calibration state setting,
i.e. setting the gamma ramps from the VCGT data. When we have
sub-surfaces to play with I was going to look at tagging subsurfaces
with different color spaces and setting up shaders to do the color
conversion to display space, but for the moment I was trying to keep
it simple.

Richard.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Setting the display calibration ramps using weston

[Richard Hughes]

On 3 April 2013 13:56, Graeme Gill grae...@argyllcms.com wrote:
 A callback makes sense if this is essentially event driven. How is the
 responder (ie. CMM) registered ?

At the moment I've just got a:

[core]
cms=colord.so

in weston.ini with the .so object setting vfuncs of a
weston_color_manager struct. It's all very CMS neutral at the moment
so it would be trivial to add a ucmm (or whatever) CMM.

 Sounds a reasonable callback mechanism, but you need
 to allow for application override of both calibration
 curves and color matching, to support calibration
 and profiling.

Right, agreed.

 I'd love to see some progress towards (or at least allowance
 for moving in the direction) of an application or CMF being able
 to talk to the display via the DDC for a given output.

I actually played with this in g-c-m and X11, so that I could reset
the you need to recalibrate timer on my HP DreamColor using DDC/CI.
It wasn't possible to do DDC/CI in a thread with X11 for obvious
reasons (DDC being dog slow) but you're right to raise this as a
feature we want with wayland/weston.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[patches] Add a color management framework to weston

[Richard Hughes]

I've attached three patches for comments and review. I'm a GLib
programmer at heart, so please be kind if I've made huge obvious
architectural mistakes :)

Patch 1 adds the framework code, patch 2 adds the ability to load CMS
modules, and patch 3 adds a CMS module written for colord. With those
three patches it actually works, and you can change the applied
profile at runtime using gnome-color-manager or colord-kde. Note, this
patchset just aims to set the calibration state, but I have left the
lcms hProfile handle available for when we want to construct 3d
shaders for the sub-surface color correction. I've also used all the
new features in colord 0.1.32, but say if you need/want me to tone
this down to something Ubuntu/whatever is shipping.

I've left a wodge of documentation in src/cms.h explaining all the
terms and the general idea on how I think this should work. There are
a ton of FIXMEs in relation to getting the EDID data, so ideas welcome
there too. Feedback very welcome, thanks.

Richard.


0003-Add-a-colord-implementation-of-a-CMS-plugin-for-west.patch
Description: Binary data


0002-Add-the-ability-to-load-CMS-implementations-from-plu.patch
Description: Binary data


0001-Add-initial-color-management-framework-code.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Setting the display calibration ramps using weston

[Richard Hughes]

On 2 April 2013 06:45, Graeme Gill grae...@argyllcms.com wrote:
 The display's ICC profile will also be stored in the _ICC_PROFILE atom,
 to make it available to the display client, so that color can be adjusted
 appropriately for that display. Using XRandR it will/should
 also be set in an output property.

There's not going to be _ICC_PROFILE atoms in weston, nor XRandR
output properties, so clients will need to query the CMM directly
(which is a good thing IMO). The grand idea is to use a sub-surface
with a tagged color profile, which means applications don't have to
care about CM unless they want to opt out a region and handle
everything themselves.

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Setting the display calibration ramps using weston

[Richard Hughes]

On 2 April 2013 02:42, Kristian Høgsberg hoegsb...@gmail.com wrote:
 So for something like weston, where we store our configuration in
 weston.ini, I'd expect that we add a config key to set the gamma (is
 this just a number like what xgamma takes? or one for red, green and
 blue?) per output in weston.ini.  So we need to parse that in
 output_section_done() and store in struct drm_configured_output, copy
 the value into struct drm_output in create_output_for_connector() and
 then apply that whenever we call drmModeSetCrtc(), generally.

I've attached a patch that works for me, I'd appreciate any comments
or a formal review. If you've got colord and lcms2 installed and you
uncomment the icc_profile item in weston.ini then the Bluish test
profile will be applied and LVDS will go a deep shade of blue. lcms2
is a totally sane dep IMO, it's used by tons of embedded and desktoppy
things already.

I'm totally up for abstracting this like John suggested, I just need
some more guidance on how you'd like me to do this. For instance, are
the plugins going to be installed by the CMF (e.g. colord installs
/usr//lib/weston/plugins/colord.so), ./configure compile arguments
(e.g. --enable-colord) or can I just link to D-Bus and make it a
runtime dep?

Richard


0001-Add-basic-ICC-color-management-calibration-state-sup.patch
Description: Binary data
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Setting the display calibration ramps using weston

[Richard Hughes]

krh mentioned in a G+ post that I should add to weston functionality
to set the output gamma ramps for color calibration. Does anyone have
any ideas on where I should start? I'm familiar with color stuff, but
the Weston internals confuse me somewhat. I need to be a position
where I can call drmModeCrtcSetGamma() on each output as it is enabled
and also at hotplug add. Any pointers very welcome, thanks!

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Sub-surface protocol

[Richard Hughes]

On 5 December 2012 14:32, Pekka Paalanen ppaala...@gmail.com wrote:
 One of the most important use cases is a video player in a window. It
 has XRGB or ARGB window decorations, usually the video content in YUV,
 and possibly an ARGB overlay for subtitles etc. Currently, the client
 has to color-convert the video, and merge it with the decorations and
 subtitles, before sending the grand ARGB buffer to the compositor.

Sorry to be late to the party. A subsurface idea was what Øyvind and I
were discussing at the OpenICC hackfest when we were talking about
tagging a surface with a known ICC profile. This would allow a toolkit
to declare a window area to be AdobeRGB or some home-created camera
profile from a jpeg that has been tagged with an embedded color
profile.

The use case are image viewers like EOG or editors like GIMP. Gimp
wants to color correct the main preview window converting from source
ICC profile to monitor ICC profile, but we really don't want to turn
off the sRGB-monitor profile for the selector widgets. By using a
subsurface we get the behaviour we want without adding tons of code.
At the moment Oyranos has some kind of xatom protocol to let the
compositor know an opt-out-region and this isn't ideal or suited at
all to wayland.

Anyway, I just wanted to say rock on and maybe to keep ICC color
correction at the back of your mind for the sub-surface. :)

Thanks,

Richard
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel