[RFC wayland-protocols v2 1/1] unstable: add color management protocol

Simon Ser contact at emersion.fr
Tue Mar 12 18:30:25 UTC 2019


Hi,

Here are a few comments, from a protocol design perspective.

On Wednesday, March 6, 2019 6:09 PM, Sebastian Wick <sebastian at sebastianwick.net> wrote:
> This protocol allows clients to attach a color space and rendering
> intent to a wl_surface. It further allows the client to be informed
> which color spaces a wl_surface was converted to on the last presented.
>
> Signed-off-by: Sebastian Wick <sebastian at sebastianwick.net>
> ---
>  Makefile.am                                   |   1 +
>  unstable/color-management/README              |   4 +
>  .../color-management-unstable-v1.xml          | 189 ++++++++++++++++++
>  3 files changed, 194 insertions(+)
>  create mode 100644 unstable/color-management/README
>  create mode 100644 unstable/color-management/color-management-unstable-v1.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 345ae6a..80abc1d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,7 @@ unstable_protocols =								\
>  	unstable/xdg-decoration/xdg-decoration-unstable-v1.xml	\
>  	unstable/linux-explicit-synchronization/linux-explicit-synchronization-unstable-v1.xml \
>  	unstable/primary-selection/primary-selection-unstable-v1.xml		\
> +	unstable/color-management/color-management-unstable-v1.xml		\
>  	$(NULL)
>
>  stable_protocols =								\
> diff --git a/unstable/color-management/README b/unstable/color-management/README
> new file mode 100644
> index 0000000..140f1e9
> --- /dev/null
> +++ b/unstable/color-management/README
> @@ -0,0 +1,4 @@
> +Color management protocol
> +
> +Maintainers:
> +Sebastian Wick <sebastian at sebastianwick.net>
> diff --git a/unstable/color-management/color-management-unstable-v1.xml b/unstable/color-management/color-management-unstable-v1.xml
> new file mode 100644
> index 0000000..47dd453
> --- /dev/null
> +++ b/unstable/color-management/color-management-unstable-v1.xml
> @@ -0,0 +1,189 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="color_management_unstable_v1">
> +
> +  <copyright>
> +    Copyright © 2019 Sebastian Wick
> +    Copyright © 2019 Erwin Burema
> +
> +    Permission is hereby granted, free of charge, to any person obtaining a
> +    copy of this software and associated documentation files (the "Software"),
> +    to deal in the Software without restriction, including without limitation
> +    the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +    and/or sell copies of the Software, and to permit persons to whom the
> +    Software is furnished to do so, subject to the following conditions:
> +
> +    The above copyright notice and this permission notice (including the next
> +    paragraph) shall be included in all copies or substantial portions of the
> +    Software.
> +
> +    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> +    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> +    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> +    DEALINGS IN THE SOFTWARE.
> +  </copyright>
> +
> +  <description summary="color management protocol">
> +    This protocol provides the ability to specify the color space
> +    of a wl_surface. If further enumerates the color spaces currently

Typo: "It"

> +    in the system and allows to query feedback about which color spaces
> +    a wl_surface was converted to on the last present.
> +    The idea behind the feedback system is to allow the client to do color
> +    conversion to a color space which will likely be used to show the surface
> +    which allows the compositor to skip the color conversion step.

Wording: the "which" repetition could be avoided by splitting this into two
sentences ("This would allow the compositor…").

> +  </description>
> +
> +  <interface name="zwp_color_manager_v1" version="1">
> +    <description summary="color manager">
> +      The color manager is a singleton global object that provides access
> +      to outputs' color spaces, let's you create new color spaces and attach

Typo: "lets"

> +      a color space to surfaces.
> +    </description>
> +
> +    <enum name="render_intent">
> +      <description summary="render intent">
> +        render intents

This could probably be expanded to at least be a sentence.

> +      </description>
> +      <entry name="absolute" value="1" summary="ICC-absolute colorimetric"/>
> +      <entry name="relative" value="2" summary="media-relative colorimetric"/>
> +      <entry name="perceptual" value="3" summary="perceptual"/>
> +      <entry name="saturation" value="4" summary="saturation"/>
> +    </enum>
> +
> +    <enum name="well_known_color_space">
> +      <description summary="well-known color spaces">
> +        Well-known color spaces
> +      </description>
> +      <entry name="none" value="0" summary="deviceRGB"/>
> +      <entry name="sRGB" value="1" summary="sRGB IEC61966-2.1"/>
> +      <entry name="opRGB" value="2" summary="opRGB IEC 61966-2-5"/>
> +      <entry name="DCI_P3D65" value="3" summary="DCI-P3D65"/>
> +      <entry name="DCI_P3DCI" value="4" summary="DCI-P3DCI (Theater)"/>
> +      <entry name="DCI_P3D60" value="5" summary="DCI-P3D60 (ACES Cinema)"/>
> +      <entry name="display_P3" value="6" summary="Display P3"/>
> +      <entry name="rec2020" value="7" summary="ITU-R BT.2020 for UHDTV"/>
> +      <entry name="rec2020_pq" value="8" summary="ITU-R BT.2020 for UHDTV with pq curve (HDR)"/>
> +      <entry name="rec2020_hlg" value="9" summary="ITU-R BT.2020 for UHDTV with HLG curve (HDR)"/>
> +      <entry name="rec709" value="10" summary="ITU-R BT.709 for HDTV"/>
> +    </enum>
> +
> +    <enum name="error">
> +      <description summary="fatal color manager errors">
> +        These fatal protocol errors may be emitted in response to
> +        illegal color management requests.
> +      </description>
> +      <entry name="invalid_icc_profile" value="0" summary="invalid ICC profile"/>
> +    </enum>
> +
> +    <request name="set_color_space">
> +      <description summary="set the color space of a surface">
> +        Set the color space of a surface. The color space is double buffered,
> +        and will be applied at the time wl_surface.commit of the corresponding
> +        wl_surface is called.

Wording: "and will be applied on the next wl_surface.commit"

> +        The render intent gives the compositor a hint what to optimize for
> +        in color space conversions.
> +
> +        If a surface has no color space set, sRGB and an arbitrary render
> +        intent will be assumed.
> +      </description>
> +      <arg name="surface" type="object" interface="wl_surface"/>
> +      <arg name="color_space" type="object" interface="zwp_color_space_v1"/>
> +      <arg name="render_intent" type="uint" enum="render_intent"/>
> +    </request>
> +
> +    <request name="color_space_feedback">
> +      <description summary="color space feedback">
> +        Request color space feedback for the current content submission
> +        on the given surface. This creates a new color_space_feedback
> +        object, which will deliver the feedback information once. If
> +        multiple color_space_feedback objects are created for the same
> +        submission, they will all deliver the same information.
> +
> +        For details on what information is returned, see the
> +        color_space_feedback interface.
> +      </description>
> +      <arg name="surface" type="object" interface="wl_surface"/>
> +      <arg name="feedback" type="new_id" interface="zwp_color_space_feedback_v1"/>
> +    </request>
> +
> +    <request name="color_space_from_icc">

Maybe this should be named "create_color_space_from_icc"?

> +      <description summary="create a color space from an ICC profile">
> +        Create a color space from an ICC profile. Only three channel
> +        display profiles are allowed.
> +      </description>
> +      <arg name="color_space" type="new_id" interface="zwp_color_space_v1"/>
> +      <arg name="icc" type="fd"/>
> +    </request>
> +
> +    <event name="color_space_added">

Generally events that create objects are just called with the name of the object
("color_space").

This uses server-side created resources. This should be done with care, see:
https://ppaalanen.blogspot.com/2014/07/wayland-protocol-design-object-lifespan.html

A way to prevent races is to add:

- A "stop" request that asks the compositor to stop sending events and destroy
  the object
- A "finished" event that notifies the client that the compositor has destroyed
  the object. The client will then be able to release resources.

> +      <description summary="color space added">
> +        Send after binding or when a new color space is added to the system.
> +      </description>
> +      <arg name="color_space" type="new_id" interface="zwp_color_space_v1"/>
> +    </event>
> +  </interface>
> +
> +  <interface name="zwp_color_space_v1" version="1">
> +    <description summary="a color space">
> +      Given two color spaces and a rendering intent it's possible to
> +      convert an image from one to the other.
> +      A color space can be attached to a wl_surface and sends
> +      information like the ICC profile to let the client perform color
> +      correction.
> +    </description>
> +
> +    <event name="information">
> +      <description summary="color space information">
> +        Information describing the color space. Send after binding.
> +
> +        The icc argument contains a fd to the corresponding ICC profile.
> +        well-known can be used to easily identify a color-space.
> +
> +        A color space can be associated with a wl_output.
> +      </description>
> +      <arg name="icc" type="fd"/>

Is this supposed to be mmap'ed? Or can this be a pipe?

> +      <arg name="well_known" type="uint" enum="well_known_color_space"/>
> +      <arg name="output" type="object" interface="wl_output" allow-null="true"/>

Could the color space be used on multiple outputs? If so, it may be better to
add an "output" event that can be sent zero, one or several times.

Even if there can be only a single output at most, a separate event may be
better.

> +    </event>
> +
> +    <event name="removed">
> +      <description summary="color space removed">
> +        This event is sent when the color space is removed from the system.
> +        When this event is received, the client must destroy the object.
> +      </description>
> +    </event>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the color space object">
> +	      Destroy the color_space object.
> +      </description>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_color_space_feedback_v1" version="1">
> +    <description summary="color space feedback">
> +      The surface content was converted to a number of color spaces
> +      on the last content update. They get listed in decreasing order
> +      of importance by the converted event.
> +      Once a color_space_feedback object has delivered a done
> +      event it is automatically destroyed.
> +    </description>
> +
> +    <event name="converted">
> +      <description summary="color space conversion">
> +        The surface was converted to a color space.
> +      </description>
> +      <arg name="color_space" type="object" interface="zwp_color_space_v1"/>
> +    </event>
> +
> +    <event name="done">
> +      <description summary="done listing color space conversions">
> +        All color space conversion have been listed.

What happens at this point?

If the feedback object is destroyed, it may be better to name this "finished".

> +      </description>
> +    </event>
> +  </interface>
> +
> +</protocol>
> --
> 2.20.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel




More information about the wayland-devel mailing list