Skip to content

Conversation

@kasper93
Copy link
Member

No description provided.

This always scalled min_luma to 0, effectivelly disabling all black
point compensation code in tone mapping, additionally, it tries to scale
to the "libplacebo space" while the requested preferred space is what
libplacebo will target.

The Wayland's target_luminance povides the luminance range that the
image description is targeting as the minimum and maximum absolute
luminance L. This is exactly what libplacebo expects.

For SDR relative mapping to reference white, it has to be rescaled, this
will be done in next commit.

This reverts commit ae211f7.
In SDR everything is relative to display, as opposed to PQ. Scale it
to 203 which is our internal reference white, to land white at 1.0 while
preserving the requested contrast in preferred colorspace.

Note that we already cheat a bit, because we clamp contrast to 1000:1
for SDR transfers, as this has been working for years, and SDR monitors
doesn't really provide this information. Some compositors defaults to
80/0.2 nits (400:1) which is taken verbatim from 1999 sRGB specification
that defined 0.2 nits as veiling glare for CRT displays at the time.
It's no longer applicable in present era, and while many current display
may have contrast in the range of 800:1, I argue that little bit of
clipping is better on those low contast targets, instead of rising the
brightness, on already low contrast anyway. The user can override this
with `--target-contrast` and `--target-peak`.

And either way, those luminance values doesn't have to refer to display
at all, they are just a dynamic range that we have to target.
Comment on lines +2152 to +2153
// In SDR output everything is relative, so rescale reference white
// to PL_COLOR_SDR_WHITE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wayland luminance values are always relative.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wp_image_description_info_v1::target_luminance

Provides the luminance range that the image description is targeting as the minimum and maximum absolute luminance L.

Relative to what? I think there is misunderstanding, based on some implementation details that I am not preview too.

The current code is not working, because it always maps min_luma to 0, which effectively makes all targets infinite contrast, which is not correct.

If ref=max, "SDR" case, we output 0-1 which maps to min and max of the display, it's display referred. 1.0 is display white, 0.0 is display black. Or colorspace volume, if you prefer to not talk display.

If ref!=max, that means we have some HDR/EDR capabilities. In which case we always switch to PQ, which is absolute scaled, where 0-1 maps to 0-10000 nits.

https://en.wikipedia.org/wiki/Perceptual_quantizer

image

Copy link
Contributor

@mahkoh mahkoh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relative to what?

Relative to the scale sent in the luminances event. The HDR headroom is always target_max/ref (we can ignore the offsets here since they don't make a difference at the upper end.)

The current code is not working, because it always maps min_luma to 0, which effectively makes all targets infinite contrast, which is not correct.

There is no way for the compositor to determine accurate target_min_luma and target_max_luma values for the output if the output is not operating in HDR mode. EDIDs do not contain these values. Assuming anything other than these values being made up will lead to incorrect results.

By treating target_max_luma/target_min_luma as the contrast of the display, you're ascribing meaning to the messages sent by the compositors that is not intended by the compositors and will lead to results that are unexpected.

I already proposed another solution that might fix this issue in practice in afaedac#diff-7cd33ead8491d06868d9ff3b01c25933493014124ad7c496a59dabeb434d97f4R2164-R2165

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relative to the scale sent in the luminances event. The HDR headroom is always target_max/ref (we can ignore the offsets here since they don't make a difference at the upper end.)

I don't really need "HDR headroom". All I need to our tone mapper is the dynamic range of the virtual target.

Could you give citation, where it says that those values are "relative" to luminances event? And what does this mean?

Say we have

min_luminance = 0.2 nits
min_target_luminance = 0.2 nits

What in what why they are relative?

Doesn't it mean that 0.0 value is referring to 0.2 nits? Whether the 0.2 nits makes sense is another question, but let's ignore this.

There is no way for the compositor to determine accurate target_min_luma and target_max_luma values for the output if the output is not operating in HDR mode. EDIDs do not contain these values. Assuming anything other than these values being made up will lead to incorrect results.

Correct. But those are the values we get. Why should mpv doubt that? After all compositor may have a tool to calibrate those values, may use other means to determine them.

Like on KDE, those values are connected to display brightness. And yes, they are made up, but scale between 200-600 nits. Of course 600 nits doesn't mean display luminance output, when it's at 10% brightness, but it's the value that when you plug into the math of tonemapper will produce the expected HDR headroom.

By treating target_max_luma/target_min_luma as the contrast of the display, you're ascribing meaning to the messages sent by the compositors that is not intended by the compositors and will lead to results that are unexpected.

It's not "contrast of the display". It's contrast of the requested target encoding. I'm not sure what you are trying to say. It's some starting value. I think better than using some hardcoded values in binary, compositors may give use better guess.

I already proposed another solution that might fix this issue in practice in afaedac#diff-7cd33ead8491d06868d9ff3b01c25933493014124ad7c496a59dabeb434d97f4R2164-R2165

This is still not correct. You are trying to convert target_luminance to some undefined space. PL_COLOR_SDR_WHITE / PL_COLOR_SDR_CONTRAST is only used as a fallback.

The color space that mpv targets is THE preferred one, i.e. c = wd->csp.hdr.min_luma.

Copy link
Contributor

@mahkoh mahkoh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give citation, where it says that those values are "relative" to luminances event? And what does this mean?

I gave the formulas below. Citation or not, this is how KDE and other compositors convert between luminance spaces.

But those are the values we get. Why should mpv doubt that?
[...]
It's contrast of the requested target encoding.

The values are not intended to be interpreted without offsetting. Again, see below.

The color space that mpv targets is THE preferred one, i.e. c = wd->csp.hdr.min_luma.

It's not because mpv doesn't forward the values from the luminances event to libplacebo and libplacebo assumes a space in which the reference luminance is 203. Nor does the vulkan API allow mpv to use that luminance space in general.

Therefore mpv is not targeting the luminance space advertised in the preferred image description.

Copy link
Contributor

@mahkoh mahkoh Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ref<max, we output PQ, which is absolute.

All wayland image descriptions are relative, including those using PQ. If you don't believe me, you can ping Xaver. Or you can look at the godot HDR implementation which treats target_max as a relative value: https://github.com/godotengine/godot/pull/102987/changes#diff-cb17fff099312adf05f6c79715f92ded45d468a8b4d57f8ab9ee79d99fe174a6R1038-R1041

float RenderingContextDriverVulkan::surface_get_hdr_output_max_value(SurfaceID p_surface) const {
	Surface *surface = (Surface *)(p_surface);
	return MAX(surface->hdr_max_luminance / MAX(surface->hdr_reference_luminance, 1.0f), 1.0f);
}

(this calculates the maximum values that should be stored in the floating point buffer used by godot)

and you can see that Xaver has reviewed this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So how does dmabuf-wayland works if you send it a PQ video? Do you have whitepaper on this Wayland PQ? We could also update Wikipedia.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works just like every other transfer function. Reference white is scaled to whatever is being rendered to.

You could just perform the experiment and see that with this PR mpv appears much darker. I would take a screenshot but SDR screenshots don't properly represent this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you argue that current master mpv is correct? It's bizarre to me that we ignore min_luminance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the upper bound, it is correct (+- a negligible amount).

At the lower bound it is less clear. It's probably fine for OLED displays but not for other displays. I feel like there is a bit of a mismatch between what the protocol exposes and what mpv works with.

It's bizarre to me that we ignore min_luminance?

I don't know what you mean by this.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

I believe this is incorrect because it will cause image descriptions that are treated as equivalent by compositors to be treated differently by mpv.

I already gave an example in #17329 (comment)

@kasper93
Copy link
Member Author

kasper93 commented Jan 27, 2026

I believe this is incorrect because it will cause image descriptions that are treated as equivalent by compositors to be treated differently by mpv.

I'm happy to be educated what is the correct way. Let's start from beginning, to avoid misconceptions.

When I get preferred descriptor to have target_luminance with say values:
max_lum: 200 nits
min_lum: 0.01 nits

What is the value 1.0 referring to?
What is the value 0.0 referring to?

Wouldn't it be that 1.0 is max, while 0.0 is min?

I already gave an example in #17329 (comment)

And this example is perfectly valid, one compositor requests 400:1 contrast, the other 20_000:1, both valid values and we respect that. But if this is wrong, please tell how to derive contrast value?

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

When I get preferred descriptor to have target_luminance with say values:
max_lum: 200 nits
min_lum: 0.01 nits

What is the value 1.0 referring to? What is the value 0.0 referring to?

Wouldn't it be that 1.0 is max, while 0.0 is min?

1.0 refers to primary_max_lum and 0.0 refers to primary_min_lum. The target_luminances are irrelevant when considering how values are encoded in the buffer. That is determined entirely by luminances, primaries, and transfer_function.

Let's assume that

  • primary_min_lum = 0.005
  • primary_ref_lum = 80
  • primary_max_lum = 10_000
  • target_min_lum = 0.2
  • target_max_lum = 400

and a linear transfer function for simplicity.

Then the maximum values that makes sense in the buffer is

$$\frac{400 - 0.005}{10000 - 0.005}$$

and the minimum value that makes sense is

$$\frac{0.2 - 0.005}{10000 - 0.005}$$

since values outside this range cannot be displayed.

If you want to convert these values to the following primary luminance space

  • primary_min_lum = 0.01
  • primary_ref_lum = 200
  • primary_max_lum = 1_000

Then you convert as follows:

$$\frac{400 - 0.005}{10000 - 0.005}\cdot\frac{10000 - 0.005}{80 - 0.005}\cdot\frac{200 - 0.01}{1000 - 0.01} = \frac{(400 - 0.005)(200 - 0.01)}{(1000 - 0.01)(80 - 0.005)}$$

and

$$\frac{0.2 - 0.005}{10000 - 0.005}\cdot\frac{10000 - 0.005}{80 - 0.005}\cdot\frac{200 - 0.01}{1000 - 0.01} = \frac{(0.2 - 0.005)(200 - 0.01)}{(1000 - 0.01)(80 - 0.005)}$$

This is the formula that KDE and other compositors use to convert buffer values in shaders.

If you solve this for the new luminances, you get the formula that mpv currently uses. Except that primary_min_lum of libplacebo is unknown. (primary_max_lum cancels out after solving for the luminance so it doesn't matter.)

For example, for target_min_lum:

$$\frac{(0.2 - 0.005)(200 - 0.01)}{(1000 - 0.01)(80 - 0.005)} = \frac{L - 0.01}{1000 - 0.01}$$

solves to

$$L = (0.2 - 0.005)\frac{200 - 0.01}{80 - 0.005} + 0.01$$

@kasper93
Copy link
Member Author

When I get preferred descriptor to have target_luminance with say values:
max_lum: 200 nits
min_lum: 0.01 nits
What is the value 1.0 referring to? What is the value 0.0 referring to?
Wouldn't it be that 1.0 is max, while 0.0 is min?

1.0 refers to primary_max_lum and 0.0 refers to primary_min_lum. The target_luminances are irrelevant when considering how values are encoded in the buffer. That is determined entirely by luminances, primaries, and transfer_function.

Let's assume that

* primary_min_lum = 0.005

* primary_ref_lum = 80

* primary_max_lum = 10_000

* target_min_lum = 0.2

* target_max_lum = 400

and a linear transfer function for simplicity.

Then the maximum values that makes sense in the buffer is
400 − 0.005 10000 − 0.005

and the minimum value that makes sense is
0.2 − 0.005 10000 − 0.005

since values outside this range cannot be displayed.

Exactly! So you confirm 400 and 0.2 are the values that we should use. 10000 is encoding range, 400 is usable range.

We should factor this offset that you show here.

Except that primary_min_lum of libplacebo is unknown.

There is no such thing. Do you mean when we get preferred gamma2.2 descriptor, when we fallback to PQ we should convert this to PQ descriptor? So primary_min_lum would be 0.005 as defined default for PQ, which Vulkan WSI will not change.

If you want to convert these values to the following primary luminance space

Yeah, but I don't want to "convert", between color spaces. We have tone mapper do do that. We need the usable range on input.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

So you confirm 400 and 0.2 are the values that we should use.

After they have been scaled as shown in the rest of my message.

Do you mean when we get preferred gamma2.2 descriptor, when we fallback to PQ we should convert this to PQ descriptor? So primary_min_lum would be 0.005 as defined default for PQ, which Vulkan WSI will not change.

Using the minimum luminance of the transfer function is something that I had considered. But I don't immediately see how it follows that this is correct.

We need the usable range on input.

The usable range is the range after scaling as discussed above.

@kasper93
Copy link
Member Author

kasper93 commented Jan 27, 2026

Using the minimum luminance of the transfer function is something that I had considered. But I don't immediately see how it follows that this is correct.

I don't see how using any other value would be correct.

I still don't see how ref value affects PQ code points. It is bizarre to me that you renormalize PQ into some other space. I understand for any other transfer, but PQ is kinda special in it's formula.

EDIT:

But if that helps, our reference is quite simple

PQ: 0-1 == 0-10000 nits
anything else: 0-1 == min_luma-max_luma (where min and max is provided externally)

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

I don't see how using any other value would be correct.

In the formula

$$L = (0.2 - 0.005)\frac{200 - 0.01}{80 - 0.005} + 0.01$$

The numerator of the fraction is PL_COLOR_SDR_WHITE - ???. AIUI you want to plug in the default primary_min_lum of the wayland transfer function here, with the assumption that libplacebo is going to choose that transfer function in vulkan. But for example for gamma22 the default values are

  • ref=max=80
  • min=0.2

So min is in a really different scale than PL_COLOR_SDR_WHITE (because PL_COLOR_SDR_WHITE != 80) so using the two in an addition or subtraction feels wrong.

I feel like afaedac#diff-7cd33ead8491d06868d9ff3b01c25933493014124ad7c496a59dabeb434d97f4R2164-R2165 is the best we can do at this point.

@kasper93
Copy link
Member Author

The numerator of the fraction is PL_COLOR_SDR_WHITE - ???. AIUI you want to plug in the default primary_min_lum of the wayland transfer function here, with the assumption that libplacebo is going to choose that transfer function in vulkan.

For PQ output if you argue that it somehow clips the PQ curve. We should rescale to natural 0-10000 nits range. PL_COLOR_SDR_WHITE have zero significance when tonemapping PQ->PQ for example. And we only need a min/max to guide tonemapping curve to compress the dynamic range. Which I still believe, sorry to say, are the absolute luminance values provided by target_luminances.

We need those values to be in the same range/space as HDR metadata of the input we provide. Of course nothing is really absolute, but you make it sound like we need to convert to "libplacebo" space, while there is no such thing. Of course internally we are normalized to some internal value, but it has zero significant on output as we encode the output according to transfer functions.

We do not support encodings where ref!=max except for PQ which is absolute. If you argue that PQ is not absolute and PQ codepoints are scaled also, that we don't support Wayland at all. But then again, neither is dmabuf-wayland which is sending the PQ image to compositor, which we do not rescale.

The values that we set have to be compatible with the values of HDR metadata, which is also explicitly stated that they are in Wayland spec.

But for example for gamma22 the default values are

ref=max=80
min=0.2

So min is in a really different scale than PL_COLOR_SDR_WHITE (because PL_COLOR_SDR_WHITE != 80) so using the two in an addition or subtraction feels wrong.

This case is easy, we just multiply by the PL_COLOR_SDR_WHITE/80 factor. The exact value doesn't matter if you normalize, when ref==max, such that 1.0 is white, 0.0 is black.

I feel like afaedac#diff-7cd33ead8491d06868d9ff3b01c25933493014124ad7c496a59dabeb434d97f4R2164-R2165 is the best we can do at this point.

This is random arbitrary value not connected with anything.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

This case is easy, we just multiply by the PL_COLOR_SDR_WHITE/80 factor. The exact value doesn't matter if you normalize, when ref==max, such that 1.0 is white, 0.0 is black.

If target_min=primary_min, which is the critical case, then the new target_min is PL_COLOR_SDR_WHITE/primary_ref*primary_min. If primary_ref=80 and primary_min=0.2, this results in target_min=0.5075 which is way too high.

@kasper93
Copy link
Member Author

kasper93 commented Jan 27, 2026

This case is easy, we just multiply by the PL_COLOR_SDR_WHITE/80 factor. The exact value doesn't matter if you normalize, when ref==max, such that 1.0 is white, 0.0 is black.

If target_min=primary_min, which is the critical case, then the new target_min is PL_COLOR_SDR_WHITE/primary_ref*primary_min. If primary_ref=80 and primary_min=0.2, this results in target_min=0.5075 which is way too high.

Exactly. Why do you think this value is not correct? We won't make it more correct by doing some magic scaling. Also as discussed on IRC, there is no "correct" value. The above value is correct for early 90' CRT, while it may not be that correct today, it's the value we get from preferred descriptor. That requests 400:1 contrast ratio.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

I don't believe using that value would deliver the best experience for mpv users.

@kasper93
Copy link
Member Author

I don't believe using that value would deliver the best experience for mpv users.

We already clamp to 1000:1 anyway, so it would fallback to that. I hope it was you on IRC, because there was long discussion about those values and why they likely never be correct.

Frankly for me the only way is to tune those values by eye for your viewing environment. Which can be done manually, with some other tool, maybe compositor can do it.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

So something like this:

diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c
index 7acc0b0ef..6d6783264 100644
--- a/video/out/wayland_common.c
+++ b/video/out/wayland_common.c
@@ -2144,6 +2144,23 @@ static void info_done(void *data, struct wp_image_description_info_v1 *image_des
     if (!wd->icc_file) {
         MP_VERBOSE(wl, "Preferred surface feedback received:\n");
         log_color_space(wl->log, wd);
+        if (fabsf(wd->csp.hdr.max_luma / wd->ref_luma - 1.0f) > 1e-4f &&
+            !pl_color_transfer_is_hdr(wd->csp.transfer)) {
+            MP_VERBOSE(wl, "Setting preferred transfer to PQ for HDR output.\n");
+            wd->csp.transfer = PL_COLOR_TRC_PQ;
+        }
+        float default_ref = 80.0f;
+        float default_min = 0.2f;
+        if (wd->csp.transfer == PL_COLOR_TRC_PQ) {
+            default_ref = 203.0f;
+            default_min = 0.005f;
+        } else if (wd->csp.transfer == PL_COLOR_TRC_BT_1886) {
+            default_ref = 100.0f;
+            default_min = 0.01f;
+        } else if (wd->csp.transfer == PL_COLOR_TRC_HLG) {
+            default_ref = 1000.0f;
+            default_min = 0.005f;
+        }
         // Wayland luminances are always in reference to the reference luminance. That is,
         // if max_luma == 2*ref_luma, then there is 2x headroom above paper white. On the
         // other hand, libplacebo hardcodes PL_COLOR_SDR_WHITE as the reference luminance.
@@ -2151,13 +2168,14 @@ static void info_done(void *data, struct wp_image_description_info_v1 *image_des
         // otherwise libplacebo will assume that there is too little or too much headroom
         // when ref_luma != PL_COLOR_SDR_WHITE.
         float a = wd->min_luma;
-        float b = (PL_COLOR_SDR_WHITE - PL_COLOR_HDR_BLACK) / (wd->ref_luma - a);
-        wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + PL_COLOR_HDR_BLACK;
-        wd->csp.hdr.max_luma = (wd->csp.hdr.max_luma - a) * b + PL_COLOR_HDR_BLACK;
+        float c = PL_COLOR_SDR_WHITE / default_ref * default_min;
+        float b = (PL_COLOR_SDR_WHITE - c) / (wd->ref_luma - a);
+        wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + c;
+        wd->csp.hdr.max_luma = (wd->csp.hdr.max_luma - a) * b + c;
         if (wd->csp.hdr.max_cll != 0)
-            wd->csp.hdr.max_cll  = (wd->csp.hdr.max_cll  - a) * b + PL_COLOR_HDR_BLACK;
+            wd->csp.hdr.max_cll  = (wd->csp.hdr.max_cll  - a) * b + c;
         if (wd->csp.hdr.max_fall != 0)
-            wd->csp.hdr.max_fall = (wd->csp.hdr.max_fall - a) * b + PL_COLOR_HDR_BLACK;
+            wd->csp.hdr.max_fall = (wd->csp.hdr.max_fall - a) * b + c;
         // Ensure that min_luma doesn't become negative.
         wd->csp.hdr.min_luma = MPMAX(wd->csp.hdr.min_luma, 0.0);
         // Since we want to do some exact comparisons of max_luma with PL_COLOR_SDR_WHITE,
@@ -2170,10 +2188,6 @@ static void info_done(void *data, struct wp_image_description_info_v1 *image_des
                 wd->csp.hdr.max_fall = MPMIN(wd->csp.hdr.max_fall, wd->csp.hdr.max_luma);
         }
         wl->preferred_csp = wd->csp;
-        if (wd->csp.hdr.max_luma != PL_COLOR_SDR_WHITE && !pl_color_transfer_is_hdr(wd->csp.transfer)) {
-            MP_VERBOSE(wl, "Setting preferred transfer to PQ for HDR output.\n");
-            wl->preferred_csp.transfer = PL_COLOR_TRC_PQ;
-        }
     } else {
         if (wl->icc_size) {
             munmap(wl->icc_file, wl->icc_size);

We already clamp to 1000:1 anyway

I don't really see how clamping is any less arbitrary.

@kasper93
Copy link
Member Author

When wd->min_luma==wd->csp.hdr.min_luma this wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + c; will result in c which is default_min (scaled).

Consider simple case of mapping sRGB to sRGB which should be identity.

For simplicity assume that PL_COLOR_SDR_WHITE==80.

Consider this example:

min_target_luminance=0.455432
min_luminance=0.455432
reference_luminance=80

wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + c;

will produce 0.2 (c) which is the default, ignoring the provided value in descriptor. This should be identity conversion and result in 0.455432.

$$ \begin{aligned} a &= 0.455432 \\ c &= \frac{80}{80}\cdot 0.2 \\ b &= \frac{80 - 0.2}{80 - 0.455432} \\ \mathrm{minluma} &= (0.455432 - 0.455432)\cdot b + 0.2 \end{aligned} $$

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

When wd->min_luma==wd->csp.hdr.min_luma this wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + c; will result in c which is default_min (scaled).

Yes, that's what I said above:

If target_min=primary_min, which is the critical case, then the new target_min is PL_COLOR_SDR_WHITE/primary_ref*primary_min.

And then you said that that was what you wanted:

Exactly. Why do you think this value is not correct?

@kasper93
Copy link
Member Author

kasper93 commented Jan 27, 2026

And then you said that that was what you wanted:

I was responding to this, and probably didn't parse what exactly you meant.

If primary_ref=80 and primary_min=0.2, this results in target_min=0.5075 which is way too high.

Because this is what this PR currently does.

wl->preferred_csp.hdr.min_luma *= PL_COLOR_SDR_WHITE / wd->ref_luma;

$$ 0.2 * 203 / 80 = 0.5075 $$

However the value depends on the provided descriptor. For my example from above, we would get:

$$ 0.455432 * 203 / 80 = 1.1556587 $$

In your solution we always will get 0.2 default (scaled), this doesn't sounds correct.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

However the value depends on the provided descriptor.

The value in the case target_min=primary_min cannot depend on the descriptor because vulkan will always use the default values which are 80/0.2 in most cases.

@kasper93
Copy link
Member Author

All wayland image descriptions are relative, including those using PQ.

I really doubt that. I'm reading KWin code.

bool TransferFunction::isRelative() const
{
    switch (type) {
    case TransferFunction::gamma22:
    case TransferFunction::sRGB:
    case TransferFunction::BT1886:
        return true;
    case TransferFunction::linear:
    case TransferFunction::PerceptualQuantizer:
        return false;
    }
    Q_UNREACHABLE();
}
TransferFunction TransferFunction::relativeScaledTo(double referenceLuminance) const
{
    if (isRelative()) {
        return TransferFunction(type, minLuminance * referenceLuminance / maxLuminance, referenceLuminance);
    } else {
        return *this;
    }
}
    // the min luminance the Wayland protocol defines for SDR is unrealistically high for most modern displays
    // normally that doesn't really matter, but with night light it can lead to increased black levels,
    // which are really noticeable when they're tinted red
    const double minSdrLuminance = 0.01;
    const auto transferFunction = effectiveHdr ? TransferFunction{TransferFunction::PerceptualQuantizer} : TransferFunction{TransferFunction::gamma22, minSdrLuminance * next.artificialHdrHeadroom, referenceLuminance * next.artificialHdrHeadroom};
    // HDR screens are weird, sending them the min. luminance from the EDID does *not* make all of them present the darkest luminance the display can show
    // to work around that, (unless overridden by the user), assume the min. luminance of the transfer function instead
    const double minBrightness = effectiveHdr ? next.minBrightnessOverride.value_or(transferFunction.minLuminance) : transferFunction.minLuminance;

    const double brightnessFactor = (!next.brightnessDevice && next.allowSdrSoftwareBrightness) || effectiveHdr ? brightness : 1.0;
    const double effectiveReferenceLuminance = 5 + (referenceLuminance - 5) * brightnessFactor;
    return std::make_shared<ColorDescription>(ColorDescription{
        containerColorimetry,
        transferFunction,
        effectiveReferenceLuminance,
        minBrightness,
        maxAverageBrightness,
        maxPeakBrightness,
        masteringColorimetry,
        sdrColorimetry,
    });

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

I really doubt that. I'm reading KWin code.

You don't need to read the code. The protocol spells it out:

Compositors should make sure that all content is anchored, meaning that an input signal level of 'reference_lum' on one image description and another input signal level of 'reference_lum' on another image description should produce the same output level, even though the 'reference_lum' on both image representations can be different.

It does not make an exception for PQ.

@kasper93
Copy link
Member Author

I really doubt that. I'm reading KWin code.

You don't need to read the code. The protocol spells it out:

Compositors should make sure that all content is anchored, meaning that an input signal level of 'reference_lum' on one image description and another input signal level of 'reference_lum' on another image description should produce the same output level, even though the 'reference_lum' on both image representations can be different.

It does not make an exception for PQ.

This doesn't say anything about PQ. Especially that it can be somehow "scaled" as you imply.

    double referenceLuminance = TransferFunction::defaultReferenceLuminanceFor(func.type);
    if (m_transferFunctionLuminances) {
        // PQ is special cased to require (max - min) == 10'000
        if (m_transferFunctionType == TransferFunction::PerceptualQuantizer) {
            func.minLuminance = m_transferFunctionLuminances->min;
            func.maxLuminance = m_transferFunctionLuminances->min + 10'000;
        } else {
            func.minLuminance = m_transferFunctionLuminances->min;
            func.maxLuminance = m_transferFunctionLuminances->max;
        }
        referenceLuminance = m_transferFunctionLuminances->reference;
    }

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

Ok. I don't really see a need to argue about this. Just hack libplacebo to use passthrough and manually create a PQ image description but with ref=100 and you'll see that the compositor increases the brightness of the image by 2x.

@mahkoh
Copy link
Contributor

mahkoh commented Jan 27, 2026

Or maybe even simpler hack mesa to set this value in the image description it creates.

diff --git a/src/vulkan/wsi/wsi_common_wayland.c b/src/vulkan/wsi/wsi_common_wayland.c
index 79ddda92f48..d15373047f0 100644
--- a/src/vulkan/wsi/wsi_common_wayland.c
+++ b/src/vulkan/wsi/wsi_common_wayland.c
@@ -1315,9 +1315,12 @@ wsi_wl_swapchain_update_colorspace(struct wsi_wl_swapchain *chain)
 
    wp_image_description_creator_params_v1_set_primaries_named(creator, primaries);
    wp_image_description_creator_params_v1_set_tf_named(creator, tf);
+   uint32_t divider = 4;
+   wp_image_description_creator_params_v1_set_luminances(creator, 50, 10000, 203 / divider);
    if (should_use_hdr_metadata) {
-      wp_image_description_creator_params_v1_set_max_cll(creator, wayland_hdr_metadata.max_cll);
-      wp_image_description_creator_params_v1_set_max_fall(creator, wayland_hdr_metadata.max_fall);
+      // also reduce the other values to prevent the compositor from tone mapping itself
+      wp_image_description_creator_params_v1_set_max_cll(creator, wayland_hdr_metadata.max_cll / divider);
+      wp_image_description_creator_params_v1_set_max_fall(creator, wayland_hdr_metadata.max_fall / divider);
       if (display->color_features.mastering_display_primaries) {
          uint32_t red_x = round(chain->color.hdr_metadata.displayPrimaryRed.x * 1000000);
          uint32_t red_y = round(chain->color.hdr_metadata.displayPrimaryRed.y * 1000000);
@@ -1337,8 +1340,8 @@ wsi_wl_swapchain_update_colorspace(struct wsi_wl_swapchain *chain)
           */
          if (wayland_hdr_metadata.max_luminance != 0) {
             wp_image_description_creator_params_v1_set_mastering_luminance(creator,
-                                                                           wayland_hdr_metadata.min_luminance,
-                                                                           wayland_hdr_metadata.max_luminance);
+                                                                           wayland_hdr_metadata.min_luminance / divider,
+                                                                           wayland_hdr_metadata.max_luminance / divider);
          }
       }
    }

@kasper93
Copy link
Member Author

kasper93 commented Jan 28, 2026

Ok. I don't really see a need to argue about this. Just hack libplacebo to use passthrough and manually create a PQ image description but with ref=100 and you'll see that the compositor increases the brightness of the image by 2x.

Ok, so PQ is absolute, but is later mapped into the same lightness based of reference value in the descriptor. Now we are clear on this.

So something like this:

This looks reasonable, and what will work mechanically to output the signal in "expected" range. However I don't think this is correct for PQ, where we would set different minimum luminance, through the WSI, so we shouldn't use default_min and instead use scaled source min.

For SDR, it will get the same scaling as if we just rescale ref/ref, but this is working fine.

I'm thinking we could simplify this to actually set the luminances value through protocol and avoid adjusting the the "default" values that mesa would set. Then we would just scale values exactly like this PR do, but set the reference on sRGB descriptor so the compositor remaps it for us. And still this is only a problem in those ref!=max sdr.

Speaking of which direct scanout is probably allowed only if we set exact descriptor as the preferred one? Setting different ref would require compositor to convert it anyway.

I could also add adjustable ref luminace in libplacebo, but frankly it's convenient to have everything in single point of reference there. Generally it's not even necessarly needed, as it would work just fine with gamma2.2 transfer and max_luma > 203, but we currently don't allow it and implicitly switch to PQ when this happens. Do you think it would be useful to support?

@kasper93
Copy link
Member Author

Will fix this in a different way.

@kasper93 kasper93 closed this Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants