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

Got sampled images working. Samplers are next #693

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Fletterio
Copy link
Contributor

Description

Writing the necessary stuff so that we can have decoupled image/samplers in bindings

Testing

I changed example 07 a little bit and it runs normally. If you remove the sampler from the equation it doesn't forward any to the vk write

TODO list:

If you still provide a sampler in the DescWrite, it will be forwarded to Vulkan (the current way stuff works is that if the type's category is image and you provide samplers, they will be sent). I think this is not an issue because if the VkDescriptorType of the write is VK_SAMPLED_IMAGE the samplers of the image info are ignored. However it makes no sense to give samplers to an image info for a sampled image descriptor. If necessary I can make it generate a warning or fail

@devshgraphicsprogramming
Copy link
Member

However it makes no sense to give samplers to an image info for a sampled image descriptor. If necessary I can make it generate a warning or fail

Its necessary for us to ignore the sampler as well, because otherwise we write it into the lifetime tracking of the pool.

@Fletterio
Copy link
Contributor Author

I'll see when it gets lifetime tracked then and have it be ignored at that point

@@ -101,35 +101,7 @@ class NBL_API2 ICPUDescriptorSet final : public IDescriptorSet<ICPUDescriptorSet
private:
static inline IDescriptor::E_CATEGORY getCategoryFromType(const IDescriptor::E_TYPE type)

Choose a reason for hiding this comment

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

I'll just remove the function all together

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

2 participants