-
Notifications
You must be signed in to change notification settings - Fork 12
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
base: master
Are you sure you want to change the base?
Changes from 35 commits
345ac7b
87d4794
7a5ea7c
5026a64
5d63d04
640e6a3
0148f60
d69a111
54bf38f
461efd3
734fea9
fc9b0bb
4a11724
734b887
7d4895a
0e3e125
cef80b3
15e489f
c646c7d
36d7097
3a70977
6addbf1
8434f20
bf08caa
b342c6c
817c4a7
7f89542
defd45e
f6f8154
64eb610
3d3d646
b4102dc
8307e92
7b5ca05
edbf8d1
dca49d2
9e28395
8b6675b
18fae9f
9b31c2c
e987452
e135e43
57e49ae
f8d50e8
d3b5765
612f0f6
cb46d82
1996cf3
36633f5
bc11b4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
|
||
include(common RESULT_VARIABLE RES) | ||
if(NOT RES) | ||
message(FATAL_ERROR "common.cmake not found. Should be in {repo_root}/cmake directory") | ||
endif() | ||
|
||
nbl_create_executable_project("" "" "" "" "${NBL_EXECUTABLE_PROJECT_CREATION_PCH_TARGET}") | ||
|
||
if(NBL_EMBED_BUILTIN_RESOURCES) | ||
set(_BR_TARGET_ ${EXECUTABLE_NAME}_builtinResourceData) | ||
set(RESOURCE_DIR "app_resources") | ||
|
||
get_filename_component(_SEARCH_DIRECTORIES_ "${CMAKE_CURRENT_SOURCE_DIR}" ABSOLUTE) | ||
get_filename_component(_OUTPUT_DIRECTORY_SOURCE_ "${CMAKE_CURRENT_BINARY_DIR}/src" ABSOLUTE) | ||
get_filename_component(_OUTPUT_DIRECTORY_HEADER_ "${CMAKE_CURRENT_BINARY_DIR}/include" ABSOLUTE) | ||
|
||
file(GLOB_RECURSE BUILTIN_RESOURCE_FILES RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}/${RESOURCE_DIR}" CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/${RESOURCE_DIR}/*") | ||
foreach(RES_FILE ${BUILTIN_RESOURCE_FILES}) | ||
LIST_BUILTIN_RESOURCE(RESOURCES_TO_EMBED "${RES_FILE}") | ||
endforeach() | ||
|
||
ADD_CUSTOM_BUILTIN_RESOURCES(${_BR_TARGET_} RESOURCES_TO_EMBED "${_SEARCH_DIRECTORIES_}" "${RESOURCE_DIR}" "nbl::this_example::builtin" "${_OUTPUT_DIRECTORY_HEADER_}" "${_OUTPUT_DIRECTORY_SOURCE_}") | ||
|
||
LINK_BUILTIN_RESOURCES_TO_TARGET(${EXECUTABLE_NAME} ${_BR_TARGET_}) | ||
endif() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Copyright (C) 2018-2024 - DevSH Graphics Programming Sp. z O.O. | ||
// This file is part of the "Nabla Engine". | ||
// For conditions of distribution and use, see copyright notice in nabla.h | ||
|
||
#ifndef _AUTOEXPOSURE_COMMON_INCLUDED_ | ||
#define _AUTOEXPOSURE_COMMON_INCLUDED_ | ||
|
||
#include "nbl/builtin/hlsl/cpp_compat.hlsl" | ||
|
||
struct AutoexposurePushData | ||
{ | ||
float meteringWindowScaleX, meteringWindowScaleY; | ||
float meteringWindowOffsetX, meteringWindowOffsetY; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't realize earlier that |
||
float lumaMin, lumaMax; | ||
float EV; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EV bias! not just EV |
||
uint32_t sampleCountX, sampleCountY; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you don't need an X and Y sample count you just need an overall one |
||
uint32_t viewportSizeX, viewportSizeY; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you need viewport sizes for ? |
||
uint64_t lumaMeterBDA; | ||
}; | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// Copyright (C) 2024-2024 - DevSH Graphics Programming Sp. z O.O. | ||
// This file is part of the "Nabla Engine". | ||
// For conditions of distribution and use, see copyright notice in nabla.h | ||
|
||
#include "nbl/builtin/hlsl/luma_meter/luma_meter.hlsl" | ||
#include "nbl/builtin/hlsl/bda/bda_accessor.hlsl" | ||
#include "app_resources/common.hlsl" | ||
|
||
[[vk::combinedImageSampler]] [[vk::binding(0, 0)]] Texture2D texture; | ||
[[vk::combinedImageSampler]] [[vk::binding(0, 0)]] SamplerState samplerState; | ||
|
||
[[vk::push_constant]] AutoexposurePushData pushData; | ||
|
||
using Ptr = nbl::hlsl::bda::__ptr < uint32_t >; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pro tip, you can actually put `using namespace nbl::hlsl; in your "main" hlsl files (the ones with entry points) |
||
using PtrAccessor = nbl::hlsl::BdaAccessor < uint32_t >; | ||
|
||
groupshared float32_t sdata[WorkgroupSize]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are you tying histogram size to WorkgroupSize ? |
||
|
||
struct SharedAccessor | ||
{ | ||
uint32_t get(const uint32_t index) | ||
{ | ||
return sdata[index]; | ||
} | ||
|
||
void set(const uint32_t index, const uint32_t value) | ||
{ | ||
sdata[index] = value; | ||
} | ||
|
||
void workgroupExecutionAndMemoryBarrier() | ||
{ | ||
nbl::hlsl::glsl::barrier(); | ||
} | ||
}; | ||
|
||
struct TexAccessor | ||
{ | ||
float32_t3 get(float32_t2 uv) { | ||
return texture.Sample(samplerState, uv).rgb; | ||
} | ||
Comment on lines
+45
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because the TexAcessor is supposed to "know" the colorspace of the input image, I'd rather have it return |
||
}; | ||
|
||
uint32_t3 nbl::hlsl::glsl::gl_WorkGroupSize() | ||
{ | ||
return uint32_t3(WorkgroupSize, 1, 1); | ||
} | ||
|
||
[numthreads(DeviceSubgroupSize, DeviceSubgroupSize, 1)] | ||
void main(uint32_t3 ID : SV_GroupThreadID, uint32_t3 GroupID : SV_GroupID) | ||
{ | ||
nbl::hlsl::luma_meter::LumaMeteringWindow luma_meter_window; | ||
luma_meter_window.meteringWindowScale = float32_t2(pushData.meteringWindowScaleX, pushData.meteringWindowScaleY); | ||
luma_meter_window.meteringWindowOffset = float32_t2(pushData.meteringWindowOffsetX, pushData.meteringWindowOffsetY); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should be using |
||
|
||
const Ptr val_ptr = Ptr::create(pushData.lumaMeterBDA); | ||
PtrAccessor val_accessor = PtrAccessor::create(val_ptr); | ||
|
||
SharedAccessor sdata; | ||
TexAccessor tex; | ||
|
||
using LumaMeter = nbl::hlsl::luma_meter::geom_luma_meter< WorkgroupSize, PtrAccessor, SharedAccessor, TexAccessor>; | ||
LumaMeter meter = LumaMeter::create(luma_meter_window, pushData.lumaMin, pushData.lumaMax); | ||
|
||
uint32_t2 sampleCount = uint32_t2(pushData.sampleCountX, pushData.sampleCountY); | ||
uint32_t2 viewportSize = uint32_t2(pushData.viewportSizeX, pushData.viewportSizeY); | ||
|
||
meter.gatherLuma(val_accessor, tex, sdata, sampleCount, viewportSize); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// Copyright (C) 2024-2024 - DevSH Graphics Programming Sp. z O.O. | ||
// This file is part of the "Nabla Engine". | ||
// For conditions of distribution and use, see copyright notice in nabla.h | ||
|
||
#pragma wave shader_stage(fragment) | ||
|
||
#include "nbl/builtin/hlsl/colorspace/EOTF.hlsl" | ||
#include "nbl/builtin/hlsl/colorspace/encodeCIEXYZ.hlsl" | ||
#include "nbl/builtin/hlsl/colorspace/decodeCIEXYZ.hlsl" | ||
#include "nbl/builtin/hlsl/colorspace/OETF.hlsl" | ||
#include "nbl/builtin/hlsl/tonemapper/operators.hlsl" | ||
#include "app_resources/common.hlsl" | ||
|
||
// vertex shader is provided by the fullScreenTriangle extension | ||
#include <nbl/builtin/hlsl/ext/FullScreenTriangle/SVertexAttributes.hlsl> | ||
using namespace nbl::hlsl::ext::FullScreenTriangle; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. slap more |
||
|
||
// binding 0 set 1 | ||
[[vk::combinedImageSampler]] [[vk::binding(0, 1)]] Texture2D texture; | ||
[[vk::combinedImageSampler]] [[vk::binding(0, 1)]] SamplerState samplerState; | ||
|
||
[[vk::push_constant]] AutoexposurePushData pushData; | ||
|
||
[[vk::location(0)]] float32_t4 main(SVertexAttributes vxAttr) : SV_Target0 | ||
{ | ||
float32_t3 color = nbl::hlsl::colorspace::oetf::sRGB(texture.Sample(samplerState, vxAttr.uv).rgb); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the texture view you bound to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P.S. also it would have been eotf not oetf IIRC |
||
float32_t3 CIEColor = mul(nbl::hlsl::colorspace::sRGBtoXYZ, color); | ||
|
||
nbl::hlsl::tonemapper::ReinhardParams params = nbl::hlsl::tonemapper::ReinhardParams::create(pushData.EV); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're not supposed to get the EV value from a push constant, this is why the OLD GLSL code used compute to do the tonemapping, because workgroups were cooperatively used to get the mid-gray value |
||
|
||
float32_t3 tonemappedColor = mul(nbl::hlsl::colorspace::decode::XYZtoscRGB, nbl::hlsl::tonemapper::reinhard(params, CIEColor)); | ||
|
||
return float32_t4(nbl::hlsl::colorspace::eotf::sRGB(tonemappedColor), 1.0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, if you're rendering to a framebuffer that has an sRGB format attachment, you don't need an oetf. You'd only need it if doign an |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't a struct for push constants shared between your Luma Meter and Tonemapper supposed to be in the extension headers?