Created concepts for samplers, added quotient_and_pdf variants to satisfy the concepts#1001
Created concepts for samplers, added quotient_and_pdf variants to satisfy the concepts#1001karimsayedre wants to merge 1 commit intomasterfrom
Conversation
…isfy the concepts
| namespace hlsl | ||
| { | ||
| scalar_type sinPhi, cosPhi; | ||
| math::sincos<scalar_type>(2.0 * numbers::pi<scalar_type> * xi.y - numbers::pi<scalar_type>, sinPhi, cosPhi); | ||
| return vector2_type(cosPhi, sinPhi) * nbl::hlsl::sqrt(-2.0 * nbl::hlsl::log(xi.x)) * stddev; | ||
| namespace sampling | ||
| { |
There was a problem hiding this comment.
no indent for namespaces
| template <typename T NBL_PRIMARY_REQUIRES(concepts::FloatingPointLikeScalar<T>) struct BoxMullerTransform | ||
| { |
There was a problem hiding this comment.
fix the indents and template + requires is always on its own line
| namespace nbl | ||
| { | ||
| namespace hlsl | ||
| { | ||
| namespace sampling | ||
| { | ||
| namespace concepts | ||
| { | ||
|
|
||
| // ============================================================================ |
There was a problem hiding this comment.
no indends for namespaces
| ((NBL_CONCEPT_REQ_TYPE)(T::domain_type)) | ||
| ((NBL_CONCEPT_REQ_TYPE)(T::codomain_type)) | ||
| ((NBL_CONCEPT_REQ_TYPE)(T::density_type)) | ||
| ((NBL_CONCEPT_REQ_TYPE)(T::sample_type)) |
There was a problem hiding this comment.
I'd also check that sample_type has a pdf or rpdf and value method (can do a separate concept SampleWithPDF and SampleWithRcpPDF)
| // Required types: | ||
| // domain_type - the input space | ||
| // codomain_type - the output space | ||
| // density_type - the density type (typically scalar) |
There was a problem hiding this comment.
density will always be scalar
| #include <nbl/builtin/hlsl/concepts/__end.hlsl> | ||
|
|
||
| // ============================================================================ | ||
| // BackwardDensitySampler |
There was a problem hiding this comment.
we need a better name like ResamplableSampler
| #define NBL_CONCEPT_NAME BackwardDensitySampler | ||
| #define NBL_CONCEPT_TPLT_PRM_KINDS (typename) | ||
| #define NBL_CONCEPT_TPLT_PRM_NAMES (T) | ||
| #define NBL_CONCEPT_PARAM_0 (sampler, T) |
There was a problem hiding this comment.
sampler is a HLSL keyword, replace with _sampler throughout the file
| // Extends TractableSampler with the ability to evaluate the PDF given | ||
| // a codomain value (i.e. without knowing the original domain input). | ||
| // | ||
| // Required methods (in addition to TractableSampler): |
There was a problem hiding this comment.
if it extends then you can simply put a clause that checks TractableSampler<T>, see how Anisotropic and Isotropic build on top of each other in the BxDF concepts
| } | ||
| namespace sampling | ||
| { | ||
|
|
There was a problem hiding this comment.
watch out for indents
| struct quotient_and_pdf | ||
| { | ||
| using this_t = quotient_and_pdf<Q, P>; |
There was a problem hiding this comment.
we really need this type, its necessary for BxDFs and without it you'd murder it, quotient_and_pdf is meant for importance sampling a function f(x) proportional to its value
The struct here stores quotient = f(sample)/forwardPdf(sample) and shouldn't be abused for anything else
you need something else for warps (which is what you're doing), which means a new header
Undo changes here.
2 participants
Examples PR
Notes:
quotient_and_pdf()methods inUniformHemisphere,UniformSphere,ProjectedHemisphere, andProjectedSphereshadow the struct typesampling::quotient_and_pdf<Q, P>fromquotient_and_pdf.hlsl. DXC can't resolve the return type because the method name takes precedence over the struct name during lookup. Fixed by fully qualifying with::nbl::hlsl::sampling::quotient_and_pdf<U, T>.