Am 31.03.2014 16:10, schrieb Pekka Paalanen: > 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.
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 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
