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)
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
| ((NBL_CONCEPT_REQ_TYPE)(T::object_handle_type)) | ||
| ((NBL_CONCEPT_REQ_TYPE)(T::intersect_data_type)) | ||
| ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((intersect.traceRay(ray, scene)), ::nbl::hlsl::is_same_v, typename T::intersect_data_type)) |
There was a problem hiding this comment.
you don't need object_handle_type but you do need different closest_hit_type (better name than intersect_data_Type and any_hit_type)
There was a problem hiding this comment.
could be an uint16, we're not gonna render paths that deep, ever
There was a problem hiding this comment.
as talked on discord, we only need deferred_pdf, the emission will be given by material system
There was a problem hiding this comment.
why do you have two interactions and duplicate parameters!?
and aniso_interaction is usable in place of your `iso_interaction!
There was a problem hiding this comment.
Also luminosityContributionHint should have already came out good an initialized #991 (comment)
| intersect_data_type intersection = intersector_type::traceRay(ray, scene); | ||
|
|
||
| hit = ray.objectID.id != -1; | ||
| hit = intersection.foundHit; |
There was a problem hiding this comment.
METHOD ! NOT a member!
There was a problem hiding this comment.
| // 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; |
| // 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.
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
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);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
4 participants
Makes changes left from #966