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

Autoexposure example restoration #137

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

Conversation

devshgraphicsprogramming
Copy link
Member

No description provided.

nipunG314 and others added 28 commits July 19, 2024 22:07
Comment on lines 12 to 17
float meteringWindowScaleX, meteringWindowScaleY;
float meteringWindowOffsetX, meteringWindowOffsetY;
float lumaMin, lumaMax;
uint32_t sampleCountX, sampleCountY;
uint32_t viewportSizeX, viewportSizeY;
uint64_t lumaMeterBDA;
Copy link
Member Author

Choose a reason for hiding this comment

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

you should pack these up intro structs in nbl::hlsl::luma_meter and friends

P.S. I see you already have, why not use things like nbl::hlsl::luma_meter::LumaMeteringWindow directly here?

Comment on lines 586 to 588
auto queue = getGraphicsQueue();
auto cmdbuf = m_graphicsCmdBufs[0].get();
cmdbuf->reset(IGPUCommandBuffer::RESET_FLAGS::NONE);
Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +658 to +663
// we don't need to wait for the transfer semaphore, because we submit everything to the same queue
const IQueue::SSubmitInfo::SSemaphoreInfo acquired[1] = { {
.semaphore = acquire.semaphore,
.value = acquire.acquireCount,
.stageMask = PIPELINE_STAGE_FLAGS::NONE
} };
Copy link
Member Author

Choose a reason for hiding this comment

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

you need to wait for the compute queue semaphore (from the luma metering) as well as the acquire

because eventually you'll compute the EV in the tonemapping pipeline and stop doing any host wait on the compute queue.

Comment on lines 677 to 687
// Wait for completion
{
const ISemaphore::SWaitInfo cmdbufDonePending[] = {
{
.semaphore = m_presentSemaphore.get(),
.value = m_submitIx
}
};
if (m_device->blockForSemaphores(cmdbufDonePending) != ISemaphore::WAIT_RESULT::SUCCESS)
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

time to remove this and do Frames in Flight

constexpr float Key = 0.18;
auto params = ToneMapperClass::Params_t<TMO>(Exposure, Key, 0.85f);
{
params.setAdaptationFactorFromFrameDelta(0.f);
Copy link
Member Author

Choose a reason for hiding this comment

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

we used this basically
https://en.wikipedia.org/wiki/Exponential_smoothing

to blend the EV value over time

Copy link
Member Author

Choose a reason for hiding this comment

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

0 was a special value to "reset" the exposure (allow it to use the instantaneous value)

@devshgraphicsprogramming
Copy link
Member Author

Whats the status on this?

@devshgraphicsprogramming
Copy link
Member Author

Before merge this needs to become example 27 or some other free number

Comment on lines +583 to +649
// transition m_tonemappedImgView to GENERAL
{
auto transitionSemaphore = m_device->createSemaphore(0);
auto queue = getGraphicsQueue();
auto cmdbuf = m_cmdBufs[0].get();
cmdbuf->reset(IGPUCommandBuffer::RESET_FLAGS::NONE);

m_api->startCapture();

cmdbuf->begin(IGPUCommandBuffer::USAGE::ONE_TIME_SUBMIT_BIT);

// TRANSITION m_outImgView to GENERAL (because of descriptorSets0 -> ComputeShader Writes into the image)
{
const IGPUCommandBuffer::SImageMemoryBarrier<IGPUCommandBuffer::SOwnershipTransferBarrier> imgBarriers[] = {
{
.barrier = {
.dep = {
.srcStageMask = PIPELINE_STAGE_FLAGS::NONE,
.dstStageMask = PIPELINE_STAGE_FLAGS::ALL_COMMANDS_BITS,
}
},
.image = m_tonemappedImgView->getCreationParameters().image.get(),
.subresourceRange = {
.aspectMask = IImage::EAF_COLOR_BIT,
.baseMipLevel = 0u,
.levelCount = 1u,
.baseArrayLayer = 0u,
.layerCount = 1u
},
.oldLayout = IImage::LAYOUT::UNDEFINED,
.newLayout = IImage::LAYOUT::GENERAL
}
};
cmdbuf->pipelineBarrier(E_DEPENDENCY_FLAGS::EDF_NONE, { .imgBarriers = imgBarriers });
}
cmdbuf->end();

const IQueue::SSubmitInfo::SSemaphoreInfo rendered[] =
{
{
.semaphore = transitionSemaphore.get(),
.value = 1,
.stageMask = PIPELINE_STAGE_FLAGS::ALL_COMMANDS_BITS
}
};
const IQueue::SSubmitInfo::SCommandBufferInfo commandBuffers[] =
{
{.cmdbuf = cmdbuf }
};
const IQueue::SSubmitInfo infos[] =
{
{
.waitSemaphores = {},
.commandBuffers = commandBuffers,
.signalSemaphores = rendered
}
};
queue->submit(infos);
const ISemaphore::SWaitInfo waits[] = {
{
.semaphore = transitionSemaphore.get(),
.value = 1
}
};
m_device->blockForSemaphores(waits);
m_api->endCapture();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

btw you could also create the m_tonemappedImgView from an empt ICPUImage using asset converter and override its layout in the callback, that would give you an already transitioned empty image

…ould the SPIR-V legalization pass just decide to kill implicit lod texture sampling operations and just warn? GLSL makes them into explicit lod with implied lod 0.
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.

3 participants