Complete bxdf tests #236
Complete bxdf tests #236
Conversation
…ng/Nabla-Examples-and-Tests into bxdf_unit_tests
…ng/Nabla-Examples-and-Tests into bxdf_unit_tests
| BET_RECIPROCITY, | ||
| BET_GENERATE_H, | ||
| BET_JACOBIAN, // jacobian * pdf != 0 | ||
| BET_PDF_EVAL_DIFF, // quotient * pdf != eval |
There was a problem hiding this comment.
thats actually allowed for some mixture/delta BxDFs
Why? cause you call quotient always on samples you've generated yourself, while eval is called on what you havent.
so if you have a delta distribution, it should be impossible to sample via a different sampler.
Example, smooth reflector BRDF returns 0 pdf and 0 eval, unless from quotient_and_pdf where it will return some exactly fresnel coloured quotient and Inf as the PDF.
| #define GLM_ENABLE_EXPERIMENTAL | ||
| #include <glm/gtx/hash.hpp> |
There was a problem hiding this comment.
this probably conflicts with how Nabla includes it
| if (!(s.isValid() && sx.isValid() && sy.isValid())) | ||
| return BET_INVALID; | ||
|
|
||
| struct SBxDFTestResources | ||
| { | ||
| static SBxDFTestResources create(uint32_t2 seed) | ||
| { | ||
| SBxDFTestResources retval; | ||
| retval.rng = nbl::hlsl::Xoroshiro64Star::construct(seed); | ||
| retval.u = float32_t3(rngUniformDist<float32_t2>(retval.rng), 0.0); | ||
| if (traits_t::type == bxdf::BT_BRDF) | ||
| { | ||
| if (s.getNdotL() <= bit_cast<float>(numeric_limits<float>::min)) | ||
| return BET_INVALID; | ||
| } | ||
| else if (traits_t::type == bxdf::BT_BSDF) | ||
| { | ||
| if (hlsl::abs(s.getNdotL()) <= bit_cast<float>(numeric_limits<float>::min)) | ||
| return BET_INVALID; | ||
| } |
There was a problem hiding this comment.
same comments as in the NDF tests
| if (checkZero<float32_t3>(bsdf, 1e-5) || checkZero<float32_t3>(pdf.quotient, 1e-5)) | ||
| return BET_NONE; // produces an "impossible" sample |
There was a problem hiding this comment.
improve the comment, this is valid behaviour
| if (checkLt<float32_t3>(bsdf, (float32_t3)0.0) || checkLt<float32_t3>(pdf.quotient, (float32_t3)0.0) || pdf.pdf < 0.0) | ||
| return BET_NEGATIVE_VAL; |
There was a problem hiding this comment.
do this as the very first check
| // get jacobian | ||
| float32_t2x2 m = float32_t2x2( | ||
| sx.getTdotL() - s.getTdotL(), sy.getTdotL() - s.getTdotL(), | ||
| sx.getBdotL() - s.getBdotL(), sy.getBdotL() - s.getBdotL() | ||
| ); | ||
| float det = nbl::hlsl::determinant<float32_t2x2>(m); | ||
|
|
||
| template<> | ||
| struct TestBxDF<bxdf::reflection::SBeckmannBxDF<sample_t, iso_cache, aniso_cache, spectral_t>> : TestBxDFBase<bxdf::reflection::SBeckmannBxDF<sample_t, iso_cache, aniso_cache, spectral_t>> | ||
| { | ||
| using base_t = TestBxDFBase<bxdf::reflection::SBeckmannBxDF<sample_t, iso_cache, aniso_cache, spectral_t>>; | ||
| if (!checkZero<float>(det * pdf.pdf / s.getNdotL(), 1e-4)) | ||
| return BET_JACOBIAN; |
There was a problem hiding this comment.
infinite PDF and zero jacobian will produce NaN, which will make you throw this error.. while its completely valid behaviour actually
| float32_t3 quo_pdf = pdf.value(); | ||
| if (!checkEq<float32_t3>(quo_pdf, bsdf, 1e-4)) |
There was a problem hiding this comment.
if pdf is infinite, you need to make sure you skip this check, then eval and quotient*pdf are allowed to be different
Think of a mixture of smooth specular and lambertian diffuse, eval will only consider the diffuse and forget about the specular.
| #include "nlohmann/json.hpp" | ||
|
|
||
| using json = nlohmann::json; |
| int main(int argc, char** argv) | ||
| { | ||
| std::cout << std::fixed << std::setprecision(4); | ||
| IApplicationFramework::GlobalsInit(); | ||
| auto system = IApplicationFramework::createSystem(); | ||
| const system::path CWD = path(argv[0]).parent_path().generic_string() + "/"; | ||
| smart_refctd_ptr<ILogger> logger = make_smart_refctd_ptr<CColoredStdoutLoggerANSI>(ILogger::DefaultLogMask()); | ||
| logger->log("Logger Created!", ILogger::ELL_INFO); | ||
| auto assetManager = make_smart_refctd_ptr<IAssetManager>(smart_refctd_ptr(system)); |
There was a problem hiding this comment.
no, if you use the application framework you shouldn't have an int main anymore
66_HLSLBxDFTests/main.cpp
Outdated
| fprintf(stderr, "[ERROR] could not open config file\n"); | ||
| logger->log("could not open config file\n", ILogger::ELL_ERROR); |
There was a problem hiding this comment.
Unformatted String Vulnerability, see https://en.wikipedia.org/wiki/Uncontrolled_format_string
|
|
||
| float bin(float a) | ||
| { | ||
| float diff = std::fmod(a, stride); |
There was a problem hiding this comment.
nitpick, use the hlsl namespace's tgmath (or intrinsics, I'm sure that @Przemog1 added such a function) fmod function for this
| if (!s.isValid()) | ||
| continue; |
There was a problem hiding this comment.
it should be an error to not generate a valid sample if 100% VNDF is guaranteed by the NDF trait
| if (pdf.pdf == bit_cast<float>(numeric_limits<float>::infinity)) | ||
| buckets[bucket] += 1; |
There was a problem hiding this comment.
explain this logic ?
Why are only infinite PDF samples added to a bucket !?
|
|
||
| sample_t s; | ||
| quotient_pdf_t pdf; | ||
| float32_t3 bsdf; |
There was a problem hiding this comment.
unused variable ?
| buckets[bucket] += 1; | ||
| } | ||
|
|
||
| #ifndef __HLSL_VERSION |
There was a problem hiding this comment.
remove the ifndef, you're in C++ header now
| if (!selective || b.second > 0) | ||
| { | ||
| math::Polar<float> polarCoords; | ||
| polarCoords.theta = b.first.x * numbers::pi<float>; | ||
| polarCoords.phi = b.first.y * 2.f * numbers::pi<float>; | ||
| const float32_t3 v = polarCoords.getCartesian(); | ||
| base_t::errMsg += std::format("({:.3f},{:.3f},{:.3f}): {}\n", v.x, v.y, v.z, b.second); | ||
| } |
There was a problem hiding this comment.
We need some documentation about this test.
I don't get why you're only counting inf PDF samples, and why your only error is an inf PDF.
Also why the error message build is done here, and the back buckets are not appended to a list to format and print later closer to cb.__call
| inline float adaptiveSimpson(const std::function<float(float)>& f, float x0, float x1, float eps = 1e-6, int depth = 6) | ||
| { | ||
| int count = 0; | ||
| std::function<float(float, float, float, float, float, float, float, float, int)> integrate = | ||
| [&](float a, float b, float c, float fa, float fb, float fc, float I, float eps, int depth) | ||
| { | ||
| float d = 0.5f * (a + b); | ||
| float e = 0.5f * (b + c); | ||
| float fd = f(d); | ||
| float fe = f(e); | ||
|
|
||
| float h = c - a; | ||
| float I0 = (1.0f / 12.0f) * h * (fa + 4 * fd + fb); | ||
| float I1 = (1.0f / 12.0f) * h * (fb + 4 * fe + fc); | ||
| float Ip = I0 + I1; | ||
| count++; | ||
|
|
||
| if (depth <= 0 || std::abs(Ip - I) < 15 * eps) | ||
| return Ip + (1.0f / 15.0f) * (Ip - I); | ||
|
|
||
| return integrate(a, d, b, fa, fd, fb, I0, .5f * eps, depth - 1) + | ||
| integrate(b, e, c, fb, fe, fc, I1, .5f * eps, depth - 1); | ||
| }; | ||
|
|
||
| float a = x0; | ||
| float b = 0.5f * (x0 + x1); | ||
| float c = x1; | ||
| float fa = f(a); | ||
| float fb = f(b); | ||
| float fc = f(c); | ||
| float I = (c - a) * (1.0f / 6.0f) * (fa + 4.f * fb + fc); | ||
| return integrate(a, b, c, fa, fb, fc, I, eps, depth); | ||
| } | ||
|
|
||
| inline float adaptiveSimpson2D(const std::function<float(float, float)>& f, float32_t2 x0, float32_t2 x1, float eps = 1e-6, int depth = 6) | ||
| { | ||
| const auto integrate = [&](float y) -> float | ||
| { | ||
| return adaptiveSimpson(std::bind(f, std::placeholders::_1, y), x0.x, x1.x, eps, depth); | ||
| }; | ||
| return adaptiveSimpson(integrate, x0.y, x1.y, eps, depth); | ||
| } |
There was a problem hiding this comment.
replace std::function with a generic template functor, right now you're doing virtual calls on every invocation.
Also it would make sense to move this to Nabla somwhere in https://github.com/Devsh-Graphics-Programming/Nabla/tree/master/include/nbl/builtin/hlsl/math/quadrature
because we have a similar function to compute integrals https://github.com/Devsh-Graphics-Programming/Nabla/blob/3f5b2c33e07d43c6bfd78193a8c75ffa11842ecf/include/nbl/builtin/hlsl/math/quadrature/gauss_legendre/gauss_legendre.hlsl#L22
You can do recursion in HLSL a slightly cursed way -> https://godbolt.org/z/6rvE8rEqW but please do compile-time depth so we don't kill register occupancy
There was a problem hiding this comment.
@Erfan-Ahmadi look at the new toys you're getting to play with
| double RLGamma(double a, double x) { | ||
| const double epsilon = 0.000000000000001; | ||
| const double big = 4503599627370496.0; | ||
| const double bigInv = 2.22044604925031308085e-16; | ||
| assert(a >= 0 && x >= 0); | ||
|
|
||
| if (x == 0) | ||
| return 0.0f; | ||
|
|
||
| double ax = (a * std::log(x)) - x - std::lgamma(a); | ||
| if (ax < -709.78271289338399) | ||
| return a < x ? 1.0 : 0.0; | ||
|
|
||
| if (x <= 1 || x <= a) | ||
| { | ||
| double r2 = a; | ||
| double c2 = 1; | ||
| double ans2 = 1; | ||
|
|
||
| do { | ||
| r2 = r2 + 1; | ||
| c2 = c2 * x / r2; | ||
| ans2 += c2; | ||
| } while ((c2 / ans2) > epsilon); | ||
|
|
||
| return std::exp(ax) * ans2 / a; | ||
| } | ||
|
|
||
| int c = 0; | ||
| double y = 1 - a; | ||
| double z = x + y + 1; | ||
| double p3 = 1; | ||
| double q3 = x; | ||
| double p2 = x + 1; | ||
| double q2 = z * x; | ||
| double ans = p2 / q2; | ||
| double error; | ||
|
|
||
| do { | ||
| c++; | ||
| y += 1; | ||
| z += 2; | ||
| double yc = y * c; | ||
| double p = (p2 * z) - (p3 * yc); | ||
| double q = (q2 * z) - (q3 * yc); | ||
|
|
||
| if (q != 0) | ||
| { | ||
| double nextans = p / q; | ||
| error = std::abs((ans - nextans) / nextans); | ||
| ans = nextans; | ||
| } | ||
| else | ||
| { | ||
| error = 1; | ||
| } | ||
|
|
||
| p3 = p2; | ||
| p2 = p; | ||
| q3 = q2; | ||
| q2 = q; | ||
|
|
||
| if (std::abs(p) > big) | ||
| { | ||
| p3 *= bigInv; | ||
| p2 *= bigInv; | ||
| q3 *= bigInv; | ||
| q2 *= bigInv; | ||
| } | ||
| } while (error > epsilon); | ||
|
|
||
| return 1.0 - (std::exp(ax) * ans); | ||
| } |
There was a problem hiding this comment.
This is https://en.wikipedia.org/wiki/Incomplete_gamma_function
Which in Boost is in this file https://www.boost.org/doc/libs/latest/libs/math/doc/html/math_toolkit/sf_gamma/igamma.html
C++ doesn't have incomplete gamma functions, but other "special functions" like Bessel, would live in tgmath (cmath, but we always template everything for good measure)
For now you can constrain the implementation to a scalar.
| smart_refctd_ptr<ICPUImage> writeToCPUImage() | ||
| { |
There was a problem hiding this comment.
nitpick, instead of burying this function a test, I'd rather have it in main.cpp as a function that does
smart_refctd_ptr<ICPUImage> writeAsCPUImage(const TestChi2<BxDF,aniso>&)| const auto format = E_FORMAT::EF_R32G32B32A32_SFLOAT; | ||
|
|
||
| IImage::SCreationParams imageParams = {}; | ||
| imageParams.flags = static_cast<asset::IImage::E_CREATE_FLAGS>(asset::IImage::ECF_MUTABLE_FORMAT_BIT | asset::IImage::ECF_EXTENDED_USAGE_BIT); |
There was a problem hiding this comment.
nitpick, you shouldn't care about this when you're not making GPU images from the CPU image via asset converter or multiple views with different formats
| // write sample count from generate, top half | ||
| for (uint64_t j = 0u; j < thetaSplits; ++j) | ||
| for (uint64_t i = 0; i < phiSplits; ++i) | ||
| { | ||
| float32_t3 pixelColor = mapColor(countFreq[j * phiSplits + i], 0.f, maxCountFreq); | ||
| double decodedPixel[4] = { pixelColor[0], pixelColor[1], pixelColor[2], 1 }; | ||
|
|
||
| const uint64_t pixelIndex = j * phiSplits + i; | ||
| asset::encodePixelsRuntime(format, bytePtr + pixelIndex * asset::getTexelOrBlockBytesize(format), decodedPixel); | ||
| } | ||
|
|
||
| // write values of pdf, bottom half | ||
| for (uint64_t j = 0u; j < thetaSplits; ++j) | ||
| for (uint64_t i = 0; i < phiSplits; ++i) | ||
| { | ||
| float32_t3 pixelColor = mapColor(integrateFreq[j * phiSplits + i], 0.f, maxIntFreq); | ||
| double decodedPixel[4] = { pixelColor[0], pixelColor[1], pixelColor[2], 1 }; | ||
|
|
||
| const uint64_t pixelIndex = (thetaSplits + j) * phiSplits + i; | ||
| asset::encodePixelsRuntime(format, bytePtr + pixelIndex * asset::getTexelOrBlockBytesize(format), decodedPixel); | ||
| } |
There was a problem hiding this comment.
I'd just write first image into RGB, and second into Alpha.
Since you're paying for RGBA full fp32 eacvh channel, and one can easily turn off certain channels or view just alpha in Renderdoc and GIMP (Can leave this as a comment)
| float32_t3 mapColor(float v, float vmin, float vmax) | ||
| { | ||
| float32_t3 c = float32_t3(1, 1, 1); | ||
| float diff = vmax - vmin; | ||
| v = clamp<float>(v, vmin, vmax); | ||
|
|
||
| if (v < (vmin + 0.25f * diff)) | ||
| { | ||
| c.r = 0; | ||
| c.g = 4.f * (v - vmin) / diff; | ||
| } | ||
| else if (v < (vmin + 0.5f * diff)) | ||
| { | ||
| c.r = 0; | ||
| c.b = 1.f + 4.f * (vmin + 0.25f * diff - v) / diff; | ||
| } | ||
| else if (v < (vmin + 0.75f * diff)) | ||
| { | ||
| c.r = 4.f * (v - vmin - 0.5f * diff) / diff; | ||
| c.b = 0; | ||
| } | ||
| else | ||
| { | ||
| c.g = 1.f + 4.f * (vmin + 0.75f * diff - v) / diff; | ||
| c.b = 0; | ||
| } | ||
|
|
||
| return c; | ||
| } |
There was a problem hiding this comment.
I'd add this to nbl::hlsl::visualization::false_color and keep our Sci-viz gradient there
@AnastaZIuk same thing for your IESes
Some color mappings have particular names in scientific literature https://docs.paraview.org/en/latest/ReferenceManual/colorMapping.html
https://pmc.ncbi.nlm.nih.gov/articles/PMC4959790/
https://www.youtube.com/watch?v=tJ6aEIB61hY & https://www.fabiocrameri.ch/colourmaps/
| if (!s.isValid()) | ||
| continue; | ||
|
|
There was a problem hiding this comment.
same comment as always about VNDF
| if (write_frequencies) | ||
| writeToEXR(assetManager); |
There was a problem hiding this comment.
we're liklely to drown in temp files, maybe have an option to only write failed tests' frequencies instead of always ?
basically turning write_frequencies into an enum
Also if you want the tests to be thread safe and parallizable, you'd need to somehow document that you expect that all base_t::rc.halfSeed to be unique (with a wide enough seed, good enough rng you shouldn't get collisions in rng output until veeeeeery large test instance counts)
No description provided.