[RFC PATCH 1/6] Add colorcorrection protocol

[Kai-Uwe Behrmann]

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.
>> +
>> +    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.
>> +  </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.

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.]

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

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

[ 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.]

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

> 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