Add VK_EXT_disable_wayland_color_management

[colinmarc]

VK_EXT_disable_wayland_color_management

This adds a small extension, allowing applications to request that WSI implementations avoid using wayland color management, so that they can do it themselves. This addresses #2307.

Background

Wayland is growing color management and HDR support, in the soon-to-be-finalized wp_color_management protocol¹. The protocol has support for tagging surfaces with colorspace information and HDR metadata, generally in ways matching up to Vulkan's VK_KHR_swapchain_colorspace and VK_EXT_hdr_metadata, but also with facilities that are not currently expressible via Vulkan or other graphics APIs, such as using an ICC profile to describe a surface's content.

As such, applications wanting to do color management or output HDR content on Wayland will roughly fall into three buckets:

1. Image viewers and other utility apps that use Wayland directly, but not Vulkan or other graphics APIs
2. Games, video players, and the like, which will use Vulkan or other graphics APIs, but not the Wayland protocol directly
3. Content creation apps, which are picky about color management and wish to use the Wayland protocol directly, but also want to use Vulkan/etc to submit GPU buffers

The protocol's design is such that only one Wayland client may do color management for a particular surface at once. Specifically, only one color_management_surface may exist for a given Wayland surface at a time. Applications in the first group will create this object themselves, while applications in the second group will use VK_KHR_swapchain_colorspace and other related extensions, and rely on WSI implementations to do the tagging for them. The third group wants to create the object themselves, but needs to prevent the WSI implementation from also creating the object, since that would be a protocol violation. Hence this extension.

Alternatives considered

1. Check whether VK_KHR_swapchain_colorspace is enabled

WSI implementations could check whether the application has enabled the extension, and only create color_management_surface objects if it is. This has a few problems:

• It uses an instance extension to signal an option that really should be per-surface, changing the behavior of all surfaces created from the same instance.
• Because the extension is disabled, the application has no access to VK_COLOR_SPACE_PASS_THROUGH_EXT, which matches the behavior they want, and must use VK_COLOR_SPACE_SRGB_NONLINEAR_KHR, which may be completely unrelated to the content of their surface, to mean that the surface should remain untagged.

2. Switch on VK_COLOR_SPACE_PASS_THROUGH_EXT

This is more intuitively correct. Applications that set PASS_THROUGH would seem to be indicating that they want the surface to be untagged. However, this is difficult to implement. By the time the swapchain is create, the wayland surface has already been created, which means the driver must delay deciding whether to create the color_management_surface object. Furthermore, if another swapchain is created with a different colorspace, the color_management_surface must be created at that point, and then destroyed if the colorspace is again set to PASS_THROUGH. All of those operations could potentially race with the application's creation or destruction of the color_management_surface object, and such shenanigans would be pretty error-prone in general.

Note

I would like to create a validation check to ensure that surfaces created with VK_WAYLAND_SURFACE_CREATE_DISABLE_COLOR_MANAGEMENT must only be used to create swapchains with VK_COLOR_SPACE_PASS_THROUGH_EXT. Should that go in this PR, or somewhere else? Should the validation be on vkCreateSwapchainKHR or on VkWaylandCreateSurafceKHR?

Footnotes

1. https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/14 ↩

[CLAassistant]

All committers have signed the CLA.

[oddhack]

@colinmarc process-wise, we require that extension number and flag bits be reserved by a commit to main prior to the extension development branch actually being able to use them. I don't know where this extension stands deployment / release-wise, but we need to separate out the reservations and accept those first, to avoid a race condition (see extension 597 for a guide to what that PR would look like). In fact because #2414 also just came up, we will have to tweak one or the other of those reservation PRs since you can't both have extension 603.

[oddhack]

If you will go ahead and create the reservation PR using extension number 604, since #2416 just reserved 603, we should be good now (at least if nobody does an internal extension reservation MR first :-)).

[r-potter]

I would like to create a validation check to ensure that surfaces created with VK_WAYLAND_SURFACE_CREATE_DISABLE_COLOR_MANAGEMENT must only be used to create swapchains with VK_COLOR_SPACE_PASS_THROUGH_EXT. Should that go in this PR, or somewhere else? Should the validation be on vkCreateSwapchainKHR or on VkWaylandCreateSurafceKHR?

The Valid Usage statement should go in this PR and be on the VkWaylandCreateSurfaceKHR structure, next to the other two VUIDs. EDIT: on reflection, it's a constraint on the value of VkSwapchainCreateInfoKHR::imageColorSpace, so should probably be validated on VkSwapchainCreateInfoKHR i.e. phrased as "If surface was created with VK_WAYLAND_SURFACE_CREATE_DISABLE_COLOR_MANAGEMENT, then imageColorSpace must equal VK_COLOR_SPACE_PASS_THROUGH_EXT".

Just write it as an undecorated bullet point, and the actual VUID tag will get assigned as part of the spec merge process.

Implementation of the check itself would then follow as a PR against https://github.com/KhronosGroup/Vulkan-validationlayers

[colinmarc]

Thanks all for the help! I reserved the extension in #2419. I also rebased and added the validation check.

[linyaa-kiwi]

cc @swick

[swick]

The wayland protocol now supports has two surface extensions. wp_color_management_feedback_surface_v1 can be instantiated multiple times and provides everything that's required until a swapchain is presenting with a VkColorSpaceKHR that's not VK_COLOR_SPACE_PASS_THROUGH_EXT or VK_COLOR_SPACE_SRGB_NONLINEAR_KHR.

I'm not convinced this extension is necessary.

[colinmarc]

@swick what mechanism should a Vulkan implementation use to determine whether or not to create a wp_color_management_surface_v1? I don't see how the feedback object helps with that. Or do you mean that the Vulkan implementation should always create one, and apps (wanting to use Vulkan) should never do so?

[swick]

I'm suggesting that you only need to create a wp_color_management_surface_v1 when a swapchain with VkColorSpace != VK_COLOR_SPACE_PASS_THROUGH_EXT is actually presenting. Just the existence of such a swapchain is irrelevant. Equally, if a swaptchain with VkColorSpace == VK_COLOR_SPACE_PASS_THROUGH_EXT is presenting, then there should be no wp_color_management_surface_v1.

[colinmarc]

That sounds a lot like alternative 2., which I outlined above, except you're suggesting delaying the creation of the surface until presentation, rather than delaying it until swapchain creation time. I don't think that offers any particular advantages over lazily creating it along with the swapchain, since the colorspace is known at swapchain creation time, and it's awkward for the implementation to wait until VkQueuePresentKHR.

That solution suffers from the race conditions I mentioned - for example, what happens if you create a swapchain with HDR10 or something and then destroy it and create another with PASSTHROUGH? Should the wp_color_management_surface_v1 be destroyed at that point? Note that swapchain destruction is usually asynchronous and follows creation and presentation on the new swapchain in most apps.

[swick]

I don't think that offers any particular advantages over lazily creating it along with the swapchain, since the colorspace is known at swapchain creation time, and it's awkward for the implementation to wait until VkQueuePresentKHR.

You could ref count the wp_color_management_surface_v1 and when no swapchain with VkColorSpace != VK_COLOR_SPACE_PASS_THROUGH_EXT exists you destroy it. Then it's up to the app to make sure that all relevant swapchains have been destroyed before creating a wp_color_management_surface_v1 itself. In practice, apps would probably never create a swapchain with VkColorSpace != VK_COLOR_SPACE_PASS_THROUGH_EXT on a surface they want to use wp_color_management_surface_v1 on.

[colinmarc]

You could ref count the wp_color_management_surface_v1 and when no swapchain with VkColorSpace != VK_COLOR_SPACE_PASS_THROUGH_EXT exists you destroy it. Then it's up to the app to make sure that all relevant swapchains have been destroyed before creating a wp_color_management_surface_v1 itself.

Perhaps, but then apps wouldn't be able to tag their surface until after the first few frames are presented on the new swapchain, which would cause flickering. And swapchain destruction happens in a separate thread in a lot of apps, which could race.

In practice, apps would probably never create a swapchain with VkColorSpace != VK_COLOR_SPACE_PASS_THROUGH_EXT on a surface they want to use wp_color_management_surface_v1 on.

I agree, so why not expressly and explicitly forbid it? That's what this extension does.

You're proposing a potential mechanism, but what Vulkan requires is policy. Formulating your suggested mechanism as policy would be difficult to understand or enforce via validation. It would be something like

Implementations must only create a wp_color_management_surface_v1 if there exists an extant swapchain with a colorspace value other than VK_COLOR_SPACE_PASS_THROUGH_EXT, and destroy it as soon as, for a given surface, there is no extant swapchain with a colorspace value other than VK_COLOR_SPACE_PASS_THROUGH_EXT.

Compare that to these other policy alternatives:

Applications must not create wp_color_management_surface_v1 objects if they are using Vulkan

(for the record, I prefer this option). or:

Implementations must not create wp_color_management_surface_v1 objects if VK_WAYLAND_SURFACE_CREATE_DISABLE_COLOR_MANAGEMENT is set on the surface

I find both of those much more understandable and enforceable.

[swick]

Implementations must only create a wp_color_management_surface_v1 if there exists an extant swapchain with a colorspace value other than VK_COLOR_SPACE_PASS_THROUGH_EXT, and destroy it as soon as, for a given surface, there is no extant swapchain with a colorspace value other than VK_COLOR_SPACE_PASS_THROUGH_EXT.

What's wrong with this?

I find it an odd choice to create an extension which removes all VkColorSpaces other than VK_COLOR_SPACE_PASS_THROUGH_EXT when the app could just itself create swapchains with VK_COLOR_SPACE_PASS_THROUGH_EXT only.

[Zamundaaa]

Ref counting non-VK_COLOR_SPACE_PASS_THROUGH_EXT swapchains sounds acceptable to me as well, though I like that this extension is a bit more explicit. In practice the difference probably doesn't matter a lot, apps that do color management themselves likely only have one swapchain per surface anyways.

[ppaalanen]

I have a small complaint about the nomenclature here. I find "disable color management" misleading. In this proposal it means "do not use color management protocol", but I believe people will intuitively understand it as "push pixel values unmodified to display". The documentation is clear, though.

How about something like VK_EXT_wayland_no_color_description? no_image_description would be straight from the Wayland extension, but I suspect "image description" could be easily confused with unrelated Vulkan terminology.

As for whether this extension as a whole is a good idea, I cannot say. I'm not familiar with Vulkan.

[swick]

If people really want such an extension, then it shouldn't be wayland specific and just disable all VkColorSpaces other than VK_COLOR_SPACE_PASS_THROUGH_EXT. It has exactly the same result but is more general.

I personally still don't really see the point though.

[swick]

Apparently VK_COLOR_SPACE_PASS_THROUGH_EXT is not very well defined. Multiple people had different interpretations of what it means.

We would have to work on defining it better and probably also explicitly define the interaction with the wayland color management protocol, or come up with a new VkColorSpace for this purpose.