[RFC wayland-protocols] Add the color-management protocol

Tue Dec 27 13:27:16 UTC 2016

On Tuesday, 20 December 2016, 14:25:02 CET, Daniel Stone wrote:
> Hi Niels,
> Just nitpicking at the protocol side of things a little bit; please
> excuse what I'm sure is a horrific abuse of the terminology, contained
> in what I'm sure is a series of stupid questions.
> 
> On 19 November 2016 at 16:29, Niels Ole Salscheider
> 
> <niels_ole at salscheider-online.de> wrote:
> > +  <interface name="zwp_color_management_v1" version="1">
> > +    <description summary="allows attaching a color space to a
> > wl_surface">
> > +      This interface allows attaching a color space to a wl_surface. The
> > +      compositor uses this information to display the colors correctly.
> > +      For this, the compositor converts any attached surfaces to the
> > blending +      color space before performing the blending operations.
> > After blending, +      the output surface is converted to the color space
> > of the output device. +      This interface also provides requests for
> > the sRGB and the blending color +      space. It further allows creation
> > of a color space from an ICC profile. +      The client is informed by an
> > event if the color space of one of the +      outputs changes.
> > +    </description>
> > +
> > +    <request name="set_colorspace">
> > +      <description summary="set the color space of a wl_surface">
> > +        With this request, the color space of a wl_surface can be set.
> > +        The sRGB colorspace is attached to a surface before
> > set_colorspace is +        called for the first time.
> > +      </description>
> > +      <arg name="surface" type="object" interface="wl_surface" />
> > +      <arg name="colorspace" type="object" interface="zwp_colorspace_v1"
> > /> +    </request>
> 
> Hm. When does this apply: is it at the next wl_surface::commit, or
> immediately? FWIW, the correct answer for this is the former. ;)

Yes, the former is right. I will make this more explicit.

> Putting explicit language in to this effect would be good; it may also
> be nice to have this new_id a separate per-surface object, so the
> application can then destroy the object to opt out, e.g. if it starts
> embedding content of an unknown colourspace, so the best option is to
> have the compositor convert from sRGB.

Can you explain what exactly you mean by per-surface objects? Do you suggest 
to add a wl_surface argument to the zwp_colorspace_v1 object or how would it 
work?

The reason why I did not do this was because I expect that many (sub)surfaces 
will use the same color profile, e. g. sRGB or the preferred (blending) color 
space of the compositor.
We can avoid the former by defining that no colorspace object implies sRGB. 
But that does not help for the preferred color space that might be used if the 
client does simple color conversions on its own.

> Another good reason to have a per-surface object is so you have
> explicit control: only one per-surface object can be active at any
> time, and that surface is free to call set_colorspace as many times as
> it wants. Without per-surface objects, you can't enforce the same
> 'only one at a time' model.

My idea was that set_colorspace can be called as often as the client wants - 
the new color profile will be used once wl_surface.commit is called and the 
old profile won't have effect anymore from there on.

> > +    <request name="colorspace_from_fd">
> > +      <description summary="creates a color space from an ICC profile">
> > +        This request allows to create a zwp_colorspace_v1 object from an
> > ICC +        profile. The fd argument is the file descriptor to the ICC
> > profile (ICC +        V2 or V4).
> > +      </description>
> > +      <arg name="fd" type="fd" />
> > +      <arg name="id" type="new_id" interface="zwp_colorspace_v1" />
> > +    </request>
> 
> This should probably take length + offset, and note explicitly whether
> it requires NULL termination or not, and how that relates to the size.
> Our experience with xkbcommon was that _not_ NULL-terminating was
> ideal.

I have never seen length and offset arguments in other wayland requests that 
take an fd (do you have an example?). But I can add it if it is useful.

> > +    <request name="srgb_colorspace">
> > +      <description summary="tell the client what blending space is used">
> > +        This request returns a zwp_colorspace_1 object for the sRGB color
> > +        space. The sRGB color space is initially attached to all
> > surfaces.
> > +      </description>
> > +      <arg name="id" type="new_id" interface="zwp_colorspace_v1" />
> > +    </request>
> 
> Why is returning this object useful? The only use I can see is to
> return the surface to the 'unset' state, which could possibly be more
> cleanly handled by destroying a per-surface object. The description is
> also copypasta.

Yes, the use case is to reset the color profile to sRGB. If we switch to per-
surface objects we can remove this.

> > +    <request name="blending_colorspace">
> > +      <description summary="tell the client what blending space is used">
> > +        This request returns a zwp_colorspace_v1 object for the blending
> > color +        space of the compositor. All surfaces are converted by the
> > compositor +        to the blending color space before the blending
> > operations. Once the +        blending is performed, a further color
> > conversion to the output color +        spaces is carried out by the
> > compositor.
> > +        A client should render in the blending color space returned by
> > this +        request if it does any color conversion on its own. This
> > allows the +        compositor to skip the color conversion to the
> > blending color space. +      </description>
> > +      <arg name="id" type="new_id" interface="zwp_colorspace_v1" />
> > +    </request>
> 
> This might be a stupid question, but is the blending colourspace
> constant, or could it feasibly be changed if you had, e.g., outputs
> with different characteristics? I understand that blending in linear
> space is a must, but are there other considerations in play which
> could suggest multiple 'blending spaces' for a single compositor? (I
> would expect that if the blending space was to be constant across
> outputs, then it should also be statically derivable by clients,
> making this event redundant.)

I really intended this to return the preferred input color space of the 
compositor and not necessarily a blending space. I have reworded this.

I could imagine that the preferred input color space might depend on the 
output on which the surface is displayed. But in this case we need a per-
surface preferred color space and an event that notifies the client when this 
changes (e. g. because the surface is displayed on a different output). Such a 
change means that the client has to set up new cLUTs and so on if it does 
color conversions on its own.

I'm not sure if it is worth the effort and added complexity or if we should 
just stick with a static preferred input color space.

> > +    <request name="output_colorspace">
> > +      <description summary="returns the color space for the requested
> > output"> +        This request returns a zwp_colorspace_v1 object for the
> > requested +        output. A client can use this when it does not want
> > its surfaces to be +        color-corrected. In this case it can attach
> > the color space of its main +        output to its surfaces.
> > +      </description>
> > +      <arg name="output" type="object" interface="wl_output" />
> > +      <arg name="id" type="new_id" interface="zwp_colorspace_v1" />
> > +    </request>
> 
> If a client doesn't want its surfaces to be colour-corrected ... won't
> the compositor still convert into blending space and back (not
> necessarily a lossless operation), if blending is required?

My next RFC will avoid this problem with device link profiles.

> > +    <event name="output_colorspace_changed">
> > +      <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. +      </description>
> > +      <arg name="output" type="object" interface="wl_output" />
> > +    </event>
> 
> These three, linked together, are somewhat awkward, but then again, so
> are server-created objects.
> 
> > +  <interface name="zwp_colorspace_v1" version="1">
> > +    <description summary="represents a color space">
> > +      This interface represents a color space that can be attached to
> > surfaces. +      It is used by the zwp_color_management_v1 interface.
> > +    </description>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroys the zwp_colorspace_v1 object">
> > +        Informs the server that the client will not be using this
> > protocol
> > +        object anymore.
> > +      </description>
> > +    </request>
> 
> What happens when destroying a colorspace object which is still
> attached to a surface?

This should probably not be allowed or we define that it falls back to sRGB.

> > +    <request name="get_profile_fd">
> > +      <description summary="get a file descriptor to the profile data">
> > +        This request will cause a profile_fd event that returns a file
> > +        descriptor to the ICC profile data of this colorspace.
> > +      </description>
> > +    </request>
> > +
> > +    <event name="profile_data">
> > +      <description summary="file descriptor to the profile data">
> > +        This event occurs after a get_profile_fd request and returns the
> > file +        descriptor to the ICC profile data of this colorspace.
> > +      </description>
> > +      <arg name="fd" type="fd" />
> > +    </event>
> 
> This also needs length + offset, no?
> 
> Cheers,
> Daniel