Conversation
| struct HierarchicalWarpSampler | ||
| { | ||
| using warp_generator_type = HierarchicalWarpGenerator<ScalarT, LuminanceAccessorT>; | ||
| using warp_sample_type = typename warp_generator_type::sample_type; |
There was a problem hiding this comment.
could just assert that your PostWarpT::domain_type is same as the HierarhicalSampler::codomain_type (current warp_generator_type::codomain_type)
| using domain_type = vector2_type; | ||
| using codomain_type = vector3_type; |
There was a problem hiding this comment.
domain_type = warp_generator_type::domain_type and codomain_type = PostWarp::codomain_type you can static_assert or require that they have vector_traits<>::dimension 2 and 3 respectively
| sample_type generate(domain_type xi) NBL_CONST_MEMBER_FUNC | ||
| { | ||
| const warp_sample_type warpSample = _warpGenerator.generate(xi); | ||
| const WarpResult<codomain_type> postWarpResult = PostWarpT::warp(warpSample.value()); | ||
| return sample_type::create(postWarpResult.dst, postWarpResult.density * warpSample.pdf()); | ||
| } | ||
|
|
||
| density_type forwardPdf(domain_type xi) NBL_CONST_MEMBER_FUNC |
There was a problem hiding this comment.
when #1001 comes, you'll need to put into this struct's cache:
- the
_warpGenerator.generate's cache members - the hierarchical output sample (since its hidden and intermediate value needed to call postWarp.forwardPdf)
postWarp's cache
| density_type backwardPdf(codomain_type codomainVal) NBL_CONST_MEMBER_FUNC | ||
| { | ||
| return PostWarpT::backwardPdf(codomainVal, _rcpAvgLuma) * _warpGenerator.backwardPdf(codomainVal); | ||
| } |
There was a problem hiding this comment.
does your test even compile, its the _warpGenerator (hierarhical sampler) that currently cares about _rcpAvgLuma not the post-warp
anyway neither should care.
There was a problem hiding this comment.
Anyway the function is wrong, because you'd need to call uv = _postWarp.generateInverse(codomainVal) to get the uv to feed into _warpGenerator.backwardPdf(uv);
| template <typename ScalarT, typename LuminanceAccessorT, typename PostWarpT | ||
| NBL_PRIMARY_REQUIRES( | ||
| is_scalar_v<ScalarT> && | ||
| hierarchical_image::LuminanceReadAccessor<LuminanceAccessorT, ScalarT> && |
There was a problem hiding this comment.
LuminanceAccessorT should tell you the PostWarpT since the luma map had to be made with luminance weighed by 1/forwardPdf of the post-warp, so the warp mustl be strictly paired with the luma map
and then in turn the PostWarpT also tells you your scalar_type (its own typedef)
So only one tempalte parameter needed
| template <typename ScalarT, typename LuminanceAccessorT, typename HierarchicalSamplerT, typename PostWarpT | ||
| NBL_PRIMARY_REQUIRES(is_scalar_v<ScalarT> && | ||
| concepts::accessors::GenericReadAccessor<LuminanceAccessorT, ScalarT, float32_t2> && | ||
| hierarchical_image::WarpAccessor<HierarchicalSamplerT, ScalarT> && | ||
| concepts::Warp<PostWarpT>) |
There was a problem hiding this comment.
I think you can pretty much just template on LuminanceAccessorT for same reasons I outlined for the ComposedHierarchicalSampler you don't need PostWarpT or ScalarT
and I think there isn't much freedom in HierarchicalSamplerT , you can construct it from the other 3
There was a problem hiding this comment.
wait, but your HierarchicalSamplerT isn't a HierarchicalWarpGenerator at alll, such a misleading name, its a WarpmapAccessorT
| using domain_type = vector2_type; | ||
| using codomain_type = vector3_type; |
There was a problem hiding this comment.
same as with the ComposedHierarchicalSampler
| using vector4_type = vector<ScalarT, 4>; | ||
| using domain_type = vector2_type; | ||
| using codomain_type = vector3_type; | ||
| using weight_type = scalar_type; |
There was a problem hiding this comment.
after PR1001 you'll also need a density_type
| if (p.x == 0) | ||
| xi.x = xi.x * scalar_type(0.5) + scalar_type(0.5); | ||
| if (p.y == 0) | ||
| xi.y = xi.y * scalar_type(0.5) + scalar_type(0.5); | ||
| if (p.x == _mapSize.x - 1) | ||
| xi.x = xi.x * scalar_type(0.5); | ||
| if (p.y == _mapSize.y - 1) | ||
| xi.y = xi.y * scalar_type(0.5); | ||
|
|
||
| const vector2_type directionUV = (vector2_type(p.x, p.y) + xi) / _mapSize; |
There was a problem hiding this comment.
there's a bit of a problem with what you're returning (corner sampled) because the PostWarp is resolution agnostic, it doesn't know about corner sampling and expects a [0,1]^2 Unit Square to map to the Sphere where the boundaries touch
So I'd divide by _mapSize - promote(1) and subtract 0.5 from the p+xi before that so you get a normalized coordinate within the corner sampling
There was a problem hiding this comment.
This also means that you return _lastTexel.x * _lastTexel.y /rcpPmf as the PDF instead because of how the UV domain gets shrunk
| LuminanceAccessorT _map; | ||
| float32_t _rcpAvgLuma; | ||
| float32_t2 _rcpWarpSize; | ||
| uint16_t2 _mapSize; |
There was a problem hiding this comment.
you shold really store _lastTexel (_mapSize-1) instead of mapSize
| WarpmapSampler<ScalarT, LuminanceAccessorT, HierarchicalSamplerT, PostWarpT> result; | ||
| result._lumaMap = lumaMap; | ||
| result._warpMap = warpMap; | ||
| result._effectiveWarpArea = (warpSize.x - 1) * (warpSize.y - 1); |
There was a problem hiding this comment.
warpMap is using a custom concept anyway, add a resolution() getter to it
There was a problem hiding this comment.
also store _lastWarpTexel instead of _effectiveWarpArea
|
|
||
| template <typename ScalarT, typename LuminanceAccessorT, typename HierarchicalSamplerT, typename PostWarpT | ||
| NBL_PRIMARY_REQUIRES(is_scalar_v<ScalarT> && | ||
| concepts::accessors::GenericReadAccessor<LuminanceAccessorT, ScalarT, float32_t2> && |
There was a problem hiding this comment.
use LuminanceRead accessor, and add a getAvgValue method to the concept
| const vector2_type interpolant; | ||
| matrix<scalar_type, 4, 2> uvs; | ||
| _warpMap.gatherUv(xi, uvs, interpolant); |
There was a problem hiding this comment.
you know the warp size, make it your job to compute the interpolant and apply corner sampling, so you give the gatherUv(coord,uvs) method a coord thats in [0.5,WarpMapSize-0.5]^2
Lesss code for user to write and get wrong, see https://github.com/Devsh-Graphics-Programming/Nabla-Examples-and-Tests/pull/257/changes#r2991829432
| const scalar_type detInterpolJacobian = determinant(matrix<scalar_type, 2, 2>( | ||
| lerp(xDiffs[0], xDiffs[1], interpolant.y), // first column dFdx | ||
| yDiff // second column dFdy | ||
| )) * _effectiveWarpArea; | ||
|
|
||
| const scalar_type pdf = abs(warpResult.density / detInterpolJacobian); |
There was a problem hiding this comment.
postWarp density, xDiffs, yDiff and interpolant.y go in the cache between generate and forwardPdf
There was a problem hiding this comment.
also the uv put into the post warp (for the forward weight function)
| weight_type forwardWeight(domain_type xi) NBL_CONST_MEMBER_FUNC | ||
| { | ||
| return generate(xi).value(); | ||
| } | ||
|
|
||
| weight_type backwardWeight(codomain_type direction) NBL_CONST_MEMBER_FUNC | ||
| { | ||
| vector2_type envmapUv = PostWarpT::inverseWarp(direction); | ||
| scalar_type luma; | ||
| _lumaMap.get(envmapUv, luma); | ||
| return luma * _rcpAvgLuma * PostWarpT::backwardDensity(direction); | ||
| } |
There was a problem hiding this comment.
no the forward and backward weights need to be identical, so:
forwardWeightmust returnlumaMap.get(generateCache.uv) * _rcpAvgLuma * generateCache.postWarpPdfbackwardWeightmust do what it does now*
- except that right now it doesn't apply corner sampling to
envmapUvwhich it must do
| uint16_t _layerIndex; | ||
| uint16_t _mip2x1 : 15; |
There was a problem hiding this comment.
max mip level is 15, you only need 4 bits to store it, leaving you with 11 bits for the layer, and it just so happens that max layer count is 2048 (where the max value needs only 11 bits) on most GPUs
all 3 members fit in one uint16
| uint16_t2 _mapSize; | ||
| uint16_t _mip2x1 : 15; | ||
| uint16_t _layerIndex; | ||
| uint16_t _lastMipLevel : 15; |
There was a problem hiding this comment.
its not the last, its the penultimate (one minus last) because you're not after the last one which is 1x1, you're tapping 2x1 or 2x2
3 participants
Description
Rework environment map importance sampling to vulkan and hlsl
Testing
Rework example 0 to use vulkan and hlsl
Unit Test Pull Request
TODO list: