On Mon, 31 Mar 2014 11:29:03 +0200 Kai-Uwe Behrmann <[email protected]> wrote:
> Am 31.03.2014 09:46, schrieb Pekka Paalanen: > > On Sun, 30 Mar 2014 13:36:32 +0200 > > Niels Ole Salscheider <[email protected]> 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 <[email protected]> > >> --- > >> 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. > > 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. 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. 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? > > 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? Thanks, pq > > 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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
