-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
video/out/gpu/context: add target_csp callback to ra_swapchain #15279
Conversation
/cc @Dudemanguy |
Download the artifacts for this pull request: |
Also auto being implemented in this fashion would make everything else (like drm) basically go back to behaving like |
The intention for now is that when the |
struct ra_swapchain_fns { | ||
// Gets the current framebuffer depth in bits (0 if unknown). Optional. | ||
int (*color_depth)(struct ra_swapchain *sw); | ||
|
||
// Target device color space. Optional. | ||
pl_color_space_t (*target_csp)(struct ra_swapchain *sw); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of nitpicky but is there a reason to return a colorspace? It seems like you only actually use the display's peak brightness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial version was using only target peak, but it turns out it is also returned for SDR displays. Like I have one with 270 nits. Correct way is to check the color space returned from the API. Also primaries might be useful in the future.
There is still one thing to fix, because I noticed that lib placebo completely fails to render SDR correctly if the target peak is not 203. I may have time tomorrow to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could only return the MaxLuminance
value if the colorspace is actually an HDR one and return 203 otherwise.
This allows to get target display parameters.
Just the same comment I had before. Why return the csp if you don't actually use it? SDR could always just return 0 for the target peak or whatever value works. |
I have also commit to support EDIT: I pushed contrast commit, if that's not ok, I will revert to target luma only. |
Ah you're using multiple values now, it's fine then and my nitpick is moot. No real point in using pointers or whatever when returning a |
Let's try it. There is possibility that those reported display values will be not optimal for some people. But we will never know if we don't test it out. |
No description provided.