KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.81k stars 469 forks source link

Color management on the Vulkan Wayland platform #2307

Open ppaalanen opened 9 months ago

ppaalanen commented 9 months ago

We are developing a color-management Wayland protocol extension, and expect Vulkan WSI implementations to use it on behalf of the application, because Vulkan already defines the extensions to do so. We need to define the rules how that works in VK_KHR_wayland_surface.

The main driver of color-management information is VkColorSpaceKHR enumeration. Other Vulkan extension may add more, e.g. VK_EXT_hdr_metadata. These values shall be converted into Wayland color-management extension protocol by a WSI.

We need to define how exactly the protocol values are derived from Vulkan values. We also need to define when Vulkan WSI shall not instantiate a wp_color_management_surface_v1 for a wl_surface. The latter is needed to allow applications drive color-management protocol themselves, because the protocol allows things not exposed in Vulkan API. The use of ICC profile files is one such feature.

Presumably VK_COLOR_SPACE_PASS_THROUGH_EXT would be the value to guarantee that no wp_color_management_surface_v1 is created.

How to handle VK_COLOR_SPACE_SRGB_NONLINEAR_KHR is an open question. There are two possibilities:

By Vulkan definition, VK_COLOR_SPACE_SRGB_NONLINEAR_KHR is sRGB. However, window systems have probably relayed such pixel data as-is to all (SDR) displays, meaning that its presentation may not have been sRGB. If its presentation is now changed to sRGB, it might appear visually different. On the other hand, if no image description is set for VK_COLOR_SPACE_SRGB_NONLINEAR_KHR, Wayland compositors may continue presenting the content like they always have, or the presentation may differ from previous, and the presentation may differ from content that is explicitly sRGB. All this is up to the policy of each Wayland compositor implementation, what their users want.

We need to document how Vulkan WSI must use Wayland color-management extension.

linyaa-kiwi commented 8 months ago

Related to https://github.com/KhronosGroup/Vulkan-Docs/issues/2312, which seeks clarification on the interaction between VkColorSpaceKHR and vkSetHdrMetadataKHR.

linyaa-kiwi commented 8 months ago

In a previous Vulkan meeting, I showed @swick where the relevant text should be added to the specification, in the vkCreateWaylandSurfaceKHR section and under an existing paragraph that discusses Wayland protocol restrictions.

However, I don't yet have opinions on what that text should say. I need to first study the MR for wp_color_management_surface_v1.

swick commented 8 months ago

Keep in mind this protocol is not finalized (but this part is very likely not going to change). The interesting part is the get_surface request (https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/c2f13ff1ff52447a40ecdc4b33936b784c4a04bc/staging/color-management/color-management-v1.xml#L341):

If a wp_color_management_surface_v1 object already exists for the given wl_surface, the protocol error surface_exists is raised.

This is done to make sure only one component can change the color properties (what we call a wp_image_description_v1) and we don't end up in an unwanted state.

colinmarc commented 7 months ago

Hi, I tried to deal with some of this in my Mesa MR and hit some problems. First, I'll try to summarize the issue to make sure I understand.

Setting VK_COLOR_SPACE_PASS_THROUGH_EXT as a way to say "don't do any color management for me" makes sense on its face. However, the problem is that that's a setting on the swapchain, whereas a given surface has multiple swapchains over its lifetime.

In other words, the driver probably has to decide whether to call get_surface at surface creation time, but the swapchain is not yet created at that point. It could do so lazily, or it could take PASSTHROUGH to mean "please destroy the color_management_surface object", but that would seem to invite race conditions.

Another option would be to never bind the global at all if the VK_EXT_swapchain_colorspace extension is not enabled. However, that also removes access to VK_COLOR_SPACE_PASS_THROUGH_EXT, which is part of the extension.

I think the best way out of this rabbit hole to just add a very small extension which extends VkWaylandSurfaceCreateFlagsKHR with an ENABLE_COLOR_MANAGEMENT_BIT flag. That would also cover the sRGB case.

ppaalanen commented 7 months ago

@colinmarc, your understanding of the issue is correct. I don't know about Vulkan to say anything more.

Binding to the global Wayland interface is always fine. It's really the get_surface request being the issue.