-
Notifications
You must be signed in to change notification settings - Fork 17
Autoexposure example restoration #137
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
Open
devshgraphicsprogramming
wants to merge
100
commits into
master
Choose a base branch
from
autoexposure_ex
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 76 commits
Commits
Show all changes
100 commits
Select commit
Hold shift + click to select a range
345ac7b
Add 26_Autoexposure
nipunG314 87d4794
Change 26_Autoexposure to SimpleWindowedApplication
nipunG314 7a5ea7c
Build a staging buffer and upload exr image
nipunG314 5026a64
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 5d63d04
Init surface and create the swapchain
nipunG314 640e6a3
Load shaders and create the pipeline for full screen triagnle
nipunG314 0148f60
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 d69a111
Set window size according to loaded image
nipunG314 54bf38f
Stop running if window is closed
nipunG314 461efd3
Acquire swapchain image and present uploaded image to it
nipunG314 734fea9
Set window size directly and use that for swapchain rendering
nipunG314 fc9b0bb
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 4a11724
m_computeSubgroupSize
nipunG314 734b887
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 7d4895a
Allocate buffer for gathered luma values
nipunG314 0e3e125
Create gpu resources for all passes
nipunG314 cef80b3
Create shaders and pipelines
nipunG314 15e489f
Allocate and create texture for tonemapping
nipunG314 c646c7d
Create separate ds for luma and present
nipunG314 36d7097
Record luma meter commands
nipunG314 3a70977
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 6addbf1
fix layout issues with compute pipeline in 26_Autoexposure example + …
AnastaZIuk 8434f20
Create two sets from common lumaPresentLayout correctly
nipunG314 bf08caa
Create compute and graphics resources separately and finish luma meter
nipunG314 b342c6c
Fix descriptor binding for luma_meter
nipunG314 817c4a7
Create separate pipeline layouts for luma and present
nipunG314 7f89542
Setup luma_meter.comp.hlsl
nipunG314 defd45e
Pass push constants
nipunG314 f6f8154
Record draw pass correctly
nipunG314 64eb610
Add a pipeline barrier to transition image layout
nipunG314 3d3d646
Record tonemapping pass
nipunG314 b4102dc
Revert "Record tonemapping pass"
nipunG314 8307e92
Remove separate tonemapping pass
nipunG314 7b5ca05
Compute final EV value on CPU
nipunG314 edbf8d1
Compute EV correctly and tonemap in fragment shader
nipunG314 dca49d2
Separate LumaMeteringWindow into a common header
nipunG314 9e28395
Simplify luma_meter naming
nipunG314 8b6675b
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 18fae9f
Update luma examples to shared accessor api
nipunG314 9b31c2c
Refactor tonemapping operators
nipunG314 e987452
Simplify push constants and remove explicit sample counts
nipunG314 e135e43
Infer sample count from viewportSize and simplify userspace HLSL
nipunG314 57e49ae
Templatize float type and add toXYZ method to TexAccessor
nipunG314 f8d50e8
Refactor the example into using a 2-compute, 1-fragment architecture
nipunG314 d3b5765
Handle image layouts correctly
nipunG314 612f0f6
Simplify type
nipunG314 cb46d82
Wait for correct semaphore value
nipunG314 1996cf3
Remove unnecessary data members
nipunG314 36633f5
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 bc11b4a
Use asset converter for images and descriptors
nipunG314 f79caed
Merge branch 'master' of github.com:Devsh-Graphics-Programming/Nabla-…
nipunG314 3a94cd4
Rewrite descriptor set logic
nipunG314 462e220
Replace dot with mul
nipunG314 9e26a74
Replace combined image sampler with RWTexture2D
nipunG314 208a58a
use a single asset converter throughout, always call `convert` to mak…
devshgraphicsprogramming 10b6690
Transition m_tonemappedImgView to GENERAL
nipunG314 21995ea
Keep direct track of m_gatherBuffer
nipunG314 06dad8c
yay another DXC bug that was an absolute joy to debug, why on earth w…
devshgraphicsprogramming dc457ca
Create a Texture2D descriptor for the fragment shader
nipunG314 e7c4160
Add median metering mode
nipunG314 3eb06b7
Use correct reinhard parameters
nipunG314 498ffd2
Fix autoexposure sample
nipunG314 2949212
merge master, fix conflicts
keptsecret 1d82130
fixes missing shader type attribute, added check for required defines
keptsecret 75c7530
fix shader compilation access violation
keptsecret 68dfd1b
Merge branch 'master' into autoexposure_ex
keptsecret a9762cd
refactor morton usage
keptsecret 570fc93
refactor luma_meter and tonemap ops reorganization
keptsecret e07df94
Merge branch 'master' into autoexposure_ex
keptsecret ea2b1b9
use new morton class
keptsecret e3f1d35
fixes (+removals) to make it work like old glsl version, use correct …
keptsecret 7680a95
Merge branch 'master' into autoexposure_ex
keptsecret 42ed25a
fixes to get it to compile after merge from master + old bugs in median
keptsecret a15508e
fixes histogram autoexposure
keptsecret 9c330d3
clean up push constant usage, more values in push constant
keptsecret 233db45
rename defines to all caps, refactor luma meter template names
keptsecret 80fd063
Merge branch 'master' into autoexposure_ex
keptsecret de38929
fix change from master that broke
keptsecret ea585f5
added temporal adaptation
keptsecret e1d1cb1
minor fixes
keptsecret 9f86c09
refactor to use workgroup2 stuff in cpp and average metering
keptsecret 5e27920
merge master, fix conflicts
keptsecret 7c5d3b4
refactor to use workgroup2 with histogram metering, some more defines…
keptsecret d46e463
fix calculating sampling area uv for luma metering
keptsecret 77ec3d5
check range on tonemap output
keptsecret 59117a8
small fix to oob bug
keptsecret d104945
refactor uv fix, minor bug fixes
keptsecret 986492e
removed semantically incorrect naming of gl_WorkgroupSize()
keptsecret 07ad5db
added in temp dithering factor, fix usage of histogram metering perce…
keptsecret 1e012c0
ping pong buffer for last frame EV, removed tonemap output oetf becau…
keptsecret df7f7ed
removed sampleCount param (unused), made subgroup size member
keptsecret 8b93ee5
implement frames in flight
keptsecret 0804333
add eotf, removed fake dither factor
keptsecret f6b1b7d
use median + aces
keptsecret 27ff451
made tonemapped image same size as input img
keptsecret 9482f15
moved push constant out to nabla
keptsecret ca0e50e
refactor minor changes to luma meter
keptsecret eebde78
ex 40 works now
devshgraphicsprogramming 6f67226
Merge branch 'master' into autoexposure_ex
keptsecret 6286a69
Merge commit 'eebde787c233367ade8eb0580bc79c0d562e97aa' into autoexpo…
keptsecret File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
|
|
||
| include(common RESULT_VARIABLE RES) | ||
| if(NOT RES) | ||
| message(FATAL_ERROR "common.cmake not found. Should be in {repo_root}/cmake directory") | ||
| endif() | ||
|
|
||
| set(NBL_INCLUDE_SERACH_DIRECTORIES | ||
| "${CMAKE_CURRENT_SOURCE_DIR}/include" | ||
| ) | ||
|
|
||
| list(APPEND NBL_LIBRARIES | ||
| Nabla::ext::FullScreenTriangle | ||
| ) | ||
|
|
||
| nbl_create_executable_project("" "" "${NBL_INCLUDE_SERACH_DIRECTORIES}" "${NBL_LIBRARIES}" "${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() |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // 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/geom_mean.hlsl" | ||
| #include "nbl/builtin/hlsl/bda/bda_accessor.hlsl" | ||
| #include "nbl/builtin/hlsl/colorspace/encodeCIEXYZ.hlsl" | ||
| #include "app_resources/common.hlsl" | ||
|
|
||
| [[vk::combinedImageSampler]] [[vk::binding(0, 0)]] Texture2D texture; | ||
| [[vk::combinedImageSampler]] [[vk::binding(0, 0)]] SamplerState samplerState; | ||
|
|
||
| using namespace nbl::hlsl; | ||
| using Ptr = bda::__ptr < uint32_t >; | ||
| using PtrAccessor = BdaAccessor < uint32_t >; | ||
|
|
||
| [[vk::push_constant]] AutoexposurePushData pushData; | ||
|
|
||
| groupshared float32_t sdata[WORKGROUP_SIZE]; | ||
| struct SharedAccessor | ||
| { | ||
| using type = float32_t; | ||
| void get(const uint32_t index, NBL_REF_ARG(uint32_t) value) | ||
| { | ||
| value = sdata[index]; | ||
| } | ||
|
|
||
| void set(const uint32_t index, const uint32_t value) | ||
| { | ||
| sdata[index] = value; | ||
| } | ||
|
|
||
| void workgroupExecutionAndMemoryBarrier() | ||
| { | ||
| glsl::barrier(); | ||
| } | ||
| }; | ||
|
|
||
| struct TexAccessor | ||
| { | ||
| static float32_t3 toXYZ(float32_t3 srgbColor) { | ||
| return dot(colorspace::sRGBtoXYZ[1], srgbColor); | ||
| } | ||
|
|
||
| float32_t3 get(float32_t2 uv) { | ||
| return texture.SampleLevel(samplerState, uv, 0.f).rgb; | ||
| } | ||
| }; | ||
|
|
||
| uint32_t3 glsl::gl_WorkGroupSize() | ||
| { | ||
| return uint32_t3(SUBGROUP_SIZE, SUBGROUP_SIZE, 1); | ||
| } | ||
|
|
||
| [numthreads(SUBGROUP_SIZE, SUBGROUP_SIZE, 1)] | ||
| [shader("compute")] | ||
| void main(uint32_t3 ID : SV_GroupThreadID, uint32_t3 GroupID : SV_GroupID) | ||
| { | ||
| const Ptr val_ptr = Ptr::create(pushData.lumaMeterBDA); | ||
| PtrAccessor val_accessor = PtrAccessor::create(val_ptr); | ||
|
|
||
| SharedAccessor sdata; | ||
| TexAccessor tex; | ||
|
|
||
| using LumaMeter = luma_meter::geom_meter< WORKGROUP_SIZE, SUBGROUP_SIZE, PtrAccessor, SharedAccessor, TexAccessor>; | ||
| LumaMeter meter = LumaMeter::create(pushData.lumaMin, pushData.lumaMax, pushData.sampleCount, pushData.rcpFirstPassWGCount); | ||
|
|
||
| uint32_t texWidth, texHeight; | ||
| texture.GetDimensions(texWidth, texHeight); | ||
| meter.sampleLuma(pushData.window, val_accessor, tex, sdata, (float32_t2)(glsl::gl_WorkGroupID() * glsl::gl_WorkGroupSize()), float32_t2(texWidth, texHeight)); | ||
| } |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // 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/geom_mean.hlsl" | ||
| #include "nbl/builtin/hlsl/bda/bda_accessor.hlsl" | ||
| #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/reinhard.hlsl" | ||
| #include "app_resources/common.hlsl" | ||
|
|
||
| [[vk::combinedImageSampler]] [[vk::binding(0, 0)]] Texture2D textureIn; | ||
| [[vk::combinedImageSampler]] [[vk::binding(0, 0)]] SamplerState samplerStateIn; | ||
| [[vk::binding(0, 3)]] RWTexture2D<float32_t4> textureOut; | ||
|
|
||
| using namespace nbl::hlsl; | ||
| using Ptr = bda::__ptr < uint32_t >; | ||
| using PtrAccessor = BdaAccessor < uint32_t >; | ||
|
|
||
| [[vk::push_constant]] AutoexposurePushData pushData; | ||
|
|
||
| groupshared float32_t sdata[WORKGROUP_SIZE]; | ||
| struct SharedAccessor | ||
| { | ||
| using type = float32_t; | ||
| void get(const uint32_t index, NBL_REF_ARG(uint32_t) value) | ||
| { | ||
| value = sdata[index]; | ||
| } | ||
|
|
||
| void set(const uint32_t index, const uint32_t value) | ||
| { | ||
| sdata[index] = value; | ||
| } | ||
|
|
||
| void workgroupExecutionAndMemoryBarrier() | ||
| { | ||
| glsl::barrier(); | ||
| } | ||
| }; | ||
|
|
||
| struct TexAccessor | ||
| { | ||
| static float32_t3 toXYZ(float32_t3 srgbColor) { | ||
| return dot(colorspace::sRGBtoXYZ[1], srgbColor); | ||
| } | ||
|
|
||
| float32_t3 get(float32_t2 uv) { | ||
| return textureIn.SampleLevel(samplerStateIn, uv, 0.f).rgb; | ||
| } | ||
| }; | ||
|
|
||
| uint32_t3 glsl::gl_WorkGroupSize() | ||
| { | ||
| return uint32_t3(SUBGROUP_SIZE, SUBGROUP_SIZE, 1); | ||
| } | ||
|
|
||
| [numthreads(SUBGROUP_SIZE, SUBGROUP_SIZE, 1)] | ||
| [shader("compute")] | ||
| void main(uint32_t3 ID : SV_GroupThreadID, uint32_t3 GroupID : SV_GroupID) | ||
| { | ||
| const Ptr val_ptr = Ptr::create(pushData.lumaMeterBDA); | ||
| PtrAccessor val_accessor = PtrAccessor::create(val_ptr); | ||
|
|
||
| SharedAccessor sdata; | ||
| TexAccessor tex; | ||
|
|
||
| using LumaMeter = luma_meter::geom_meter< WORKGROUP_SIZE, SUBGROUP_SIZE, PtrAccessor, SharedAccessor, TexAccessor>; | ||
| LumaMeter meter = LumaMeter::create(pushData.lumaMin, pushData.lumaMax, pushData.sampleCount, pushData.rcpFirstPassWGCount); | ||
|
|
||
| float32_t EV = meter.gatherLuma(val_accessor); | ||
|
|
||
| uint32_t tid = workgroup::SubgroupContiguousIndex(); | ||
| morton::code<false, 32, 2> mc; | ||
| mc.value = tid; | ||
| uint32_t2 coord = _static_cast<uint32_t2>(mc); | ||
|
|
||
| uint32_t2 pos = (glsl::gl_WorkGroupID() * glsl::gl_WorkGroupSize()).xy + coord; | ||
| float32_t2 uv = (float32_t2)(pos) / pushData.viewportSize; | ||
| float32_t3 color = colorspace::eotf::sRGB(tex.get(uv).rgb); | ||
| float32_t3 CIEColor = mul(colorspace::sRGBtoXYZ, color); | ||
| tonemapper::Reinhard<float32_t> reinhard = tonemapper::Reinhard<float32_t>::create(EV, 0.18f, 0.85f); | ||
| float32_t3 tonemappedColor = mul(colorspace::decode::XYZtoscRGB, reinhard(CIEColor)); | ||
|
|
||
| textureOut[pos] = float32_t4(colorspace::oetf::sRGB(tonemappedColor), 1.0f); | ||
| } | ||
Oops, something went wrong.
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.
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.
No bounds guard and we have OOB write. We go 1280x720 and dispatch is
ceil(Dimensions/SubgroupSize). WithSubgroupSize32 it is 1280x736 coverageNabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 46 in 233db45
Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Lines 841 to 844 in 233db45
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.
resolved
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.
I'm not sure about it, could you please confirm the expected metering region is$uv\in[0.1,0.9]^2$ ? If so then with current inputs
Dimensions=(1280,720)Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 47 in 59117a8
MeteringMinUV=(0.1,0.1)Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 48 in 59117a8
MeteringMaxUV=(0.9,0.9)Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 49 in 59117a8
SamplingFactor=2Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 50 in 59117a8
SubgroupSize=m_physicalDevice->getLimits().subgroupSizeNabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 791 in 59117a8
and flow like
meteringUVRange = MeteringMaxUV - MeteringMinUVNabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 821 in 59117a8
dispatchSize = ceil(dim * meteringUVRange / (SubgroupSize * SamplingFactor))Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 827 in 59117a8
window = create(meteringUVRange / (dispatchSize * SubgroupSize), MeteringMinUV)Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 829 in 59117a8
sampleLuma(..., tileOffset=WorkGroupID*WorkGroupSize, viewportSize=(texWidth,texHeight))Nabla-Examples-and-Tests/26_Autoexposure/app_resources/avg_luma_meter.comp.hlsl
Line 71 in 59117a8
shiftedCoord = (tileOffset + coord) / viewportSizehttps://github.com/Devsh-Graphics-Programming/Nabla/blob/79fad2f7d16ba75347c94a8b4327042b4d786547/include/nbl/builtin/hlsl/luma_meter/geom_mean.hlsl#L126
uvPos = shiftedCoord * window.meteringWindowScale + window.meteringWindowOffsethttps://github.com/Devsh-Graphics-Programming/Nabla/blob/79fad2f7d16ba75347c94a8b4327042b4d786547/include/nbl/builtin/hlsl/luma_meter/geom_mean.hlsl#L77
we have
dispatchSize=(16,9)windowScale=(0.8/(16*32),0.8/(9*32))=(0.0015625,0.002777...)(tileOffset+coord)spans[0,511]x[0,287]and then sampled UV is approximately
this is a tiny rectangle near
(0.1,0.1)not the intended (I guess) metering window because the coordinates are effectively normalized twice, first byviewportSizeinshiftedCoord = (tileOffset + coord) / viewportSizeand then scaled again bywindowScale = meteringUVRange / (dispatchSize * SubgroupSize).https://github.com/Devsh-Graphics-Programming/Nabla/blob/79fad2f7d16ba75347c94a8b4327042b4d786547/include/nbl/builtin/hlsl/luma_meter/geom_mean.hlsl#L126
Nabla-Examples-and-Tests/26_Autoexposure/main.cpp
Line 829 in 59117a8
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.
resolved