-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
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
Vulkan sampler fix #6001
Closed
martin-ejdestig
wants to merge
2
commits into
ocornut:master
from
martin-ejdestig:vulkan_sampler_fix
Closed
Vulkan sampler fix #6001
martin-ejdestig
wants to merge
2
commits into
ocornut:master
from
martin-ejdestig:vulkan_sampler_fix
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sampler, descriptor set layout and pipeline layout are created in exact same way directly in ImGui_ImplVulkan_CreateDeviceObjects(). The removed functions are local and only has call chain that starts in ImGui_ImplVulkan_CreateDeviceObjects(), so will always do early return.
From https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDescriptorSetLayoutBinding.html : "pImmutableSamplers affects initialization of samplers. If descriptorType specifies a VK_DESCRIPTOR_TYPE_SAMPLER or VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER type descriptor, then pImmutableSamplers can be used to initialize a set of immutable samplers. Immutable samplers are permanently bound into the set layout and must not be changed; updating a VK_DESCRIPTOR_TYPE_SAMPLER descriptor with immutable samplers is not allowed and updates to a VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER descriptor with immutable samplers does not modify the samplers (the image views are updated, but the sampler updates are ignored)." So setting it means the same sampler will be used for all textures, basically making sampler argument to ImGui_ImplVulkan_AddTexture() useless. Fixes ocornut#5502 .
ocornut
pushed a commit
that referenced
this pull request
Jan 2, 2023
…6001) Sampler, descriptor set layout and pipeline layout are created in exact same way directly in ImGui_ImplVulkan_CreateDeviceObjects(). The removed functions are local and only has call chain that starts in ImGui_ImplVulkan_CreateDeviceObjects(), so will always do early return.
ocornut
pushed a commit
that referenced
this pull request
Jan 2, 2023
…6001) Sampler, descriptor set layout and pipeline layout are created in exact same way directly in ImGui_ImplVulkan_CreateDeviceObjects(). The removed functions are local and only has call chain that starts in ImGui_ImplVulkan_CreateDeviceObjects(), so will always do early return.
Thanks for this. Catching up with a few PR now. I've merged the "dead code removal" commit as 14d4b36 and - wow :) |
ocornut
pushed a commit
that referenced
this pull request
Jan 2, 2023
…6001) Sampler, descriptor set layout and pipeline layout are created in exact same way directly in ImGui_ImplVulkan_CreateDeviceObjects(). The removed functions are local and only has call chain that starts in ImGui_ImplVulkan_CreateDeviceObjects(), so will always do early return.
ocornut
pushed a commit
that referenced
this pull request
Jan 2, 2023
And merged second commit as e5d5186. Thanks a lot @martin-ejdestig and @rytisss! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Do not set VkDescriptorSetLayoutBinding::pImmutableSamplers
From https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkDescriptorSetLayoutBinding.html :
"pImmutableSamplers affects initialization of samplers. If descriptorType specifies a VK_DESCRIPTOR_TYPE_SAMPLER or VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER type descriptor, then pImmutableSamplers can be used to initialize a set of immutable samplers. Immutable samplers are permanently bound into the set layout and must not be changed; updating a VK_DESCRIPTOR_TYPE_SAMPLER descriptor with immutable samplers is not allowed and updates to a VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER descriptor with immutable samplers does not modify the samplers (the image views are updated, but the sampler updates are ignored)."
So setting it means the same sampler will be used for all textures, basically making sampler argument to
ImGui_ImplVulkan_AddTexture()
useless.Fixes #5502 .
(Also removes some dead code, see first commit, to not have to make same fix in two places.)