Skip to content
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

Add support for NV12 buffers #166

Merged
merged 9 commits into from Apr 20, 2021
Merged

Add support for NV12 buffers #166

merged 9 commits into from Apr 20, 2021

Conversation

emersion
Copy link
Collaborator

Make it clear the ID refers to a Wayland object.
Make sure we don't overwrite an existing surf->wlr. We don't really
care about the Wayland surface ID here.
This allows gamescope-specific Wayland clients to connect to the
Wayland server, while preventing regular Wayland clients from doing
so.
Try to import multi-planar textures even if we don't have a
modifier. This is the case for e.g. DMA-BUFs coming from VA-API.
@emersion
Copy link
Collaborator Author

emersion commented Apr 16, 2021

The colors displayed by Vulkan for NV12 buffers are still incorrect, even though I think the VkFormat is correct. We likely need to setup VK_KHR_sampler_ycbcr_conversion.

@Joshua-Ashton
Copy link
Collaborator

Joshua-Ashton commented Apr 16, 2021

What colours are you seeing?

Sampler conversions only do stuff for image views/samplers so if you aren't compositing it shouldn't do anything? (unless I am missing something)

@emersion
Copy link
Collaborator Author

I'm seeing a red-ish image: https://l.sr.ht/XwZA.png

We might need some swizzles as well. I've tried one at random (just switch the bNeedsSwizzle field) and it makes the image blue-ish.

@emersion emersion changed the title Add support for YUV direct scan-out Add support for NV12 buffers Apr 16, 2021
@emersion
Copy link
Collaborator Author

Sampler conversions only do stuff for image views/samplers so if you aren't compositing it shouldn't do anything? (unless I am missing something)

I'm not well versed into the Vulkan compute things we're doing in gamescope, but I've seen VkSampler usage.

@Joshua-Ashton
Copy link
Collaborator

Joshua-Ashton commented Apr 16, 2021

Looking at where this is used yes there is an image view so we definitely need sampler conversion for that.

(Sorry, I was under the assumption that this would bypass composite always)

@Joshua-Ashton
Copy link
Collaborator

Joshua-Ashton commented Apr 16, 2021

You also have to use a combined image sampler and immutable samplers for ycbcr conversion iirc which would be annoying.

@Joshua-Ashton
Copy link
Collaborator

Let me know if you need any help with that or want me to do the rework to move to COMBINED and immutable samplers

@Plagman
Copy link
Member

Plagman commented Apr 19, 2021

Bypassing composite is the goal, but we'll need Vulkan composite support for dev and as a potential fallback. We should still merge it even with incomplete Vulkan support, as long as it's not going to be crashy, which it doesn't appear to be.

@Plagman
Copy link
Member

Plagman commented Apr 20, 2021

I guess I didn't strictly answer your question, Josh - feel free to work on the Vulkan stuff, but it shouldn't be blocking this MR and it can be merged as-is, in my opinion (at least as described).

@Joshua-Ashton
Copy link
Collaborator

👍

This doesn't yet display correct colors, and tiling artifacts are
visible.
@emersion emersion marked this pull request as ready for review April 20, 2021 07:11
@emersion
Copy link
Collaborator Author

Alright, let's merge this then. Proper Vulkan support can be built on top of existing commits.

@emersion emersion merged commit 910fba1 into ValveSoftware:master Apr 20, 2021
@emersion emersion deleted the nv12 branch April 20, 2021 07:13
@emersion
Copy link
Collaborator Author

emersion commented May 4, 2021

The tiling artifacts on the NV12 buffers should be fixed by this patch: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10623

(This doesn't fix the red-ish taint we also have on linear buffers.)

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.

None yet

3 participants