mac/vulkan: add retrieval of color depth and return auto (0)

[Akemi]

see #13767.

@Dudemanguy is that what you had in mind?

should i optimise the sw->fns->color_depth(sw) call so it isn't called twice in the worst case?

[github-actions[bot]]

Download the artifacts for this pull request:

Windows

• mpv-i686-w64-mingw32
• mpv-x86_64-w64-mingw32
• mpv-x86_64-windows-msvc

macOS

• mpv-macos-13-intel
• mpv-macos-14-arm
• mpv-macos-15-arm

[kasper93]

Correct me if I'm wrong, but this whole change looks like a hack to disable another hack?

Specifically this part

        } else if (!pl_color_transfer_is_hdr(target->color.transfer)) {
            dither_depth = 8;

which is just wrong and if it causes problems it should be reworked to do this only on affected platforms.

EDIT: I don't think we should add a workaround to platforms that works as expected and instead contain focus on others that are affected.

EDIT2: After reading the code again, I think for now we can implement

static int mac_vk_color_depth(struct ra_ctx *ctx)
{
    return 0;
}

to use default behavior of dithering to output swap chain. And avoid duplicating format checks.

[Akemi]

yeah i think the pl_color_transfer_is_hdr() else if case is problematic. deciding the colour depth by source isn't optimal. anyway this is also the reason why i changed the if like i did, see above.

[edit]

which is just wrong and if it causes problems it should be reworked to do this only on affected platforms.

maybe i misunderstand, but that's what this does atm. previously this was always the case for auto on vulkan. with the changes of this PR it will only hit on platforms/contexts that don't implement color_depth(), so as before but without macvk.

[kasper93]

this is also the reason why i changed the if like i did, see above.

Yes, I understand, I was confused. See my suggested changes, should make auto work fine for mac_vk, but not the rest vulkan.

[Akemi]

thanks @kasper93 should have done this in the first place, but for whatever reason i went with the first approach. circumventing the else if case of picking the dither depth by source and instead choosing based on the output surface/pixel format is really what should have been done.

anyway i tested it a bit and it does what is expected, eg it picks rgb10a2Unorm and dithers to 10bit on my end. just as an example.

...
[vo/gpu-next/libplacebo] Picked surface configuration 28: VK_FORMAT_A2B10G10R10_UNORM_PACK32 + VK_COLOR_SPACE_PASS_THROUGH_EXT
...
[vo/gpu-next/libplacebo] Dithering to 10 bit depth
...

[kasper93]

Looks ok now. I was confused too, because we have double lookup for vulkan context. The assumption of original code was that if color_depth is present (!= NULL) it should be valid, but looks like we need special -1 case. It's fine.

[Akemi]

sry that i didn't notice and ask beforehand. -1 is also a valid value, eg no dithering. should we rather use -2 or something else in that case?

[kasper93]

sry that i didn't notice and ask beforehand. -1 is also a valid value, eg no dithering. should we rather use -2 or something else in that case?

I think color_depth return value, doesn't have to follow dither-depth value directly, but if you prefer -2 works too.