src/mirror-extcopy: implement color management

[mahkoh]

This change allows wl-mirror to losslessly mirror an HDR monitor to another HDR monitor.

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/448

[Ferdi265]

Thank you for your contribution, this is amazing! I was wondering how HDR/color-management is gonna work with wl-mirror. I'm not 100% sure the rendering/GL side of wl-mirror is ready for HDR yet, I don't know if we have to add anything there or of it will just adapt to the color format of whatever we import into the texture.

I don't have an HDR monitor, so I can't really test this for HDR, but I can test that it at least doesn't break normal operation.

I think we'll best leave this PR unmerged until the protocol you added is merged into wl-protocols or at least one desktop starts shipping it.

At a first glance the implementation looks great! You integrated the protocol binding and additional color management things into the core wayland.c and extcopy backend really well.

I know the repetetive nature of adding new protocols to wayland.c is quite clunky, but I already have a patch for that in the pipeline that will clean this up a lot and will make it much more declarative (basically you add one struct member for the protocol object and add the interface name, min and max version, and a 'required' boolean to an array), I just haven't gotten to fully integrating it because of work and other things. (Once that lands I'll post a patch for your PR to avoid the merge conflict in wayland.c)

[mahkoh]

I'm not 100% sure the rendering/GL side of wl-mirror is ready for HDR yet, I don't know if we have to add anything there or of it will just adapt to the color format of whatever we import into the texture.

I haven't looked into what you're doing on the opengl side since everything just worked out of the box after I got this far. I assume you're just copying the pixels and potentially scaling them. That should mostly be fine as is.

The invert-colors functionality does not work because it would require the shader to actually understand how the colors are encoded. It would be very difficult to implement.

The biggest problem that I encountered was that the extcopy-dmabuf code just selects the first format supported by the compositor. Since my compositor supports a large number of formats, that just doesn't work. More often than not wl-mirror selects a format that the opengl renderer does not support and then crashes.

Making it easier for clients to select a good format has been discussed here: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/issues/224. For HDR content in particular, the client needs to select a sufficiently high bit depth format since otherwise HDR content will not be accurately reproduced. In lieu of the compositor informing you of the native format, you would have to add a bit-depth flag to wl-mirror and then select a matching format from those advertised by the compositor.

[Ferdi265]

The biggest problem that I encountered was that the extcopy-dmabuf code just selects the first format supported by the compositor. Since my compositor supports a large number of formats, that just doesn't work. More often than not wl-mirror selects a format that the opengl renderer does not support and then crashes.

Thank you for this! I noticed that wl-mirror's DMABUF support was flaky with other compositors at times (which has gotten better without changing anything in wl-mirror), but there's definitely something I need to improve here. I'll try to figure out how to check if a format is supported by the GL implementation and choose the first format that is actually supported. As far as I understand, the formats should be ordered by order of compositor preference, but if you have a better idea of how to choose a format, I'll gladly implement it.

[mahkoh]

As far as I understand, the formats should be ordered by order of compositor preference

The formats are unordered. See for example https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/398

ext_image_copy_capture_session_v1 sends all supported buffer constraints which the client combines with its own internal constraints to allocate an appropriate buffer for use with capture frames. The buffer constraints have no order.

[Ferdi265]

Making it easier for clients to select a good format has been discussed here: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/issues/224. For HDR content in particular, the client needs to select a sufficiently high bit depth format since otherwise HDR content will not be accurately reproduced. In lieu of the compositor informing you of the native format, you would have to add a bit-depth flag to wl-mirror and then select a matching format from those advertised by the compositor.

Interesting. I agree with the arguments from the wayland-protocols PR that the format negotiation system currently in the protocol(s) is suboptimal. I wonder how it works in pipewire and if there are any learnings Wayland can take from that (i.e. using a similar known-working solution or intentionally avoiding making the same mistakes).

[Ferdi265]

As far as I understand, the formats should be ordered by order of compositor preference

The formats are unordered. See for example https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/398

ext_image_copy_capture_session_v1 sends all supported buffer constraints which the client combines with its own internal constraints to allocate an appropriate buffer for use with capture frames. The buffer constraints have no order.

Right, so as it currently stands we'd also have to do a client-side 'rating' of the formats in order to not use a nonensical offered format as long as there isn't a good way to know the native format of the compositor.

Thanks for your insights! This is very helpful!