Address path tracer merge leftover comments#991
Conversation
…mek's remove_core_matrix branch, added create from mat3x3
…c cast to/from truncated quat
|
|
include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl
Outdated
Show resolved
Hide resolved
include/nbl/builtin/hlsl/sampling/projected_spherical_triangle.hlsl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
most parameters should be members #966 (comment)
There was a problem hiding this comment.
Also #966 (comment)
| return retval; | ||
| } | ||
|
|
||
| vector4_type computeBilinearPatch(const vector3_type receiverNormal, bool isBSDF) |
There was a problem hiding this comment.
why is this returning vector4_type instead of sampling::bilinear
| static ProjectedSphericalTriangle<T> create(NBL_CONST_REF_ARG(shapes::SphericalTriangle<T>) tri) | ||
| { | ||
| ProjectedSphericalTriangle<T> retval; | ||
| retval.tri = tri; | ||
| return retval; | ||
| } |
There was a problem hiding this comment.
remove, see #966 (comment)
| vector3_type cos_vertices, sin_vertices; | ||
| const scalar_type solidAngle = tri.solidAngleOfTriangle(cos_vertices, sin_vertices, cos_a, cos_c, csc_b, csc_c); | ||
| const scalar_type solidAngle = tri.solidAngle(cos_vertices, sin_vertices); |
There was a problem hiding this comment.
should be precomputed and stored as members #966 (comment)
There was a problem hiding this comment.
again function with bazillion parameters that are really local members
| // TODO: probably will only work with isotropic surfaces, need to do aniso | ||
| bool closestHitProgram(uint32_t depth, uint32_t _sample, NBL_REF_ARG(ray_type) ray, NBL_CONST_REF_ARG(scene_type) scene) | ||
| // TODO: will only work with isotropic surfaces, need to do aniso | ||
| bool closestHitProgram(uint32_t depth, uint32_t _sample, NBL_REF_ARG(ray_type) ray, NBL_CONST_REF_ARG(intersect_data_type) intersectData, NBL_CONST_REF_ARG(scene_type) scene) |
There was a problem hiding this comment.
you're storing the scene, so why are you passing it by reference to the function !?
| ray_dir_info_type V; | ||
| V.setDirection(-ray.direction); | ||
| isotropic_interaction_type iso_interaction = isotropic_interaction_type::create(V, N); | ||
| const vector3_type intersection = intersectData.intersection; |
| anisotropic_interaction_type interaction = anisotropic_interaction_type::create(iso_interaction); | ||
| interaction.isotropic.luminosityContributionHint = colorspace::scRGBtoXYZ[1]; | ||
|
|
||
| vector3_type throughput = ray.payload.throughput; |
| // emissive | ||
| const uint32_t lightID = glsl::bitfieldExtract(bsdfLightIDs, 16, 16); | ||
| if (lightID != light_type::INVALID_ID) | ||
| typename scene_type::mat_light_id_type matLightID = scene.getMatLightIDs(ray.objectID); |
There was a problem hiding this comment.
why is what you hit stored in the ray!?
Please make the ray be a CONST reference, this stuff belongs in the intersection data!
There was a problem hiding this comment.
we still add to ray.payload.accumulation throughout though
| nee_ray.direction = nee_sample.getL().getDirection(); | ||
| nee_ray.intersectionT = t; | ||
| if (bsdf_quotient_pdf.pdf < numeric_limits<scalar_type>::max && getLuma(neeContrib_pdf.quotient) > lumaContributionThreshold && intersector_type::traceRay(nee_ray, scene).id == -1) | ||
| if (bsdf_quotient_pdf.pdf < numeric_limits<scalar_type>::max && getLuma(neeContrib_pdf.quotient) > lumaContributionThreshold && !intersector_type::traceRay(nee_ray, scene).foundHit) |
There was a problem hiding this comment.
you want an anyhit ray, and you want the anyhit ray return type to have an operator scalar_t() to give you a shadow percentage
| ray_type nee_ray; | ||
| nee_ray.origin = intersection + nee_sample.getL().getDirection() * t * Tolerance<scalar_type>::getStart(depth); | ||
| nee_ray.direction = nee_sample.getL().getDirection(); | ||
| nee_ray.intersectionT = t; |
There was a problem hiding this comment.
I'd make a Tolerance<scalar_type>::adjust(&ray,geoNormalAtOrigin,depth); so it can do whatever internally, and you call like this
ray_type nee_ray;
nee_ray.origin = intersection;
nee_ray.direction = nee_sample.getL().getDirection();
nee_ray.intersectionT = t;
Tolerance<scalar_type>::adjust(ray,geoNormalAtOrigin,depth);| ray_type ray = rayGen.generate(uvw); | ||
| ray.initPayload(); | ||
|
|
||
| nee.scene = scene; |
There was a problem hiding this comment.
okay, because of the copying, I'll allow passing the scene as NBL_CONST_REF_ARG to nee's methods as the first parameter
| ray.normalAtOrigin = interaction.getN(); | ||
| ray.wasBSDFAtOrigin = isBSDF; | ||
| } | ||
| vector3_type origin = intersection + bxdfSample * (1.0/*kSceneSize*/) * Tolerance<scalar_type>::getStart(depth); |
There was a problem hiding this comment.
do the tolerance stuff with adjust as per previous comment
| vector3_type origin = intersection + bxdfSample * (1.0/*kSceneSize*/) * Tolerance<scalar_type>::getStart(depth); | ||
| vector3_type direction = bxdfSample; | ||
|
|
||
| ray.initData(origin, direction, interaction.getN(), isBSDF); |
There was a problem hiding this comment.
pass whole interaction and something about the material syustem like ID or other stuff instead of just passing the isBSDF
Makes changes left from #966