[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.
>>>> +
>>>> + 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.
> 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.
true
>>> 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
Kai-Uwe
>>> 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