Add 'output render_bit_depth [8|10]' command #6475

mstoeckl · 2021-09-03T03:49:11Z

~~Companion to wlroots MR !3333.~~ ~~See commit message (and the documentation for the introduced command in the man page) for what this does.~~

This PR provides an output <name> render_bit_depth [8|10] command which can be used to make sway render window surfaces onto a 10 bit-per-channel buffer, if sway's backend and rendering system support it. The default bit depth remains 8, because that is supported essentially everywhere.

This is a rather specific interface; it does not guarantee that 10 bpc will be used, and even if a 10-bpc render buffer is used, it is not guaranteed that a monitor will be sent 10 bpc color data. Also, direct scan out for fullscreen windows might temporarily change the format of the buffers being sent to an output.

I am not tied to the design of the render_bit_depth command; if anyone has a better alternative, please suggest it.

This command can easily be generalized to set 5-bit (RGB565) or 16-float-bit channel depths; however:

Essentially all programs produce 8 bpc data, so 5 bpc will lose color information; also, as far as I know, all monitors already accept 8 bpc color data. There could in theory be monitors where sending 6bpc instead of 8bpc allows for higher frame rates, but I don't know of any.
16f used to work for me with the wlroots drm backend, but started failing (with frozen display) a few months ago, possibly after a kernel or Mesa change. This bit depth can be added later by someone who can test that it doesn't break anything in Sway or wlroots.

emersion · 2021-11-21T20:12:25Z

sway/config/output.c

@@ -351,6 +356,22 @@ static int compute_default_scale(struct wlr_output *output) {
 	return 2;
 }

+/* Lists of formats to try, in order, when a specific render bit depth has
+ * been asked for. The last format in the list should always be XRGB8888,
+ * as a reliable backup in case the others are not available. */


I'd prefer to not fallback to 8-bit when the user explicitly asks for 10-bit. Similar to how we handle modes which cannot be enabled.

Similar to how we handle modes which cannot be enabled.

It looks like sway does different things here, depending on the backend.

With the DRM backend, it picks the closest listed mode to the one asked for; config/output.c#L257-L275.

Setting a a modeline, tries to set the specified mode (and possibly freezes the screen if the mode isn't allowed.)

With the Wayland backend, it always accepts the mode.

not fallback to 8-bit when the user explicitly asks for 10-bit.

If I pick this behavior, then Sway will crash if no 10 bit buffer formats are supported, because it does not have any buffers to render onto. This is an experimental command, so it might be fine to do this as long as the documentation warns about it. Shall I go for it?

It looks like sway does different things here, depending on the backend.

Hm that's a good point. I forgot the bits about the DRM backend. We should look at a way to unify all of this. Since black screens aren't great, maybe we should always fallback to something that works, and integrate with swaynag to print warnings "your output mode couldn't be enabled" and "10-bit not available for this output".

Ideally we should also add a render_bit_depth field in swaymsg -t get_outputs, to let users query the current bit depth. But this can come later.

Also there's the wlr-output-management protocol, which never expects such fallbacks. It expects the configuration to fail if it's not possible. The wlr-output-management client itself will then try something else.

If I pick this behavior, then Sway will crash if no 10 bit buffer formats are supported, because it does not have any buffers to render onto.

I wouldn't expect it to crash, but maybe it won't light up outputs configured with 10-bit render formats? But per the above, I've changed my mind.

sway/config/output.c

emersion · 2021-11-21T20:23:18Z

sway/sway-output.5.scd

@@ -157,6 +157,20 @@ must be separated by one space. For example:
 	adaptive sync can improve latency, but can cause flickering on some
 	hardware.

+*output* <name> render_bit_depth 8|10


Config design question: do we know yet how this will interact with other related options? Like KMS "max bpc" (we don't have "force bpc" or "current bpc" yet unfortunately, but it's been discussed), potential HDR options, etc?

Is the bit depth a good thing to let users configure? Are there some other more meaningful or high level config options we could let the user configure and which would dictate low-level things such as render format and KMS "max bpc"?

Maybe it's a bit too soon to ask these questions. Also, I don't know a lot about color management and HDR. If everything is blurry still, maybe we can label this with an "experimental" disclaimer, saying it can go away or change in the future.

Some relevant comments were in swaywm/wlroots#3162 (comment). I'm not sure I like the mode string extension idea too much, I'd prefer not to end up with hard-to-decipher mode strings, and instead use clearer output commands, even if multiple commands might change the hw limitations and such.

On X11, DefaultDepth 30 just extends sRGB (or whatever profile the X11 display claims to have, I think it's a property, either way see https://www.freedesktop.org/software/colord/intro.html) all over the 30 bits. Only Chromium is being a jerk and needs to have the profile set, or it becomes super low contrast, anyway.

I have no clue about HDR content.

Color management should not otherwise differ between 10-bit and 8-bit, I fail to see how.

which supports color management might prefer higher bpc formats

Definitely. When there is a mismatch between the content colour space and the display, any transformation will typically lose noticeable information when using the low output bit depth of 8.

I'm curious, what reason do you have in mind with screen recording?

I'm curious, what reason do you have in mind with screen recording?

As mentioned in wlroots#3162 (comment), sometimes programs produce 10 or 16 bit buffers; rendering to a higher depth, and taking a screenshot with the depth, will avoid losing color information from what the program provided.

Screen sharing: if computer A has an 8 bit monitor, but computer B has a 10 bit monitor, then when looking at computer A's screen from computer B, it would be nice to have colors available at full precision, or gradients drawn without banding.

The RGB->YUV conversion when recording a video from a screen often loses a very small amount of information; capturing a 10 bpc screen, instead of an 8bpc screen, can slightly improve quality.

I see, thanks for the explanation.

To elaborate a bit on the use case I personally care about and have in mind here, your "typical" image viewer will: extract an ICC profile from an image, read out the profile of the screen (Apple, e.g., now uses Display P3, and in the past used gamma 1.8 "sRGB") or just assume sRGB, and use a library such as Little-CMS to convert the data. Guaranteed to merge values visibly, with 8-bit channels.

@mstoeckl, this design looks good to me, thanks!

By the way, I've found this, it seems to explain HDR well https://www.x.org/wiki/Events/XDC2016/Program/xdc-2016-hdr.pdf

This makes it possible to hint to the renderer and backends how many bits per channel the buffers that the compositor draws windows onto should have. Renderers and backends may deviate from this if they do not support the formats with higher bit depth.

emersion

LGTM, thanks!

emersion · 2021-11-23T14:54:16Z

Opened #6681 and #6682 as follow-up issues.

mstoeckl mentioned this pull request Sep 3, 2021

output: Add function to set preferred render bit depth swaywm/wlroots#3162

Closed

mstoeckl marked this pull request as draft September 3, 2021 04:07

mstoeckl force-pushed the bit-depth-setting branch 2 times, most recently from 528e08d to ccfd92f Compare September 11, 2021 17:31

mstoeckl force-pushed the bit-depth-setting branch from ccfd92f to 2dda72b Compare November 12, 2021 00:22

mstoeckl force-pushed the bit-depth-setting branch 2 times, most recently from cbdaba8 to 5ebe68c Compare November 20, 2021 20:54

mstoeckl changed the title ~~WIP: Add 'output render_bit_depth [8|10]' command~~ Add 'output render_bit_depth [8|10]' command Nov 20, 2021

mstoeckl marked this pull request as ready for review November 20, 2021 20:55

mstoeckl mentioned this pull request Nov 20, 2021

Support for high bit depth screencopy buffers ammen99/wf-recorder#155

Closed

emersion reviewed Nov 21, 2021

View reviewed changes

sway/config/output.c Outdated Show resolved Hide resolved

emersion reviewed Nov 21, 2021

View reviewed changes

sway/config/output.c Outdated Show resolved Hide resolved

emersion reviewed Nov 21, 2021

View reviewed changes

mstoeckl force-pushed the bit-depth-setting branch from 5ebe68c to aecc6e0 Compare November 21, 2021 20:56

emersion approved these changes Nov 23, 2021

View reviewed changes

emersion merged commit a23cdbb into swaywm:master Nov 23, 2021

This was referenced Nov 23, 2021

Add render_bit_depth field to get_outputs IPC command #6681

Open

Unify output configuration failure modes, report swaynag warnings #6682

Open

freundTech mentioned this pull request Oct 13, 2022

Deep color support (10bit per color)? hyprwm/Hyprland#845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'output render_bit_depth [8|10]' command #6475

Add 'output render_bit_depth [8|10]' command #6475

mstoeckl commented Sep 3, 2021 •

edited

Loading

emersion Nov 21, 2021

mstoeckl Nov 21, 2021 •

edited

Loading

emersion Nov 23, 2021

emersion Nov 21, 2021

emersion Nov 21, 2021

pjanx Nov 21, 2021

pjanx Nov 21, 2021 •

edited

Loading

pjanx Nov 21, 2021

pjanx Nov 21, 2021

mstoeckl Nov 21, 2021 •

edited

Loading

pjanx Nov 21, 2021 •

edited

Loading

emersion Nov 23, 2021

pjanx Nov 25, 2021

emersion left a comment

emersion commented Nov 23, 2021

Add 'output render_bit_depth [8|10]' command #6475

Add 'output render_bit_depth [8|10]' command #6475

Conversation

mstoeckl commented Sep 3, 2021 • edited Loading

Choose a reason for hiding this comment

mstoeckl Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjanx Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mstoeckl Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

pjanx Nov 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emersion left a comment

Choose a reason for hiding this comment

emersion commented Nov 23, 2021

mstoeckl commented Sep 3, 2021 •

edited

Loading

mstoeckl Nov 21, 2021 •

edited

Loading

pjanx Nov 21, 2021 •

edited

Loading

mstoeckl Nov 21, 2021 •

edited

Loading

pjanx Nov 21, 2021 •

edited

Loading