Skip to content

Commit

Permalink
fix layout issues with compute pipeline in 26_Autoexposure example + …
Browse files Browse the repository at this point in the history
…update luma DSes
  • Loading branch information
AnastaZIuk committed Aug 13, 2024
1 parent 3a70977 commit 6addbf1
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 24 deletions.
5 changes: 3 additions & 2 deletions 26_Autoexposure/app_resources/luma_meter.comp.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
#include "nbl/builtin/hlsl/luma_meter/luma_meter.hlsl"
#include "app_resources/common.hlsl"

[[vk::combinedImageSampler]] [[vk::binding(0)]] Texture2D texture;
[[vk::combinedImageSampler]] [[vk::binding(0)]] SamplerState samplerState;
// shared accross frag & compute - binding 0 set 3
[[vk::combinedImageSampler]] [[vk::binding(0, 3)]] Texture2D texture;
[[vk::combinedImageSampler]] [[vk::binding(0, 3)]] SamplerState samplerState;

[[vk::push_constant]] AutoexposurePushData pushData;

Expand Down
1 change: 1 addition & 0 deletions 26_Autoexposure/app_resources/present.frag.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <nbl/builtin/hlsl/ext/FullScreenTriangle/SVertexAttributes.hlsl>
using namespace nbl::hlsl::ext::FullScreenTriangle;

// shared accross frag & compute - binding 0 set 3
[[vk::combinedImageSampler]] [[vk::binding(0, 3)]] Texture2D texture;
[[vk::combinedImageSampler]] [[vk::binding(0, 3)]] SamplerState samplerState;

Expand Down
37 changes: 15 additions & 22 deletions 26_Autoexposure/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,15 @@ class AutoexposureApp final : public examples::SimpleWindowedApplication, public

// create the descriptor sets and with enough room
{
constexpr uint32_t lumaPresentSetCount = 2, tonemapperSetCount = 1;
auto lumaPresentPool = m_device->createDescriptorPoolForDSLayouts(
IDescriptorPool::E_CREATE_FLAGS::ECF_NONE,
{ &lumaPresentDSLayout.get(), 1 },
&lumaPresentSetCount
);
constexpr uint32_t tonemapperSetCount = 1;

core::smart_refctd_ptr<IDescriptorPool> lumaPresentPool;
{
const video::IGPUDescriptorSetLayout* const layouts[] = { nullptr, nullptr, nullptr, lumaPresentDSLayout.get() };
const uint32_t setCounts[] = { 0u, 0u, 0u, 1u }; // leaving you one for 3th set, but you can increase if you really want 2 separate DSs but I think you want single to be shared (then you also need to create 2 DSes as you did)
lumaPresentPool = m_device->createDescriptorPoolForDSLayouts(IDescriptorPool::E_CREATE_FLAGS::ECF_NONE, layouts, setCounts);
}

auto tonemapperPool = m_device->createDescriptorPoolForDSLayouts(
IDescriptorPool::E_CREATE_FLAGS::ECF_NONE,
{ &tonemapperDSLayout.get(), 1 },
Expand All @@ -136,9 +139,9 @@ class AutoexposureApp final : public examples::SimpleWindowedApplication, public
if (!lumaPresentPool || !tonemapperPool)
return logFail("Failed to Create Descriptor Pools");

// why do you need 2 separate DSs for combined sampler? from stage flags it looks like you want them shared between compute & fragment
m_lumaPresentDS[0] = lumaPresentPool->createDescriptorSet(core::smart_refctd_ptr(lumaPresentDSLayout));
m_lumaPresentDS[1] = lumaPresentPool->createDescriptorSet(core::smart_refctd_ptr(lumaPresentDSLayout));
if (!m_lumaPresentDS[0] || !m_lumaPresentDS[1])
if (!m_lumaPresentDS[0])
return logFail("Could not create Descriptor Set: lumaPresentDS!");
m_tonemapperDS[0] = tonemapperPool->createDescriptorSet(core::smart_refctd_ptr(tonemapperDSLayout));
if (!m_tonemapperDS[0])
Expand Down Expand Up @@ -228,7 +231,8 @@ class AutoexposureApp final : public examples::SimpleWindowedApplication, public

smart_refctd_ptr<IGPUPipelineLayout> layout;
{
layout = m_device->createPipelineLayout({ &pcRange,1 });
layout = m_device->createPipelineLayout({ &pcRange,1 }, nullptr, nullptr, nullptr, core::smart_refctd_ptr(lumaPresentDSLayout)); // dont forget your compute uses combinedImageSampler, cause of your cmd buffer errors is here

IGPUComputePipeline::SCreationParams params = {};
params.layout = layout.get();
params.shader.shader = shader.get();
Expand Down Expand Up @@ -483,28 +487,17 @@ class AutoexposureApp final : public examples::SimpleWindowedApplication, public
info1.info.image.imageLayout = IImage::LAYOUT::READ_ONLY_OPTIMAL;
info1.desc = m_gpuImgView;

IGPUDescriptorSet::SDescriptorInfo info2 = {};
info2.info.image.imageLayout = IImage::LAYOUT::READ_ONLY_OPTIMAL;
info2.desc = m_gpuImgView;

IGPUDescriptorSet::SWriteDescriptorSet writeDescriptors[] = {
{
.dstSet = m_lumaPresentDS[0].get(),
.binding = 0,
.arrayElement = 0,
.count = 1,
.info = &info1
},
{
.dstSet = m_lumaPresentDS[1].get(),
.binding = 0,
.arrayElement = 0,
.count = 1,
.info = &info2
}
};

m_device->updateDescriptorSets(2, writeDescriptors, 0, nullptr);
m_device->updateDescriptorSets(1, writeDescriptors, 0, nullptr);

queue->endCapture();
}
Expand Down Expand Up @@ -533,7 +526,7 @@ class AutoexposureApp final : public examples::SimpleWindowedApplication, public
cmdbuf->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT);

cmdbuf->bindComputePipeline(m_lumaMeterPipeline.get());
cmdbuf->bindDescriptorSets(nbl::asset::EPBP_GRAPHICS, m_lumaMeterPipeline->getLayout(), 0, 1, &ds);
cmdbuf->bindDescriptorSets(nbl::asset::EPBP_GRAPHICS, m_lumaMeterPipeline->getLayout(), 3, 1, &ds); // also if you created DS Set with 3th index you need to respect it here - firstSet tells you the index of set and count tells you what range from this index it should update, useful if you had 2 DS with lets say set index 2,3, then you can bind both with single call setting firstSet to 2, count to 2 and last argument would be pointet to your DS pointers

This comment has been minimized.

Copy link
@AnastaZIuk

AnastaZIuk Aug 13, 2024

Author Member

pointer not pointet, typo

cmdbuf->dispatch(1 + (SampleCount[0] - 1) / SubgroupSize, 1 + (SampleCount[1] - 1) / SubgroupSize);
cmdbuf->end();
}
Expand Down

0 comments on commit 6addbf1

Please sign in to comment.