[RFC wayland-protocols v3 1/1] unstable: add color management protocol
Pekka Paalanen
ppaalanen at gmail.com
Tue Mar 19 11:23:00 UTC 2019
On Mon, 18 Mar 2019 02:12:59 +0100
Sebastian Wick <sebastian at sebastianwick.net> wrote:
> This protocol allows clients to attach a color space, rendering intent
> and HDR information to surfaces and to query outputs about their color
> spaces and HDR capabilities.
>
> Signed-off-by: Sebastian Wick <sebastian at sebastianwick.net>
Hi Sebastian,
this is looking very good to me. I only pointed out some corner cases
and clarifications below. Nice work!
> ---
> Makefile.am | 1 +
> unstable/color-management/README | 4 +
> .../color-management-unstable-v1.xml | 228 ++++++++++++++++++
> 3 files changed, 233 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>
Can we get more people to sign up for this?
I would otherwise except I'm not sure my understanding of color
management is up to par quite yet.
I suppose Ankit will join when the HDR information is integrated?
What about Erwin who has copyright below?
> 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..7b4d08e
> --- /dev/null
> +++ b/unstable/color-management/color-management-unstable-v1.xml
> @@ -0,0 +1,228 @@
> +<?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 specifies a way for a client to set the color space and
> + HDR metadata of a surface and to get information about the color spaces
> + and HDR capabilities of outputs.
> + </description>
> +
> + <interface name="zwp_color_manager_v1" version="1">
> + <description summary="color manager">
> + A global interface used for getting color management surface and color
> + management output objects as well as creating color spaces from ICC
> + profiles.
> + </description>
> +
> + <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="create_color_space">
> + <description summary="create a color space object">
> + Create a color space object from an ICC profile.
> +
> + Only three channel display profiles are allowed. The file descriptor
> + must be mmap-able. If the conditions are not met a protocol error
> + "invalid_icc_profile" is raised by the compositor.
> +
> + See the zwp_color_space interface for more details about the created
> + object.
> +
> + See the ICC specification for more details about ICC profiles.
> + </description>
> + <arg name="id" type="new_id" interface="zwp_color_space_v1"/>
> + <arg name="icc" type="fd"/>
mmap() requires size. Shouldn't there be a size sent along with the fd?
All Wayland core uses of an mmappable fd carry a size, and I suspect
there might be a reason for that instead of just fstat()'ing the fd to
get a size.
Could also add that the data behind the fd is read-only for the
compositor and must not be mutated by the client after this request (as
long as the zwp_color_space_v1 object exists?). We've been perhaps too
lax in defining these in Wayland before, leading to problems with using
e.g. memfd and seals in some cases.
> + </request>
> +
> + <request name="get_color_management_output">
> + <description summary="create a color management output from a wl_output">
> + This creates a new color zwp_color_management_output object for the
> + given wl_output.
> +
> + See the zwp_color_management_output interface for more details.
> + </description>
> + <arg name="id" type="new_id" interface="zwp_color_management_output_v1"/>
> + <arg name="output" type="object" interface="wl_output"/>
> + </request>
> +
> + <request name="get_color_management_surface">
> + <description summary="create a color management surface from a wl_surface">
> + This creates a new color zwp_color_management_surface object for the
> + given wl_surface.
> +
> + See the zwp_color_management_surface interface for more details.
> + </description>
> + <arg name="id" type="new_id" interface="zwp_color_management_surface_v1"/>
> + <arg name="surface" type="object" interface="wl_surface"/>
> + </request>
> +
> + <request name="destroy" type="destructor">
> + <description summary="destroy the color manager">
> + Destroy the zwp_color_manager object.
I would add: "This does not affect any other objects in any way."
> + </description>
> + </request>
> + </interface>
> +
> + <interface name="zwp_color_management_output_v1" version="1">
> + <description summary="output color properties">
> + A zwp_color_management_output describes the color properties of an
> + output.
What happens if the wl_output (the proxy) is destroyed? Inert, perhaps?
> + </description>
> +
> + <event name="color_space_changed">
> + <description summary="color space changed">
> + The color_space_changed event is sent after creating an zwp_color_management_output
> + (see zwp_color_manager.get_color_management_output) and whenever the color
> + space of the output changed.
> + </description>
> + </event>
> +
> + <request name="get_color_space">
> + <description summary="get the color space of the output">
> + This creates a new zwp_color_space object for the current color space of
> + the output. There always is exactly one color space active so the client
> + should destroy the color space created by earlier invocations of this
> + request. This is usually called as a reaction to the color_space_changed
> + event.
> +
> + See the zwp_color_space interface for more details.
> + </description>
> + <arg name="id" type="new_id" interface="zwp_color_space_v1"/>
> + </request>
> +
> + <!-- TODO: HDR capabilities event -->
> +
> + <request name="destroy" type="destructor">
> + <description summary="destroy the color management output">
> + Destroy the color zwp_color_management_output object.
> + </description>
> + </request>
> + </interface>
> +
> + <interface name="zwp_color_management_surface_v1" version="1">
> + <description summary="set color management information of a surface">
> + A zwp_color_management_surface allows the client to set the color space and
> + HDR properties of a surface.
What happens if the wl_surface is destroyed?
I would guess this object becomes "inert", as is the technical term we
tend to use in Wayland protocol: all requests except destroy are
ignored as much as possible and no events will be emitted.
An alternative would be to forbid destruction of wl_surface, but
implementation-wise it probably would not make anything easier, because
on disconnect all remaining objects will be torn down in arbitrary
order.
> + </description>
> +
> + <enum name="render_intent">
> + <description summary="render intent">
> + Rendering intent allow the client to hint at how to perform color space
> + transformations.
> +
> + See the ICC specification for more details about rendering intent.
> + </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>
> +
> + <request name="set_color_space">
> + <description summary="set the color space">
> + Set the color space of the underlying surface. The color space is double
> + buffered, and will be applied at the time wl_surface.commit of the
> + corresponding wl_surface is called.
> +
> + The render intent gives the compositor a hint what to optimize for in
> + color space transformations.
> +
> + The corresponding buffer is expected to contain un-premultiplied pixels when
> + a color space is set with this request.
This is explicit, good.
> +
> + If a surface has no color space set, sRGB and an arbitrary render intent
> + will be assumed.
Is it un-premultiplied also in this case, or does it follow the
original Wayland convention of premultiplied?
> +
> + If the color space of the surface matches the color space of an output
> + it is shown on the performance and color accuracy might improve. To find
> + those color spaces the client can listen to the preferred_color_space or
> + the wl_surface.enter/leave events.
> + </description>
> + <arg name="color_space" type="object" interface="zwp_color_space_v1"/>
> + <arg name="render_intent" type="uint" enum="render_intent"/>
> + </request>
> +
> + <!-- TODO: HDR metadata request -->
> +
> + <event name="preferred_color_space">
> + <description summary="preferred color space">
> + The preferred_color_space event is sent after creating an
> + zwp_color_management_surface (see zwp_color_manager.get_color_management_surface)
> + and whenever the preferred color space changed.
This means that a compositor will need to pick a preferred color space
for a surface before it knows where to map it, that is, which outputs
it might be shown on. The alternative would be to not send the event
until the compositor has a preference, but then the client needs to
guess what color space to start with to get the surface mapped.
I think this is good enough as is.
Removing the initial uncertainty would require quite some more protocol
and would apply to much more than just color space.
> +
> + The event does not carry a zwp_color_space but a wl_output object. The
> + concret zwp_color_space can be created by calling
> + zwp_color_management_output.get_color_space on the output.
> +
> + This is only a hint and clients can set any valid color space with
> + set_color_space but there might be performance and color accuracy
> + improvements by providing the surface in the preferred color space.
> + </description>
> + <arg name="output" type="object" interface="wl_output"/>
> + </event>
> +
> + <request name="destroy" type="destructor">
> + <description summary="destroy the color management surface">
> + Destroy the zwp_color_management_surface object.
I presume this has implications to the wl_surface color space setting?
Does the wl_surface revert to the defaults as without this extension
the next time the surface state is applied?
> + </description>
> + </request>
> + </interface>
> +
> + <interface name="zwp_color_space_v1" version="1">
> + <description summary="color space">
> + Describes a color space which can be attached to a surface
> + (zwp_color_management_surface.set_color_space) and provides information
> + like the ICC profile to alow clients to do color space transformations.
> +
> + The client can create a zwp_color_space object from an ICC profile by
> + calling zwp_color_manager.create_color_space or from an output by
> + calling zwp_color_management_output.get_color_space.
> + </description>
> +
> + <event name="information">
> + <description summary="color space information">
> + Information describing the color space is send once after binding.
"after binding"?
If a client creates a color space object from an ICC profile, there is
no need to send the ICC file immediately back to the client.
Should sending this event be tied to the request that created this
object, or should this object have an explicit request to emit this
event?
I think an explicit request would be better because... it is explicit.
It does not add any roundtrips that are not already there. In the cases
where a client likely wants to get the ICC file, it has already had to
create the color space protocol object itself and sending another
request on the object goes in the same message burst as creating it.
> +
> + The icc argument contains a mmap-able fd to the corresponding ICC
> + profile.
> + </description>
> + <arg name="icc" type="fd"/>
The same comments here as for the earlier fd argument, just in the
opposite direction.
> + </event>
> +
> + <request name="destroy" type="destructor">
> + <description summary="destroy the color space">
> + Destroy the zwp_color_space object.
If this object is set on a wl_surface, what happens?
If destroying the color space object frees the client to modify the ICC
file contents again, this needs to have some implications to the
wl_surface, similar to how destroying a wl_buffer does.
Which way should it be designed: once a client has sent an ICC file fd
to the compositor, it will never be able to modify the file contents
again; or this?
I don't think the "never be able to modify again" would be a problem in
practise, the client would just create another file and leave the
previous file to be freed by the kernel when the last fd and mmap of it
go away.
> + </description>
> + </request>
> + </interface>
> +
> +</protocol>
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20190319/4dc83d5e/attachment.sig>
More information about the wayland-devel
mailing list