Skip to content

Conversation

@njdom24
Copy link
Contributor

@njdom24 njdom24 commented Nov 5, 2025

Describe your PR, what does it fix/add?

Simpler alternative to #12204

When tonemapping from HDR to SDR, scale the HDR source's reference luminance (203) to SDR (80). Prevents HDR screenshare from blowing out the brightness.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Unlike #12204, this method makes games typically look closer to how they do in SDR, at the risk of losing highlight detail that a game's native tonemapper might prioritize better.

Screenshots are of an extremely bright area, sort of a worst-case scenario, to emphasize the difference.

HDR-to-SDR on master:
image

HDR-to-SDR with this PR:
image

Native SDR:
image

Also brings in a quick fix for #12094, which made screensharing too dark when render:cm_sdr_eotf > 0, which became apparent when testing this.

Is it ready for merging, or does it need work?

Heavily WIP

@UjinT34
Copy link
Contributor

UjinT34 commented Nov 5, 2025

Does it work as expected with overriden monitorv2 luminances?

vec4 tonemap(vec4 color, mat3 dstXYZ) {
if (maxLuminance < dstMaxLuminance * 1.01)
return vec4(clamp(color.rgb, vec3(0.0), vec3(dstMaxLuminance)), color[3]);
mat3 toLMS = BT2020toLMS * dstXYZ;
mat3 fromLMS = inverse(dstXYZ) * LMStoBT2020;
vec3 lms = fromLinear(vec4((toLMS * color.rgb) / HDR_MAX_LUMINANCE, 1.0), CM_TRANSFER_FUNCTION_ST2084_PQ).rgb;
vec3 ICtCp = ICtCpPQ * lms;
float E = pow(clamp(ICtCp[0], 0.0, 1.0), PQ_INV_M2);
float luminance = pow(
(max(E - PQ_C1, 0.0)) / (PQ_C2 - PQ_C3 * E),
PQ_INV_M1
) * HDR_MAX_LUMINANCE;
float srcScale = maxLuminance / dstRefLuminance;
float dstScale = dstMaxLuminance / dstRefLuminance;
float minScale = min(srcScale, 1.5);
float dimming = 1.0 / clamp(minScale / dstScale, 1.0, minScale);
float refLuminance = dstRefLuminance * dimming;
float low = min(luminance * dimming, refLuminance);
float highlight = clamp((luminance / dstRefLuminance - 1.0) / (srcScale - 1.0), 0.0, 1.0);
float high = log(highlight * (M_E - 1.0) + 1.0) * (dstMaxLuminance - refLuminance);
luminance = low + high;
E = pow(clamp(ICtCp[0], 0.0, 1.0), PQ_M1);
ICtCp[0] = pow(
(PQ_C1 + PQ_C2 * E) / (1.0 + PQ_C3 * E),
PQ_M2
) / HDR_MAX_LUMINANCE;
return vec4(fromLMS * toLinear(vec4(ICtCpPQInv * ICtCp, 1.0), CM_TRANSFER_FUNCTION_ST2084_PQ).rgb * HDR_MAX_LUMINANCE, color[3]);
}
does some scaling, looks like it should do the same trick. Maybe some math or constants aren't correct or missing.

@njdom24
Copy link
Contributor Author

njdom24 commented Nov 5, 2025

@UjinT34 We throw that info away in the cmBackToSRGB block:

imageDescription.luminances = {};

I rocked the boat more with #12204, but even then I forced the luminanances to match the transfer function with

imageDescription.luminances = {
                .min = imageDescription.getTFMinLuminance(-1), .max = imageDescription.getTFMaxLuminance(-1), .reference = imageDescription.getTFRefLuminance(-1)};

Because monitorv2 settings would make an impact otherwise, and I'm not sure it'd ever be in a good way.

Edit: It'd probably be better to do the same luminance override in this branch

@njdom24 njdom24 marked this pull request as draft November 5, 2025 12:40
@njdom24
Copy link
Contributor Author

njdom24 commented Nov 5, 2025

Do the monitorv2 min_luminance, max_luminance, max_avg_luminance serve a purpose? It feels like they're just worked around with getTFMinLuminance, getTFMaxLuminance, and now getTFRefLuminance.

@UjinT34
Copy link
Contributor

UjinT34 commented Nov 5, 2025

Do the monitorv2 min_luminance, max_luminance, max_avg_luminance serve a purpose? It feels like they're just worked around with getTFMinLuminance, getTFMaxLuminance, and now getTFRefLuminance.

A monitor should provide those values through EDID. Sometimes EDID can't be fully parsed or the values are incorrect. Those monitorv2 rules are meant to fix that. getTF* values should be used for the source, EDID/monitorv2 values are for the output. Screen capture uses the main fb iirc and it's encoded with the EDID/monitorv2 values.

@s0mebodyhelpme
Copy link

Interestingly enough, sharing a screen with HDR on works for normal windows with this PR, but in Marvel's Spider-Man Remastered on Steam the game looks undersaturated in the screenshare when focused on the window and slightly overly bright when focused away with cm = srgb and cm_fs_passthrough = 1 with auto hdr on. Same is true for cm = hdr setting. Also, mpv isn't affected by cm_fs_passthrough = 2 with cm = hdr and auto hdr off. This makes it an issue to use cm_fs_passthrough = 1 with hdr on because you can't fullscreen normal windows without making them blown out or oversaturated and dark.

@njdom24
Copy link
Contributor Author

njdom24 commented Nov 5, 2025

@s0mebodyhelpme I've seen most of the issues you described with cm_fs_passthrough, but that's out of the scope of this PR.

These changes just address better conversion when we correctly make the determination that we want to share HDR, but it does seem like there's an issue with making that determination somewhere in the pipeline.

I'm surprised to see the issue with cm = hdr, though. That one's new to me. If the issue you're seeing with mpv is similar to what I saw with VLC, I did #12127 to address it, but I need to take another look at the passthrough settings to see if it's handling them correctly. Personally, I've never had cm_fs_passthrough = 1 work.

@s0mebodyhelpme
Copy link

s0mebodyhelpme commented Nov 5, 2025

@njdom24 im not sure if the issue with mpv is actually to do with direct scanout, as I have that set to 0, but I can try using both patches to see if that resolves the issue on my end. On another note, though, I am glad you're working on this. Willing to test out any future changes and report back if you welcome it.

@njdom24
Copy link
Contributor Author

njdom24 commented Nov 5, 2025

@s0mebodyhelpme If you don't have direct scanout enabled, the linked PR won't improve things, unfortunately. I'll see if I can reproduce it.

@njdom24 njdom24 force-pushed the screenshare-cm-brightness-scale branch from 383ff89 to 499c2e1 Compare November 6, 2025 03:08
@s0mebodyhelpme
Copy link

s0mebodyhelpme commented Nov 6, 2025

@njdom24 Update: cm_fs_passthrough = 2 does work, but when cm is srgb and/or (i'm guessing) not set to hdr. Something about how hdr mode is handled prevents it from working?

Edit: Also, screensharing an HDR game like Spider-Man Remastered looks right ONLY if focus is away from the window?

@njdom24
Copy link
Contributor Author

njdom24 commented Nov 6, 2025

@s0mebodyhelpme Are you sharing the window or the screen? All of my testing has been with the screen

@s0mebodyhelpme
Copy link

@njdom24 i noticed this on sharing a window, but it also becomes undersaturated when focused on the same app during sharing the screen (I know because I viewed the stream on a separate device).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants