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

Vulkan Backend image rendering. Image sampler always 'REPEAT' #5502

Closed
rytisss opened this issue Jul 23, 2022 · 9 comments
Closed

Vulkan Backend image rendering. Image sampler always 'REPEAT' #5502

rytisss opened this issue Jul 23, 2022 · 9 comments

Comments

@rytisss
Copy link

rytisss commented Jul 23, 2022

Version/Branch of Dear ImGui:

Version: 1.88
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_vulkan.cpp
Compiler: Visual Studio C++ 2022
Operating System: Windows 10

I have a problem with texture visualization, I am trying to visualize the texture (picture) and clamp the image to the border. However, as much as I try to change the parameters in the sampler, I still get the repetitive pattern:

image

The code for the sampler:

        VkSamplerCreateInfo samplerInfo{};
	samplerInfo.sType = VK_STRUCTURE_TYPE_SAMPLER_CREATE_INFO;
	samplerInfo.magFilter = VK_FILTER_LINEAR;
	samplerInfo.minFilter = VK_FILTER_LINEAR;
	samplerInfo.addressModeU = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER;
	samplerInfo.addressModeV = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER;
	samplerInfo.addressModeW = VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER;
	samplerInfo.anisotropyEnable = VK_FALSE;
	samplerInfo.maxAnisotropy = 1.0f;
	samplerInfo.borderColor = VK_BORDER_COLOR_INT_OPAQUE_BLACK;
	samplerInfo.unnormalizedCoordinates = VK_FALSE;
	samplerInfo.compareEnable = VK_FALSE;
	samplerInfo.compareOp = VK_COMPARE_OP_ALWAYS;
	samplerInfo.mipmapMode = VK_SAMPLER_MIPMAP_MODE_NEAREST;
	samplerInfo.mipLodBias = 0.0f;
	samplerInfo.minLod = 0.0f;
	samplerInfo.maxLod = 0.0f;
	if (vkCreateSampler(m_info.Device, &samplerInfo, nullptr, &m_textureSampler) != VK_SUCCESS)
	{
		throw std::runtime_error("failed to create texture sampler!");
	}

Even when VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER or VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_EDGE is set, the results are the same (see image above). It seems that sampling is all the time VK_SAMPLER_ADDRESS_MODE_REPEAT.

To create a descriptor set (VkDescriptorSet), I am using:
m_descriptorSet = ImGui_ImplVulkan_AddTexture(m_textureSampler, m_textureImageView, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);

And to add an image:

ImGui::PushID((void*)(intptr_t)m_descriptorSet);
ImGui::GetWindowDrawList()->AddImageQuad(
					(ImTextureID)(m_descriptorSet),
					m_canvasTopLeft,
					m_canvasTopRight,
					m_canvasBottomRight,
					m_canvasBottomLeft,
					m_uv_0,
					m_uv_1,
					m_uv_2,
					m_uv_3,
					IM_COL32_WHITE);
ImGui::PopID();

It is sampling outside of image regions (outside should be with defined color), however, I get repetitive image patterns all the time (similar to repeat sampling mode).

I am wondering if there are any options set by default in the ImGui Vulkan backend that might interfere with this texture sampling (or override it)? Any help and hints are appreciated!

Might be related to #914

@rytisss
Copy link
Author

rytisss commented Jul 23, 2022

Just a note, that same image visualization approach with OpenGL3 work perfectly fine in ImGUI with borders clamping:

image

@ghost
Copy link

ghost commented Aug 13, 2022

Hi, @rytisss @ocornut
I also notify this problem, the mistake is that ImGui Vulkan backend use a common DescriptorSetLayout to create combiner image descriptorset, and when create DescriptorSetLayout, imgui fill font's sampler as a pImmutableSamplers.

But in vulkan spec, when pImmutableSamplers no NULL, pipeline will default use these init sampler and never dynamic switch sampler.

So the resolve method is just keep pImmutableSamplers is NULL like this:
image

@rytisss
Copy link
Author

rytisss commented Aug 13, 2022

Hi @qiutanguu ,
Thank You so much for your comment and suggestion. And indeed it fixes the problem! I commented out the line in imgui_impl_vulkan.cpp as suggested and it is working as expected now with the clamp to boarders!

Line commented (

binding[0].pImmutableSamplers = sampler;
):
image

Result:
image

@rytisss rytisss closed this as completed Aug 13, 2022
@ocornut
Copy link
Owner

ocornut commented Aug 14, 2022

It would be good to find a solution that doesn’t requires making a modification to the backend otherwise you aren’t going to easily update your copy.

this may need to be designed into a backend change.

@rytisss
Copy link
Author

rytisss commented Aug 15, 2022

Indeed, totally agree. This is just a temporary fix. I will reopen the issue.

Maybe someone who is more experienced in Vulkan backend could give a suggestion if it is the proper way of solving this issue.

@rytisss rytisss reopened this Aug 15, 2022
@martin-ejdestig
Copy link
Contributor

I think that is all there is to it, pImmutableSamplers just needs to not be set. 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).

(There are two vkCreateDescriptorSetLayout() calls though. Not sure why. Should be removed in both places, I think.)

@epajarre
Copy link

Just a a note, but I think I also met this problem. In my case have not noticed any visual issues, but when I show buttons with custom Texture in them, Vulkan validation layers will show an error:

VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358(ERROR / SPEC): msgNum: -507995293 - Validation Error: [ VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358 ] Object 0: handle = 0x67d8fc00000f097d, type = VK_OBJECT_TYPE_DESCRIPTOR_SET; | MessageID = 0xe1b89b63 | vkCmdBindDescriptorSets(): descriptorSet #0 being bound is not compatible with overlapping descriptorSetLayout at index 0 of VkPipelineLayout 0xb76ed0000000074b[] due to: Immutable samplers from binding 0 in pipeline layout VkDescriptorSetLayout 0x2193b1000000074a[] do not match the immutable samplers in the layout currently bound (VkDescriptorSetLayout 0xb12fb2000000002c[]). The Vulkan spec states: Each element of pDescriptorSets must have been allocated with a VkDescriptorSetLayout that matches (is the same as, or identically defined as) the VkDescriptorSetLayout at set n in layout, where n is the sum of firstSet and the index into pDescriptorSets (https://vulkan.lunarg.com/doc/view/1.3.224.1/windows/1.3-extensions/vkspec.html#VUID-vkCmdBindDescriptorSets-pDescriptorSets-00358)

Dropping the setting of pImmutableSamplers in backend fixes the validation error, and I have so far not noticed any problems with that.

martin-ejdestig added a commit to martin-ejdestig/imgui that referenced this issue Dec 16, 2022
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 issue Jan 2, 2023
…Samplers, allow changing sampler. (#6001, #5502, #914)

Follow up to c9aef16 which removec three funtions worth of duplicate code.
@ocornut
Copy link
Owner

ocornut commented Jan 2, 2023

Merged fix e5d5186. Thanks a lot @martin-ejdestig and @rytisss!

@ocornut
Copy link
Owner

ocornut commented Oct 7, 2024

I have changed all samplers to default to clamp: 42206b3
See #7230 (comment) for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants