-
Notifications
You must be signed in to change notification settings - Fork 67
Rework environment map importance sampling to vulkan and hlsl #946
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| #ifndef _NBL_BUILTIN_HLSL_CONCEPTS_ACCESSORS_ENVMAP_INCLUDED_ | ||
| #define _NBL_BUILTIN_HLSL_CONCEPTS_ACCESSORS_ENVMAP_INCLUDED_ | ||
|
|
||
| #include "nbl/builtin/hlsl/concepts/accessors/generic_shared_data.hlsl" | ||
|
|
||
| namespace nbl | ||
| { | ||
| namespace hlsl | ||
| { | ||
| namespace workgroup | ||
| { | ||
| namespace envmap | ||
| { | ||
| // declare concept | ||
| #define NBL_CONCEPT_NAME LuminanceReadAccessor | ||
| #define NBL_CONCEPT_TPLT_PRM_KINDS (typename) | ||
| #define NBL_CONCEPT_TPLT_PRM_NAMES (U) | ||
| // not the greatest syntax but works | ||
| #define NBL_CONCEPT_PARAM_0 (a,U) | ||
| #define NBL_CONCEPT_PARAM_1 (uv,uint32_t2) | ||
| #define NBL_CONCEPT_PARAM_2 (level,uint32_t) | ||
| #define NBL_CONCEPT_PARAM_3 (offset,uint32_t2) | ||
| // start concept | ||
| NBL_CONCEPT_BEGIN(4) | ||
| // need to be defined AFTER the concept begins | ||
| #define a NBL_CONCEPT_PARAM_T NBL_CONCEPT_PARAM_0 | ||
| #define uv NBL_CONCEPT_PARAM_T NBL_CONCEPT_PARAM_1 | ||
| #define level NBL_CONCEPT_PARAM_T NBL_CONCEPT_PARAM_2 | ||
| #define offset NBL_CONCEPT_PARAM_T NBL_CONCEPT_PARAM_3 | ||
| NBL_CONCEPT_END( | ||
| ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((a.template get(uv,level,offset)) , ::nbl::hlsl::is_same_v, float32_t4>)) | ||
|
Member
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. this is the same as the make the |
||
| ); | ||
| #undef offset | ||
| #undef level | ||
| #undef uv | ||
| #undef a | ||
| #include <nbl/builtin/hlsl/concepts/__end.hlsl> | ||
|
|
||
| template <typename T> | ||
| NBL_BOOL_CONCEPT WarpmapWriteAccessor = concepts::accessors::GenericWriteAccessor<T, float32_t2, uint32_t2>; | ||
|
|
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
|
|
||
| #ifndef _NBL_BUILTIN_HLSL_WORKGROUP_ENVMAP_IMPORTANCE_SAMPLING_INCLUDED_ | ||
| #define _NBL_BUILTIN_HLSL_WORKGROUP_ENVMAP_IMPORTANCE_SAMPLING_INCLUDED_ | ||
|
|
||
| namespace nbl | ||
| { | ||
| namespace hlsl | ||
| { | ||
| namespace workgroup | ||
| { | ||
| namespace envmap | ||
|
Comment on lines
+9
to
+11
Member
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 in workgroup anything, also
Member
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. actually |
||
| { | ||
| namespace impl | ||
| { | ||
| bool choseSecond(float first, float second, NBL_REF_ARG(float) xi) | ||
| { | ||
| // numerical resilience against IEEE754 | ||
| float firstProb = 1.0f / (1.0f + second / first); | ||
| float dummy = 0.0f; | ||
| return math::partitionRandVariable(firstProb, xi, dummy); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #ifdef __HLSL_VERSION | ||
| namespace nbl | ||
| { | ||
| namespace hlsl | ||
| { | ||
| namespace workgroup | ||
| { | ||
| namespace envmap | ||
| { | ||
|
|
||
| struct WarpmapGeneration | ||
|
Member
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. rename to warpmap just caches the results in a LUT for a grid of |
||
| { | ||
|
|
||
| template <typename LuminanceAccessor, typename OutputAccessor NBL_FUNC_REQUIRES (envmap::LuminanceReadAccessor<LuminanceAccessor> && envmap::WarpmapWriteAccessor<OutputAccessor>) | ||
| // TODO(kevinyu): Should lumapMapSize and warpMapSize provided by Accessor? | ||
|
Member
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. not necessary, this is fine, make the
Member
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. Take the
Member
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. also no write accessor necessary |
||
| static void __call(NBL_CONST_REF_ARG(LuminanceAccessor) luminanceAccessor, NBL_REF_ARG(OutputAcessor) outputAccessor, uint32_t2 lumaMapSize, uint32_t2 warpMapSize) | ||
| { | ||
| const uint32_t threadID = uint32_t(SubgroupContiguousIndex()); | ||
| const uint32_t lastWarpMapPixel = warpMapSize - uint32_t2(1, 1); | ||
|
|
||
| if (all(threadID < warpMapSize)) | ||
| { | ||
| float32_t2 xi = float32_t2(threadID) / float32_t2(lastWarpMapPixel); | ||
|
Comment on lines
+47
to
+52
Member
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. nope, threadID is 1D, AND its in 0,512 There's nothing "workgroup" about this. Take the
Member
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. also when you move the code outside, leave comments about why the |
||
|
|
||
| uint32_t2 p; | ||
| p.y = 0; | ||
|
|
||
| // TODO(kevinyu): Implement findMSB | ||
| const uint32_t2 mip2x1 = findMSB(lumaMapSize.x) - 1; | ||
|
Comment on lines
+57
to
+58
Member
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. maybe ask for the |
||
| // do one split in the X axis first cause penultimate full mip would have been 2x1 | ||
| p.x = impl::choseSecond(luminanceAccessor.get(uint32_t2(0, 0), mip2x1, uint32_t2(0, 0)), luminanceAccessor.get(uint32_t2(0, 0), mip2x1, uint32_t2(1, 0), xi.x) ? 1 : 0; | ||
|
Comment on lines
+59
to
+60
Member
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. to support octahedral maps (which are 1:1 apsect raito) only do this split |
||
| for (uint32_t i = mip2x1; i != 0;) | ||
| { | ||
| --i; | ||
| p <<= 1; | ||
| const float32_t4 values = float32_t4( | ||
| luminanceAccessor.get(p, i, uint32_t2(0, 1)), | ||
| luminanceAccessor.get(p, i, uint32_t2(1, 1)), | ||
| luminanceAccessor.get(p, i, uint32_t2(1, 0)), | ||
| luminanceAccessor.get(p, i, uint32_t2(0, 0)) | ||
| ); | ||
|
Comment on lines
+65
to
+70
Member
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. the |
||
|
|
||
| float32_t wx_0, wx_1; | ||
| { | ||
| const float32_t wy_0 = values[3] + values[2]; | ||
| const float32_t wy_1 = values[1] + values[0]; | ||
| if (impl::choseSecond(wy_0, wy_1, xi.y)) | ||
| { | ||
| p.y |= 1; | ||
| wx_0 = values[0]; | ||
| wx_1 = values[1]; | ||
| } | ||
| else | ||
| { | ||
| wx_0 = values[3]; | ||
| wx_1 = values[2]; | ||
| } | ||
| } | ||
|
|
||
| if (impl::choseSecond(wx_0, wx_1, xi.x)) | ||
| { | ||
| p.x |= 1; | ||
| } | ||
| } | ||
|
|
||
| const float32_t2 directionUV = (float32_t2(p.x, p.y) + xi) / float32_t2(lumaMapSize); | ||
|
Member
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. comments, comments, lets not let the discord discussions go to waste |
||
| outputAccessor.set(threadID, directionUV); | ||
|
Member
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. don't use an output accessor here because we just want a sampling function, so just return |
||
| } | ||
| } | ||
|
|
||
| }; | ||
|
|
||
| } | ||
| } | ||
| } | ||
| } | ||
| #endif | ||
|
|
||
| #endif | ||
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.
wrong namespace
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.
nbl::hlsl::concepts::accessors