[RFC PATCH 1/6] Add colorcorrection protocol

Kai-Uwe Behrmann ku.b-list at gmx.de
Mon Mar 31 12:48:28 PDT 2014

Am 31.03.2014 16:10, schrieb Pekka Paalanen:
> On Mon, 31 Mar 2014 11:29:03 +0200
> Kai-Uwe Behrmann <ku.b-list at gmx.de> wrote:
>> Am 31.03.2014 09:46, schrieb Pekka Paalanen:
>>> On Sun, 30 Mar 2014 13:36:32 +0200
>>> Niels Ole Salscheider <niels_ole at salscheider-online.de> wrote:
>>>> The color correction protocol allows to attach an ICC profile to a
>>>> surface. It also tells the client about the blending color space and
>>>> the color spaces of all outputs.
>>>> Signed-off-by: Niels Ole Salscheider <niels_ole at salscheider-online.de>
>>>> ---
>>>>  Makefile.am                  | 15 ++++++--
>>>>  protocol/colorcorrection.xml | 87 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 98 insertions(+), 4 deletions(-)
>>>>  create mode 100644 protocol/colorcorrection.xml
>>>> diff --git a/Makefile.am b/Makefile.am
>>>> index 5ff4f83..ec0a30b 100644
>>>> --- a/Makefile.am
>>>> +++ b/Makefile.am
>>>> @@ -76,7 +76,9 @@ nodist_weston_SOURCES =					\
>>>>  	protocol/workspaces-protocol.c			\
>>>>  	protocol/workspaces-server-protocol.h		\
>>>>  	protocol/scaler-protocol.c			\
>>>> -	protocol/scaler-server-protocol.h
>>>> +	protocol/scaler-server-protocol.h		\
>>>> +	protocol/colorcorrection-protocol.c		\
>>>> +	protocol/colorcorrection-server-protocol.h
>>>>  BUILT_SOURCES += $(nodist_weston_SOURCES)
>>>> @@ -426,7 +428,9 @@ nodist_libtoytoolkit_la_SOURCES =			\
>>>>  	protocol/workspaces-protocol.c			\
>>>>  	protocol/workspaces-client-protocol.h		\
>>>>  	protocol/xdg-shell-protocol.c			\
>>>> -	protocol/xdg-shell-client-protocol.h
>>>> +	protocol/xdg-shell-client-protocol.h		\
>>>> +	protocol/colorcorrection-protocol.c		\
>>>> +	protocol/colorcorrection-client-protocol.h
>>>>  BUILT_SOURCES += $(nodist_libtoytoolkit_la_SOURCES)
>>>> @@ -606,7 +610,9 @@ BUILT_SOURCES +=					\
>>>>  	protocol/workspaces-client-protocol.h		\
>>>>  	protocol/workspaces-protocol.c			\
>>>>  	protocol/xdg-shell-protocol.c			\
>>>> -	protocol/xdg-shell-client-protocol.h
>>>> +	protocol/xdg-shell-client-protocol.h		\
>>>> +	protocol/colorcorrection-protocol.c		\
>>>> +	protocol/colorcorrection-client-protocol.h
>>>>  westondatadir = $(datadir)/weston
>>>> @@ -920,7 +926,8 @@ EXTRA_DIST +=					\
>>>>  	protocol/text-cursor-position.xml	\
>>>>  	protocol/wayland-test.xml		\
>>>>  	protocol/xdg-shell.xml			\
>>>> -	protocol/scaler.xml
>>>> +	protocol/scaler.xml			\
>>>> +	protocol/colorcorrection.xml
>>>>  man_MANS = weston.1 weston.ini.5
>>>> diff --git a/protocol/colorcorrection.xml b/protocol/colorcorrection.xml
>>>> new file mode 100644
>>>> index 0000000..7986e93
>>>> --- /dev/null
>>>> +++ b/protocol/colorcorrection.xml
>>>> @@ -0,0 +1,87 @@
>>>> +<?xml version="1.0" encoding="UTF-8"?>
>>>> +<protocol name="colorcorrection">
>>>> +
>>>> +  <copyright>
>>>> +    Copyright © 2014 Niels Ole Salscheider
>>>> +
>>>> +    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.
>>>> +
>>>> +    THIS SOFTWARE.
>>>> +  </copyright>
>>>> +
>>>> +  <interface name="wl_colorcorrection" version="1">
>>>> +    <description summary="allows to attach a color profile to a wl_surface">
>>>> +      This interface allows to attach a color profile to a wl_surface.
>>>> +      This is used by the compositor to display the colors correctly.
>>>> +      The client is informed by two events about the blending space used
>>>> +      by the compositor and the color spaces of the outputs.
>>>> +    </description>
>>>> +
>>>> +    <request name="set_profile">
>>>> +      <description summary="set the color profile of a wl_surface">
>>>> +	With this request, the color profile of a wl_surface can be set.
>>>> +	When mode is set to "profile", an ICC profile can be passed in the
>>>> +	"profile_data" argument. In all other cases, "profile_data" is
>>>> +	ignored.
>>>> +	"mode" should only be set to "uncorrected" for fullscreen applications
>>>> +	or applications that really require uncorrected output (e. g. color
>>>> +	profiling tools).
>>>> +      </description>
>>>> +      <arg name="surface" type="object" interface="wl_surface"/>
>>>> +      <arg name="mode" type="uint" />
>>>> +      <arg name="profile_data" type="array"/>
>>> Hi,
>>> how much data can an ICC profile be?
>>> I'm wondering whether it makes sense to send it inline in the protocol
>>> stream like this. E.g. we transmit XKB keymaps by sending only an fd,
>>> and then reading the actual data from the fd. If the receiver can mmap
>>> the fd, it could read the data zero-copy, if the profile data in a file
>>> is the very same, i.e. the sender does not need to transform it.
>> Not sure if that can work in every form of session. E.g. a client side
>> in memory generated profile might not be visible to the compositor.
> Sure it can, just like wl_shm buffers work. Yes, the sender may have to
> copy from a some malloc'd piece of memory into a mmap'd file, but
> that's only one copy, compared to the copies with array here: into
> protocol buffer, socket buffer, receiving socket buffer(?), receiving
> protocol buffer, and maybe the 5th time into malloc'd memory.
>> IMO what would make sense here, is to provide additional information
>> about identifying the profile (ICC profile ID - a 16byte binary hash or
>> 32 byte textual) and a mechanism skip or to request the not yet server
>> cached profile from the client.
>> [That could help clients, which want to show a collection of images each
>> one colour corrected but most of the images have the same source profile.]
> That would help for cross-client coalescing, sure. OTOH, if
> transmission is zero-copy, the hashing could be an internal detail of
> the server and would not need to specify it at the protocol level.


>>> 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.
>> Agreed, caching is certainly needed on both ends. The question is,
>> should caching somehow be defined inside the protocol to avoid resending
>> the same data? I think it would be helpful.
> We can do that with just protocol objects, I think. Zoxc in irc had
> some ideas: https://github.com/Zoxc/weston/blob/cms/protocol/cms.xml
> wl_color_space object represents a profile.
>>> If 'mode' is 'blending_space', what does it mean? Explaining 'srgb'
>>> would be good, too, for completeness.
>>> Did we have specified somewhere already, what is the surface's default
>>> 'mode', if this request is never used?
>>>> +    </request>
>>>> +
>>>> +    <event name="blending_space">
>>>> +      <description summary="tell the client what blending space is used">
>>>> +	This event will be sent when the blending space of the compositor
>>>> +	is changed. A client can use this information to output its surface
>>>> +	in the blending space of the compositor so that only one color
>>>> +	transform (from blending to output space) has to be performed by the
>>>> +	compositor. This can result in better performance.
>>>> +      </description>
>>>> +      <arg name="profile_data" type="array"/>
>>> The same comments here about data size and using an fd.
>>>> +    </event>
>>>> +
>>>> +    <event name="output_space">
>>>> +      <description summary="tell the client what color space an output has">
>>>> +	This event will be sent when the color space of an output is changed.
>>>> +	A client can use this information e. g. when it sets the mode to
>>>> +	"uncorrected".
>>>> +      </description>
>>>> +      <arg name="output_id" type="uint"/>
>>> What is this output_id? Isn't there a way we could use the existing
>>> wl_output objects, like send this event for every wl_output instance
>>> the client has created by wl_registry.bind, and also every time when a
>>> client creates a new wl_output object?
>>> That would allow us to use wl_output object as the argument here, while
>>> also guaranteeing that when outputs come and go, and clients bind to
>>> them at random times, they would not miss any information.
>>>> +      <arg name="profile_data" type="array"/>
>>> The same comments here about data size and using an fd.
>>>> +    </event>
>>>> +
>>>> +    <enum name="error">
>>>> +      <entry name="invalid_mode" value="0"
>>>> +	     summary="mode is set to an invalid value"/>
>>>> +      <entry name="invalid_profile" value="1"
>>>> +	     summary="the passed icc data is invalid"/>
>>>> +    </enum>
>>>> +
>>>> +    <enum name="mode">
>>>> +      <entry name="srgb" value="0"/>
>>>> +      <entry name="blending_space" value="1"/>
>>>> +      <entry name="uncorrected" value="2"/>
>>>> +      <entry name="profile" value="3"/>
>>>> +    </enum>
>>>> +  </interface>
>>>> +</protocol>
>>> You asked about double-buffering the color profile/mode.
>>> Double-buffering is needed, if you expect that clients should be able
>>> to change the profile/mode while the surface is mapped. If you do not
>>> double-buffer, then clients cannot flickerlessly change the
>>> profile/mode without unmapping the surface.
>>> In the very old plans, like the sketch I did in
>>> http://people.collabora.com/~pq/wayland-color-management-proposal.png
>>> we had emphasis on clients doing the hard work. This was trying to
>>> reduce the burden on the compositor, while still allowing to implement
>>> high-quality compositors as needed.
>> With per subsurface colour correction things are much more inviting to
>> implementors. That situation is not remotly compareable to the time your
>> early CM effort started. Even though I think it was a very helpful step.
> Sorry, what has changed? Sub-surfaces are just wl_surfaces like any.

To get colour correction to portions of a client window working, a
mechanism like subsurface is ideal to deal with the details. I think
that a per texture color shader is much simpler than multiple shaders
per texture. Why would a client want different colour conversions
applied to one window? Toolbars, icons and so on are most likely exposed
in a different colour space (sRGB) like image (AdobeRGB) or video
content (Rec709). Telling a toolkit to attach one profile to a image
widget, that can be passed internally as subsurface to the compositor
appears pretty easy.

> I you refer to my diagram, that was just a sketch, not even an
> effort. :-)
>> [ I had quite some trouble to implement the client side per window
>> concept in CompICC / KWin and stalled it immediatly after reading about
>> the subsurface plans. Given that very few people code at all on colour
>> management compositor stuff, IMO it makes sense to do put the work in
>> one place and lessen the reuirements to compositor implementors.]
> What do you mean? On the Wayland protocol level, I see no difference
> between a single-surface window and a sub-surface wrt. color management.

It is awesome that for Wayland is so few difference for single-surface
compared to sub-surface color management. On clients side coding it
makes a rather big difference.

Compositing into a single surface on client side is not straight
forward. A blending colour space must be decided about. The icons,
possibly overlays etc. must be rendered into a offline buffer and then
blended into one texture for sending as window to the display
compositor. I experimented with that in one project. But the necessary
logic makes the code looking complicated.

Contrary sub-surface work like per region profiles. It is very easy once
the toolkit maps each image widget with a ICC profile to a sub-surface.

> Were you trying to add some notion of "this sub-region is this color
> space, the outside of it is other color space" kind of stuff?

That is the most common case for clients. Video playback with overlays +
menue: Firefox, VLC. Image editor with side bar: Krita, Inkscape. Most
colour managed image display applications have different colour spaces
in one window.

>>> Compared to that, I see your protocol adds the option for clients to
>>> provide a custom ICC profile to the server, and expects the server to
>>> process it properly.
>> ... a no brainer for compositors.
> Somehow when we were talking about color management a long time ago, I
> understood that there are so many (wrong) ways to implement it, that it
> is better left for the color-aware clients or libraries to do it
> properly, so they get exactly what they want. Was that a
> misunderstanding, or maybe I just remember wrong?

You remember correctly. Niels solved this by adding a "uncorrected" mode:

+ <enum name="mode">
+ <entry name="srgb" value="0"/>
+ <entry name="blending_space" value="1"/>
+ <entry name="uncorrected" value="2"/>
+ <entry name="profile" value="3"/>
+ </enum>

That allows expert clients to do *the* right thing, being it profilers,
advanced colour managed applications with e.g. print simulation
capabilities etc. without interference of later undesired colour management.

> Thanks,
> pq

kind regards

>>> Is that a reasonable requirement for all compositors that support
>>> per-output ICC profiles?
>> CM with per subsurface shaders is much easier to implement for
>> compositors. Compared to that the code paths without subsurfaces is
>> considerable in CompICC and KWin/KolorManager.
>>> Or, should we perhaps have a way for a compositor to advertise the
>>> ability to use custom, per-surface ICC profiles separately? So that
>>> clients would not use them, if the compositor does not want to do that
>>> work.
>> Such a protocol is helpful for extending over the most basic
>> capabilites. Oyranos use it in several parts of the stack to react to
>> divergine colour server feature sets.
>>> Your proposal also adds the explicit notion of the blending space into
>>> the protocol. Should that be somehow optional? What if the compositor
>>> blends in whatever like sRGB, should it still send a complete profile
>>> describing it? Should we have special shorthands for sRGB and "current
>>> output's profile"? Would "linear" be one, too, or is "linear" ambiguous?
>> linear + sRGB primaries is non ambiguous for floating point buffers.
>>> Thanks,
>>> pq
>> Niels, thanks for you work.
>> kind regards
>> Kai-Uwe

More information about the wayland-devel mailing list